Merge pull request #74831 from fabriziopandini/remove-discovery-flags

Remove discovery flags from kubeadm join phases (when possible)

cmd/kubeadm/app/cmd/BUILD

@@ -58,6 +58,7 @@ go_library(
         "//staging/src/k8s.io/apimachinery/pkg/util/sets:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/version:go_default_library",
         "//staging/src/k8s.io/client-go/kubernetes:go_default_library",
+        "//staging/src/k8s.io/client-go/tools/clientcmd:go_default_library",
         "//staging/src/k8s.io/client-go/tools/clientcmd/api:go_default_library",
         "//staging/src/k8s.io/cluster-bootstrap/token/api:go_default_library",
         "//staging/src/k8s.io/cluster-bootstrap/token/util:go_default_library",

cmd/kubeadm/app/cmd/join.go

@@ -28,6 +28,7 @@ import (
 	flag "github.com/spf13/pflag"
 	"k8s.io/apimachinery/pkg/util/sets"
 	clientset "k8s.io/client-go/kubernetes"
+	clientcmd "k8s.io/client-go/tools/clientcmd"
 	clientcmdapi "k8s.io/client-go/tools/clientcmd/api"
 	"k8s.io/klog"
 	kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
@@ -308,44 +309,57 @@ func newJoinOptions() *joinOptions {
 // newJoinData returns a new joinData struct to be used for the execution of the kubeadm join workflow.
 // This func takes care of validating joinOptions passed to the command, and then it converts
 // options into the internal JoinConfiguration type that is used as input all the phases in the kubeadm join workflow
-func newJoinData(cmd *cobra.Command, args []string, options *joinOptions, out io.Writer) (*joinData, error) {
+func newJoinData(cmd *cobra.Command, args []string, opt *joinOptions, out io.Writer) (*joinData, error) {
 	// Re-apply defaults to the public kubeadm API (this will set only values not exposed/not set as a flags)
-	kubeadmscheme.Scheme.Default(options.externalcfg)
+	kubeadmscheme.Scheme.Default(opt.externalcfg)
 
 	// Validate standalone flags values and/or combination of flags and then assigns
 	// validated values to the public kubeadm config API when applicable
 
 	// if a token is provided, use this value for both discovery-token and tls-bootstrap-token when those values are not provided
-	if len(options.token) > 0 {
+	if len(opt.token) > 0 {
-		if len(options.externalcfg.Discovery.TLSBootstrapToken) == 0 {
+		if len(opt.externalcfg.Discovery.TLSBootstrapToken) == 0 {
-			options.externalcfg.Discovery.TLSBootstrapToken = options.token
+			opt.externalcfg.Discovery.TLSBootstrapToken = opt.token
 		}
-		if len(options.externalcfg.Discovery.BootstrapToken.Token) == 0 {
+		if len(opt.externalcfg.Discovery.BootstrapToken.Token) == 0 {
-			options.externalcfg.Discovery.BootstrapToken.Token = options.token
+			opt.externalcfg.Discovery.BootstrapToken.Token = opt.token
 		}
 	}
 
 	// if a file or URL from which to load cluster information was not provided, unset the Discovery.File object
-	if len(options.externalcfg.Discovery.File.KubeConfigPath) == 0 {
+	if len(opt.externalcfg.Discovery.File.KubeConfigPath) == 0 {
-		options.externalcfg.Discovery.File = nil
+		opt.externalcfg.Discovery.File = nil
 	}
 
 	// if an APIServerEndpoint from which to retrive cluster information was not provided, unset the Discovery.BootstrapToken object
 	if len(args) == 0 {
-		options.externalcfg.Discovery.BootstrapToken = nil
+		opt.externalcfg.Discovery.BootstrapToken = nil
 	} else {
-		if len(options.cfgPath) == 0 && len(args) > 1 {
+		if len(opt.cfgPath) == 0 && len(args) > 1 {
 			klog.Warningf("[join] WARNING: More than one API server endpoint supplied on command line %v. Using the first one.", args)
 		}
-		options.externalcfg.Discovery.BootstrapToken.APIServerEndpoint = args[0]
+		opt.externalcfg.Discovery.BootstrapToken.APIServerEndpoint = args[0]
 	}
 
 	// if not joining a control plane, unset the ControlPlane object
-	if !options.controlPlane {
+	if !opt.controlPlane {
-		options.externalcfg.ControlPlane = nil
+		opt.externalcfg.ControlPlane = nil
 	}
 
-	ignorePreflightErrorsSet, err := validation.ValidateIgnorePreflightErrors(options.ignorePreflightErrors)
+	// if the admin.conf file already exists, use it for skipping the discovery process.
+	// NB. this case can happen when we are joining a control-plane node only (and phases are invoked atomically)
+	var adminKubeConfigPath = kubeadmconstants.GetAdminKubeConfigPath()
+	var tlsBootstrapCfg *clientcmdapi.Config
+	if _, err := os.Stat(adminKubeConfigPath); err == nil && opt.controlPlane {
+		// use the admin.conf as tlsBootstrapCfg, that is the kubeconfig file used for reading the kubeadm-config during discovery
+		klog.V(1).Infof("[join] found %s. Use it for skipping discovery", adminKubeConfigPath)
+		tlsBootstrapCfg, err = clientcmd.LoadFromFile(adminKubeConfigPath)
+		if err != nil {
+			return nil, errors.Wrapf(err, "Error loading %s", adminKubeConfigPath)
+		}
+	}
+
+	ignorePreflightErrorsSet, err := validation.ValidateIgnorePreflightErrors(opt.ignorePreflightErrors)
 	if err != nil {
 		return nil, err
 	}
@@ -356,25 +370,35 @@ func newJoinData(cmd *cobra.Command, args []string, options *joinOptions, out io
 
 	// Either use the config file if specified, or convert public kubeadm API to the internal JoinConfiguration
 	// and validates JoinConfiguration
-	if options.externalcfg.NodeRegistration.Name == "" {
+	if opt.externalcfg.NodeRegistration.Name == "" {
 		klog.V(1).Infoln("[join] found NodeName empty; using OS hostname as NodeName")
 	}
 
-	if options.externalcfg.ControlPlane != nil && options.externalcfg.ControlPlane.LocalAPIEndpoint.AdvertiseAddress == "" {
+	if opt.externalcfg.ControlPlane != nil && opt.externalcfg.ControlPlane.LocalAPIEndpoint.AdvertiseAddress == "" {
 		klog.V(1).Infoln("[join] found advertiseAddress empty; using default interface's IP address as advertiseAddress")
 	}
 
-	cfg, err := configutil.LoadOrDefaultJoinConfiguration(options.cfgPath, options.externalcfg)
+	// in case the command doesn't have flags for discovery, makes the join cfg validation pass checks on discovery
+	if cmd.Flags().Lookup(options.FileDiscovery) == nil {
+		if _, err := os.Stat(adminKubeConfigPath); os.IsNotExist(err) {
+			return nil, errors.Errorf("File %s does not exists. Please use 'kubeadm join phase control-plane-prepare' subcommands to generate it.", adminKubeConfigPath)
+		}
+		klog.V(1).Infof("[join] found discovery flags missing for this command. using FileDiscovery: %s", adminKubeConfigPath)
+		opt.externalcfg.Discovery.File = &kubeadmapiv1beta1.FileDiscovery{KubeConfigPath: adminKubeConfigPath}
+		opt.externalcfg.Discovery.BootstrapToken = nil //NB. this could be removed when we get better control on args (e.g. phases without discovery should have NoArgs )
+	}
+
+	cfg, err := configutil.LoadOrDefaultJoinConfiguration(opt.cfgPath, opt.externalcfg)
 	if err != nil {
 		return nil, err
 	}
 
-	// override node name and CRI socket from the command line options
+	// override node name and CRI socket from the command line opt
-	if options.externalcfg.NodeRegistration.Name != "" {
+	if opt.externalcfg.NodeRegistration.Name != "" {
-		cfg.NodeRegistration.Name = options.externalcfg.NodeRegistration.Name
+		cfg.NodeRegistration.Name = opt.externalcfg.NodeRegistration.Name
 	}
-	if options.externalcfg.NodeRegistration.CRISocket != "" {
+	if opt.externalcfg.NodeRegistration.CRISocket != "" {
-		cfg.NodeRegistration.CRISocket = options.externalcfg.NodeRegistration.CRISocket
+		cfg.NodeRegistration.CRISocket = opt.externalcfg.NodeRegistration.CRISocket
 	}
 
 	if cfg.ControlPlane != nil {
@@ -385,9 +409,10 @@ func newJoinData(cmd *cobra.Command, args []string, options *joinOptions, out io
 
 	return &joinData{
 		cfg:                   cfg,
+		tlsBootstrapCfg:       tlsBootstrapCfg,
 		ignorePreflightErrors: ignorePreflightErrorsSet,
 		outputWriter:          out,
-		certificateKey:        options.certificateKey,
+		certificateKey:        opt.certificateKey,
 	}, nil
 }
 

cmd/kubeadm/app/cmd/phases/join/controlplanejoin.go

@@ -44,12 +44,6 @@ func getControlPlaneJoinPhaseFlags() []string {
 		options.CfgPath,
 		options.ControlPlane,
 		options.NodeName,
-		options.FileDiscovery,
-		options.TokenDiscovery,
-		options.TokenDiscoveryCAHash,
-		options.TokenDiscoverySkipCAHash,
-		options.TLSBootstrapToken,
-		options.TokenStr,
 	}
 }
 

cmd/kubeadm/app/cmd/phases/join/controlplaneprepare.go

@@ -43,9 +43,7 @@ func NewControlPlanePreparePhase() workflow.Phase {
 			{
 				Name:           "all",
 				Short:          "Prepares the machine for serving a control plane.",
-				InheritFlags: append(getControlPlanePreparePhaseFlags(),
+				InheritFlags:   getControlPlanePreparePhaseFlags("all"),
-					options.CertificateKey,
-				),
 				RunAllSiblings: true,
 			},
 			newControlPlanePrepareDownloadCertsSubphase(),
@@ -56,20 +54,30 @@ func NewControlPlanePreparePhase() workflow.Phase {
 	}
 }
 
-func getControlPlanePreparePhaseFlags() []string {
+func getControlPlanePreparePhaseFlags(name string) []string {
-	return []string{
+	flags := []string{
 		options.APIServerAdvertiseAddress,
 		options.APIServerBindPort,
 		options.CfgPath,
 		options.ControlPlane,
 		options.NodeName,
+	}
+	if name != "manifests" {
+		flags = append(flags,
 			options.FileDiscovery,
 			options.TokenDiscovery,
 			options.TokenDiscoveryCAHash,
 			options.TokenDiscoverySkipCAHash,
 			options.TLSBootstrapToken,
 			options.TokenStr,
+		)
 	}
+	if name == "all" || name == "download-certs" {
+		flags = append(flags,
+			options.CertificateKey,
+		)
+	}
+	return flags
 }
 
 func newControlPlanePrepareDownloadCertsSubphase() workflow.Phase {
@@ -78,9 +86,7 @@ func newControlPlanePrepareDownloadCertsSubphase() workflow.Phase {
 		Short:        fmt.Sprintf("Download certificates from %s", kubeadmconstants.KubeadmCertsSecret),
 		Long:         cmdutil.MacroCommandLongDescription,
 		Run:          runControlPlanePrepareDownloadCertsPhaseLocal,
-		InheritFlags: append(getControlPlanePreparePhaseFlags(),
+		InheritFlags: getControlPlanePreparePhaseFlags("download-certs"),
-			options.CertificateKey,
-		),
 	}
 }
 
@@ -88,8 +94,8 @@ func newControlPlanePrepareCertsSubphase() workflow.Phase {
 	return workflow.Phase{
 		Name:         "certs",
 		Short:        "Generates the certificates for the new control plane components",
-		Run:          runControlPlanePrepareCertsPhaseLocal,
+		Run:          runControlPlanePrepareCertsPhaseLocal, //NB. eventually in future we would like to break down this in sub phases for each cert or add the --csr option
-		InheritFlags: getControlPlanePreparePhaseFlags(), //NB. eventually in future we would like to break down this in sub phases for each cert or add the --csr option
+		InheritFlags: getControlPlanePreparePhaseFlags("certs"),
 	}
 }
 
@@ -97,8 +103,8 @@ func newControlPlanePrepareKubeconfigSubphase() workflow.Phase {
 	return workflow.Phase{
 		Name:         "kubeconfig",
 		Short:        "Generates the kubeconfig for the new control plane components",
-		Run:          runControlPlanePrepareKubeconfigPhaseLocal,
+		Run:          runControlPlanePrepareKubeconfigPhaseLocal, //NB. eventually in future we would like to break down this in sub phases for each kubeconfig
-		InheritFlags: getControlPlanePreparePhaseFlags(), //NB. eventually in future we would like to break down this in sub phases for each kubeconfig
+		InheritFlags: getControlPlanePreparePhaseFlags("kubeconfig"),
 	}
 }
 
@@ -106,8 +112,8 @@ func newControlPlanePrepareManifestsSubphases() workflow.Phase {
 	return workflow.Phase{
 		Name:         "manifests",
 		Short:        "Generates the manifests for the new control plane components",
-		Run:          runControlPlanePrepareManifestsSubphase,
+		Run:          runControlPlanePrepareManifestsSubphase, //NB. eventually in future we would like to break down this in sub phases for each component
-		InheritFlags: getControlPlanePreparePhaseFlags(), //NB. eventually in future we would like to break down this in sub phases for each component
+		InheritFlags: getControlPlanePreparePhaseFlags("manifests"),
 	}
 }