Address comments in review

cmd/kubeadm/README.md

-# Kubernetes Cluster Bootstrap Made Easy
-
-## Usage
-
-### `kubeadm init`
-
-It's usually enough to run `kubeadm init`, but in some case you might like to override the
-default behaviour. The flags used for said purpose are described below.
-
-- `--token=<token>`
-
-  By default, a token is generated, but if you are to automate cluster deployment, you will want to
-  set the token ahead of time. Read the docs for more information on the token format.
-
-- `--api-advertise-addresses=<ips>` (multiple values are allowed by having multiple flag declarations or multiple values separated by comma)
-- `--api-external-dns-names=<domain>` (multiple values are allowed by having multiple flag declarations or multiple values separated by comma)
-
-  By default, `kubeadm` will auto detect IP addresses and use that to generate API server certificates.
-  If you would like to access the API via any external IPs and/or hostnames, which it might not be able
-  to detect, you can use `--api-advertise-addresses` and `--api-external-dns-names` to add multiple
-  different IP addresses and hostnames (DNS).
-
-- `--service-cidr=<cidr>` (default: "100.64.0.0/12")
-
-  By default, `kubeadm` sets `100.64.0.0/12` as the subnet for services. This means when a service is created, its cluster IP, if not manually specified,
-  will be automatically assigned from the services subnet. If you would like to set a different one, use `--service-cidr`.
-
-- `--service-dns-domain=<domain>` (default: "cluster.local")
-
-  By default, `kubeadm` sets `cluster.local` as the cluster DNS domain. If you would like to set a different one, use `--service-dns-domain`.
-
-- `--schedule-pods-here=<bool>` (default: "false")
-
-  By default, `kubeadm` sets the master node kubelet as non-schedulable for workloads. This means the master node won't run your pods. If you want to change that,
-  use `--schedule-pods-here=true`.
-
-- `--cloud-provider=<cloud provider>`
-
-  By default, `kubeadm` doesn't perform auto-detection of the current cloud provider. If you want to specify it, use `--cloud-provider`. Possible values are
-  the ones supported by controller-manager, namely `"aws"`, `"azure"`, `"cloudstack"`, `"gce"`, `"mesos"`, `"openstack"`, `"ovirt"`, `"rackspace"`, `"vsphere"`.
-
-***TODO(phase1+)***
-
-- `--api-bind-address=<ip>`
-- `--api-bind-port=<port>`
-
-***TODO(phase2)***
-
-- `--api-bind-loopback-unsecure=<bool>`
-
-- `--prefer-private-network=<bool>`
-- `--prefer-public-network=<bool>`
-
-### `kubeadm join`
-
-`kubeadm join` has one mandatory flag, the token used to secure cluster bootstrap, and one mandatory argument, the master IP address.
-Here's an example on how to use it:
-
-`kubeadm join --token=the_secret_token 192.168.1.1`
-
-- `--token=<token>`
-
-By default, when `kubeadm init` runs, a token is generated and revealed in the output. That's the token you should use here.

cmd/kubeadm/app/api/types.go

@@ -21,6 +21,7 @@ import (
 )
 
 // KubeadmConfig TODO add description
