Cleanup some low-hanging fruits and review TODOs

cmd/kubeadm/README.md

@@ -1,4 +1,4 @@
-# Kubernetes Cluster Boostrap Made Easy
+# Kubernetes Cluster Bootstrap Made Easy
 
 ## Usage
 
@@ -9,35 +9,35 @@ 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
+  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.
+  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.
+  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
+  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
+  to detect, you can use `--api-advertise-addresses` and `--api-external-dns-names` to add multiple
-different IP addresses and hostnames (DNS).
+  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, 
+  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`.
+  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`.
+  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-workload=<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, 
+  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-workload=true`.
+  use `--schedule-workload=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
+  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"`.
+  the ones supported by controller-manager, namely `"aws"`, `"azure"`, `"cloudstack"`, `"gce"`, `"mesos"`, `"openstack"`, `"ovirt"`, `"rackspace"`, `"vsphere"`.
 
 ***TODO(phase1+)***
 
@@ -61,37 +61,3 @@ Here's an example on how to use it:
 - `--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.
-
-# User Experience Considerations
-
-> ***TODO*** _Move this into the design document
-
-a) `kube-apiserver` will listen on `0.0.0.0:443` and `127.0.0.1:8080`, which is not configurable right now and make things a bit easier for the MVP
-b) there is `kube-discovery`, which will listen on `0.0.0.0:9898`
-
-
-from the point of view of `kubeadm init`, we need to have
-a) a primary IP address as will be seen by the nodes and needs to go into the cert and `kube-discovery` configuration secret
-b) some other names and addresses API server may be known by (e.g. external DNS and/or LB and/or NAT)
-
-from that perspective we don’t can assume default ports for now, but for all the address we really only care about two ports (i.e.  443 and 9898)
-
-we should make ports configurable and expose some way of making API server bind to a specific address/interface
-
-but I think for the MVP we need solve the issue with hardcode IPs and DNS names in the certs
-
-so it sounds rather simple enough to introduce  `--api-advertise-addr=<ip>` and `--api-external-dns-name=<domain>`, and allowing multiple of those sounds also simple enough
-
-from the `kubeadm join` perspective, it cares about the two ports we mentioned, and we can make those configurable too
-
-but for now it’s easier to pass just the IP address
-
-plust it’s also require, so passing it without a named flag sounds convenient, and it’s something users are familiar with
-
-that’s what Consul does, what what Weave does, and now Docker SwarmKit does the same thing also (edited)
-
-flags will differ, as there are some Kubernetes-specifics aspects to what join does, but basic join semantics will remain _familiar_
-
-if we do marry `kube-discovery` with the API, we might do `kubeadm join host:port`, as we’d end-up with a single port to care about
-
-but we haven’t yet

cmd/kubeadm/app/api/types.go

@@ -90,11 +90,12 @@ func init() {
 // JoinFlags holds values for "kubeadm join" command flags.
 type JoinFlags struct {
 	MasterAddrs []net.IP
+	// TODO(phase1+) add manual mode flags here, e.g. RootCACertPath
 }
 
 // ClusterInfo TODO add description
 type ClusterInfo struct {
-	// TODO(phase1?) this may become simply `api.Config`
+	// TODO(phase1+) this may become simply `api.Config`
 	CertificateAuthorities []string `json:"certificateAuthorities"`
 	Endpoints              []string `json:"endpoints"`
 }

cmd/kubeadm/app/cmd/cmd.go

@@ -22,10 +22,9 @@ import (
 	"github.com/renstrom/dedent"
 	"github.com/spf13/cobra"
 
+	kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/api"
 	cmdutil "k8s.io/kubernetes/pkg/kubectl/cmd/util"
 	"k8s.io/kubernetes/pkg/util/flag"
-
-	kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/api"
 )
 
 func NewKubeadmCommand(f *cmdutil.Factory, in io.Reader, out, err io.Writer, envParams map[string]string) *cobra.Command {
@@ -66,9 +65,6 @@ func NewKubeadmCommand(f *cmdutil.Factory, in io.Reader, out, err io.Writer, env
 	}
 	// TODO(phase2+) figure out how to avoid running as root
 	//
-	// TODO(phase1) also print the alpha warning when running any commands, as well as
-	// in the help text.
-	//
 	// TODO(phase2) detect interactive vs non-interactive use and adjust output accordingly
 	// i.e. make it automation friendly
 	//

cmd/kubeadm/app/cmd/init.go

@@ -49,7 +49,7 @@ func NewCmdInit(out io.Writer, s *kubeadmapi.KubeadmConfig) *cobra.Command {
 		Short: "Run this on the first machine.",
 		Run: func(cmd *cobra.Command, args []string) {
 			err := RunInit(out, cmd, args, s, advertiseAddrs)
-			cmdutil.CheckErr(err) // TODO(phase1+) append alpha warning with bugs URL etc
+			cmdutil.CheckErr(err)
 		},
 	}
 
@@ -72,8 +72,7 @@ func NewCmdInit(out io.Writer, s *kubeadmapi.KubeadmConfig) *cobra.Command {
 	)
 	cmd.PersistentFlags().IPNetVar(
 		&s.InitFlags.PodNetwork.CIDR, "pod-network-cidr", net.IPNet{},
-		`(optional) Specify range of IP addresses for the pod overlay network, e.g. 10.3.0.0/16.`+
+		`(optional) Specify range of IP addresses for the pod network. If set, the control plane will automatically allocate CIDRs for every node.`,
-			`If set, the control plane will automatically attempt to allocate Pod network CIDRs for every node.`,
 	)
 	cmd.PersistentFlags().StringVar(
 		&s.InitFlags.Services.DNSDomain, "service-dns-domain", kubeadmapi.DefaultServiceDNSDomain,
@@ -154,6 +153,16 @@ func RunInit(out io.Writer, cmd *cobra.Command, args []string, s *kubeadmapi.Kub
 	if err != nil {
 		return err
 	}
+
+	// kubeadm is responsible for writing the following kubeconfig file, which
+	// kubelet should be waiting for. Help user avoid foot-shooting by refusing to
+	// write a file that has already been written (the kubelet will be up and
+	// running in that case - they'd need to stop the kubelet, remove the file, and
+	// start it again in that case).
+	// TODO(phase1+) this is no longer the right place to guard agains foo-shooting,
+	// we need to decide how to handle existing files (it may be handy to support
+	// importing existing files, may be we could even make our command idempotant,
+	// or at least allow for external PKI and stuff)
 	for name, kubeconfig := range kubeconfigs {
 		if err := kubeadmutil.WriteKubeconfigIfNotExists(s, name, kubeconfig); err != nil {
 			return err

cmd/kubeadm/app/cmd/join.go

@@ -62,7 +62,7 @@ func NewCmdJoin(out io.Writer, s *kubeadmapi.KubeadmConfig) *cobra.Command {
 
 // RunJoin executes worked node provisioning and tries to join an existing cluster.
 func RunJoin(out io.Writer, cmd *cobra.Command, args []string, s *kubeadmapi.KubeadmConfig) error {
-	// TODO this we are missing args from the help text, there should be a way to tell cobra about it
+	// TODO(phase1+) this we are missing args from the help text, there should be a way to tell cobra about it
 	if len(args) == 0 {
 		return fmt.Errorf("<cmd/join> must specify master IP address (see --help)")
 	}

cmd/kubeadm/app/images/images.go

@@ -42,7 +42,7 @@ const (
 )
 
 // TODO(phase1): Make this configurable + default to a v1.4 value fetched from: https://storage.googleapis.com/kubernetes-release/release/stable.txt
-var DefaultKubeVersion = "v1.4.0-beta.6"
+var DefaultKubeVersion = "v1.4.0-beta.8"
 
 func GetCoreImage(image string, overrideImage string) string {
 	if overrideImage != "" {

cmd/kubeadm/app/kubeadm.go

@@ -21,6 +21,7 @@ import (
 	"os"
 	"strings"
 
+	"github.com/renstrom/dedent"
 	"github.com/spf13/pflag"
 
 	"k8s.io/kubernetes/cmd/kubeadm/app/cmd"
@@ -28,12 +29,18 @@ import (
 	"k8s.io/kubernetes/pkg/util/logs"
 )
 
+var AlphaWarningOnExit = dedent.Dedent(`
+	kubeadm: I am an alpha version, my authors welcome your feedback and bug reports
+	kubeadm: please create issue an using https://github.com/kubernetes/kubernetes/issues/new
+	kubeadm: and make sure to mention @kubernetes/sig-cluster-lifecycle. Thank you!
+`)
+
 // TODO(phase2) use componentconfig
 // we need some params for testing etc, let's keep these hidden for now
 func getEnvParams() map[string]string {
 
 	envParams := map[string]string{
-		// TODO(phase1): Mode prefix and host_pki_path to another place as constants, and use them everywhere
+		// TODO(phase1+): Mode prefix and host_pki_path to another place as constants, and use them everywhere
 		// Right now they're used here and there, but not consequently
 		"kubernetes_dir":     "/etc/kubernetes",
 		"host_pki_path":      "/etc/kubernetes/pki",

cmd/kubeadm/app/master/apiclient.go

@@ -31,6 +31,8 @@ import (
 	"k8s.io/kubernetes/pkg/util/wait"
 )
 
+const apiCallRetryInterval = 500 * time.Millisecond
+
 func CreateClientAndWaitForAPI(adminConfig *clientcmdapi.Config) (*clientset.Clientset, error) {
 	adminClientConfig, err := clientcmd.NewDefaultClientConfig(
 		*adminConfig,
@@ -50,12 +52,12 @@ func CreateClientAndWaitForAPI(adminConfig *clientcmdapi.Config) (*clientset.Cli
 	fmt.Println("<master/apiclient> created API client, waiting for the control plane to become ready")
 
 	start := time.Now()
-	wait.PollInfinite(500*time.Millisecond, func() (bool, error) {
+	wait.PollInfinite(apiCallRetryInterval, func() (bool, error) {
 		cs, err := client.ComponentStatuses().List(api.ListOptions{})
 		if err != nil {
 			return false, nil
 		}
-		// TODO revisit this when we implement HA
+		// TODO(phase2) must revisit this when we implement HA
 		if len(cs.Items) < 3 {
 			fmt.Println("<master/apiclient> not all control plane components are ready yet")
 			return false, nil
@@ -75,14 +77,13 @@ func CreateClientAndWaitForAPI(adminConfig *clientcmdapi.Config) (*clientset.Cli
 
 	fmt.Println("<master/apiclient> waiting for at least one node to register and become ready")
 	start = time.Now()
-	wait.PollInfinite(500*time.Millisecond, func() (bool, error) {
+	wait.PollInfinite(apiCallRetryInterval, func() (bool, error) {
 		nodeList, err := client.Nodes().List(api.ListOptions{})
 		if err != nil {
 			fmt.Println("<master/apiclient> temporarily unable to list nodes (will retry)")
 			return false, nil
 		}
 		if len(nodeList.Items) < 1 {
-			//fmt.Printf("<master/apiclient> %d nodes have registered so far", len(nodeList.Items))
 			return false, nil
 		}
 		n := &nodeList.Items[0]
@@ -146,7 +147,7 @@ func NewDeployment(deploymentName string, replicas int32, podSpec api.PodSpec) *
 }
 
 // It's safe to do this for alpha, as we don't have HA and there is no way we can get
-// more then one node here (TODO find a way to determine owr own node name)
+// more then one node here (TODO(phase1+) use os.Hostname)
 func findMyself(client *clientset.Clientset) (*api.Node, error) {
 	nodeList, err := client.Nodes().List(api.ListOptions{})
 	if err != nil {
@@ -175,7 +176,7 @@ func attemptToUpdateMasterRoleLabelsAndTaints(client *clientset.Clientset, sched
 	if _, err := client.Nodes().Update(n); err != nil {
 		if apierrs.IsConflict(err) {
 			fmt.Println("<master/apiclient> temporarily unable to update master node metadata due to conflict (will retry)")
-			time.Sleep(500 * time.Millisecond)
+			time.Sleep(apiCallRetryInterval)
 			attemptToUpdateMasterRoleLabelsAndTaints(client, schedulable)
 		} else {
 			return err

cmd/kubeadm/app/master/discovery.go

@@ -42,10 +42,6 @@ const (
 )
 
 func encodeKubeDiscoverySecretData(s *kubeadmapi.KubeadmConfig, caCert *x509.Certificate) map[string][]byte {
-	// TODO ListenIP is probably not the right now, although it's best we have right now
-	// if user provides a DNS name, or anything else, we should use that, may be it's really
-	// the list of all SANs (minus internal DNS names and service IP)?
-
 	var (
 		data         = map[string][]byte{}
 		endpointList = []string{}
@@ -75,7 +71,7 @@ func newKubeDiscoveryPodSpec(s *kubeadmapi.KubeadmConfig) api.PodSpec {
 		Containers: []api.Container{{
 			Name:    kubeDiscoveryName,
 			Image:   s.EnvParams["discovery_image"],
-			Command: []string{"/usr/bin/kube-discovery"},
+			Command: []string{"/usr/local/bin/kube-discovery"},
 			VolumeMounts: []api.VolumeMount{{
 				Name:      kubeDiscoverySecretName,
 				MountPath: "/tmp/secret", // TODO use a shared constant
@@ -124,9 +120,8 @@ func CreateDiscoveryDeploymentAndSecret(s *kubeadmapi.KubeadmConfig, client *cli
 
 	fmt.Println("<master/discovery> created essential addon: kube-discovery, waiting for it to become ready")
 
-	// wait for the pod to become ready
 	start := time.Now()
-	wait.PollInfinite(500*time.Millisecond, func() (bool, error) {
+	wait.PollInfinite(apiCallRetryInterval, func() (bool, error) {
 		d, err := client.Extensions().Deployments(api.NamespaceSystem).Get(kubeDiscoveryName)
 		if err != nil {
 			return false, nil

cmd/kubeadm/app/master/manifests.go

@@ -49,43 +49,18 @@ const (
 	kubeControllerManager = "kube-controller-manager"
 	kubeScheduler         = "kube-scheduler"
 	kubeProxy             = "kube-proxy"
+	pkiDir                = "/etc/kubernetes/pki"
 )
 
-// TODO look into what this really means, scheduler prints it for some reason
-//
-//E0817 17:53:22.242658       1 event.go:258] Could not construct reference to: '&api.Endpoints{TypeMeta:unversioned.TypeMeta{Kind:"", APIVersion:""}, ObjectMeta:api.ObjectMeta{Name:"kube-scheduler", GenerateName:"", Namespace:"kube-system", SelfLink:"", UID:"", ResourceVersion:"", Generation:0, CreationTimestamp:unversioned.Time{Time:time.Time{sec:0, nsec:0, loc:(*time.Location)(nil)}}, DeletionTimestamp:(*unversioned.Time)(nil), DeletionGracePeriodSeconds:(*int64)(nil), Labels:map[string]string(nil), Annotations:map[string]string(nil), OwnerReferences:[]api.OwnerReference(nil), Finalizers:[]string(nil)}, Subsets:[]api.EndpointSubset(nil)}' due to: 'selfLink was empty, can't make reference'. Will not report event: 'Normal' '%v became leader' 'moby'
-
 // WriteStaticPodManifests builds manifest objects based on user provided configuration and then dumps it to disk
 // where kubelet will pick and schedule them.
 func WriteStaticPodManifests(s *kubeadmapi.KubeadmConfig) error {
-	// Placeholder for kube-apiserver pod spec command
-	apiServerCommand := getComponentCommand(apiServer, s)
-
-	// Check if the user decided to use an external etcd cluster
-	if len(s.InitFlags.API.Etcd.ExternalEndpoints) > 0 {
-		arg := fmt.Sprintf("--etcd-servers=%s", strings.Join(s.InitFlags.API.Etcd.ExternalEndpoints, ","))
-		apiServerCommand = append(apiServerCommand, arg)
-	} else {
-		apiServerCommand = append(apiServerCommand, "--etcd-servers=http://127.0.0.1:2379")
-	}
-
-	// Is etcd secured?
-	if s.InitFlags.API.Etcd.ExternalCAFile != "" {
-		etcdCAFileArg := fmt.Sprintf("--etcd-cafile=%s", s.InitFlags.API.Etcd.ExternalCAFile)
-		apiServerCommand = append(apiServerCommand, etcdCAFileArg)
-	}
-	if s.InitFlags.API.Etcd.ExternalCertFile != "" && s.InitFlags.API.Etcd.ExternalKeyFile != "" {
-		etcdClientFileArg := fmt.Sprintf("--etcd-certfile=%s", s.InitFlags.API.Etcd.ExternalCertFile)
-		etcdKeyFileArg := fmt.Sprintf("--etcd-keyfile=%s", s.InitFlags.API.Etcd.ExternalKeyFile)
-		apiServerCommand = append(apiServerCommand, etcdClientFileArg, etcdKeyFileArg)
-	}
-
 	// Prepare static pod specs
 	staticPodSpecs := map[string]api.Pod{
 		kubeAPIServer: componentPod(api.Container{
 			Name:          kubeAPIServer,
 			Image:         images.GetCoreImage(images.KubeAPIServerImage, s.EnvParams["hyperkube_image"]),
-			Command:       apiServerCommand,
+			Command:       getComponentCommand(apiServer, s),
 			VolumeMounts:  []api.VolumeMount{certsVolumeMount(), k8sVolumeMount()},
 			LivenessProbe: componentProbe(8080, "/healthz"),
 			Resources:     componentResources("250m"),
@@ -163,7 +138,7 @@ func certsVolume(s *kubeadmapi.KubeadmConfig) api.Volume {
 	return api.Volume{
 		Name: "certs",
 		VolumeSource: api.VolumeSource{
-			// TODO make path configurable
+			// TODO(phase1+) make path configurable
 			HostPath: &api.HostPathVolumeSource{Path: "/etc/ssl/certs"},
 		},
 	}
@@ -235,9 +210,6 @@ func componentPod(container api.Container, volumes ...api.Volume) api.Pod {
 }
 
 func getComponentCommand(component string, s *kubeadmapi.KubeadmConfig) (command []string) {
-	// TODO: make a global constant of this
-	pkiDir := "/etc/kubernetes/pki"
-
 	baseFlags := map[string][]string{
 		apiServer: []string{
 			"--address=127.0.0.1",
@@ -253,7 +225,7 @@ func getComponentCommand(component string, s *kubeadmapi.KubeadmConfig) (command
 			"--allow-privileged",
 		},
 		controllerManager: []string{
-			// TODO: consider adding --address=127.0.0.1 in order to not expose the cm port to the rest of the world
+			// TODO(phase1+): consider adding --address=127.0.0.1 in order to not expose the cm port to the rest of the world
 			"--leader-elect",
 			"--master=127.0.0.1:8080",
 			"--cluster-name=" + DefaultClusterName,
@@ -264,7 +236,7 @@ func getComponentCommand(component string, s *kubeadmapi.KubeadmConfig) (command
 			"--insecure-experimental-approve-all-kubelet-csrs-for-group=system:kubelet-bootstrap",
 		},
 		scheduler: []string{
-			// TODO: consider adding --address=127.0.0.1 in order to not expose the scheduler port to the rest of the world
+			// TODO(phase1+): consider adding --address=127.0.0.1 in order to not expose the scheduler port to the rest of the world
 			"--leader-elect",
 			"--master=127.0.0.1:8080",
 		},
@@ -280,19 +252,39 @@ func getComponentCommand(component string, s *kubeadmapi.KubeadmConfig) (command
 	command = append(command, s.EnvParams["component_loglevel"])
 	command = append(command, baseFlags[component]...)
 
+	if component == apiServer {
+		// Check if the user decided to use an external etcd cluster
+		if len(s.InitFlags.API.Etcd.ExternalEndpoints) > 0 {
+			command = append(command, fmt.Sprintf("--etcd-servers=%s", strings.Join(s.InitFlags.API.Etcd.ExternalEndpoints, ",")))
+		} else {
+			command = append(command, "--etcd-servers=http://127.0.0.1:2379")
+		}
+
+		// Is etcd secured?
+		if s.InitFlags.API.Etcd.ExternalCAFile != "" {
+			command = append(command, fmt.Sprintf("--etcd-cafile=%s", s.InitFlags.API.Etcd.ExternalCAFile))
+		}
+		if s.InitFlags.API.Etcd.ExternalCertFile != "" && s.InitFlags.API.Etcd.ExternalKeyFile != "" {
+			etcdClientFileArg := fmt.Sprintf("--etcd-certfile=%s", s.InitFlags.API.Etcd.ExternalCertFile)
+			etcdKeyFileArg := fmt.Sprintf("--etcd-keyfile=%s", s.InitFlags.API.Etcd.ExternalKeyFile)
+			command = append(command, etcdClientFileArg, etcdKeyFileArg)
+		}
+	}
+
 	if component == controllerManager {
 		if s.InitFlags.CloudProvider != "" {
 			command = append(command, "--cloud-provider="+s.InitFlags.CloudProvider)
 
 			// Only append the --cloud-config option if there's a such file
+			// TODO(phase1+) this won't work unless it's in one of the few directories we bind-mount
 			if _, err := os.Stat(DefaultCloudConfigPath); err == nil {
 				command = append(command, "--cloud-config="+DefaultCloudConfigPath)
 			}
 		}
 
 		if s.InitFlags.PodNetwork.CIDR.IP != nil {
-			// Let the controller-manager allocate Node CIDRs for the Pod overlay network.
+			// Let the controller-manager allocate Node CIDRs for the Pod network.
-			// Each node will get a subspace of the address CIDR provided with --cluster-cidr.
+			// Each node will get a subspace of the address CIDR provided with --pod-network-cidr.
 			command = append(command, "--allocate-node-cidrs=true", "--cluster-cidr="+s.InitFlags.PodNetwork.CIDR.String())
 		}
 	}

cmd/kubeadm/app/master/pki.go

@@ -175,7 +175,7 @@ func CreatePKIAssets(s *kubeadmapi.KubeadmConfig) (*rsa.PrivateKey, *x509.Certif
 		return nil, nil, fmt.Errorf("<master/pki> failure while saving service account singing keys - %s", err)
 	}
 
-	// TODO(phase1+) print a summary of SANs used and checksums (signatures) of each of the certiicates
+	// TODO(phase1+) print a summary of SANs used and checksums (signatures) of each of the certificates
 	fmt.Printf("<master/pki> created keys and certificates in %q\n", pkiPath)
 	return caKey, caCert, nil
 }

cmd/kubeadm/app/node/csr.go

@@ -18,9 +18,7 @@ package node
 
 import (
 	"fmt"
-	"io/ioutil"
 	"os"
-	"strings"
 
 	kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/api"
 	kubeadmutil "k8s.io/kubernetes/cmd/kubeadm/app/util"
@@ -36,10 +34,9 @@ import (
 
 // PerformTLSBootstrap creates a RESTful client in order to execute certificate signing request.
 func PerformTLSBootstrap(s *kubeadmapi.KubeadmConfig, apiEndpoint string, caCert []byte) (*clientcmdapi.Config, error) {
-	// TODO try all the api servers until we find one that works
+	// TODO(phase1+) try all the api servers until we find one that works
 	bareClientConfig := kubeadmutil.CreateBasicClientConfig("kubernetes", apiEndpoint, caCert)
 
-	// Try to fetch the hostname of the node
 	nodeName, err := os.Hostname()
 	if err != nil {
 		return nil, fmt.Errorf("<node/csr> failed to get node hostname [%v]", err)
@@ -55,18 +52,20 @@ func PerformTLSBootstrap(s *kubeadmapi.KubeadmConfig, apiEndpoint string, caCert
 		return nil, fmt.Errorf("<node/csr> failed to create API client configuration [%s]", err)
 	}
 
-	err = checkCertsAPI(bootstrapClientConfig)
-
-	if err != nil {
-		return nil, fmt.Errorf("<node/csr> API compatibility error [%s]", err)
-	}
-
 	client, err := unversionedcertificates.NewForConfig(bootstrapClientConfig)
 	if err != nil {
 		return nil, fmt.Errorf("<node/csr> failed to create API client [%s]", err)
 	}
 	csrClient := client.CertificateSigningRequests()
 
+	// TODO(phase1+) checkCertsAPI() has a side-effect of making first attempt of communicating with the API,
+	// we should _make it more explicit_ and have a user-settable _retry timeout_ to account for potential connectivity issues
+	// (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)
+	}
+
 	fmt.Println("<node/csr> created API client to obtain unique certificate for this node, generating keys and certificate signing request")
 
 	key, err := certutil.MakeEllipticPrivateKeyPEM()
@@ -79,7 +78,7 @@ func PerformTLSBootstrap(s *kubeadmapi.KubeadmConfig, apiEndpoint string, caCert
 		return nil, fmt.Errorf("<node/csr> failed to request signed certificate from the API server [%s]", err)
 	}
 
-	// TODO print some basic info about the cert
+	// TODO(phase1+) print some basic info about the cert
 	fmt.Println("<node/csr> received signed certificate from the API server, generating kubelet configuration")
 
 	finalConfig := kubeadmutil.MakeClientConfigWithCerts(
@@ -110,14 +109,9 @@ func checkCertsAPI(config *restclient.Config) error {
 	}
 
 	version, err := discoveryClient.ServerVersion()
-	serverVersion := version.String()
-
 	if err != nil {
-		serverVersion = "N/A"
+		return fmt.Errorf("unable to obtain API version [%s]", err)
 	}
 
-	// Due to changes in API namespaces for certificates
+	return fmt.Errorf("API version %s does not support certificates API, use v1.4.0 or newer", version.String())
-	// https://github.com/kubernetes/kubernetes/pull/31887/
-	// it is compatible only with versions released after v1.4.0-beta.0
-	return fmt.Errorf("installed Kubernetes API server version \"%s\" does not support certificates signing request use v1.4.0 or newer", serverVersion)
 }

cmd/kubeadm/app/node/discovery.go

@@ -68,11 +68,10 @@ func RetrieveTrustedClusterInfo(s *kubeadmapi.KubeadmConfig) (*clientcmdapi.Conf
 		return nil, fmt.Errorf("<node/discovery> cluster info object is invalid - no endpoint(s) and/or root CA certificate(s) found")
 	}
 
-	// TODO print checksum of the CA certificate
+	// TODO(phase1+) print summary info about the CA certificate, along with the the checksum signature
+	// we also need an ability for the user to configure the client to validate recieved CA cert agains a checksum
 	fmt.Printf("<node/discovery> cluster info signature and contents are valid, will use API endpoints %v\n", clusterInfo.Endpoints)
 
-	// TODO we need to configure the client to validate the server
-	// if it is signed by any of the returned certificates
 	apiServer := clusterInfo.Endpoints[0]
 	caCert := []byte(clusterInfo.CertificateAuthorities[0])
 

cmd/kubeadm/app/util/kubeconfig.go

@@ -75,12 +75,6 @@ func MakeClientConfigWithToken(config *clientcmdapi.Config, clusterName string,
 	return newConfig
 }
 
-// kubeadm is responsible for writing the following kubeconfig file, which
-// kubelet should be waiting for. Help user avoid foot-shooting by refusing to
-// write a file that has already been written (the kubelet will be up and
-// running in that case - they'd need to stop the kubelet, remove the file, and
-// start it again in that case).
-
 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)
@@ -88,11 +82,7 @@ func WriteKubeconfigIfNotExists(s *kubeadmapi.KubeadmConfig, name string, kubeco
 
 	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(
+	f, err := os.OpenFile(filename, os.O_CREATE|os.O_WRONLY|os.O_EXCL, 0600)
-		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)
 	}

cmd/kubeadm/app/util/tokens.go

@@ -66,8 +66,8 @@ func UseGivenTokenIfValid(s *kubeadmapi.KubeadmConfig) (bool, error) {
 	}
 	fmt.Println("<util/tokens> validating provided token")
 	givenToken := strings.Split(strings.ToLower(s.Secrets.GivenToken), ".")
-	// TODO print desired format
+	// TODO(phase1+) print desired format
-	// TODO could also print more specific messages in each case
+	// TODO(phase1+) could also print more specific messages in each case
 	invalidErr := "<util/tokens> provided token is invalid - %s"
 	if len(givenToken) != 2 {
 		return false, fmt.Errorf(invalidErr, "not in 2-part dot-separated format")

cmd/kubeadm/kubeadm.go

@@ -17,14 +17,16 @@ limitations under the License.
 package main
 
 import (
+	"fmt"
 	"os"
 
 	"k8s.io/kubernetes/cmd/kubeadm/app"
 )
 
-// TODO(phase1): check for root
+// TODO(phase1+): check for root
 func main() {
 	if err := app.Run(); err != nil {
+		fmt.Printf(app.AlphaWarningOnExit)
 		os.Exit(1)
 	}
 	os.Exit(0)