Merge pull request kubernetes#75847 from fabriziopandini/fix-external-etcd

kubeadm: fix join control-plane with external-etcd

Author: k8s-ci-robot

cmd/kubeadm/app/cmd/phases/init/preflight.go

@@ -57,7 +57,7 @@ func runPreflight(c workflow.RunData) error {
 	}
 
 	fmt.Println("[preflight] Running pre-flight checks")
-	if err := preflight.RunInitNodeChecks(utilsexec.New(), data.Cfg(), data.IgnorePreflightErrors(), false); err != nil {
+	if err := preflight.RunInitNodeChecks(utilsexec.New(), data.Cfg(), data.IgnorePreflightErrors(), false, false); err != nil {
 		return err
 	}
 

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

@@ -120,7 +120,8 @@ func runPreflight(c workflow.RunData) error {
 
 		// run kubeadm init preflight checks for checking all the prerequisites
 		fmt.Println("[preflight] Running pre-flight checks before initializing the new control plane instance")
-		if err := preflight.RunInitNodeChecks(utilsexec.New(), initCfg, j.IgnorePreflightErrors(), true); err != nil {
+
+		if err := preflight.RunInitNodeChecks(utilsexec.New(), initCfg, j.IgnorePreflightErrors(), true, hasCertificateKey); err != nil {
 			return err
 		}
 

cmd/kubeadm/app/preflight/checks.go

@@ -884,8 +884,9 @@ func (r IPVSProxierCheck) Name() string {
 
 // RunInitNodeChecks executes all individual, applicable to control-plane node checks.
 // The boolean flag 'isSecondaryControlPlane' controls whether we are running checks in a --join-control-plane scenario.
+// The boolean flag 'downloadCerts' controls whether we should skip checks on certificates because we are downloading them.
 // If the flag is set to true we should skip checks already executed by RunJoinNodeChecks and RunOptionalJoinNodeChecks.
-func RunInitNodeChecks(execer utilsexec.Interface, cfg *kubeadmapi.InitConfiguration, ignorePreflightErrors sets.String, isSecondaryControlPlane bool) error {
+func RunInitNodeChecks(execer utilsexec.Interface, cfg *kubeadmapi.InitConfiguration, ignorePreflightErrors sets.String, isSecondaryControlPlane bool, downloadCerts bool) error {
 	if !isSecondaryControlPlane {
 		// First, check if we're root separately from the other preflight checks and fail fast
 		if err := RunRootCheckOnly(ignorePreflightErrors); err != nil {
@@ -927,19 +928,25 @@ func RunInitNodeChecks(execer utilsexec.Interface, cfg *kubeadmapi.InitConfigura
 				)
 			}
 		}
+
+		// if using an external etcd
+		if cfg.Etcd.External != nil {
+			// Check external etcd version before creating the cluster
+			checks = append(checks, ExternalEtcdVersionCheck{Etcd: cfg.Etcd})
+		}
 	}
 
 	if cfg.Etcd.Local != nil {
-		// Only do etcd related checks when no external endpoints were specified
+		// Only do etcd related checks when required to install a local etcd
 		checks = append(checks,
 			PortOpenCheck{port: kubeadmconstants.EtcdListenClientPort},
 			PortOpenCheck{port: kubeadmconstants.EtcdListenPeerPort},
 			DirAvailableCheck{Path: cfg.Etcd.Local.DataDir},
 		)
 	}
 
-	if cfg.Etcd.External != nil {
-		// Only check etcd version when external endpoints are specified
+	if cfg.Etcd.External != nil && !(isSecondaryControlPlane && downloadCerts) {
+		// Only check etcd certificates when using an external etcd and not joining with automatic download of certs
 		if cfg.Etcd.External.CAFile != "" {
 			checks = append(checks, FileExistingCheck{Path: cfg.Etcd.External.CAFile, Label: "ExternalEtcdClientCertificates"})
 		}
@@ -949,7 +956,6 @@ func RunInitNodeChecks(execer utilsexec.Interface, cfg *kubeadmapi.InitConfigura
 		if cfg.Etcd.External.KeyFile != "" {
 			checks = append(checks, FileExistingCheck{Path: cfg.Etcd.External.KeyFile, Label: "ExternalEtcdClientCertificates"})
 		}
-		checks = append(checks, ExternalEtcdVersionCheck{Etcd: cfg.Etcd})
 	}
 
 	return RunChecks(checks, os.Stderr, ignorePreflightErrors)

cmd/kubeadm/app/preflight/checks_test.go

@@ -186,9 +186,11 @@ func (pfct preflightCheckTest) Check() (warning, errorList []error) {
 
 func TestRunInitNodeChecks(t *testing.T) {
 	var tests = []struct {
-		name     string
-		cfg      *kubeadmapi.InitConfiguration
-		expected bool
+		name                    string
+		cfg                     *kubeadmapi.InitConfiguration
+		expected                bool
+		isSecondaryControlPlane bool
+		downloadCerts           bool
 	}{
 		{name: "Test valid advertised address",
 			cfg: &kubeadmapi.InitConfiguration{
@@ -197,7 +199,7 @@ func TestRunInitNodeChecks(t *testing.T) {
 			expected: false,
 		},
 		{
-			name: "Test CA file exists if specfied",
+			name: "Test CA file exists if specified",
 			cfg: &kubeadmapi.InitConfiguration{
 				ClusterConfiguration: kubeadmapi.ClusterConfiguration{
 					Etcd: kubeadmapi.Etcd{External: &kubeadmapi.ExternalEtcd{CAFile: "/foo"}},
@@ -206,7 +208,18 @@ func TestRunInitNodeChecks(t *testing.T) {
 			expected: false,
 		},
 		{
-			name: "Test Cert file exists if specfied",
+			name: "Skip test CA file exists if specified/download certs",
+			cfg: &kubeadmapi.InitConfiguration{
+				ClusterConfiguration: kubeadmapi.ClusterConfiguration{
+					Etcd: kubeadmapi.Etcd{External: &kubeadmapi.ExternalEtcd{CAFile: "/foo"}},
+				},
+			},
+			expected:                true,
+			isSecondaryControlPlane: true,
+			downloadCerts:           true,
+		},
+		{
+			name: "Test Cert file exists if specified",
 			cfg: &kubeadmapi.InitConfiguration{
 				ClusterConfiguration: kubeadmapi.ClusterConfiguration{
 					Etcd: kubeadmapi.Etcd{External: &kubeadmapi.ExternalEtcd{CertFile: "/foo"}},
@@ -215,7 +228,7 @@ func TestRunInitNodeChecks(t *testing.T) {
 			expected: false,
 		},
 		{
-			name: "Test Key file exists if specfied",
+			name: "Test Key file exists if specified",
 			cfg: &kubeadmapi.InitConfiguration{
 				ClusterConfiguration: kubeadmapi.ClusterConfiguration{
 					Etcd: kubeadmapi.Etcd{External: &kubeadmapi.ExternalEtcd{CertFile: "/foo"}},
@@ -232,7 +245,7 @@ func TestRunInitNodeChecks(t *testing.T) {
 	}
 	for _, rt := range tests {
 		// TODO: Make RunInitNodeChecks accept a ClusterConfiguration object instead of InitConfiguration
-		actual := RunInitNodeChecks(exec.New(), rt.cfg, sets.NewString(), false)
+		actual := RunInitNodeChecks(exec.New(), rt.cfg, sets.NewString(), rt.isSecondaryControlPlane, rt.downloadCerts)
 		if (actual == nil) != rt.expected {
 			t.Errorf(
 				"failed RunInitNodeChecks:\n\texpected: %t\n\t  actual: %t\n\t error: %v",