From 4947e5aaf8a5354cfada9816e837420ff7a75ca7 Mon Sep 17 00:00:00 2001 From: Matthew Fisher Date: Thu, 25 Jan 2018 23:16:44 -0800 Subject: [PATCH] fix helm init --upgrade logic --- cmd/helm/installer/install.go | 25 ++++-- cmd/helm/installer/install_test.go | 123 +++++++++++++++++++++++++++++ 2 files changed, 140 insertions(+), 8 deletions(-) diff --git a/cmd/helm/installer/install.go b/cmd/helm/installer/install.go index f2c0d232d..230c7b39b 100644 --- a/cmd/helm/installer/install.go +++ b/cmd/helm/installer/install.go @@ -63,8 +63,8 @@ func Upgrade(client kubernetes.Interface, opts *Options) error { if err != nil { return err } - existingImage := obj.Spec.Template.Spec.Containers[0].Image - if !isNewerVersion(existingImage) && !opts.ForceUpgrade { + tillerImage := obj.Spec.Template.Spec.Containers[0].Image + if semverCompare(tillerImage) == -1 && !opts.ForceUpgrade { return errors.New("current Tiller version is newer, use --force-upgrade to downgrade") } obj.Spec.Template.Spec.Containers[0].Image = opts.selectImage() @@ -82,15 +82,24 @@ func Upgrade(client kubernetes.Interface, opts *Options) error { return err } -// isNewerVersion returns whether the current version is newer than the give image's version -func isNewerVersion(image string) bool { +// semverCompare returns whether the client's version is older, equal or newer than the given image's version. +func semverCompare(image string) int { split := strings.Split(image, ":") if len(split) < 2 { - // If we don't know the version, we consider the current version newer - return true + // If we don't know the version, we consider the client version newer. + return 1 } - imageVersion := split[1] - return semver.MustParse(version.Version).GreaterThan(semver.MustParse(imageVersion)) + tillerVersion, err := semver.NewVersion(split[1]) + if err != nil { + // same thing with unparsable tiller versions (e.g. canary releases). + return 1 + } + clientVersion, err := semver.NewVersion(version.Version) + if err != nil { + // aaaaaand same thing with unparsable helm versions (e.g. canary releases). + return 1 + } + return clientVersion.Compare(tillerVersion) } // createDeployment creates the Tiller Deployment resource. diff --git a/cmd/helm/installer/install_test.go b/cmd/helm/installer/install_test.go index 4a6f5f8eb..eaea05870 100644 --- a/cmd/helm/installer/install_test.go +++ b/cmd/helm/installer/install_test.go @@ -476,6 +476,129 @@ func TestUgrade_newerVersion(t *testing.T) { } } +func TestUpgrade_identical(t *testing.T) { + image := "gcr.io/kubernetes-helm/tiller:v2.0.0" + serviceAccount := "newServiceAccount" + existingDeployment, _ := deployment(&Options{ + Namespace: v1.NamespaceDefault, + ImageSpec: "imageToReplace:v2.0.0", + ServiceAccount: "serviceAccountToReplace", + UseCanary: false, + }) + existingService := service(v1.NamespaceDefault) + + fc := &fake.Clientset{} + fc.AddReactor("get", "deployments", func(action testcore.Action) (bool, runtime.Object, error) { + return true, existingDeployment, nil + }) + fc.AddReactor("update", "deployments", func(action testcore.Action) (bool, runtime.Object, error) { + obj := action.(testcore.UpdateAction).GetObject().(*v1beta1.Deployment) + i := obj.Spec.Template.Spec.Containers[0].Image + if i != image { + t.Errorf("expected image = '%s', got '%s'", image, i) + } + sa := obj.Spec.Template.Spec.ServiceAccountName + if sa != serviceAccount { + t.Errorf("expected serviceAccountName = '%s', got '%s'", serviceAccount, sa) + } + return true, obj, nil + }) + fc.AddReactor("get", "services", func(action testcore.Action) (bool, runtime.Object, error) { + return true, existingService, nil + }) + + opts := &Options{Namespace: v1.NamespaceDefault, ImageSpec: image, ServiceAccount: serviceAccount} + if err := Upgrade(fc, opts); err != nil { + t.Errorf("unexpected error: %#+v", err) + } + + if actions := fc.Actions(); len(actions) != 3 { + t.Errorf("unexpected actions: %v, expected 3 actions got %d", actions, len(actions)) + } +} + +func TestUpgrade_canaryClient(t *testing.T) { + image := "gcr.io/kubernetes-helm/tiller:canary" + serviceAccount := "newServiceAccount" + existingDeployment, _ := deployment(&Options{ + Namespace: v1.NamespaceDefault, + ImageSpec: "imageToReplace:v1.0.0", + ServiceAccount: "serviceAccountToReplace", + UseCanary: false, + }) + existingService := service(v1.NamespaceDefault) + + fc := &fake.Clientset{} + fc.AddReactor("get", "deployments", func(action testcore.Action) (bool, runtime.Object, error) { + return true, existingDeployment, nil + }) + fc.AddReactor("update", "deployments", func(action testcore.Action) (bool, runtime.Object, error) { + obj := action.(testcore.UpdateAction).GetObject().(*v1beta1.Deployment) + i := obj.Spec.Template.Spec.Containers[0].Image + if i != image { + t.Errorf("expected image = '%s', got '%s'", image, i) + } + sa := obj.Spec.Template.Spec.ServiceAccountName + if sa != serviceAccount { + t.Errorf("expected serviceAccountName = '%s', got '%s'", serviceAccount, sa) + } + return true, obj, nil + }) + fc.AddReactor("get", "services", func(action testcore.Action) (bool, runtime.Object, error) { + return true, existingService, nil + }) + + opts := &Options{Namespace: v1.NamespaceDefault, ImageSpec: image, ServiceAccount: serviceAccount} + if err := Upgrade(fc, opts); err != nil { + t.Errorf("unexpected error: %#+v", err) + } + + if actions := fc.Actions(); len(actions) != 3 { + t.Errorf("unexpected actions: %v, expected 3 actions got %d", actions, len(actions)) + } +} + +func TestUpgrade_canaryServer(t *testing.T) { + image := "gcr.io/kubernetes-helm/tiller:v2.0.0" + serviceAccount := "newServiceAccount" + existingDeployment, _ := deployment(&Options{ + Namespace: v1.NamespaceDefault, + ImageSpec: "imageToReplace:canary", + ServiceAccount: "serviceAccountToReplace", + UseCanary: false, + }) + existingService := service(v1.NamespaceDefault) + + fc := &fake.Clientset{} + fc.AddReactor("get", "deployments", func(action testcore.Action) (bool, runtime.Object, error) { + return true, existingDeployment, nil + }) + fc.AddReactor("update", "deployments", func(action testcore.Action) (bool, runtime.Object, error) { + obj := action.(testcore.UpdateAction).GetObject().(*v1beta1.Deployment) + i := obj.Spec.Template.Spec.Containers[0].Image + if i != image { + t.Errorf("expected image = '%s', got '%s'", image, i) + } + sa := obj.Spec.Template.Spec.ServiceAccountName + if sa != serviceAccount { + t.Errorf("expected serviceAccountName = '%s', got '%s'", serviceAccount, sa) + } + return true, obj, nil + }) + fc.AddReactor("get", "services", func(action testcore.Action) (bool, runtime.Object, error) { + return true, existingService, nil + }) + + opts := &Options{Namespace: v1.NamespaceDefault, ImageSpec: image, ServiceAccount: serviceAccount} + if err := Upgrade(fc, opts); err != nil { + t.Errorf("unexpected error: %#+v", err) + } + + if actions := fc.Actions(); len(actions) != 3 { + t.Errorf("unexpected actions: %v, expected 3 actions got %d", actions, len(actions)) + } +} + func tlsTestFile(t *testing.T, path string) string { const tlsTestDir = "../../../testdata" path = filepath.Join(tlsTestDir, path)