Merge pull request #50692 from fabriziopandini/kubeadm-phases-small-cleanups

Automatic merge from submit-queue (batch tested with PRs 50692, 50727)

kubeadm: Small cleanups from the phases refactoring

**What this PR does / why we need it**:
Small cleanups on kubeadm phases

**Which issue this PR fixes**: 
fixes pending comments in [#49419](https://github.com/kubernetes/kubernetes/pull/49419)
fixes [#376](https://github.com/kubernetes/kubeadm/issues/376)

**Special notes for your reviewer**:
cc @luxas

cmd/kubeadm/app/cmd/phases/controlplane.go

@@ -85,17 +85,17 @@ func getControlPlaneSubCommands(outDir string) []*cobra.Command {
 		}
 
 		// Add flags to the command
-		cmd.Flags().StringVar(&cfg.CertificatesDir, "cert-dir", cfg.CertificatesDir, `The path where certificates are stored`)
+		cmd.Flags().StringVar(&cfg.CertificatesDir, "cert-dir", cfg.CertificatesDir, `The path where certificates are stored.`)
-		cmd.Flags().StringVar(&cfg.KubernetesVersion, "kubernetes-version", cfg.KubernetesVersion, `Choose a specific Kubernetes version for the control plane`)
+		cmd.Flags().StringVar(&cfg.KubernetesVersion, "kubernetes-version", cfg.KubernetesVersion, `Choose a specific Kubernetes version for the control plane.`)
 
 		if properties.use == "all" || properties.use == "apiserver" {
-			cmd.Flags().StringVar(&cfg.API.AdvertiseAddress, "apiserver-advertise-address", cfg.API.AdvertiseAddress, "The IP address the API Server will advertise it's listening on. 0.0.0.0 means the default network interface's address.")
+			cmd.Flags().StringVar(&cfg.API.AdvertiseAddress, "apiserver-advertise-address", cfg.API.AdvertiseAddress, "The IP address or DNS name the API Server is accessible on.")
-			cmd.Flags().Int32Var(&cfg.API.BindPort, "apiserver-bind-port", cfg.API.BindPort, "Port for the API Server to bind to")
+			cmd.Flags().Int32Var(&cfg.API.BindPort, "apiserver-bind-port", cfg.API.BindPort, "The port the API Server is accessible on.")
-			cmd.Flags().StringVar(&cfg.Networking.ServiceSubnet, "service-cidr", cfg.Networking.ServiceSubnet, "Use alternative range of IP address for service VIPs")
+			cmd.Flags().StringVar(&cfg.Networking.ServiceSubnet, "service-cidr", cfg.Networking.ServiceSubnet, "The range of IP address used for service VIPs.")
 		}
 
 		if properties.use == "all" || properties.use == "controller-manager" {
-			cmd.Flags().StringVar(&cfg.Networking.PodSubnet, "pod-network-cidr", cfg.Networking.PodSubnet, "Specify range of IP addresses for the pod network; if set, the control plane will automatically allocate CIDRs for every node")
+			cmd.Flags().StringVar(&cfg.Networking.PodSubnet, "pod-network-cidr", cfg.Networking.PodSubnet, "The range of IP addresses used for the pod network.")
 		}
 
 		cmd.Flags().StringVar(&cfgPath, "config", cfgPath, "Path to kubeadm config file (WARNING: Usage of a configuration file is experimental)")

cmd/kubeadm/app/cmd/phases/kubeconfig.go

@@ -26,8 +26,6 @@ import (
 	kubeadmapiext "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1alpha1"
 	kubeadmconstants "k8s.io/kubernetes/cmd/kubeadm/app/constants"
 	kubeconfigphase "k8s.io/kubernetes/cmd/kubeadm/app/phases/kubeconfig"
-	kubeadmutil "k8s.io/kubernetes/cmd/kubeadm/app/util"
-	configutil "k8s.io/kubernetes/cmd/kubeadm/app/util/config"
 	"k8s.io/kubernetes/pkg/api"
 )
 
@@ -107,22 +105,22 @@ func getKubeConfigSubCommands(out io.Writer, outDir string) []*cobra.Command {
 		cmd := &cobra.Command{
 			Use:   properties.use,
 			Short: properties.short,
-			Run:   runCmdFuncKubeConfig(properties.cmdFunc, &outDir, &cfgPath, cfg),
+			Run:   runCmdPhase(properties.cmdFunc, &outDir, &cfgPath, cfg),
 		}
 
 		// Add flags to the command
 		if properties.use != "user" {
 			cmd.Flags().StringVar(&cfgPath, "config", cfgPath, "Path to kubeadm config file (WARNING: Usage of a configuration file is experimental)")
 		}
-		cmd.Flags().StringVar(&cfg.CertificatesDir, "cert-dir", cfg.CertificatesDir, "The path where to save and store the certificates")
+		cmd.Flags().StringVar(&cfg.CertificatesDir, "cert-dir", cfg.CertificatesDir, "The path where certificates are stored.")
-		cmd.Flags().StringVar(&cfg.API.AdvertiseAddress, "apiserver-advertise-address", cfg.API.AdvertiseAddress, "The IP address the API Server will advertise it's listening on. 0.0.0.0 means the default network interface's address.")
+		cmd.Flags().StringVar(&cfg.API.AdvertiseAddress, "apiserver-advertise-address", cfg.API.AdvertiseAddress, "The IP address or DNS name the API Server is accessible on.")
-		cmd.Flags().Int32Var(&cfg.API.BindPort, "apiserver-bind-port", cfg.API.BindPort, "Port for the API Server to bind to")
+		cmd.Flags().Int32Var(&cfg.API.BindPort, "apiserver-bind-port", cfg.API.BindPort, "The port the API Server is accessible on.")
 		if properties.use == "all" || properties.use == "kubelet" {
-			cmd.Flags().StringVar(&cfg.NodeName, "node-name", cfg.NodeName, `Specify the node name`)
+			cmd.Flags().StringVar(&cfg.NodeName, "node-name", cfg.NodeName, `The node name that the kubelet client cert should use.`)
 		}
 		if properties.use == "user" {
-			cmd.Flags().StringVar(&token, "token", token, "The path to the directory where the certificates are.")
+			cmd.Flags().StringVar(&token, "token", token, "The token that should be used as the authentication mechanism for this kubeconfig.")
-			cmd.Flags().StringVar(&clientName, "client-name", clientName, "The name of the client for which the KubeConfig file will be generated.")
+			cmd.Flags().StringVar(&clientName, "client-name", clientName, "The name of the KubeConfig user that will be created. Will also be used as the CN if client certs are created.")
 		}
 
 		subCmds = append(subCmds, cmd)
@@ -130,23 +128,3 @@ func getKubeConfigSubCommands(out io.Writer, outDir string) []*cobra.Command {
 
 	return subCmds
 }
-
-// runCmdFuncKubeConfig creates a cobra.Command Run function, by composing the call to the given cmdFunc with necessary additional steps (e.g preparation of input parameters)
-func runCmdFuncKubeConfig(cmdFunc func(outDir string, cfg *kubeadmapi.MasterConfiguration) error, outDir, cfgPath *string, cfg *kubeadmapiext.MasterConfiguration) func(cmd *cobra.Command, args []string) {
-
-	// the following statement build a clousure that wraps a call to a CreateKubeConfigFunc, binding
-	// the function itself with the specific parameters of each sub command.
-	// Please note that specific parameter should be passed as value, while other parameters - passed as reference -
-	// are shared between sub commands and gets access to current value e.g. flags value.
-
-	return func(cmd *cobra.Command, args []string) {
-
-		// This call returns the ready-to-use configuration based on the configuration file that might or might not exist and the default cfg populated by flags
-		internalcfg, err := configutil.ConfigFileAndDefaultsToInternalConfig(*cfgPath, cfg)
-		kubeadmutil.CheckErr(err)
-
-		// Execute the cmdFunc
-		err = cmdFunc(*outDir, internalcfg)
-		kubeadmutil.CheckErr(err)
-	}
-}

cmd/kubeadm/app/cmd/phases/markmaster.go

@@ -31,7 +31,7 @@ func NewCmdMarkMaster() *cobra.Command {
 	var kubeConfigFile string
 	cmd := &cobra.Command{
 		Use:     "mark-master <node-name>",
-		Short:   "Create KubeConfig files from given credentials.",
+		Short:   "Mark a node as master.",
 		Aliases: []string{"markmaster"},
 		RunE: func(_ *cobra.Command, args []string) error {
 			err := validateExactArgNumber(args, []string{"node-name"})

cmd/kubeadm/app/phases/kubeconfig/kubeconfig.go

@@ -37,7 +37,7 @@ import (
 
 // clientCertAuth struct holds info required to build a client certificate to provide authentication info in a kubeconfig object
 type clientCertAuth struct {
-	CaKey         *rsa.PrivateKey
+	CAKey         *rsa.PrivateKey
 	Organizations []string
 }
 
@@ -48,7 +48,7 @@ type tokenAuth struct {
 
 // kubeConfigSpec struct holds info required to build a KubeConfig object
 type kubeConfigSpec struct {
-	CaCert         *x509.Certificate
+	CACert         *x509.Certificate
 	APIServer      string
 	ClientName     string
 	TokenAuth      *tokenAuth
@@ -117,8 +117,7 @@ func createKubeConfigFiles(outDir string, cfg *kubeadmapi.MasterConfiguration, k
 		}
 
 		// writes the KubeConfig to disk if it not exists
-		err = createKubeConfigFileIfNotExists(outDir, kubeConfigFileName, config)
+		if err = createKubeConfigFileIfNotExists(outDir, kubeConfigFileName, config); err != nil {
-		if err != nil {
 			return err
 		}
 	}
@@ -137,37 +136,37 @@ func getKubeConfigSpecs(cfg *kubeadmapi.MasterConfiguration) (map[string]*kubeCo
 
 	var kubeConfigSpec = map[string]*kubeConfigSpec{
 		kubeadmconstants.AdminKubeConfigFileName: {
-			CaCert:     caCert,
+			CACert:     caCert,
 			APIServer:  cfg.GetMasterEndpoint(),
 			ClientName: "kubernetes-admin",
 			ClientCertAuth: &clientCertAuth{
-				CaKey:         caKey,
+				CAKey:         caKey,
 				Organizations: []string{kubeadmconstants.MastersGroup},
 			},
 		},
 		kubeadmconstants.KubeletKubeConfigFileName: {
-			CaCert:     caCert,
+			CACert:     caCert,
 			APIServer:  cfg.GetMasterEndpoint(),
 			ClientName: fmt.Sprintf("system:node:%s", cfg.NodeName),
 			ClientCertAuth: &clientCertAuth{
-				CaKey:         caKey,
+				CAKey:         caKey,
 				Organizations: []string{kubeadmconstants.NodesGroup},
 			},
 		},
 		kubeadmconstants.ControllerManagerKubeConfigFileName: {
-			CaCert:     caCert,
+			CACert:     caCert,
 			APIServer:  cfg.GetMasterEndpoint(),
 			ClientName: kubeadmconstants.ControllerManagerUser,
 			ClientCertAuth: &clientCertAuth{
-				CaKey: caKey,
+				CAKey: caKey,
 			},
 		},
 		kubeadmconstants.SchedulerKubeConfigFileName: {
-			CaCert:     caCert,
+			CACert:     caCert,
 			APIServer:  cfg.GetMasterEndpoint(),
 			ClientName: kubeadmconstants.SchedulerUser,
 			ClientCertAuth: &clientCertAuth{
-				CaKey: caKey,
+				CAKey: caKey,
 			},
 		},
 	}
@@ -178,14 +177,14 @@ func getKubeConfigSpecs(cfg *kubeadmapi.MasterConfiguration) (map[string]*kubeCo
 // buildKubeConfigFromSpec creates a kubeconfig object for the given kubeConfigSpec
 func buildKubeConfigFromSpec(spec *kubeConfigSpec) (*clientcmdapi.Config, error) {
 
-	// If this kubeconfing should use token
+	// If this kubeconfig should use token
 	if spec.TokenAuth != nil {
 		// create a kubeconfig with a token
 		return kubeconfigutil.CreateWithToken(
 			spec.APIServer,
 			"kubernetes",
 			spec.ClientName,
-			certutil.EncodeCertPEM(spec.CaCert),
+			certutil.EncodeCertPEM(spec.CACert),
 			spec.TokenAuth.Token,
 		), nil
 	}
@@ -196,7 +195,7 @@ func buildKubeConfigFromSpec(spec *kubeConfigSpec) (*clientcmdapi.Config, error)
 		Organization: spec.ClientCertAuth.Organizations,
 		Usages:       []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth},
 	}
-	clientCert, clientKey, err := pkiutil.NewCertAndKey(spec.CaCert, spec.ClientCertAuth.CaKey, clientCertConfig)
+	clientCert, clientKey, err := pkiutil.NewCertAndKey(spec.CACert, spec.ClientCertAuth.CAKey, clientCertConfig)
 	if err != nil {
 		return nil, fmt.Errorf("failure while creating %s client certificate: %v", spec.ClientName, err)
 	}
@@ -206,7 +205,7 @@ func buildKubeConfigFromSpec(spec *kubeConfigSpec) (*clientcmdapi.Config, error)
 		spec.APIServer,
 		"kubernetes",
 		spec.ClientName,
-		certutil.EncodeCertPEM(spec.CaCert),
+		certutil.EncodeCertPEM(spec.CACert),
 		certutil.EncodePrivateKeyPEM(clientKey),
 		certutil.EncodeCertPEM(clientCert),
 	), nil
@@ -270,9 +269,9 @@ func WriteKubeConfigWithClientCert(out io.Writer, cfg *kubeadmapi.MasterConfigur
 	spec := &kubeConfigSpec{
 		ClientName: clientName,
 		APIServer:  cfg.GetMasterEndpoint(),
-		CaCert:     caCert,
+		CACert:     caCert,
 		ClientCertAuth: &clientCertAuth{
-			CaKey: caKey,
+			CAKey: caKey,
 		},
 	}
 
@@ -291,7 +290,7 @@ func WriteKubeConfigWithToken(out io.Writer, cfg *kubeadmapi.MasterConfiguration
 	spec := &kubeConfigSpec{
 		ClientName: clientName,
 		APIServer:  cfg.GetMasterEndpoint(),
-		CaCert:     caCert,
+		CACert:     caCert,
 		TokenAuth: &tokenAuth{
 			Token: token,
 		},

cmd/kubeadm/app/phases/kubeconfig/kubeconfig_test.go

@@ -122,11 +122,11 @@ func TestGetKubeConfigSpecs(t *testing.T) {
 			}
 
 			// Asserts CA certs and CA keys loaded into specs
-			if spec.CaCert == nil {
+			if spec.CACert == nil {
-				t.Errorf("getKubeConfigSpecs didn't loaded CaCert into spec for %s!", assertion.kubeConfigFile)
+				t.Errorf("getKubeConfigSpecs didn't loaded CACert into spec for %s!", assertion.kubeConfigFile)
 			}
-			if spec.ClientCertAuth == nil || spec.ClientCertAuth.CaKey == nil {
+			if spec.ClientCertAuth == nil || spec.ClientCertAuth.CAKey == nil {
-				t.Errorf("getKubeConfigSpecs didn't loaded CaKey into spec for %s!", assertion.kubeConfigFile)
+				t.Errorf("getKubeConfigSpecs didn't loaded CAKey into spec for %s!", assertion.kubeConfigFile)
 			}
 		} else {
 			t.Errorf("getKubeConfigSpecs didn't create spec for %s ", assertion.kubeConfigFile)
@@ -398,11 +398,11 @@ func TestWriteKubeConfig(t *testing.T) {
 // setupdKubeConfigWithClientAuth is a test utility function that wraps buildKubeConfigFromSpec for building a KubeConfig object With ClientAuth
 func setupdKubeConfigWithClientAuth(t *testing.T, caCert *x509.Certificate, caKey *rsa.PrivateKey, APIServer, clientName string, organizations ...string) *clientcmdapi.Config {
 	spec := &kubeConfigSpec{
-		CaCert:     caCert,
+		CACert:     caCert,
 		APIServer:  APIServer,
 		ClientName: clientName,
 		ClientCertAuth: &clientCertAuth{
-			CaKey:         caKey,
+			CAKey:         caKey,
 			Organizations: organizations,
 		},
 	}
@@ -418,7 +418,7 @@ func setupdKubeConfigWithClientAuth(t *testing.T, caCert *x509.Certificate, caKe
 // setupdKubeConfigWithClientAuth is a test utility function that wraps buildKubeConfigFromSpec for building a KubeConfig object With Token
 func setupdKubeConfigWithTokenAuth(t *testing.T, caCert *x509.Certificate, APIServer, clientName, token string) *clientcmdapi.Config {
 	spec := &kubeConfigSpec{
-		CaCert:     caCert,
+		CACert:     caCert,
 		APIServer:  APIServer,
 		ClientName: clientName,
 		TokenAuth: &tokenAuth{