From c8b7e5739c818fb5d11e89bf8df34b9ca8520bed Mon Sep 17 00:00:00 2001 From: "Rostislav M. Georgiev" Date: Mon, 23 Mar 2020 18:09:46 +0200 Subject: [PATCH] kubeadm: Use image tag as version of stacked etcd kubeadm uses image tags (such as `v3.4.3-0`) to specify the version of etcd. However, the upgrade code in kubeadm uses the etcd client API to fetch the currently deployed version. The result contains only the etcd version without the additional information (such as image revision) that is normally found in the tag. As a result it would refuse an upgrade where the etcd versions match and the only difference is the image revision number (`v3.4.3-0` to `v3.4.3-1`). To fix the above issue, the following changes are done: - Replace the existing etcd version querying code, that uses the etcd client library, with code that returns the etcd image tag from the local static pod manifest file. - If an etcd `imageTag` is specified in the ClusterConfiguration during upgrade, use that tag instead. This is done regardless if the tag was specified in the configuration stored in the cluster or with a new configuration supplied by the `--config` command line parameter. If no custom tag is specified, kubeadm will select one depending on the desired Kubernetes version. - `kubeadm upgrade plan` no longer prints upgrade information about external etcd. It's the user's responsibility to manage it in that case. Signed-off-by: Rostislav M. Georgiev --- cmd/kubeadm/app/cmd/upgrade/BUILD | 1 - cmd/kubeadm/app/cmd/upgrade/plan.go | 29 +--- cmd/kubeadm/app/cmd/upgrade/plan_test.go | 6 +- cmd/kubeadm/app/phases/upgrade/BUILD | 1 + cmd/kubeadm/app/phases/upgrade/compute.go | 27 +-- .../app/phases/upgrade/compute_test.go | 156 ++++++++---------- cmd/kubeadm/app/phases/upgrade/staticpods.go | 60 ++++--- .../app/phases/upgrade/staticpods_test.go | 53 +++--- cmd/kubeadm/app/util/BUILD | 1 + cmd/kubeadm/app/util/etcd/etcd.go | 37 ----- cmd/kubeadm/app/util/image/BUILD | 28 ++++ cmd/kubeadm/app/util/image/image.go | 38 +++++ cmd/kubeadm/app/util/image/image_test.go | 47 ++++++ 13 files changed, 273 insertions(+), 211 deletions(-) create mode 100644 cmd/kubeadm/app/util/image/BUILD create mode 100644 cmd/kubeadm/app/util/image/image.go create mode 100644 cmd/kubeadm/app/util/image/image_test.go diff --git a/cmd/kubeadm/app/cmd/upgrade/BUILD b/cmd/kubeadm/app/cmd/upgrade/BUILD index 958b345a937..a8ea89fc999 100644 --- a/cmd/kubeadm/app/cmd/upgrade/BUILD +++ b/cmd/kubeadm/app/cmd/upgrade/BUILD @@ -29,7 +29,6 @@ go_library( "//cmd/kubeadm/app/util/apiclient:go_default_library", "//cmd/kubeadm/app/util/config:go_default_library", "//cmd/kubeadm/app/util/dryrun:go_default_library", - "//cmd/kubeadm/app/util/etcd:go_default_library", "//cmd/kubeadm/app/util/kubeconfig:go_default_library", "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library", diff --git a/cmd/kubeadm/app/cmd/upgrade/plan.go b/cmd/kubeadm/app/cmd/upgrade/plan.go index a07d8646a7d..753d6da60fd 100644 --- a/cmd/kubeadm/app/cmd/upgrade/plan.go +++ b/cmd/kubeadm/app/cmd/upgrade/plan.go @@ -32,7 +32,6 @@ import ( outputapiv1alpha1 "k8s.io/kubernetes/cmd/kubeadm/app/apis/output/v1alpha1" "k8s.io/kubernetes/cmd/kubeadm/app/constants" "k8s.io/kubernetes/cmd/kubeadm/app/phases/upgrade" - etcdutil "k8s.io/kubernetes/cmd/kubeadm/app/util/etcd" ) type planFlags struct { @@ -73,28 +72,13 @@ func runPlan(flags *planFlags, userVersion string) error { return err } - var etcdClient etcdutil.ClusterInterrogator - // Currently this is the only method we have for distinguishing // external etcd vs static pod etcd isExternalEtcd := cfg.Etcd.External != nil - if isExternalEtcd { - etcdClient, err = etcdutil.New( - cfg.Etcd.External.Endpoints, - cfg.Etcd.External.CAFile, - cfg.Etcd.External.CertFile, - cfg.Etcd.External.KeyFile) - } else { - // Connects to local/stacked etcd existing in the cluster - etcdClient, err = etcdutil.NewFromCluster(client, cfg.CertificatesDir) - } - if err != nil { - return err - } // Compute which upgrade possibilities there are klog.V(1).Infoln("[upgrade/plan] computing upgrade possibilities") - availUpgrades, err := upgrade.GetAvailableUpgrades(versionGetter, flags.allowExperimentalUpgrades, flags.allowRCUpgrades, etcdClient, cfg.DNS.Type, client) + availUpgrades, err := upgrade.GetAvailableUpgrades(versionGetter, flags.allowExperimentalUpgrades, flags.allowRCUpgrades, isExternalEtcd, cfg.DNS.Type, client, constants.GetStaticPodDirectory()) if err != nil { return errors.Wrap(err, "[upgrade/versions] FATAL") } @@ -161,10 +145,6 @@ func genUpgradePlan(up *upgrade.Upgrade, isExternalEtcd bool) (*outputapiv1alpha components := []outputapiv1alpha1.ComponentUpgradePlan{} - if isExternalEtcd && up.CanUpgradeEtcd() { - components = append(components, newComponentUpgradePlan(constants.Etcd, up.Before.EtcdVersion, up.After.EtcdVersion)) - } - if up.CanUpgradeKubelets() { // The map is of the form :. Here all the keys are put into a slice and sorted // in order to always get the right order. Then the map value is extracted separately @@ -204,11 +184,8 @@ func printUpgradePlan(up *upgrade.Upgrade, plan *outputapiv1alpha1.UpgradePlan, printManualUpgradeHeader := true for _, component := range plan.Components { if isExternalEtcd && component.Name == constants.Etcd { - fmt.Fprintln(w, "External components that should be upgraded manually before you upgrade the control plane with 'kubeadm upgrade apply':") - fmt.Fprintln(tabw, "COMPONENT\tCURRENT\tAVAILABLE") - fmt.Fprintf(tabw, "%s\t%s\t%s\n", component.Name, component.CurrentVersion, component.NewVersion) - // end of external components table - endOfTable() + // Don't print etcd if it's external + continue } else if component.Name == constants.Kubelet { if printManualUpgradeHeader { fmt.Fprintln(w, "Components that must be upgraded manually after you have upgraded the control plane with 'kubeadm upgrade apply':") diff --git a/cmd/kubeadm/app/cmd/upgrade/plan_test.go b/cmd/kubeadm/app/cmd/upgrade/plan_test.go index 560e6aa6120..8e9026c2920 100644 --- a/cmd/kubeadm/app/cmd/upgrade/plan_test.go +++ b/cmd/kubeadm/app/cmd/upgrade/plan_test.go @@ -429,11 +429,7 @@ _____________________________________________________________________ }, }, externalEtcd: true, - expectedBytes: []byte(`External components that should be upgraded manually before you upgrade the control plane with 'kubeadm upgrade apply': -COMPONENT CURRENT AVAILABLE -etcd 3.0.17 3.1.12 - -Components that must be upgraded manually after you have upgraded the control plane with 'kubeadm upgrade apply': + expectedBytes: []byte(`Components that must be upgraded manually after you have upgraded the control plane with 'kubeadm upgrade apply': COMPONENT CURRENT AVAILABLE kubelet 1 x v1.9.2 v1.9.3 diff --git a/cmd/kubeadm/app/phases/upgrade/BUILD b/cmd/kubeadm/app/phases/upgrade/BUILD index 98061f26e61..620b5f16067 100644 --- a/cmd/kubeadm/app/phases/upgrade/BUILD +++ b/cmd/kubeadm/app/phases/upgrade/BUILD @@ -34,6 +34,7 @@ go_library( "//cmd/kubeadm/app/util/apiclient:go_default_library", "//cmd/kubeadm/app/util/dryrun:go_default_library", "//cmd/kubeadm/app/util/etcd:go_default_library", + "//cmd/kubeadm/app/util/image:go_default_library", "//cmd/kubeadm/app/util/staticpod:go_default_library", "//staging/src/k8s.io/api/apps/v1:go_default_library", "//staging/src/k8s.io/api/batch/v1:go_default_library", diff --git a/cmd/kubeadm/app/phases/upgrade/compute.go b/cmd/kubeadm/app/phases/upgrade/compute.go index 440f330a1ea..c41f40fb216 100644 --- a/cmd/kubeadm/app/phases/upgrade/compute.go +++ b/cmd/kubeadm/app/phases/upgrade/compute.go @@ -26,7 +26,6 @@ import ( kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm" kubeadmconstants "k8s.io/kubernetes/cmd/kubeadm/app/constants" "k8s.io/kubernetes/cmd/kubeadm/app/phases/addons/dns" - etcdutil "k8s.io/kubernetes/cmd/kubeadm/app/util/etcd" ) // Upgrade defines an upgrade possibility to upgrade from a current version to a new one @@ -75,7 +74,7 @@ type ClusterState struct { // GetAvailableUpgrades fetches all versions from the specified VersionGetter and computes which // kinds of upgrades can be performed -func GetAvailableUpgrades(versionGetterImpl VersionGetter, experimentalUpgradesAllowed, rcUpgradesAllowed bool, etcdClient etcdutil.ClusterInterrogator, dnsType kubeadmapi.DNSAddOnType, client clientset.Interface) ([]Upgrade, error) { +func GetAvailableUpgrades(versionGetterImpl VersionGetter, experimentalUpgradesAllowed, rcUpgradesAllowed, externalEtcd bool, dnsType kubeadmapi.DNSAddOnType, client clientset.Interface, manifestsDir string) ([]Upgrade, error) { fmt.Println("[upgrade] Fetching available versions to upgrade to") // Collect the upgrades kubeadm can do in this list @@ -111,10 +110,13 @@ func GetAvailableUpgrades(versionGetterImpl VersionGetter, experimentalUpgradesA return upgrades, err } - // Get current etcd version - etcdVersion, err := etcdClient.GetVersion() - if err != nil { - return upgrades, err + // Get current stacked etcd version on the local node + var etcdVersion string + if !externalEtcd { + etcdVersion, err = GetEtcdImageTagFromStaticPod(manifestsDir) + if err != nil { + return upgrades, err + } } currentDNSType, dnsVersion, err := dns.DeployedDNSAddon(client) @@ -174,7 +176,7 @@ func GetAvailableUpgrades(versionGetterImpl VersionGetter, experimentalUpgradesA DNSType: dnsType, DNSVersion: kubeadmconstants.GetDNSVersion(dnsType), KubeadmVersion: newKubeadmVer, - EtcdVersion: getSuggestedEtcdVersion(patchVersionStr), + EtcdVersion: getSuggestedEtcdVersion(externalEtcd, patchVersionStr), // KubeletVersions is unset here as it is not used anywhere in .After }, }) @@ -191,7 +193,7 @@ func GetAvailableUpgrades(versionGetterImpl VersionGetter, experimentalUpgradesA DNSType: dnsType, DNSVersion: kubeadmconstants.GetDNSVersion(dnsType), KubeadmVersion: stableVersionStr, - EtcdVersion: getSuggestedEtcdVersion(stableVersionStr), + EtcdVersion: getSuggestedEtcdVersion(externalEtcd, stableVersionStr), // KubeletVersions is unset here as it is not used anywhere in .After }, }) @@ -239,7 +241,7 @@ func GetAvailableUpgrades(versionGetterImpl VersionGetter, experimentalUpgradesA DNSType: dnsType, DNSVersion: kubeadmconstants.GetDNSVersion(dnsType), KubeadmVersion: previousBranchLatestVersionStr, - EtcdVersion: getSuggestedEtcdVersion(previousBranchLatestVersionStr), + EtcdVersion: getSuggestedEtcdVersion(externalEtcd, previousBranchLatestVersionStr), // KubeletVersions is unset here as it is not used anywhere in .After }, }) @@ -266,7 +268,7 @@ func GetAvailableUpgrades(versionGetterImpl VersionGetter, experimentalUpgradesA DNSType: dnsType, DNSVersion: unstableKubeDNSVersion, KubeadmVersion: unstableKubeVersion, - EtcdVersion: getSuggestedEtcdVersion(unstableKubeVersion), + EtcdVersion: getSuggestedEtcdVersion(externalEtcd, unstableKubeVersion), // KubeletVersions is unset here as it is not used anywhere in .After }, }) @@ -300,7 +302,10 @@ func minorUpgradePossibleWithPatchRelease(stableVersion, patchVersion *versionut return patchVersion.LessThan(stableVersion) } -func getSuggestedEtcdVersion(kubernetesVersion string) string { +func getSuggestedEtcdVersion(externalEtcd bool, kubernetesVersion string) string { + if externalEtcd { + return "" + } etcdVersion, warning, err := kubeadmconstants.EtcdSupportedVersion(kubeadmconstants.SupportedEtcdVersion, kubernetesVersion) if err != nil { klog.Warningf("[upgrade/versions] could not retrieve an etcd version for the target Kubernetes version: %v", err) diff --git a/cmd/kubeadm/app/phases/upgrade/compute_test.go b/cmd/kubeadm/app/phases/upgrade/compute_test.go index 23d696c21c1..185bc4ab52e 100644 --- a/cmd/kubeadm/app/phases/upgrade/compute_test.go +++ b/cmd/kubeadm/app/phases/upgrade/compute_test.go @@ -18,12 +18,12 @@ package upgrade import ( "fmt" + "io/ioutil" + "os" "reflect" "strings" "testing" - "time" - "github.com/pkg/errors" apps "k8s.io/api/apps/v1" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -31,7 +31,6 @@ import ( clientsetfake "k8s.io/client-go/kubernetes/fake" kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm" "k8s.io/kubernetes/cmd/kubeadm/app/constants" - etcdutil "k8s.io/kubernetes/cmd/kubeadm/app/util/etcd" ) type fakeVersionGetter struct { @@ -72,54 +71,18 @@ func (f *fakeVersionGetter) KubeletVersions() (map[string]uint16, error) { } const fakeCurrentEtcdVersion = "3.1.12" - -type fakeEtcdClient struct { - TLS bool - mismatchedVersions bool -} - -func (f fakeEtcdClient) WaitForClusterAvailable(retries int, retryInterval time.Duration) (bool, error) { - return true, nil -} - -func (f fakeEtcdClient) CheckClusterHealth() error { - return nil -} - -func (f fakeEtcdClient) GetVersion() (string, error) { - versions, _ := f.GetClusterVersions() - if f.mismatchedVersions { - return "", errors.Errorf("etcd cluster contains endpoints with mismatched versions: %v", versions) - } - return fakeCurrentEtcdVersion, nil -} - -func (f fakeEtcdClient) GetClusterVersions() (map[string]string, error) { - if f.mismatchedVersions { - return map[string]string{ - "foo": fakeCurrentEtcdVersion, - "bar": "3.2.0", - }, nil - } - return map[string]string{ - "foo": fakeCurrentEtcdVersion, - "bar": fakeCurrentEtcdVersion, - }, nil -} - -func (f fakeEtcdClient) Sync() error { return nil } - -func (f fakeEtcdClient) AddMember(name string, peerAddrs string) ([]etcdutil.Member, error) { - return []etcdutil.Member{}, nil -} - -func (f fakeEtcdClient) GetMemberID(peerURL string) (uint64, error) { - return 0, nil -} - -func (f fakeEtcdClient) RemoveMember(id uint64) ([]etcdutil.Member, error) { - return []etcdutil.Member{}, nil -} +const etcdStaticPod = `apiVersion: v1 +kind: Pod +metadata: + labels: + component: etcd + tier: control-plane + name: etcd + namespace: kube-system +spec: + containers: + - name: etcd + image: k8s.gcr.io/etcd:` + fakeCurrentEtcdVersion func getEtcdVersion(v *versionutil.Version) string { return constants.SupportedEtcdVersion[uint8(v.Minor())] @@ -154,14 +117,13 @@ func TestGetAvailableUpgrades(t *testing.T) { v1Z0rc1 := v1Z0.WithPreRelease("rc.1") v1Z1 := v1Z0.WithPatch(1) - etcdClient := fakeEtcdClient{} tests := []struct { name string vg VersionGetter expectedUpgrades []Upgrade allowExperimental, allowRCs bool errExpected bool - etcdClient etcdutil.ClusterInterrogator + externalEtcd bool beforeDNSType kubeadmapi.DNSAddOnType beforeDNSVersion string dnsType kubeadmapi.DNSAddOnType @@ -182,7 +144,6 @@ func TestGetAvailableUpgrades(t *testing.T) { expectedUpgrades: []Upgrade{}, allowExperimental: false, errExpected: false, - etcdClient: etcdClient, }, { name: "simple patch version upgrade", @@ -221,7 +182,45 @@ func TestGetAvailableUpgrades(t *testing.T) { }, allowExperimental: false, errExpected: false, - etcdClient: etcdClient, + }, + { + name: "simple patch version upgrade with external etcd", + vg: &fakeVersionGetter{ + clusterVersion: v1Y1.String(), + kubeletVersion: v1Y1.String(), // the kubelet are on the same version as the control plane + kubeadmVersion: v1Y2.String(), + + stablePatchVersion: v1Y3.String(), + stableVersion: v1Y3.String(), + }, + beforeDNSType: kubeadmapi.CoreDNS, + beforeDNSVersion: fakeCurrentCoreDNSVersion, + dnsType: kubeadmapi.CoreDNS, + externalEtcd: true, + expectedUpgrades: []Upgrade{ + { + Description: fmt.Sprintf("version in the v%d.%d series", v1Y0.Major(), v1Y0.Minor()), + Before: ClusterState{ + KubeVersion: v1Y1.String(), + KubeletVersions: map[string]uint16{ + v1Y1.String(): 1, + }, + KubeadmVersion: v1Y2.String(), + DNSType: kubeadmapi.CoreDNS, + DNSVersion: fakeCurrentCoreDNSVersion, + EtcdVersion: "", + }, + After: ClusterState{ + KubeVersion: v1Y3.String(), + KubeadmVersion: v1Y3.String(), + DNSType: kubeadmapi.CoreDNS, + DNSVersion: constants.CoreDNSVersion, + EtcdVersion: "", + }, + }, + }, + allowExperimental: false, + errExpected: false, }, { name: "no version provided to offline version getter does not change behavior", @@ -260,7 +259,6 @@ func TestGetAvailableUpgrades(t *testing.T) { }, allowExperimental: false, errExpected: false, - etcdClient: etcdClient, }, { name: "minor version upgrade only", @@ -299,7 +297,6 @@ func TestGetAvailableUpgrades(t *testing.T) { }, allowExperimental: false, errExpected: false, - etcdClient: etcdClient, }, { name: "both minor version upgrade and patch version upgrade available", @@ -358,7 +355,6 @@ func TestGetAvailableUpgrades(t *testing.T) { }, allowExperimental: false, errExpected: false, - etcdClient: etcdClient, }, { name: "allow experimental upgrades, but no upgrade available", @@ -377,7 +373,6 @@ func TestGetAvailableUpgrades(t *testing.T) { expectedUpgrades: []Upgrade{}, allowExperimental: true, errExpected: false, - etcdClient: etcdClient, }, { name: "upgrade to an unstable version should be supported", @@ -417,7 +412,6 @@ func TestGetAvailableUpgrades(t *testing.T) { }, allowExperimental: true, errExpected: false, - etcdClient: etcdClient, }, { name: "upgrade from an unstable version to an unstable version should be supported", @@ -457,7 +451,6 @@ func TestGetAvailableUpgrades(t *testing.T) { }, allowExperimental: true, errExpected: false, - etcdClient: etcdClient, }, { name: "v1.X.0-alpha.0 should be ignored", @@ -498,7 +491,6 @@ func TestGetAvailableUpgrades(t *testing.T) { }, allowExperimental: true, errExpected: false, - etcdClient: etcdClient, }, { name: "upgrade to an RC version should be supported", @@ -539,7 +531,6 @@ func TestGetAvailableUpgrades(t *testing.T) { }, allowRCs: true, errExpected: false, - etcdClient: etcdClient, }, { name: "it is possible (but very uncommon) that the latest version from the previous branch is an rc and the current latest version is alpha.0. In that case, show the RC", @@ -580,7 +571,6 @@ func TestGetAvailableUpgrades(t *testing.T) { }, allowExperimental: true, errExpected: false, - etcdClient: etcdClient, }, { name: "upgrade to an RC version should be supported. There may also be an even newer unstable version.", @@ -642,22 +632,6 @@ func TestGetAvailableUpgrades(t *testing.T) { allowRCs: true, allowExperimental: true, errExpected: false, - etcdClient: etcdClient, - }, - { - name: "Upgrades with external etcd with mismatched versions should not be allowed.", - vg: &fakeVersionGetter{ - clusterVersion: v1Y3.String(), - kubeletVersion: v1Y3.String(), - kubeadmVersion: v1Y3.String(), - stablePatchVersion: v1Y3.String(), - stableVersion: v1Y3.String(), - }, - allowRCs: false, - allowExperimental: false, - etcdClient: fakeEtcdClient{mismatchedVersions: true}, - expectedUpgrades: []Upgrade{}, - errExpected: true, }, { name: "offline version getter", @@ -666,7 +640,6 @@ func TestGetAvailableUpgrades(t *testing.T) { kubeletVersion: v1Y0.String(), kubeadmVersion: v1Y1.String(), }, v1Z1.String()), - etcdClient: etcdClient, beforeDNSType: kubeadmapi.CoreDNS, beforeDNSVersion: fakeCurrentCoreDNSVersion, dnsType: kubeadmapi.CoreDNS, @@ -703,7 +676,6 @@ func TestGetAvailableUpgrades(t *testing.T) { stablePatchVersion: v1Z0.String(), stableVersion: v1Z0.String(), }, - etcdClient: etcdClient, beforeDNSType: kubeadmapi.KubeDNS, beforeDNSVersion: fakeCurrentKubeDNSVersion, dnsType: kubeadmapi.CoreDNS, @@ -740,7 +712,6 @@ func TestGetAvailableUpgrades(t *testing.T) { stablePatchVersion: v1Z0.String(), stableVersion: v1Z0.String(), }, - etcdClient: etcdClient, beforeDNSType: kubeadmapi.KubeDNS, beforeDNSVersion: fakeCurrentKubeDNSVersion, dnsType: kubeadmapi.KubeDNS, @@ -804,13 +775,24 @@ func TestGetAvailableUpgrades(t *testing.T) { }, }) - actualUpgrades, actualErr := GetAvailableUpgrades(rt.vg, rt.allowExperimental, rt.allowRCs, rt.etcdClient, rt.dnsType, client) + manifestsDir, err := ioutil.TempDir("", "GetAvailableUpgrades-test-manifests") + if err != nil { + t.Fatalf("Unable to create temporary directory: %v", err) + } + defer os.RemoveAll(manifestsDir) + + if err = ioutil.WriteFile(constants.GetStaticPodFilepath(constants.Etcd, manifestsDir), []byte(etcdStaticPod), 0644); err != nil { + t.Fatalf("Unable to create test static pod manifest: %v", err) + } + + actualUpgrades, actualErr := GetAvailableUpgrades(rt.vg, rt.allowExperimental, rt.allowRCs, rt.externalEtcd, rt.dnsType, client, manifestsDir) if !reflect.DeepEqual(actualUpgrades, rt.expectedUpgrades) { t.Errorf("failed TestGetAvailableUpgrades\n\texpected upgrades: %v\n\tgot: %v", rt.expectedUpgrades, actualUpgrades) } - if (actualErr != nil) != rt.errExpected { - fmt.Printf("Hello error") - t.Errorf("failed TestGetAvailableUpgrades\n\texpected error: %t\n\tgot error: %t", rt.errExpected, (actualErr != nil)) + if rt.errExpected && actualErr == nil { + t.Error("unexpected success") + } else if !rt.errExpected && actualErr != nil { + t.Errorf("unexpected failure: %v", actualErr) } if !reflect.DeepEqual(actualUpgrades, rt.expectedUpgrades) { t.Errorf("failed TestGetAvailableUpgrades\n\texpected upgrades: %v\n\tgot: %v", rt.expectedUpgrades, actualUpgrades) diff --git a/cmd/kubeadm/app/phases/upgrade/staticpods.go b/cmd/kubeadm/app/phases/upgrade/staticpods.go index b90aaf105f9..510ae744383 100644 --- a/cmd/kubeadm/app/phases/upgrade/staticpods.go +++ b/cmd/kubeadm/app/phases/upgrade/staticpods.go @@ -20,7 +20,6 @@ import ( "fmt" "os" "path/filepath" - "strings" "time" "github.com/pkg/errors" @@ -39,6 +38,7 @@ import ( "k8s.io/kubernetes/cmd/kubeadm/app/util/apiclient" dryrunutil "k8s.io/kubernetes/cmd/kubeadm/app/util/dryrun" etcdutil "k8s.io/kubernetes/cmd/kubeadm/app/util/etcd" + "k8s.io/kubernetes/cmd/kubeadm/app/util/image" "k8s.io/kubernetes/cmd/kubeadm/app/util/staticpod" ) @@ -282,37 +282,38 @@ func performEtcdStaticPodUpgrade(certsRenewMgr *renewal.Manager, client clientse return true, errors.Wrap(err, "failed to back up etcd data") } - // Need to check currently used version and version from constants, if differs then upgrade - desiredEtcdVersion, warning, err := constants.EtcdSupportedVersion(constants.SupportedEtcdVersion, cfg.KubernetesVersion) - if err != nil { - return true, errors.Wrap(err, "failed to retrieve an etcd version for the target Kubernetes version") - } - if warning != nil { - klog.Warningf("[upgrade/etcd] %v", warning) + // Get the desired etcd version. That's either the one specified by the user in cfg.Etcd.Local.ImageTag + // or the kubeadm preferred one for the desired Kubernetes version + var desiredEtcdVersion *version.Version + if cfg.Etcd.Local.ImageTag != "" { + desiredEtcdVersion, err = version.ParseSemantic(cfg.Etcd.Local.ImageTag) + if err != nil { + return true, errors.Wrapf(err, "failed to parse tag %q as a semantic version", cfg.Etcd.Local.ImageTag) + } + } else { + // Need to check currently used version and version from constants, if differs then upgrade + var warning error + desiredEtcdVersion, warning, err = constants.EtcdSupportedVersion(constants.SupportedEtcdVersion, cfg.KubernetesVersion) + if err != nil { + return true, errors.Wrap(err, "failed to retrieve an etcd version for the target Kubernetes version") + } + if warning != nil { + klog.Warningf("[upgrade/etcd] %v", warning) + } } - // gets the etcd version of the local/stacked etcd member running on the current machine - currentEtcdVersions, err := oldEtcdClient.GetClusterVersions() + // Get the etcd version of the local/stacked etcd member running on the current machine + currentEtcdVersionStr, err := GetEtcdImageTagFromStaticPod(pathMgr.RealManifestDir()) if err != nil { return true, errors.Wrap(err, "failed to retrieve the current etcd version") } - currentEtcdVersionStr, ok := currentEtcdVersions[etcdutil.GetClientURL(&cfg.LocalAPIEndpoint)] - if !ok { - return true, errors.Wrap(err, "failed to retrieve the current etcd version") - } - currentEtcdVersion, err := version.ParseSemantic(currentEtcdVersionStr) + cmpResult, err := desiredEtcdVersion.Compare(currentEtcdVersionStr) if err != nil { - return true, errors.Wrapf(err, "failed to parse the current etcd version(%s)", currentEtcdVersionStr) + return true, errors.Wrapf(err, "failed comparing the current etcd version %q to the desired one %q", currentEtcdVersionStr, desiredEtcdVersion) } - - // Comparing current etcd version with desired to catch the same version or downgrade condition and fail on them. - if desiredEtcdVersion.LessThan(currentEtcdVersion) { - return false, errors.Errorf("the desired etcd version for this Kubernetes version %q is %q, but the current etcd version is %q. Won't downgrade etcd, instead just continue", cfg.KubernetesVersion, desiredEtcdVersion.String(), currentEtcdVersion.String()) - } - // For the case when desired etcd version is the same as current etcd version - if strings.Compare(desiredEtcdVersion.String(), currentEtcdVersion.String()) == 0 { - return false, nil + if cmpResult < 1 { + return false, errors.Errorf("the desired etcd version %q is not newer than the currently installed %q. Skipping etcd upgrade", desiredEtcdVersion, currentEtcdVersionStr) } beforeEtcdPodHash, err := waiter.WaitForStaticPodSingleHash(cfg.NodeRegistration.Name, constants.Etcd) @@ -633,3 +634,14 @@ func DryRunStaticPodUpgrade(kustomizeDir string, internalcfg *kubeadmapi.InitCon return dryrunutil.PrintDryRunFiles(files, os.Stdout) } + +// GetEtcdImageTagFromStaticPod returns the image tag of the local etcd static pod +func GetEtcdImageTagFromStaticPod(manifestDir string) (string, error) { + realPath := constants.GetStaticPodFilepath(constants.Etcd, manifestDir) + pod, err := staticpod.ReadStaticPodFromDisk(realPath) + if err != nil { + return "", err + } + + return image.TagFromImage(pod.Spec.Containers[0].Image), nil +} diff --git a/cmd/kubeadm/app/phases/upgrade/staticpods_test.go b/cmd/kubeadm/app/phases/upgrade/staticpods_test.go index ab31893bf3b..479ac129558 100644 --- a/cmd/kubeadm/app/phases/upgrade/staticpods_test.go +++ b/cmd/kubeadm/app/phases/upgrade/staticpods_test.go @@ -244,16 +244,6 @@ func (c fakeTLSEtcdClient) CheckClusterHealth() error { return nil } -func (c fakeTLSEtcdClient) GetClusterVersions() (map[string]string, error) { - return map[string]string{ - "https://1.2.3.4:2379": "3.1.12", - }, nil -} - -func (c fakeTLSEtcdClient) GetVersion() (string, error) { - return "3.1.12", nil -} - func (c fakeTLSEtcdClient) Sync() error { return nil } func (c fakeTLSEtcdClient) AddMember(name string, peerAddrs string) ([]etcdutil.Member, error) { @@ -285,16 +275,6 @@ func (c fakePodManifestEtcdClient) CheckClusterHealth() error { return err } -func (c fakePodManifestEtcdClient) GetClusterVersions() (map[string]string, error) { - return map[string]string{ - "https://1.2.3.4:2379": "3.1.12", - }, nil -} - -func (c fakePodManifestEtcdClient) GetVersion() (string, error) { - return "3.1.12", nil -} - func (c fakePodManifestEtcdClient) Sync() error { return nil } func (c fakePodManifestEtcdClient) AddMember(name string, peerAddrs string) ([]etcdutil.Member, error) { @@ -989,3 +969,36 @@ func TestGetPathManagerForUpgrade(t *testing.T) { } } + +func TestGetEtcdImageTagFromStaticPod(t *testing.T) { + const expectedEtcdVersion = "3.1.12" + const etcdStaticPod = `apiVersion: v1 +kind: Pod +metadata: + labels: + component: etcd + tier: control-plane + name: etcd + namespace: kube-system +spec: + containers: + - name: etcd + image: k8s.gcr.io/etcd:` + expectedEtcdVersion + + manifestsDir, err := ioutil.TempDir("", "GetEtcdImageTagFromStaticPod-test-manifests") + if err != nil { + t.Fatalf("Unable to create temporary directory: %v", err) + } + defer os.RemoveAll(manifestsDir) + + if err = ioutil.WriteFile(constants.GetStaticPodFilepath(constants.Etcd, manifestsDir), []byte(etcdStaticPod), 0644); err != nil { + t.Fatalf("Unable to create test static pod manifest: %v", err) + } + + got, err := GetEtcdImageTagFromStaticPod(manifestsDir) + if err != nil { + t.Errorf("unexpected error: %v", err) + } else if got != expectedEtcdVersion { + t.Errorf("unexpected result:\n\tgot: %q\n\texpected: %q", got, expectedEtcdVersion) + } +} diff --git a/cmd/kubeadm/app/util/BUILD b/cmd/kubeadm/app/util/BUILD index 8dd4045baa7..f7a417b5c0b 100644 --- a/cmd/kubeadm/app/util/BUILD +++ b/cmd/kubeadm/app/util/BUILD @@ -83,6 +83,7 @@ filegroup( "//cmd/kubeadm/app/util/crypto:all-srcs", "//cmd/kubeadm/app/util/dryrun:all-srcs", "//cmd/kubeadm/app/util/etcd:all-srcs", + "//cmd/kubeadm/app/util/image:all-srcs", "//cmd/kubeadm/app/util/initsystem:all-srcs", "//cmd/kubeadm/app/util/kubeconfig:all-srcs", "//cmd/kubeadm/app/util/kustomize:all-srcs", diff --git a/cmd/kubeadm/app/util/etcd/etcd.go b/cmd/kubeadm/app/util/etcd/etcd.go index 0c99cd8c9d6..56f925b8745 100644 --- a/cmd/kubeadm/app/util/etcd/etcd.go +++ b/cmd/kubeadm/app/util/etcd/etcd.go @@ -53,8 +53,6 @@ var etcdBackoff = wait.Backoff{ // ClusterInterrogator is an interface to get etcd cluster related information type ClusterInterrogator interface { CheckClusterHealth() error - GetClusterVersions() (map[string]string, error) - GetVersion() (string, error) WaitForClusterAvailable(retries int, retryInterval time.Duration) (bool, error) Sync() error AddMember(name string, peerAddrs string) ([]Member, error) @@ -409,41 +407,6 @@ func (c *Client) AddMember(name string, peerAddrs string) ([]Member, error) { return ret, nil } -// GetVersion returns the etcd version of the cluster. -// An error is returned if the version of all endpoints do not match -func (c *Client) GetVersion() (string, error) { - var clusterVersion string - - versions, err := c.GetClusterVersions() - if err != nil { - return "", err - } - for _, v := range versions { - if clusterVersion != "" && clusterVersion != v { - return "", errors.Errorf("etcd cluster contains endpoints with mismatched versions: %v", versions) - } - clusterVersion = v - } - if clusterVersion == "" { - return "", errors.New("could not determine cluster etcd version") - } - return clusterVersion, nil -} - -// GetClusterVersions returns a map of the endpoints and their associated versions -func (c *Client) GetClusterVersions() (map[string]string, error) { - versions := make(map[string]string) - statuses, err := c.getClusterStatus() - if err != nil { - return versions, err - } - - for ep, status := range statuses { - versions[ep] = status.Version - } - return versions, nil -} - // CheckClusterHealth returns nil for status Up or error for status Down func (c *Client) CheckClusterHealth() error { _, err := c.getClusterStatus() diff --git a/cmd/kubeadm/app/util/image/BUILD b/cmd/kubeadm/app/util/image/BUILD new file mode 100644 index 00000000000..f61adb81c4a --- /dev/null +++ b/cmd/kubeadm/app/util/image/BUILD @@ -0,0 +1,28 @@ +load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") + +go_library( + name = "go_default_library", + srcs = ["image.go"], + importpath = "k8s.io/kubernetes/cmd/kubeadm/app/util/image", + visibility = ["//visibility:public"], +) + +go_test( + name = "go_default_test", + srcs = ["image_test.go"], + embed = [":go_default_library"], +) + +filegroup( + name = "package-srcs", + srcs = glob(["**"]), + tags = ["automanaged"], + visibility = ["//visibility:private"], +) + +filegroup( + name = "all-srcs", + srcs = [":package-srcs"], + tags = ["automanaged"], + visibility = ["//visibility:public"], +) diff --git a/cmd/kubeadm/app/util/image/image.go b/cmd/kubeadm/app/util/image/image.go new file mode 100644 index 00000000000..0a3c2bb6cb7 --- /dev/null +++ b/cmd/kubeadm/app/util/image/image.go @@ -0,0 +1,38 @@ +/* +Copyright 2020 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package image + +import "regexp" + +var ( + // tagMatcher is the regex used to match a tag. + // Basically we presume an image can be made of `[domain][:port][path][:tag][@sha256:digest]` + // We are obvously interested only in the tag, but for the purpose of properly matching it, we also match the digest + // (if present). All the parts before the tag we match in a single match everything (but not greedy) group. + // All matched sub-groups, except the tag one, get thrown away. Hence, in a result of FindStringSubmatch, if a tag + // matches, it's going to be the second returned element (after the full match). + tagMatcher = regexp.MustCompile(`^(?U:.*)(?::([[:word:]][[:word:].-]*))?(?:@sha256:[a-fA-F0-9]{64})?$`) +) + +// TagFromImage extracts a tag from image. An empty string is returned if no tag is discovered. +func TagFromImage(image string) string { + matches := tagMatcher.FindStringSubmatch(image) + if len(matches) >= 2 { + return matches[1] + } + return "" +} diff --git a/cmd/kubeadm/app/util/image/image_test.go b/cmd/kubeadm/app/util/image/image_test.go new file mode 100644 index 00000000000..dd9ee2779bd --- /dev/null +++ b/cmd/kubeadm/app/util/image/image_test.go @@ -0,0 +1,47 @@ +/* +Copyright 2020 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package image + +import "testing" + +func TestTagFromImage(t *testing.T) { + tests := map[string]string{ + "kindest/node": "", + "kindest/node:latest": "latest", + "kindest/node:v1.17.0": "v1.17.0", + "kindest/node:v1.17.0@sha256:9512edae126da271b66b990b6fff768fbb7cd786c7d39e86bdf55906352fdf62": "v1.17.0", + "kindest/node@sha256:9512edae126da271b66b990b6fff768fbb7cd786c7d39e86bdf55906352fdf62": "", + + "example.com/kindest/node": "", + "example.com/kindest/node:latest": "latest", + "example.com/kindest/node:v1.17.0": "v1.17.0", + "example.com/kindest/node:v1.17.0@sha256:9512edae126da271b66b990b6fff768fbb7cd786c7d39e86bdf55906352fdf62": "v1.17.0", + "example.com/kindest/node@sha256:9512edae126da271b66b990b6fff768fbb7cd786c7d39e86bdf55906352fdf62": "", + + "example.com:3000/kindest/node": "", + "example.com:3000/kindest/node:latest": "latest", + "example.com:3000/kindest/node:v1.17.0": "v1.17.0", + "example.com:3000/kindest/node:v1.17.0@sha256:9512edae126da271b66b990b6fff768fbb7cd786c7d39e86bdf55906352fdf62": "v1.17.0", + "example.com:3000/kindest/node@sha256:9512edae126da271b66b990b6fff768fbb7cd786c7d39e86bdf55906352fdf62": "", + } + for in, expected := range tests { + out := TagFromImage(in) + if out != expected { + t.Errorf("TagFromImage(%q) = %q, expected %q instead", in, out, expected) + } + } +}