From 5610ac3c9c7ac55d1d8854d9bc747af80220abea Mon Sep 17 00:00:00 2001 From: yuexiao-wang Date: Wed, 5 Dec 2018 16:39:14 +0800 Subject: [PATCH] cleanup upgrade from non-TLS etcd to TLS etcd Signed-off-by: yuexiao-wang --- .../app/phases/upgrade/compute_test.go | 2 - cmd/kubeadm/app/phases/upgrade/staticpods.go | 59 ++++++++----------- .../app/phases/upgrade/staticpods_test.go | 9 --- cmd/kubeadm/app/util/etcd/etcd.go | 6 -- 4 files changed, 25 insertions(+), 51 deletions(-) diff --git a/cmd/kubeadm/app/phases/upgrade/compute_test.go b/cmd/kubeadm/app/phases/upgrade/compute_test.go index fd99f637e4..3aa67640b9 100644 --- a/cmd/kubeadm/app/phases/upgrade/compute_test.go +++ b/cmd/kubeadm/app/phases/upgrade/compute_test.go @@ -76,8 +76,6 @@ type fakeEtcdClient struct { mismatchedVersions bool } -func (f fakeEtcdClient) HasTLS() bool { return f.TLS } - func (f fakeEtcdClient) ClusterAvailable() (bool, error) { return true, nil } func (f fakeEtcdClient) WaitForClusterAvailable(delay time.Duration, retries int, retryInterval time.Duration) (bool, error) { diff --git a/cmd/kubeadm/app/phases/upgrade/staticpods.go b/cmd/kubeadm/app/phases/upgrade/staticpods.go index cc47654937..7e54de2be0 100644 --- a/cmd/kubeadm/app/phases/upgrade/staticpods.go +++ b/cmd/kubeadm/app/phases/upgrade/staticpods.go @@ -165,7 +165,7 @@ func (spm *KubeStaticPodPathManager) CleanupDirs() error { return nil } -func upgradeComponent(component string, waiter apiclient.Waiter, pathMgr StaticPodPathManager, cfg *kubeadmapi.InitConfiguration, beforePodHash string, recoverManifests map[string]string, isTLSUpgrade bool) error { +func upgradeComponent(component string, waiter apiclient.Waiter, pathMgr StaticPodPathManager, cfg *kubeadmapi.InitConfiguration, beforePodHash string, recoverManifests map[string]string) error { // Special treatment is required for etcd case, when rollbackOldManifests should roll back etcd // manifests only for the case when component is Etcd recoverEtcd := false @@ -173,20 +173,19 @@ func upgradeComponent(component string, waiter apiclient.Waiter, pathMgr StaticP if component == constants.Etcd { recoverEtcd = true } - if isTLSUpgrade { - // We currently depend on getting the Etcd mirror Pod hash from the KubeAPIServer; - // Upgrading the Etcd protocol takes down the apiserver, so we can't verify component restarts if we restart Etcd independently. - // Skip waiting for Etcd to restart and immediately move on to updating the apiserver. - if component == constants.Etcd { - waitForComponentRestart = false - } - // Normally, if an Etcd upgrade is successful, but the apiserver upgrade fails, Etcd is not rolled back. - // In the case of a TLS upgrade, the old KubeAPIServer config is incompatible with the new Etcd confg, so we rollback Etcd - // if the APIServer upgrade fails. - if component == constants.KubeAPIServer { - recoverEtcd = true - fmt.Printf("[upgrade/staticpods] The %s manifest will be restored if component %q fails to upgrade\n", constants.Etcd, component) - } + + // We currently depend on getting the Etcd mirror Pod hash from the KubeAPIServer; + // Upgrading the Etcd protocol takes down the apiserver, so we can't verify component restarts if we restart Etcd independently. + // Skip waiting for Etcd to restart and immediately move on to updating the apiserver. + if component == constants.Etcd { + waitForComponentRestart = false + } + // Normally, if an Etcd upgrade is successful, but the apiserver upgrade fails, Etcd is not rolled back. + // In the case of a TLS upgrade, the old KubeAPIServer config is incompatible with the new Etcd confg, so we rollback Etcd + // if the APIServer upgrade fails. + if component == constants.KubeAPIServer { + recoverEtcd = true + fmt.Printf("[upgrade/staticpods] The %s manifest will be restored if component %q fails to upgrade\n", constants.Etcd, component) } if err := renewCerts(cfg, component); err != nil { @@ -252,7 +251,7 @@ func upgradeComponent(component string, waiter apiclient.Waiter, pathMgr StaticP } // performEtcdStaticPodUpgrade performs upgrade of etcd, it returns bool which indicates fatal error or not and the actual error. -func performEtcdStaticPodUpgrade(client clientset.Interface, waiter apiclient.Waiter, pathMgr StaticPodPathManager, cfg *kubeadmapi.InitConfiguration, recoverManifests map[string]string, isTLSUpgrade bool, oldEtcdClient, newEtcdClient etcdutil.ClusterInterrogator) (bool, error) { +func performEtcdStaticPodUpgrade(client clientset.Interface, waiter apiclient.Waiter, pathMgr StaticPodPathManager, cfg *kubeadmapi.InitConfiguration, recoverManifests map[string]string, oldEtcdClient, newEtcdClient etcdutil.ClusterInterrogator) (bool, error) { // Add etcd static pod spec only if external etcd is not configured if cfg.Etcd.External != nil { return false, errors.New("external etcd detected, won't try to change any etcd state") @@ -313,20 +312,18 @@ func performEtcdStaticPodUpgrade(client clientset.Interface, waiter apiclient.Wa } // Waiter configurations for checking etcd status + // If we are upgrading TLS we need to wait for old static pod to be removed. + // This is needed because we are not able to currently verify that the static pod + // has been updated through the apiserver across an etcd TLS upgrade. + // This value is arbitrary but seems to be long enough in manual testing. noDelay := 0 * time.Second - podRestartDelay := noDelay - if isTLSUpgrade { - // If we are upgrading TLS we need to wait for old static pod to be removed. - // This is needed because we are not able to currently verify that the static pod - // has been updated through the apiserver across an etcd TLS upgrade. - // This value is arbitrary but seems to be long enough in manual testing. - podRestartDelay = 30 * time.Second - } + podRestartDelay := 30 * time.Second + retries := 10 retryInterval := 15 * time.Second // Perform etcd upgrade using common to all control plane components function - if err := upgradeComponent(constants.Etcd, waiter, pathMgr, cfg, beforeEtcdPodHash, recoverManifests, isTLSUpgrade); err != nil { + if err := upgradeComponent(constants.Etcd, waiter, pathMgr, cfg, beforeEtcdPodHash, recoverManifests); err != nil { fmt.Printf("[upgrade/etcd] Failed to upgrade etcd: %v\n", err) // Since upgrade component failed, the old etcd manifest has either been restored or was never touched // Now we need to check the health of etcd cluster if it is up with old manifest @@ -404,7 +401,6 @@ func performEtcdStaticPodUpgrade(client clientset.Interface, waiter apiclient.Wa // StaticPodControlPlane upgrades a static pod-hosted control plane func StaticPodControlPlane(client clientset.Interface, waiter apiclient.Waiter, pathMgr StaticPodPathManager, cfg *kubeadmapi.InitConfiguration, etcdUpgrade bool, oldEtcdClient, newEtcdClient etcdutil.ClusterInterrogator) error { recoverManifests := map[string]string{} - var isTLSUpgrade bool var isExternalEtcd bool beforePodHashMap, err := waiter.WaitForStaticPodControlPlaneHashes(cfg.NodeRegistration.Name) @@ -442,16 +438,11 @@ func StaticPodControlPlane(client clientset.Interface, waiter apiclient.Waiter, // etcd upgrade is done prior to other control plane components if !isExternalEtcd && etcdUpgrade { - previousEtcdHasTLS := oldEtcdClient.HasTLS() - // set the TLS upgrade flag for all components - isTLSUpgrade = !previousEtcdHasTLS - if isTLSUpgrade { - fmt.Printf("[upgrade/etcd] Upgrading to TLS for %s\n", constants.Etcd) - } + fmt.Printf("[upgrade/etcd] Upgrading to TLS for %s\n", constants.Etcd) // Perform etcd upgrade using common to all control plane components function - fatal, err := performEtcdStaticPodUpgrade(client, waiter, pathMgr, cfg, recoverManifests, isTLSUpgrade, oldEtcdClient, newEtcdClient) + fatal, err := performEtcdStaticPodUpgrade(client, waiter, pathMgr, cfg, recoverManifests, oldEtcdClient, newEtcdClient) if err != nil { if fatal { return err @@ -468,7 +459,7 @@ func StaticPodControlPlane(client clientset.Interface, waiter apiclient.Waiter, } for _, component := range constants.MasterComponents { - if err = upgradeComponent(component, waiter, pathMgr, cfg, beforePodHashMap[component], recoverManifests, isTLSUpgrade); err != nil { + if err = upgradeComponent(component, waiter, pathMgr, cfg, beforePodHashMap[component], recoverManifests); err != nil { return err } } diff --git a/cmd/kubeadm/app/phases/upgrade/staticpods_test.go b/cmd/kubeadm/app/phases/upgrade/staticpods_test.go index 270a6f6964..99f40cbb15 100644 --- a/cmd/kubeadm/app/phases/upgrade/staticpods_test.go +++ b/cmd/kubeadm/app/phases/upgrade/staticpods_test.go @@ -228,10 +228,6 @@ func (spm *fakeStaticPodPathManager) CleanupDirs() error { type fakeTLSEtcdClient struct{ TLS bool } -func (c fakeTLSEtcdClient) HasTLS() bool { - return c.TLS -} - func (c fakeTLSEtcdClient) ClusterAvailable() (bool, error) { return true, nil } func (c fakeTLSEtcdClient) WaitForClusterAvailable(delay time.Duration, retries int, retryInterval time.Duration) (bool, error) { @@ -263,11 +259,6 @@ func (c fakeTLSEtcdClient) AddMember(name string, peerAddrs string) ([]etcdutil. type fakePodManifestEtcdClient struct{ ManifestDir, CertificatesDir string } -func (c fakePodManifestEtcdClient) HasTLS() bool { - hasTLS, _ := etcdutil.PodManifestsHaveTLS(c.ManifestDir) - return hasTLS -} - func (c fakePodManifestEtcdClient) ClusterAvailable() (bool, error) { return true, nil } func (c fakePodManifestEtcdClient) WaitForClusterAvailable(delay time.Duration, retries int, retryInterval time.Duration) (bool, error) { diff --git a/cmd/kubeadm/app/util/etcd/etcd.go b/cmd/kubeadm/app/util/etcd/etcd.go index 5fccce82a3..5aac683e14 100644 --- a/cmd/kubeadm/app/util/etcd/etcd.go +++ b/cmd/kubeadm/app/util/etcd/etcd.go @@ -43,7 +43,6 @@ type ClusterInterrogator interface { GetClusterStatus() (map[string]*clientv3.StatusResponse, error) GetClusterVersions() (map[string]string, error) GetVersion() (string, error) - HasTLS() bool WaitForClusterAvailable(delay time.Duration, retries int, retryInterval time.Duration) (bool, error) Sync() error AddMember(name string, peerAddrs string) ([]Member, error) @@ -55,11 +54,6 @@ type Client struct { TLS *tls.Config } -// HasTLS returns true if etcd is configured for TLS -func (c Client) HasTLS() bool { - return c.TLS != nil -} - // PodManifestsHaveTLS reads the etcd staticpod manifest from disk and returns false if the TLS flags // are missing from the command list. If all the flags are present it returns true. func PodManifestsHaveTLS(ManifestDir string) (bool, error) {