+// TODO(phase1+) @krousey: Please don't embed structs. It obfuscates the source of the fields and doesn't really buy you anything.
 type KubeadmConfig struct {
 	InitFlags
 	JoinFlags
@@ -35,6 +36,11 @@ type KubeadmConfig struct {
 
 // TODO(phase2) should we add validation functions for these structs?
 
+// TODO(phase1+) refactor token handling
+// - https://github.com/kubernetes/kubernetes/pull/33262/files#r80333662
+// - https://github.com/kubernetes/kubernetes/pull/33262/files#r80336374
+// - https://github.com/kubernetes/kubernetes/pull/33262/files#r80333982
+
 // InitFlags holds values for "kubeadm init" command flags.
 type InitFlags struct {
 	API struct {
@@ -63,7 +69,7 @@ type InitFlags struct {
 
 const (
 	DefaultServiceDNSDomain   = "cluster.local"
-	DefaultServicesCIDRString = "100.64.0.0/12"
+	DefaultServicesCIDRString = "100.64.0.0/12" // Carrier-grade NAT range (RFC 6598)
 	DefaultKubernetesVersion  = "v1.4.0-beta.10"
 )
 

cmd/kubeadm/app/cmd/init.go

@@ -20,6 +20,7 @@ import (
 	"fmt"
 	"io"
 	"net"
+	"strings"
 
 	"github.com/renstrom/dedent"
 	"github.com/spf13/cobra"
@@ -43,10 +44,10 @@ var (
 
 // NewCmdInit returns "kubeadm init" command.
 func NewCmdInit(out io.Writer, s *kubeadmapi.KubeadmConfig) *cobra.Command {
-	advertiseAddrs := &[]string{}
+	advertiseAddrs := &[]string{} // TODO(pahse1+) make it work somehow else, custom flag or whatever
 	cmd := &cobra.Command{
 		Use:   "init",
-		Short: "Run this on the first machine.",
+		Short: "Run this in order to set up the Kubernetes master.",
 		Run: func(cmd *cobra.Command, args []string) {
 			err := RunInit(out, cmd, args, s, advertiseAddrs)
 			cmdutil.CheckErr(err)
@@ -55,58 +56,58 @@ func NewCmdInit(out io.Writer, s *kubeadmapi.KubeadmConfig) *cobra.Command {
 
 	cmd.PersistentFlags().StringVar(
 		&s.Secrets.GivenToken, "token", "",
-		`(optional) Shared secret used to secure cluster bootstrap. If none is provided, one will be generated for you.`,
+		"Shared secret used to secure cluster bootstrap; if none is provided, one will be generated for you",
 	)
 	cmd.PersistentFlags().StringSliceVar(
 		advertiseAddrs, "api-advertise-addresses", []string{},
-		`(optional) The IP addresses to advertise, in case autodetection fails.`,
+		"The IP addresses to advertise, in case autodetection fails",
 	)
 	cmd.PersistentFlags().StringSliceVar(
 		&s.InitFlags.API.ExternalDNSNames, "api-external-dns-names", []string{},
-		`(optional) The DNS names to advertise, in case you have configured them yourself.`,
+		"The DNS names to advertise, in case you have configured them yourself",
 	)
 	cmd.PersistentFlags().IPNetVar(
 		&s.InitFlags.Services.CIDR, "service-cidr", *kubeadmapi.DefaultServicesCIDR,
-		`(optional) Use alterantive range of IP address for service VIPs, defaults to `+
+		`Use alterantive range of IP address for service VIPs, defaults to `+
 			kubeadmapi.DefaultServicesCIDRString,
 	)
 	cmd.PersistentFlags().IPNetVar(
 		&s.InitFlags.PodNetwork.CIDR, "pod-network-cidr", net.IPNet{},
-		`(optional) Specify range of IP addresses for the pod network. If set, the control plane will automatically allocate CIDRs for every node.`,
+		"Specify range of IP addresses for the pod network; if set, the control plane will automatically allocate CIDRs for every node",
 	)
 	cmd.PersistentFlags().StringVar(
 		&s.InitFlags.Services.DNSDomain, "service-dns-domain", kubeadmapi.DefaultServiceDNSDomain,
-		`(optional) Use alternative domain for services, e.g. "myorg.internal"`,
+		`Use alternative domain for services, e.g. "myorg.internal"`,
 	)
 	cmd.PersistentFlags().StringVar(
 		&s.InitFlags.CloudProvider, "cloud-provider", "",
-		`(optional) Enable a specific cloud provider features (external load-balancers, storage, etc), e.g. "gce"`,
+		`Enable cloud provider features (external load-balancers, storage, etc), e.g. "gce"`,
 	)
 	cmd.PersistentFlags().BoolVar(
 		&s.InitFlags.Schedulable, "schedule-pods-here", false,
-		`(optional) Allow to schedule workload to the node`,
+		`Allow to schedule workload to the node`,
 	)
 	cmd.PersistentFlags().StringVar(
 		&s.InitFlags.Versions.Kubernetes, "use-kubernetes-version", kubeadmapi.DefaultKubernetesVersion,
-		`(optional) Choose a specific Kubernetes version for the control plane`,
+		`Choose a specific Kubernetes version for the control plane`,
 	)
 
 	// TODO (phase1+) @errordeveloper make the flags below not show up in --help but rather on --advanced-help
 	cmd.PersistentFlags().StringSliceVar(
 		&s.InitFlags.API.Etcd.ExternalEndpoints, "external-etcd-endpoints", []string{},
-		`(optional) etcd endpoints to use, in case you have an external cluster.`,
+		"etcd endpoints to use, in case you have an external cluster",
 	)
 	cmd.PersistentFlags().StringVar(
 		&s.InitFlags.API.Etcd.ExternalCAFile, "external-etcd-cafile", "",
-		`(optional) etcd certificate authority certificate file."`,
+		"etcd certificate authority certificate file",
 	)
 	cmd.PersistentFlags().StringVar(
 		&s.InitFlags.API.Etcd.ExternalCertFile, "external-etcd-certfile", "",
-		`(optional) etcd client certificate file."`,
+		"etcd client certificate file",
 	)
 	cmd.PersistentFlags().StringVar(
 		&s.InitFlags.API.Etcd.ExternalKeyFile, "external-etcd-keyfile", "",
-		`(optional) etcd client key file."`,
+		"etcd client key file",
 	)
 
 	return cmd
@@ -126,14 +127,15 @@ func RunInit(out io.Writer, cmd *cobra.Command, args []string, s *kubeadmapi.Kub
 		for _, i := range *advertiseAddrs {
 			addr := net.ParseIP(i)
 			if addr == nil {
-				return fmt.Errorf("<cmd/init> failed to parse flag (%q) as an IP address", "--api-advertise-addresses="+i)
+				// TODO(phase1+) custom flag will help to get this error message into a better place
+				return fmt.Errorf("<cmd/init> failed to parse %q (in %q) as an IP address", "--api-advertise-addresses="+strings.Join(*advertiseAddrs, ","))
 			}
 			s.InitFlags.API.AdvertiseAddrs = append(s.InitFlags.API.AdvertiseAddrs, addr)
 		}
 	}
 
+	// TODO(phase1+) create a custom flag
 	if s.InitFlags.CloudProvider != "" {
-		// TODO(phase2) we should be able to auto-detect it and check whether things like IAM roles are correct
 		if _, ok := kubeadmapi.SupportedCloudProviders[s.InitFlags.CloudProvider]; !ok {
 			return fmt.Errorf("<cmd/init> cloud provider %q is not supported, you can use any of %v, or leave it unset", s.InitFlags.CloudProvider, kubeadmapi.ListOfCloudProviders)
 		}

cmd/kubeadm/app/cmd/join.go

@@ -54,7 +54,7 @@ func NewCmdJoin(out io.Writer, s *kubeadmapi.KubeadmConfig) *cobra.Command {
 
 	cmd.PersistentFlags().StringVar(
 		&s.Secrets.GivenToken, "token", "",
-		`Shared secret used to secure bootstrap. Must match the output of 'kubeadm init'.`,
+		"(required) Shared secret used to secure bootstrap. Must match the output of 'kubeadm init'",
 	)
 
 	return cmd
@@ -77,7 +77,7 @@ func RunJoin(out io.Writer, cmd *cobra.Command, args []string, s *kubeadmapi.Kub
 	ok, err := kubeadmutil.UseGivenTokenIfValid(s)
 	if !ok {
 		if err != nil {
-			return fmt.Errorf("<cmd/join> %s (see --help)\n", err)
+			return fmt.Errorf("<cmd/join> %v (see --help)\n", err)
 		}
 		return fmt.Errorf("Must specify --token (see --help)\n")
 	}

cmd/kubeadm/app/master/addons.go

@@ -56,8 +56,8 @@ func createKubeProxyPodSpec(s *kubeadmapi.KubeadmConfig, architecture string) ap
 					// and accepts `--master` at the same time
 					//
 					// clever options include:
-					//  - do CSR dance and create kubeconfig and mount it as secrete
-					//  - create a service account with a second secret enconding kubeconfig
+					//  - do CSR dance and create kubeconfig and mount it as a secret
+					//  - create a service account with a second secret encoding kubeconfig
 					//  - use init container to convert known information to kubeconfig
 					//  - ...whatever
 					Name:      "kubeconfig",
@@ -215,7 +215,7 @@ func createKubeDNSPodSpec(s *kubeadmapi.KubeadmConfig) api.PodSpec {
 func createKubeDNSServiceSpec(s *kubeadmapi.KubeadmConfig) (*api.ServiceSpec, error) {
 	ip, err := ipallocator.GetIndexedIP(&s.InitFlags.Services.CIDR, 10)
 	if err != nil {
-		return nil, fmt.Errorf("unable to allocate IP address for kube-dns addon from the given CIDR (%q) [%s]", s.InitFlags.Services.CIDR, err)
+		return nil, fmt.Errorf("unable to allocate IP address for kube-dns addon from the given CIDR (%q) [%v]", s.InitFlags.Services.CIDR, err)
 	}
 
 	svc := &api.ServiceSpec{
@@ -238,7 +238,7 @@ func CreateEssentialAddons(s *kubeadmapi.KubeadmConfig, client *clientset.Client
 		SetMasterTaintTolerations(&kubeProxyDaemonSet.Spec.Template.ObjectMeta)
 
 		if _, err := client.Extensions().DaemonSets(api.NamespaceSystem).Create(kubeProxyDaemonSet); err != nil {
-			return fmt.Errorf("<master/addons> failed creating essential kube-proxy addon [%s]", err)
+			return fmt.Errorf("<master/addons> failed creating essential kube-proxy addon [%v]", err)
 		}
 	}
 
@@ -248,17 +248,17 @@ func CreateEssentialAddons(s *kubeadmapi.KubeadmConfig, client *clientset.Client
 	SetMasterTaintTolerations(&kubeDNSDeployment.Spec.Template.ObjectMeta)
 
 	if _, err := client.Extensions().Deployments(api.NamespaceSystem).Create(kubeDNSDeployment); err != nil {
-		return fmt.Errorf("<master/addons> failed creating essential kube-dns addon [%s]", err)
+		return fmt.Errorf("<master/addons> failed creating essential kube-dns addon [%v]", err)
 	}
 
 	kubeDNSServiceSpec, err := createKubeDNSServiceSpec(s)
 	if err != nil {
-		return fmt.Errorf("<master/addons> failed creating essential kube-dns addon - %s", err)
+		return fmt.Errorf("<master/addons> failed creating essential kube-dns addon - %v", err)
 	}
 
 	kubeDNSService := NewService("kube-dns", *kubeDNSServiceSpec)
 	if _, err := client.Services(api.NamespaceSystem).Create(kubeDNSService); err != nil {
-		return fmt.Errorf("<master/addons> failed creating essential kube-dns addon [%s]", err)
+		return fmt.Errorf("<master/addons> failed creating essential kube-dns addon [%v]", err)
 	}
 
 	fmt.Println("<master/addons> created essential addon: kube-dns")

cmd/kubeadm/app/master/apiclient.go

@@ -39,14 +39,14 @@ func CreateClientAndWaitForAPI(adminConfig *clientcmdapi.Config) (*clientset.Cli
 		&clientcmd.ConfigOverrides{},
 	).ClientConfig()
 	if err != nil {
-		return nil, fmt.Errorf("<master/apiclient> failed to create API client configuration [%s]", err)
+		return nil, fmt.Errorf("<master/apiclient> failed to create API client configuration [%v]", err)
 	}
 
 	fmt.Println("<master/apiclient> created API client configuration")
 
 	client, err := clientset.NewForConfig(adminClientConfig)
 	if err != nil {
-		return nil, fmt.Errorf("<master/apiclient> failed to create API client [%s]", err)
+		return nil, fmt.Errorf("<master/apiclient> failed to create API client [%v]", err)
 	}
 
 	fmt.Println("<master/apiclient> created API client, waiting for the control plane to become ready")
@@ -151,7 +151,7 @@ func NewDeployment(deploymentName string, replicas int32, podSpec api.PodSpec) *
 func findMyself(client *clientset.Clientset) (*api.Node, error) {
 	nodeList, err := client.Nodes().List(api.ListOptions{})
 	if err != nil {
-		return nil, fmt.Errorf("unable to list nodes [%s]", err)
+		return nil, fmt.Errorf("unable to list nodes [%v]", err)
 	}
 	if len(nodeList.Items) < 1 {
 		return nil, fmt.Errorf("no nodes found")
@@ -187,9 +187,10 @@ func attemptToUpdateMasterRoleLabelsAndTaints(client *clientset.Clientset, sched
 }
 
 func UpdateMasterRoleLabelsAndTaints(client *clientset.Clientset, schedulable bool) error {
+	// TODO(phase1+) use iterate instead of recursion
 	err := attemptToUpdateMasterRoleLabelsAndTaints(client, schedulable)
 	if err != nil {
-		return fmt.Errorf("<master/apiclient> failed to update master node - %s", err)
+		return fmt.Errorf("<master/apiclient> failed to update master node - %v", err)
 	}
 	return nil
 }

cmd/kubeadm/app/master/discovery.go

@@ -111,10 +111,10 @@ func CreateDiscoveryDeploymentAndSecret(s *kubeadmapi.KubeadmConfig, client *cli
 	kd := newKubeDiscovery(s, caCert)
 
 	if _, err := client.Extensions().Deployments(api.NamespaceSystem).Create(kd.Deployment); err != nil {
-		return fmt.Errorf("<master/discovery> failed to create %q deployment [%s]", kubeDiscoveryName, err)
+		return fmt.Errorf("<master/discovery> failed to create %q deployment [%v]", kubeDiscoveryName, err)
 	}
 	if _, err := client.Secrets(api.NamespaceSystem).Create(kd.Secret); err != nil {
-		return fmt.Errorf("<master/discovery> failed to create %q secret [%s]", kubeDiscoverySecretName, err)
+		return fmt.Errorf("<master/discovery> failed to create %q secret [%v]", kubeDiscoverySecretName, err)
 	}
 
 	fmt.Println("<master/discovery> created essential addon: kube-discovery, waiting for it to become ready")

cmd/kubeadm/app/master/kubeconfig.go

@@ -44,7 +44,7 @@ func CreateCertsAndConfigForClients(s *kubeadmapi.KubeadmConfig, clientNames []s
 	for _, client := range clientNames {
 		key, cert, err := newClientKeyAndCert(caCert, caKey)
 		if err != nil {
-			return nil, fmt.Errorf("<master/kubeconfig> failure while creating %s client certificate - %s", client, err)
+			return nil, fmt.Errorf("<master/kubeconfig> failure while creating %s client certificate - %v", client, err)
 		}
 		config := kubeadmutil.MakeClientConfigWithCerts(
 			basicClientConfig,

cmd/kubeadm/app/master/manifests.go

@@ -33,8 +33,7 @@ import (
 	"k8s.io/kubernetes/pkg/util/intstr"
 )
 
-// Static pod definitions in golang form are included below so that `kubeadm
-// init master` and `kubeadm manual bootstrap master` can get going.
+// Static pod definitions in golang form are included below so that `kubeadm init` can get going.
 
 const (
 	DefaultClusterName     = "kubernetes"
@@ -110,16 +109,16 @@ func WriteStaticPodManifests(s *kubeadmapi.KubeadmConfig) error {
 
 	manifestsPath := path.Join(s.EnvParams["kubernetes_dir"], "manifests")
 	if err := os.MkdirAll(manifestsPath, 0700); err != nil {
-		return fmt.Errorf("<master/manifests> failed to create directory %q [%s]", manifestsPath, err)
+		return fmt.Errorf("<master/manifests> failed to create directory %q [%v]", manifestsPath, err)
 	}
 	for name, spec := range staticPodSpecs {
 		filename := path.Join(manifestsPath, name+".json")
 		serialized, err := json.MarshalIndent(spec, "", "  ")
 		if err != nil {
-			return fmt.Errorf("<master/manifests> failed to marshall manifest for %q to JSON [%s]", name, err)
+			return fmt.Errorf("<master/manifests> failed to marshall manifest for %q to JSON [%v]", name, err)
 		}
 		if err := cmdutil.DumpReaderToFile(bytes.NewReader(serialized), filename); err != nil {
-			return fmt.Errorf("<master/manifests> failed to create static pod manifest file for %q (%q) [%s]", name, filename, err)
+			return fmt.Errorf("<master/manifests> failed to create static pod manifest file for %q (%q) [%v]", name, filename, err)
 		}
 	}
 	return nil

cmd/kubeadm/app/master/pki.go

@@ -30,7 +30,7 @@ import (
 func newCertificateAuthority() (*rsa.PrivateKey, *x509.Certificate, error) {
 	key, err := certutil.NewPrivateKey()
 	if err != nil {
-		return nil, nil, fmt.Errorf("unable to create private key [%s]", err)
+		return nil, nil, fmt.Errorf("unable to create private key [%v]", err)
 	}
 
 	config := certutil.Config{
@@ -39,7 +39,7 @@ func newCertificateAuthority() (*rsa.PrivateKey, *x509.Certificate, error) {
 
 	cert, err := certutil.NewSelfSignedCACert(config, key)
 	if err != nil {
-		return nil, nil, fmt.Errorf("unable to create self-singed certificate [%s]", err)
+		return nil, nil, fmt.Errorf("unable to create self-signed certificate [%v]", err)
 	}
 
 	return key, cert, nil
@@ -48,7 +48,7 @@ func newCertificateAuthority() (*rsa.PrivateKey, *x509.Certificate, error) {
 func newServerKeyAndCert(s *kubeadmapi.KubeadmConfig, caCert *x509.Certificate, caKey *rsa.PrivateKey, altNames certutil.AltNames) (*rsa.PrivateKey, *x509.Certificate, error) {
 	key, err := certutil.NewPrivateKey()
 	if err != nil {
-		return nil, nil, fmt.Errorf("unabel to create private key [%s]", err)
+		return nil, nil, fmt.Errorf("unabel to create private key [%v]", err)
 	}
 
 	internalAPIServerFQDN := []string{
@@ -60,7 +60,7 @@ func newServerKeyAndCert(s *kubeadmapi.KubeadmConfig, caCert *x509.Certificate,
 
 	internalAPIServerVirtualIP, err := ipallocator.GetIndexedIP(&s.InitFlags.Services.CIDR, 1)
 	if err != nil {
-		return nil, nil, fmt.Errorf("unable to allocate IP address for the API server from the given CIDR (%q) [%s]", &s.InitFlags.Services.CIDR, err)
+		return nil, nil, fmt.Errorf("unable to allocate IP address for the API server from the given CIDR (%q) [%v]", &s.InitFlags.Services.CIDR, err)
 	}
 
 	altNames.IPs = append(altNames.IPs, internalAPIServerVirtualIP)
@@ -72,7 +72,7 @@ func newServerKeyAndCert(s *kubeadmapi.KubeadmConfig, caCert *x509.Certificate,
 	}
 	cert, err := certutil.NewSignedCert(config, key, caCert, caKey)
 	if err != nil {
-		return nil, nil, fmt.Errorf("unable to sing certificate [%s]", err)
+		return nil, nil, fmt.Errorf("unable to sign certificate [%v]", err)
 	}
 
 	return key, cert, nil
@@ -81,7 +81,7 @@ func newServerKeyAndCert(s *kubeadmapi.KubeadmConfig, caCert *x509.Certificate,
 func newClientKeyAndCert(caCert *x509.Certificate, caKey *rsa.PrivateKey) (*rsa.PrivateKey, *x509.Certificate, error) {
 	key, err := certutil.NewPrivateKey()
 	if err != nil {
-		return nil, nil, fmt.Errorf("unable to create private key [%s]", err)
+		return nil, nil, fmt.Errorf("unable to create private key [%v]", err)
 	}
 
 	config := certutil.Config{
@@ -89,7 +89,7 @@ func newClientKeyAndCert(caCert *x509.Certificate, caKey *rsa.PrivateKey) (*rsa.
 	}
 	cert, err := certutil.NewSignedCert(config, key, caCert, caKey)
 	if err != nil {
-		return nil, nil, fmt.Errorf("unable to sign certificate [%s]", err)
+		return nil, nil, fmt.Errorf("unable to sign certificate [%v]", err)
 	}
 
 	return key, cert, nil
@@ -104,20 +104,20 @@ func writeKeysAndCert(pkiPath string, name string, key *rsa.PrivateKey, cert *x5
 
 	if key != nil {
 		if err := certutil.WriteKey(privateKeyPath, certutil.EncodePrivateKeyPEM(key)); err != nil {
-			return fmt.Errorf("unable to write private key file (%q) [%s]", privateKeyPath, err)
+			return fmt.Errorf("unable to write private key file (%q) [%v]", privateKeyPath, err)
 		}
 		if pubKey, err := certutil.EncodePublicKeyPEM(&key.PublicKey); err == nil {
 			if err := certutil.WriteKey(publicKeyPath, pubKey); err != nil {
-				return fmt.Errorf("unable to write public key file (%q) [%s]", publicKeyPath, err)
+				return fmt.Errorf("unable to write public key file (%q) [%v]", publicKeyPath, err)
 			}
 		} else {
-			return fmt.Errorf("unable to encode public key to PEM [%s]", err)
+			return fmt.Errorf("unable to encode public key to PEM [%v]", err)
 		}
 	}
 
 	if cert != nil {
 		if err := certutil.WriteCert(certificatePath, certutil.EncodeCertPEM(cert)); err != nil {
-			return fmt.Errorf("unable to write certificate file (%q) [%s]", certificatePath, err)
+			return fmt.Errorf("unable to write certificate file (%q) [%v]", certificatePath, err)
 		}
 	}
 
@@ -132,47 +132,46 @@ func newServiceAccountKey() (*rsa.PrivateKey, error) {
 	return key, nil
 }
 
+// CreatePKIAssets will create and write to disk all PKI assets necessary to establish the control plane.
+// It first generates a self-signed CA certificate, a server certificate (signed by the CA) and a key for
+// signing service account tokens. It returns CA key and certificate, which is convenient for use with
+// client config funcs.
 func CreatePKIAssets(s *kubeadmapi.KubeadmConfig) (*rsa.PrivateKey, *x509.Certificate, error) {
 	var (
 		err      error
 		altNames certutil.AltNames
 	)
 
-	if len(s.InitFlags.API.AdvertiseAddrs) > 0 {
 	altNames.IPs = append(altNames.IPs, s.InitFlags.API.AdvertiseAddrs...)
-	}
-
-	if len(s.InitFlags.API.ExternalDNSNames) > 0 {
 	altNames.DNSNames = append(altNames.DNSNames, s.InitFlags.API.ExternalDNSNames...)
-	}
 
 	pkiPath := path.Join(s.EnvParams["host_pki_path"])
 
 	caKey, caCert, err := newCertificateAuthority()
 	if err != nil {
-		return nil, nil, fmt.Errorf("<master/pki> failure while creating CA keys and certificate - %s", err)
+		return nil, nil, fmt.Errorf("<master/pki> failure while creating CA keys and certificate - %v", err)
 	}
 
 	if err := writeKeysAndCert(pkiPath, "ca", caKey, caCert); err != nil {
-		return nil, nil, fmt.Errorf("<master/pki> failure while saving CA keys and certificate - %s", err)
+		return nil, nil, fmt.Errorf("<master/pki> failure while saving CA keys and certificate - %v", err)
 	}
 
 	apiKey, apiCert, err := newServerKeyAndCert(s, caCert, caKey, altNames)
 	if err != nil {
-		return nil, nil, fmt.Errorf("<master/pki> failure while creating API server keys and certificate - %s", err)
+		return nil, nil, fmt.Errorf("<master/pki> failure while creating API server keys and certificate - %v", err)
 	}
 
 	if err := writeKeysAndCert(pkiPath, "apiserver", apiKey, apiCert); err != nil {
-		return nil, nil, fmt.Errorf("<master/pki> failure while saving API server keys and certificate - %s", err)
+		return nil, nil, fmt.Errorf("<master/pki> failure while saving API server keys and certificate - %v", err)
 	}
 
 	saKey, err := newServiceAccountKey()
 	if err != nil {
-		return nil, nil, fmt.Errorf("<master/pki> failure while creating service account signing keys [%s]", err)
+		return nil, nil, fmt.Errorf("<master/pki> failure while creating service account signing keys [%v]", err)
 	}
 
 	if err := writeKeysAndCert(pkiPath, "sa", saKey, nil); err != nil {
-		return nil, nil, fmt.Errorf("<master/pki> failure while saving service account singing keys - %s", err)
+		return nil, nil, fmt.Errorf("<master/pki> failure while saving service account signing keys - %v", err)
 	}
 
 	// TODO(phase1+) print a summary of SANs used and checksums (signatures) of each of the certificates

cmd/kubeadm/app/master/tokens.go

@@ -30,6 +30,7 @@ import (
 
 func generateTokenIfNeeded(s *kubeadmapi.KubeadmConfig) error {
 	ok, err := kubeadmutil.UseGivenTokenIfValid(s)
+	// TODO(phase1+) @krousey: I know it won't happen with the way it is currently implemented, but this doesn't handle case where ok is true and err is non-nil.
 	if !ok {
 		if err != nil {
 			return err
@@ -49,14 +50,15 @@ func generateTokenIfNeeded(s *kubeadmapi.KubeadmConfig) error {
 func CreateTokenAuthFile(s *kubeadmapi.KubeadmConfig) error {
 	tokenAuthFilePath := path.Join(s.EnvParams["host_pki_path"], "tokens.csv")
 	if err := generateTokenIfNeeded(s); err != nil {
-		return fmt.Errorf("<master/tokens> failed to generate token(s) [%s]", err)
+		return fmt.Errorf("<master/tokens> failed to generate token(s) [%v]", err)
 	}
 	if err := os.MkdirAll(s.EnvParams["host_pki_path"], 0700); err != nil {
-		return fmt.Errorf("<master/tokens> failed to create directory %q [%s]", s.EnvParams["host_pki_path"], err)
+		return fmt.Errorf("<master/tokens> failed to create directory %q [%v]", s.EnvParams["host_pki_path"], err)
 	}
 	serialized := []byte(fmt.Sprintf("%s,kubeadm-node-csr,%s,system:kubelet-bootstrap\n", s.Secrets.BearerToken, uuid.NewUUID()))
+	// DumpReaderToFile create a file with mode 0600
 	if err := cmdutil.DumpReaderToFile(bytes.NewReader(serialized), tokenAuthFilePath); err != nil {
-		return fmt.Errorf("<master/tokens> failed to save token auth file (%q) [%s]", tokenAuthFilePath, err)
+		return fmt.Errorf("<master/tokens> failed to save token auth file (%q) [%v]", tokenAuthFilePath, err)
 	}
 	return nil
 }

cmd/kubeadm/app/node/csr.go

@@ -49,12 +49,12 @@ func PerformTLSBootstrap(s *kubeadmapi.KubeadmConfig, apiEndpoint string, caCert
 		&clientcmd.ConfigOverrides{},
 	).ClientConfig()
 	if err != nil {
-		return nil, fmt.Errorf("<node/csr> failed to create API client configuration [%s]", err)
+		return nil, fmt.Errorf("<node/csr> failed to create API client configuration [%v]", err)
 	}
 
 	client, err := unversionedcertificates.NewForConfig(bootstrapClientConfig)
 	if err != nil {
-		return nil, fmt.Errorf("<node/csr> failed to create API client [%s]", err)
+		return nil, fmt.Errorf("<node/csr> failed to create API client [%v]", err)
 	}
 	csrClient := client.CertificateSigningRequests()
 
@@ -63,19 +63,19 @@ func PerformTLSBootstrap(s *kubeadmapi.KubeadmConfig, apiEndpoint string, caCert
 	// (for example user may be bringing up machines in parallel and for some reasons master is slow to boot)
 
 	if err := checkCertsAPI(bootstrapClientConfig); err != nil {
-		return nil, fmt.Errorf("<node/csr> fialed to proceed due to API compatibility issue - %s", err)
+		return nil, fmt.Errorf("<node/csr> fialed to proceed due to API compatibility issue - %v", err)
 	}
 
 	fmt.Println("<node/csr> created API client to obtain unique certificate for this node, generating keys and certificate signing request")
 
 	key, err := certutil.MakeEllipticPrivateKeyPEM()
 	if err != nil {
-		return nil, fmt.Errorf("<node/csr> failed to generating private key [%s]", err)
+		return nil, fmt.Errorf("<node/csr> failed to generating private key [%v]", err)
 	}
 
 	cert, err := csr.RequestNodeCertificate(csrClient, key, nodeName)
 	if err != nil {
-		return nil, fmt.Errorf("<node/csr> failed to request signed certificate from the API server [%s]", err)
+		return nil, fmt.Errorf("<node/csr> failed to request signed certificate from the API server [%v]", err)
 	}
 
 	// TODO(phase1+) print some basic info about the cert
@@ -93,13 +93,13 @@ func checkCertsAPI(config *restclient.Config) error {
 	discoveryClient, err := discovery.NewDiscoveryClientForConfig(config)
 
 	if err != nil {
-		return fmt.Errorf("failed to create API discovery client [%s]", err)
+		return fmt.Errorf("failed to create API discovery client [%v]", err)
 	}
 
 	serverGroups, err := discoveryClient.ServerGroups()
 
 	if err != nil {
-		return fmt.Errorf("failed to retrieve a list of supported API objects [%s]", err)
+		return fmt.Errorf("failed to retrieve a list of supported API objects [%v]", err)
 	}
 
 	for _, group := range serverGroups.Groups {
@@ -110,7 +110,7 @@ func checkCertsAPI(config *restclient.Config) error {
 
 	version, err := discoveryClient.ServerVersion()
 	if err != nil {
-		return fmt.Errorf("unable to obtain API version [%s]", err)
+		return fmt.Errorf("unable to obtain API version [%v]", err)
 	}
 
 	return fmt.Errorf("API version %s does not support certificates API, use v1.4.0 or newer", version.String())

cmd/kubeadm/app/node/discovery.go

@@ -33,14 +33,14 @@ func RetrieveTrustedClusterInfo(s *kubeadmapi.KubeadmConfig) (*clientcmdapi.Conf
 	requestURL := fmt.Sprintf("http://%s:%d/cluster-info/v1/?token-id=%s", host, port, s.Secrets.TokenID)
 	req, err := http.NewRequest("GET", requestURL, nil)
 	if err != nil {
-		return nil, fmt.Errorf("<node/discovery> failed to consturct an HTTP request [%s]", err)
+		return nil, fmt.Errorf("<node/discovery> failed to consturct an HTTP request [%v]", err)
 	}
 
 	fmt.Printf("<node/discovery> created cluster info discovery client, requesting info from %q\n", requestURL)
 
 	res, err := http.DefaultClient.Do(req)
 	if err != nil {
-		return nil, fmt.Errorf("<node/discovery> failed to request cluster info [%s]", err)
+		return nil, fmt.Errorf("<node/discovery> failed to request cluster info [%v]", err)
 	}
 	buf := new(bytes.Buffer)
 	io.Copy(buf, res.Body)
@@ -48,20 +48,20 @@ func RetrieveTrustedClusterInfo(s *kubeadmapi.KubeadmConfig) (*clientcmdapi.Conf
 
 	object, err := jose.ParseSigned(buf.String())
 	if err != nil {
-		return nil, fmt.Errorf("<node/discovery> failed to parse response as JWS object [%s]", err)
+		return nil, fmt.Errorf("<node/discovery> failed to parse response as JWS object [%v]", err)
 	}
 
 	fmt.Println("<node/discovery> cluster info object received, verifying signature using given token")
 
 	output, err := object.Verify(s.Secrets.Token)
 	if err != nil {
-		return nil, fmt.Errorf("<node/discovery> failed to verify JWS signature of received cluster info object [%s]", err)
+		return nil, fmt.Errorf("<node/discovery> failed to verify JWS signature of received cluster info object [%v]", err)
 	}
 
 	clusterInfo := kubeadmapi.ClusterInfo{}
 
 	if err := json.Unmarshal(output, &clusterInfo); err != nil {
-		return nil, fmt.Errorf("<node/discovery> failed to decode received cluster info object [%s]", err)
+		return nil, fmt.Errorf("<node/discovery> failed to decode received cluster info object [%v]", err)
 	}
 
 	if len(clusterInfo.CertificateAuthorities) == 0 || len(clusterInfo.Endpoints) == 0 {

cmd/kubeadm/app/util/kubeconfig.go

@@ -77,19 +77,19 @@ func MakeClientConfigWithToken(config *clientcmdapi.Config, clusterName string,
 
 func WriteKubeconfigIfNotExists(s *kubeadmapi.KubeadmConfig, name string, kubeconfig *clientcmdapi.Config) error {
 	if err := os.MkdirAll(s.EnvParams["kubernetes_dir"], 0700); err != nil {
-		return fmt.Errorf("<util/kubeconfig> failed to create directory %q [%s]", s.EnvParams["kubernetes_dir"], err)
+		return fmt.Errorf("<util/kubeconfig> failed to create directory %q [%v]", s.EnvParams["kubernetes_dir"], err)
 	}
 
 	filename := path.Join(s.EnvParams["kubernetes_dir"], fmt.Sprintf("%s.conf", name))
 	// Create and open the file, only if it does not already exist.
 	f, err := os.OpenFile(filename, os.O_CREATE|os.O_WRONLY|os.O_EXCL, 0600)
 	if err != nil {
-		return fmt.Errorf("<util/kubeconfig> failed to create %q, it already exists [%s]", filename, err)
+		return fmt.Errorf("<util/kubeconfig> failed to create %q, it already exists [%v]", filename, err)
 	}
 	f.Close()
 
 	if err := clientcmd.WriteToFile(*kubeconfig, filename); err != nil {
-		return fmt.Errorf("<util/kubeconfig> failed to write to %q [%s]", filename, err)
+		return fmt.Errorf("<util/kubeconfig> failed to write to %q [%v]", filename, err)
 	}
 
 	fmt.Printf("<util/kubeconfig> created %q\n", filename)