Merge pull request kubernetes#86861 from SataQiu/fix-kubeadm-20200106

kubeadm upgrades always persist the etcd backup for stacked

Author: k8s-ci-robot

cmd/kubeadm/app/cmd/upgrade/apply.go

@@ -220,6 +220,5 @@ func PerformControlPlaneUpgrade(flags *applyFlags, client clientset.Interface, w
 		return upgrade.DryRunStaticPodUpgrade(flags.kustomizeDir, internalcfg)
 	}
 
-	// Don't save etcd backup directory if etcd is HA, as this could cause corruption
 	return upgrade.PerformStaticPodUpgrade(client, waiter, internalcfg, flags.etcdUpgrade, flags.renewCerts, flags.kustomizeDir)
 }

cmd/kubeadm/app/phases/upgrade/staticpods.go

@@ -591,8 +591,8 @@ func renewCertsByComponent(cfg *kubeadmapi.InitConfiguration, component string,
 
 // GetPathManagerForUpgrade returns a path manager properly configured for the given InitConfiguration.
 func GetPathManagerForUpgrade(kubernetesDir, kustomizeDir string, internalcfg *kubeadmapi.InitConfiguration, etcdUpgrade bool) (StaticPodPathManager, error) {
-	isHAEtcd := etcdutil.CheckConfigurationIsHA(&internalcfg.Etcd)
-	return NewKubeStaticPodPathManagerUsingTempDirs(kubernetesDir, kustomizeDir, true, etcdUpgrade && !isHAEtcd)
+	isExternalEtcd := internalcfg.Etcd.External != nil
+	return NewKubeStaticPodPathManagerUsingTempDirs(kubernetesDir, kustomizeDir, true, etcdUpgrade && !isExternalEtcd)
 }
 
 // PerformStaticPodUpgrade performs the upgrade of the control plane components for a static pod hosted cluster

cmd/kubeadm/app/phases/upgrade/staticpods_test.go

@@ -900,7 +900,7 @@ func getEmbeddedCerts(tmpDir, kubeConfig string) ([]*x509.Certificate, error) {
 
 func TestGetPathManagerForUpgrade(t *testing.T) {
 
-	haEtcd := &kubeadmapi.InitConfiguration{
+	externalEtcd := &kubeadmapi.InitConfiguration{
 		ClusterConfiguration: kubeadmapi.ClusterConfiguration{
 			Etcd: kubeadmapi.Etcd{
 				External: &kubeadmapi.ExternalEtcd{
@@ -910,7 +910,7 @@ func TestGetPathManagerForUpgrade(t *testing.T) {
 		},
 	}
 
-	noHAEtcd := &kubeadmapi.InitConfiguration{}
+	stackedEtcd := &kubeadmapi.InitConfiguration{}
 
 	tests := []struct {
 		name             string
@@ -919,23 +919,29 @@ func TestGetPathManagerForUpgrade(t *testing.T) {
 		shouldDeleteEtcd bool
 	}{
 		{
-			name:             "ha etcd but no etcd upgrade",
-			cfg:              haEtcd,
+			name:             "external etcd but no etcd upgrade",
+			cfg:              externalEtcd,
 			etcdUpgrade:      false,
 			shouldDeleteEtcd: true,
 		},
 		{
-			name:             "non-ha etcd with etcd upgrade",
-			cfg:              noHAEtcd,
+			name:             "external etcd with etcd upgrade",
+			cfg:              externalEtcd,
 			etcdUpgrade:      true,
-			shouldDeleteEtcd: false,
+			shouldDeleteEtcd: true,
 		},
 		{
-			name:             "ha etcd and etcd upgrade",
-			cfg:              haEtcd,
-			etcdUpgrade:      true,
+			name:             "stacked etcd but no etcd upgrade",
+			cfg:              stackedEtcd,
+			etcdUpgrade:      false,
 			shouldDeleteEtcd: true,
 		},
+		{
+			name:             "stacked etcd with etcd upgrade",
+			cfg:              stackedEtcd,
+			etcdUpgrade:      true,
+			shouldDeleteEtcd: false,
+		},
 	}
 
 	for _, test := range tests {

cmd/kubeadm/app/util/etcd/etcd.go

@@ -421,11 +421,6 @@ func (c *Client) WaitForClusterAvailable(retries int, retryInterval time.Duratio
 	return false, errors.New("timeout waiting for etcd cluster to be available")
 }
 
-// CheckConfigurationIsHA returns true if the given InitConfiguration etcd block appears to be an HA configuration.
-func CheckConfigurationIsHA(cfg *kubeadmapi.Etcd) bool {
-	return cfg.External != nil && len(cfg.External.Endpoints) > 1
-}
-
 // GetClientURL creates an HTTPS URL that uses the configured advertise
 // address and client port for the API controller
 func GetClientURL(localEndpoint *kubeadmapi.APIEndpoint) string {

cmd/kubeadm/app/util/etcd/etcd_test.go

@@ -25,53 +25,6 @@ import (
 	"k8s.io/kubernetes/cmd/kubeadm/app/constants"
 )
 
-func TestCheckConfigurationIsHA(t *testing.T) {
-	var tests = []struct {
-		name     string
-		cfg      *kubeadmapi.Etcd
-		expected bool
-	}{
-		{
-			name: "HA etcd",
-			cfg: &kubeadmapi.Etcd{
-				External: &kubeadmapi.ExternalEtcd{
-					Endpoints: []string{"10.100.0.1:2379", "10.100.0.2:2379", "10.100.0.3:2379"},
-				},
-			},
-			expected: true,
-		},
-		{
-			name: "single External etcd",
-			cfg: &kubeadmapi.Etcd{
-				External: &kubeadmapi.ExternalEtcd{
-					Endpoints: []string{"10.100.0.1:2379"},
-				},
-			},
-			expected: false,
-		},
-		{
-			name: "local etcd",
-			cfg: &kubeadmapi.Etcd{
-				Local: &kubeadmapi.LocalEtcd{},
-			},
-			expected: false,
-		},
-		{
-			name:     "empty etcd struct",
-			cfg:      &kubeadmapi.Etcd{},
-			expected: false,
-		},
-	}
-
-	for _, test := range tests {
-		t.Run(test.name, func(t *testing.T) {
-			if isHA := CheckConfigurationIsHA(test.cfg); isHA != test.expected {
-				t.Errorf("expected isHA to be %v, got %v", test.expected, isHA)
-			}
-		})
-	}
-}
-
 func testGetURL(t *testing.T, getURLFunc func(*kubeadmapi.APIEndpoint) string, port int) {
 	portStr := strconv.Itoa(port)
 	var tests = []struct {