From 1a707c4bd5dd24d632bee18331c4ebda3cf14c3b Mon Sep 17 00:00:00 2001 From: Jan Schlicht Date: Tue, 23 Jun 2020 13:19:51 +0200 Subject: [PATCH] KEP-29: Handle changed dependencies when upgrading operators (#1558) When upgrading operators with dependencies, its dependencies could have changed. In this case the Operator and OperatorVersion resources of the new dependencies are installed. Outdated OperatorVersion resources aren't removed. The code was refactored to decouple dependency handling from package installation and have separate packages to install and upgrade KUDO resources. Signed-off-by: Jan Schlicht Co-authored-by: Aleksey Dukhovniy --- .../instance/instance_controller.go | 26 +- pkg/kudoctl/cmd/upgrade.go | 6 +- pkg/kudoctl/packages/install/operator.go | 59 ----- pkg/kudoctl/packages/install/package.go | 57 +---- pkg/kudoctl/resources/dependencies/doc.go | 3 + .../dependencies/resolve.go} | 10 +- .../dependencies/resolve_test.go} | 4 +- .../install/instance.go | 7 +- pkg/kudoctl/resources/install/operator.go | 153 ++++++++++++ .../resources/upgrade/operatorversion.go | 95 ++++++++ .../resources/upgrade/operatorversion_test.go | 228 ++++++++++++++++++ pkg/kudoctl/util/kudo/upgrade.go | 63 ----- pkg/kudoctl/util/kudo/upgrade_test.go | 101 -------- .../upgrade-with-dependencies/00-assert.yaml | 11 + .../upgrade-with-dependencies/00-errors.yaml | 9 + .../upgrade-with-dependencies/00-install.yaml | 5 + .../upgrade-with-dependencies/01-assert.yaml | 21 ++ .../upgrade-with-dependencies/01-upgrade.yaml | 5 + .../dependency/operator.yaml | 24 ++ .../dependency/params.yaml | 6 + .../operator-1.0/operator.yaml | 24 ++ .../operator-1.0/params.yaml | 6 + .../operator-2.0/operator.yaml | 25 ++ .../operator-2.0/params.yaml | 6 + 24 files changed, 657 insertions(+), 297 deletions(-) delete mode 100644 pkg/kudoctl/packages/install/operator.go create mode 100644 pkg/kudoctl/resources/dependencies/doc.go rename pkg/kudoctl/{packages/install/dependencies.go => resources/dependencies/resolve.go} (94%) rename pkg/kudoctl/{packages/install/dependencies_test.go => resources/dependencies/resolve_test.go} (98%) rename pkg/kudoctl/{packages => resources}/install/instance.go (79%) create mode 100644 pkg/kudoctl/resources/install/operator.go create mode 100644 pkg/kudoctl/resources/upgrade/operatorversion.go create mode 100644 pkg/kudoctl/resources/upgrade/operatorversion_test.go delete mode 100644 pkg/kudoctl/util/kudo/upgrade.go delete mode 100644 pkg/kudoctl/util/kudo/upgrade_test.go create mode 100644 test/integration/upgrade-with-dependencies/00-assert.yaml create mode 100644 test/integration/upgrade-with-dependencies/00-errors.yaml create mode 100644 test/integration/upgrade-with-dependencies/00-install.yaml create mode 100644 test/integration/upgrade-with-dependencies/01-assert.yaml create mode 100644 test/integration/upgrade-with-dependencies/01-upgrade.yaml create mode 100644 test/integration/upgrade-with-dependencies/dependency/operator.yaml create mode 100644 test/integration/upgrade-with-dependencies/dependency/params.yaml create mode 100644 test/integration/upgrade-with-dependencies/operator-1.0/operator.yaml create mode 100644 test/integration/upgrade-with-dependencies/operator-1.0/params.yaml create mode 100644 test/integration/upgrade-with-dependencies/operator-2.0/operator.yaml create mode 100644 test/integration/upgrade-with-dependencies/operator-2.0/params.yaml diff --git a/pkg/controller/instance/instance_controller.go b/pkg/controller/instance/instance_controller.go index 16988bb1f..5979d6fb0 100644 --- a/pkg/controller/instance/instance_controller.go +++ b/pkg/controller/instance/instance_controller.go @@ -50,8 +50,7 @@ import ( "github.com/kudobuilder/kudo/pkg/engine/renderer" "github.com/kudobuilder/kudo/pkg/engine/task" "github.com/kudobuilder/kudo/pkg/engine/workflow" - "github.com/kudobuilder/kudo/pkg/kudoctl/packages" - "github.com/kudobuilder/kudo/pkg/kudoctl/packages/install" + "github.com/kudobuilder/kudo/pkg/kudoctl/resources/dependencies" "github.com/kudobuilder/kudo/pkg/util/convert" ) @@ -199,15 +198,13 @@ func (r *Reconciler) Reconcile(request ctrl.Request) (ctrl.Result, error) { r.Recorder.Event(instance, "Normal", "PlanStarted", fmt.Sprintf("Execution of plan %s started", plan)) } - // if this is a top-level instance, check for dependency cycles - if instance.IsTopLevelInstance() { - err := r.resolveDependencies(instance, ov) - if err != nil { - planStatus.SetWithMessage(kudov1beta1.ExecutionFatalError, err.Error()) - instance.UpdateInstanceStatus(planStatus, &metav1.Time{Time: time.Now()}) - err = r.handleError(err, instance, oldInstance) - return reconcile.Result{}, err - } + // check if all the dependencies can be resolved (if necessary) + err = r.resolveDependencies(instance, ov) + if err != nil { + planStatus.SetWithMessage(kudov1beta1.ExecutionFatalError, err.Error()) + instance.UpdateInstanceStatus(planStatus, &metav1.Time{Time: time.Now()}) + err = r.handleError(err, instance, oldInstance) + return reconcile.Result{}, err } // ---------- 3. Execute the scheduled plan ---------- @@ -255,10 +252,13 @@ func (r *Reconciler) Reconcile(request ctrl.Request) (ctrl.Result, error) { } func (r *Reconciler) resolveDependencies(i *kudov1beta1.Instance, ov *kudov1beta1.OperatorVersion) error { + // no need to check the dependencies if this is a child-level instance, as the top-level instance will take care of that + if i.IsChildInstance() { + return nil + } resolver := &InClusterResolver{ns: i.Namespace, c: r.Client} - root := packages.Resources{OperatorVersion: ov} - _, err := install.ResolveDependencies(root, resolver) + _, err := dependencies.Resolve(ov, resolver) if err != nil { return engine.ExecutionError{Err: fmt.Errorf("%w%v", engine.ErrFatalExecution, err), EventName: "CircularDependency"} } diff --git a/pkg/kudoctl/cmd/upgrade.go b/pkg/kudoctl/cmd/upgrade.go index c25f84c1a..3fdef4d93 100644 --- a/pkg/kudoctl/cmd/upgrade.go +++ b/pkg/kudoctl/cmd/upgrade.go @@ -11,7 +11,7 @@ import ( "github.com/kudobuilder/kudo/pkg/kudoctl/cmd/params" "github.com/kudobuilder/kudo/pkg/kudoctl/env" pkgresolver "github.com/kudobuilder/kudo/pkg/kudoctl/packages/resolver" - "github.com/kudobuilder/kudo/pkg/kudoctl/util/kudo" + "github.com/kudobuilder/kudo/pkg/kudoctl/resources/upgrade" "github.com/kudobuilder/kudo/pkg/kudoctl/util/repo" ) @@ -109,5 +109,7 @@ func runUpgrade(args []string, options *options, fs afero.Fs, settings *env.Sett resources := pkg.Resources - return kudo.UpgradeOperatorVersion(kc, resources.OperatorVersion, options.InstanceName, settings.Namespace, options.Parameters) + resources.OperatorVersion.SetNamespace(settings.Namespace) + + return upgrade.OperatorVersion(kc, resources.OperatorVersion, options.InstanceName, options.Parameters, resolver) } diff --git a/pkg/kudoctl/packages/install/operator.go b/pkg/kudoctl/packages/install/operator.go deleted file mode 100644 index d64c1bcec..000000000 --- a/pkg/kudoctl/packages/install/operator.go +++ /dev/null @@ -1,59 +0,0 @@ -package install - -import ( - "fmt" - - "github.com/thoas/go-funk" - - "github.com/kudobuilder/kudo/pkg/kudoctl/clog" - "github.com/kudobuilder/kudo/pkg/kudoctl/packages" - "github.com/kudobuilder/kudo/pkg/kudoctl/util/kudo" -) - -func installOperatorAndOperatorVersion(client *kudo.Client, resources packages.Resources) error { - if !client.OperatorExistsInCluster(resources.Operator.Name, resources.Operator.Namespace) { - if _, err := client.InstallOperatorObjToCluster(resources.Operator, resources.Operator.Namespace); err != nil { - return fmt.Errorf( - "failed to install operator %s/%s: %v", - resources.Operator.Namespace, - resources.Operator.Name, - err) - } - clog.Printf( - "operator %s/%s created", - resources.Operator.Namespace, - resources.Operator.Name) - } - - versionsInstalled, err := client.OperatorVersionsInstalled(resources.Operator.Name, resources.Operator.Namespace) - if err != nil { - return fmt.Errorf( - "failed to retrieve existing operator versions of operator %s/%s: %v", - resources.Operator.Namespace, - resources.Operator.Name, - err) - } - - if !funk.ContainsString(versionsInstalled, resources.OperatorVersion.Spec.Version) { - if _, err := client.InstallOperatorVersionObjToCluster( - resources.OperatorVersion, - resources.OperatorVersion.Namespace); err != nil { - return fmt.Errorf( - "failed to install operatorversion %s/%s: %v", - resources.OperatorVersion.Namespace, - resources.OperatorVersion.Name, - err) - } - clog.Printf( - "operatorversion %s/%s created", - resources.OperatorVersion.Namespace, - resources.OperatorVersion.Name) - } else { - clog.Printf( - "operatorversion %s/%s already installed", - resources.OperatorVersion.Namespace, - resources.OperatorVersion.Name) - } - - return nil -} diff --git a/pkg/kudoctl/packages/install/package.go b/pkg/kudoctl/packages/install/package.go index d3d07c2dc..011fd8361 100644 --- a/pkg/kudoctl/packages/install/package.go +++ b/pkg/kudoctl/packages/install/package.go @@ -5,10 +5,10 @@ import ( "time" "github.com/kudobuilder/kudo/pkg/apis/kudo/v1beta1" - engtask "github.com/kudobuilder/kudo/pkg/engine/task" "github.com/kudobuilder/kudo/pkg/kudoctl/clog" "github.com/kudobuilder/kudo/pkg/kudoctl/packages" "github.com/kudobuilder/kudo/pkg/kudoctl/packages/resolver" + "github.com/kudobuilder/kudo/pkg/kudoctl/resources/install" "github.com/kudobuilder/kudo/pkg/kudoctl/util/kudo" ) @@ -60,35 +60,8 @@ func Package( } } - dependencies, err := ResolveDependencies(resources, resolver) - if err != nil { - return err - } - - // The KUDO controller will create Instances for the dependencies. For this - // it needs to resolve the dependencies again from 'KudoOperatorTaskSpec'. - // But it cannot resolve packages like the CLI, because it may - // not have access to the referenced local files or URLs. - // It can however resolve the OperatorVersion from the name of the operator - // dependency. For this, we overwrite the 'Package' field describing - // dependencies in 'KudoOperatorTaskSpec' with the operator name of the - // dependency. This has to be done for the operator to install as well as in - // all of its dependencies. - - updateKudoOperatorTaskPackageNames(dependencies, resources.OperatorVersion) - - for _, dependency := range dependencies { - dependency.Operator.SetNamespace(namespace) - dependency.OperatorVersion.SetNamespace(namespace) - - updateKudoOperatorTaskPackageNames(dependencies, dependency.OperatorVersion) - - if err := installOperatorAndOperatorVersion(client, dependency.Resources); err != nil { - return err - } - } - - if err := installOperatorAndOperatorVersion(client, resources); err != nil { + if err := install.OperatorAndOperatorVersion( + client, resources.Operator, resources.OperatorVersion, resolver); err != nil { return err } @@ -96,12 +69,12 @@ func Package( return nil } - if err := installInstance(client, resources.Instance); err != nil { + if err := install.Instance(client, resources.Instance); err != nil { return err } if options.Wait != nil { - if err := waitForInstance(client, resources.Instance, *options.Wait); err != nil { + if err := install.WaitForInstance(client, resources.Instance, *options.Wait); err != nil { return err } } @@ -154,23 +127,3 @@ func validateParameters(instance v1beta1.Instance, parameters []v1beta1.Paramete return nil } - -// updateKudoOperatorTaskPackageNames sets the 'Package' and 'OperatorName' -// fields of the 'KudoOperatorTaskSpec' of an 'OperatorVersion' to the operator name -// initially referenced in the 'Package' field. -func updateKudoOperatorTaskPackageNames(pkgs []Dependency, operatorVersion *v1beta1.OperatorVersion) { - tasks := operatorVersion.Spec.Tasks - - for i := range tasks { - if tasks[i].Kind == engtask.KudoOperatorTaskKind { - for _, pkg := range pkgs { - if tasks[i].Spec.KudoOperatorTaskSpec.Package == pkg.PackageName { - tasks[i].Spec.KudoOperatorTaskSpec.Package = pkg.Operator.Name - break - } - } - } - } - - operatorVersion.Spec.Tasks = tasks -} diff --git a/pkg/kudoctl/resources/dependencies/doc.go b/pkg/kudoctl/resources/dependencies/doc.go new file mode 100644 index 000000000..5f06907e0 --- /dev/null +++ b/pkg/kudoctl/resources/dependencies/doc.go @@ -0,0 +1,3 @@ +// Package dependencies provides functions to resolve operator dependencies of +// operator packages. +package dependencies diff --git a/pkg/kudoctl/packages/install/dependencies.go b/pkg/kudoctl/resources/dependencies/resolve.go similarity index 94% rename from pkg/kudoctl/packages/install/dependencies.go rename to pkg/kudoctl/resources/dependencies/resolve.go index f760b6604..ead2461f8 100644 --- a/pkg/kudoctl/packages/install/dependencies.go +++ b/pkg/kudoctl/resources/dependencies/resolve.go @@ -1,4 +1,4 @@ -package install +package dependencies import ( "fmt" @@ -49,10 +49,14 @@ type Dependency struct { PackageName string } -// ResolveDependencies resolved all dependencies of a package. +// Resolve resolves all dependencies of an OperatorVersion. // Dependencies are resolved recursively. // Cyclic dependencies are detected and result in an error. -func ResolveDependencies(root packages.Resources, resolver pkgresolver.Resolver) ([]Dependency, error) { +func Resolve(operatorVersion *v1beta1.OperatorVersion, resolver pkgresolver.Resolver) ([]Dependency, error) { + root := packages.Resources{ + OperatorVersion: operatorVersion, + } + dependencies := []Dependency{ {Resources: root}, } diff --git a/pkg/kudoctl/packages/install/dependencies_test.go b/pkg/kudoctl/resources/dependencies/resolve_test.go similarity index 98% rename from pkg/kudoctl/packages/install/dependencies_test.go rename to pkg/kudoctl/resources/dependencies/resolve_test.go index 74825ca9a..121080a24 100644 --- a/pkg/kudoctl/packages/install/dependencies_test.go +++ b/pkg/kudoctl/resources/dependencies/resolve_test.go @@ -1,4 +1,4 @@ -package install +package dependencies import ( "fmt" @@ -188,7 +188,7 @@ func TestResolve(t *testing.T) { tt := tt t.Run(tt.name, func(t *testing.T) { resolver := nameResolver{tt.pkgs} - got, err := ResolveDependencies(*tt.pkgs[0].Resources, resolver) + got, err := Resolve(tt.pkgs[0].Resources.OperatorVersion, resolver) assert.Equal(t, err == nil, tt.wantErr == "") diff --git a/pkg/kudoctl/packages/install/instance.go b/pkg/kudoctl/resources/install/instance.go similarity index 79% rename from pkg/kudoctl/packages/install/instance.go rename to pkg/kudoctl/resources/install/instance.go index 82c0f6fa6..21e70c141 100644 --- a/pkg/kudoctl/packages/install/instance.go +++ b/pkg/kudoctl/resources/install/instance.go @@ -12,7 +12,9 @@ import ( "github.com/kudobuilder/kudo/pkg/kudoctl/util/kudo" ) -func installInstance(client *kudo.Client, instance *v1beta1.Instance) error { +// Instance installs a KUDO instance to a cluster. +// It returns an error if the namespace already contains an instance with the same name. +func Instance(client *kudo.Client, instance *v1beta1.Instance) error { existingInstance, err := client.GetInstance(instance.Name, instance.Namespace) if err != nil { return fmt.Errorf("failed to verify existing instance: %v", err) @@ -37,7 +39,8 @@ func installInstance(client *kudo.Client, instance *v1beta1.Instance) error { return nil } -func waitForInstance(client *kudo.Client, instance *v1beta1.Instance, timeout time.Duration) error { +// WaitForInstance waits for an amount of time for an instance to be "complete". +func WaitForInstance(client *kudo.Client, instance *v1beta1.Instance, timeout time.Duration) error { err := client.WaitForInstance(instance.Name, instance.Namespace, nil, timeout) if errors.Is(err, pollwait.ErrWaitTimeout) { clog.Printf("timeout waiting for instance %s/%s", instance.Namespace, instance.Name) diff --git a/pkg/kudoctl/resources/install/operator.go b/pkg/kudoctl/resources/install/operator.go new file mode 100644 index 000000000..0c6f4c523 --- /dev/null +++ b/pkg/kudoctl/resources/install/operator.go @@ -0,0 +1,153 @@ +package install + +import ( + "fmt" + + "github.com/thoas/go-funk" + + "github.com/kudobuilder/kudo/pkg/apis/kudo/v1beta1" + engtask "github.com/kudobuilder/kudo/pkg/engine/task" + "github.com/kudobuilder/kudo/pkg/kudoctl/clog" + "github.com/kudobuilder/kudo/pkg/kudoctl/packages/resolver" + deps "github.com/kudobuilder/kudo/pkg/kudoctl/resources/dependencies" + "github.com/kudobuilder/kudo/pkg/kudoctl/util/kudo" +) + +// OperatorAndOperatorVersion installs both of these object to a cluster. +// Operators can contain dependencies on other operators. In this case +// the O/OV of dependencies are installed as well. +func OperatorAndOperatorVersion( + client *kudo.Client, + operator *v1beta1.Operator, + operatorVersion *v1beta1.OperatorVersion, + resolver resolver.Resolver) error { + if !client.OperatorExistsInCluster(operator.Name, operator.Namespace) { + if _, err := client.InstallOperatorObjToCluster(operator, operator.Namespace); err != nil { + return fmt.Errorf( + "failed to install operator %s/%s: %v", + operator.Namespace, + operator.Name, + err) + } + clog.Printf("operator %s/%s created", operator.Namespace, operator.Name) + } + + versionsInstalled, err := client.OperatorVersionsInstalled(operator.Name, operator.Namespace) + if err != nil { + return fmt.Errorf( + "failed to retrieve existing operator versions of operator %s/%s: %v", + operator.Namespace, + operator.Name, + err) + } + + if !funk.ContainsString(versionsInstalled, operatorVersion.Spec.Version) { + if err := installDependencies(client, operatorVersion, resolver); err != nil { + return fmt.Errorf( + "failed to install dependencies of operatorversion %s/%s: %v", + operatorVersion.Namespace, + operatorVersion.Name, + err) + } + + if _, err := client.InstallOperatorVersionObjToCluster( + operatorVersion, + operatorVersion.Namespace); err != nil { + return fmt.Errorf( + "failed to install operatorversion %s/%s: %v", + operatorVersion.Namespace, + operatorVersion.Name, + err) + } + clog.Printf("operatorversion %s/%s created", operatorVersion.Namespace, operatorVersion.Name) + } else { + clog.Printf("operatorversion %s/%s already installed", operatorVersion.Namespace, operatorVersion.Name) + } + + return nil +} + +func installDependencies(client *kudo.Client, ov *v1beta1.OperatorVersion, resolver resolver.Resolver) error { + dependencies, err := deps.Resolve(ov, resolver) + if err != nil { + return err + } + + // The KUDO controller will create Instances for the dependencies. For this + // it needs to resolve the dependencies again from 'KudoOperatorTaskSpec'. + // But it cannot resolve packages like the CLI, because it may + // not have access to the referenced local files or URLs. + // It can however resolve the OperatorVersion from the name of the operator + // dependency. For this, we overwrite the 'Package' field describing + // dependencies in 'KudoOperatorTaskSpec' with the operator name of the + // dependency. This has to be done for the operator to upgrade as well as in + // all of its new dependencies. + + updateKudoOperatorTaskPackageNames(dependencies, ov) + + for _, dependency := range dependencies { + dependency.Operator.SetNamespace(ov.Namespace) + dependency.OperatorVersion.SetNamespace(ov.Namespace) + + if !client.OperatorExistsInCluster(dependency.Operator.Name, dependency.Operator.Namespace) { + if _, err := client.InstallOperatorObjToCluster(dependency.Operator, dependency.Operator.Namespace); err != nil { + return fmt.Errorf( + "failed to install operator %s/%s: %v", + dependency.Operator.Namespace, + dependency.Operator.Name, + err) + } + clog.Printf("operator %s/%s created", dependency.Operator.Namespace, dependency.Operator.Name) + } + + installed, err := client.OperatorVersionsInstalled(dependency.Operator.Name, dependency.Operator.Namespace) + if err != nil { + return fmt.Errorf( + "failed to retrieve operatorversion of dependency %s/%s: %v", + dependency.Operator.Namespace, + dependency.Operator.Name, + err) + } + + if !funk.ContainsString(installed, dependency.OperatorVersion.Spec.Version) { + updateKudoOperatorTaskPackageNames(dependencies, dependency.OperatorVersion) + + if _, err := client.InstallOperatorVersionObjToCluster( + dependency.OperatorVersion, + dependency.OperatorVersion.Namespace); err != nil { + return fmt.Errorf( + "failed to install operatorversion %s/%s: %v", + dependency.OperatorVersion.Namespace, + dependency.OperatorVersion.Name, + err) + } + clog.Printf( + "operatorversion %s/%s created", + dependency.OperatorVersion.Namespace, + dependency.OperatorVersion.Name) + } + } + + return nil +} + +// updateKudoOperatorTaskPackageNames sets the 'Package' and 'OperatorName' +// fields of the 'KudoOperatorTaskSpec' of an 'OperatorVersion' to the operator name +// initially referenced in the 'Package' field. +func updateKudoOperatorTaskPackageNames( + dependencies []deps.Dependency, operatorVersion *v1beta1.OperatorVersion) { + tasks := operatorVersion.Spec.Tasks + + for i := range tasks { + if tasks[i].Kind == engtask.KudoOperatorTaskKind { + for _, dependency := range dependencies { + if tasks[i].Spec.KudoOperatorTaskSpec.Package == dependency.PackageName { + tasks[i].Spec.KudoOperatorTaskSpec.Package = dependency.Operator.Name + break + } + } + } + } + + operatorVersion.Spec.Tasks = tasks +} diff --git a/pkg/kudoctl/resources/upgrade/operatorversion.go b/pkg/kudoctl/resources/upgrade/operatorversion.go new file mode 100644 index 000000000..ad137278a --- /dev/null +++ b/pkg/kudoctl/resources/upgrade/operatorversion.go @@ -0,0 +1,95 @@ +package upgrade + +import ( + "fmt" + + "github.com/Masterminds/semver" + + "github.com/kudobuilder/kudo/pkg/apis/kudo/v1beta1" + "github.com/kudobuilder/kudo/pkg/kudoctl/clog" + "github.com/kudobuilder/kudo/pkg/kudoctl/packages/resolver" + "github.com/kudobuilder/kudo/pkg/kudoctl/resources/install" + "github.com/kudobuilder/kudo/pkg/kudoctl/util/kudo" + "github.com/kudobuilder/kudo/pkg/util/convert" +) + +// OperatorVersion upgrades an OperatorVersion and its Instance. +// For the updated Instance, new parameters can be provided. +func OperatorVersion( + kc *kudo.Client, + newOv *v1beta1.OperatorVersion, + instanceName string, + parameters map[string]string, + resolver resolver.Resolver) error { + + ov, err := operatorVersionFromInstance(kc, instanceName, newOv.Namespace) + if err != nil { + return err + } + + if err := compareVersions(ov.Spec.Version, newOv.Spec.Version); err != nil { + return err + } + + o, err := kc.GetOperator(newOv.Spec.Operator.Name, newOv.Namespace) + if err != nil { + return fmt.Errorf("failed to retrieve operator %s/%s: %v", newOv.Namespace, newOv.Spec.Operator.Name, err) + } + + if err := install.OperatorAndOperatorVersion(kc, o, newOv, resolver); err != nil { + return fmt.Errorf("failed to install new operatorversion %s/%s: %v", newOv.Namespace, newOv.Name, err) + } + + if err = kc.UpdateInstance(instanceName, newOv.Namespace, convert.StringPtr(newOv.Name), parameters, nil, false, 0); err != nil { + return fmt.Errorf("failed to update instance for new operatorversion %s/%s: %v", newOv.Namespace, newOv.Name, err) + } + + clog.Printf("instance %s/%s updated", newOv.Namespace, instanceName) + return nil +} + +func operatorVersionFromInstance( + kc *kudo.Client, + instanceName string, + namespace string) (*v1beta1.OperatorVersion, error) { + instance, err := kc.GetInstance(instanceName, namespace) + if err != nil { + return nil, fmt.Errorf("failed to get instance: %v", err) + } + + if instance == nil { + return nil, fmt.Errorf("instance %s/%s does not exist in the cluster", namespace, instanceName) + } + + operatorVersionName := instance.Spec.OperatorVersion.Name + + operatorVersion, err := kc.GetOperatorVersion(operatorVersionName, namespace) + if err != nil { + return nil, fmt.Errorf( + "failed to retrieve existing operatorversion of instance %s/%s: %v", namespace, instanceName, err) + } + + if operatorVersion == nil { + return nil, fmt.Errorf("operatorversion %s/%s does not exist in the cluster", namespace, operatorVersionName) + } + + return operatorVersion, nil +} + +func compareVersions(old string, new string) error { + oldVersion, err := semver.NewVersion(old) + if err != nil { + return fmt.Errorf("failed to parse %s as semver: %v", old, err) + } + + newVersion, err := semver.NewVersion(new) + if err != nil { + return fmt.Errorf("failed to parse %s as semver: %v", new, err) + } + + if !oldVersion.LessThan(newVersion) { + return fmt.Errorf("upgraded version %s is the same or smaller as current version %s -> not upgrading", new, old) + } + + return nil +} diff --git a/pkg/kudoctl/resources/upgrade/operatorversion_test.go b/pkg/kudoctl/resources/upgrade/operatorversion_test.go new file mode 100644 index 000000000..9338766de --- /dev/null +++ b/pkg/kudoctl/resources/upgrade/operatorversion_test.go @@ -0,0 +1,228 @@ +package upgrade + +import ( + "fmt" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + v1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + kubefake "k8s.io/client-go/kubernetes/fake" + + "github.com/kudobuilder/kudo/pkg/apis/kudo/v1beta1" + "github.com/kudobuilder/kudo/pkg/client/clientset/versioned/fake" + engtask "github.com/kudobuilder/kudo/pkg/engine/task" + "github.com/kudobuilder/kudo/pkg/kudoctl/packages" + "github.com/kudobuilder/kudo/pkg/kudoctl/util/kudo" + util "github.com/kudobuilder/kudo/pkg/util/kudo" +) + +const ( + installNamespace = "default" +) + +func Test_UpgradeOperatorVersion(t *testing.T) { + testO := v1beta1.Operator{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "kudo.dev/v1beta1", + Kind: "Operator", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + }, + } + + testOv := v1beta1.OperatorVersion{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "kudo.dev/v1beta1", + Kind: "OperatorVersion", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-1.0", + }, + Spec: v1beta1.OperatorVersionSpec{ + Version: "1.0", + Operator: v1.ObjectReference{ + Name: "test", + }, + }, + } + + testInstance := v1beta1.Instance{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "kudo.dev/v1beta1", + Kind: "Instance", + }, + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + util.OperatorLabel: "test", + }, + Name: "test", + }, + Spec: v1beta1.InstanceSpec{ + OperatorVersion: v1.ObjectReference{ + Name: "test-1.0", + }, + }, + } + + tests := []struct { + name string + newVersion string + instanceExists bool + ovExists bool + errMessageContains string + }{ + {"instance does not exist", "1.1.1", false, true, "instance default/test does not exist in the cluster"}, + {"operatorversion does not exist", "1.1.1", true, false, "operatorversion default/test-1.0 does not exist in the cluster"}, + {"upgrade to same version", "1.0", true, true, "upgraded version 1.0 is the same or smaller"}, + {"upgrade to smaller version", "0.1", true, true, "upgraded version 0.1 is the same or smaller"}, + {"upgrade to smaller version", "1.1.1", true, true, ""}, + } + + for _, tt := range tests { + c := kudo.NewClientFromK8s(fake.NewSimpleClientset(), kubefake.NewSimpleClientset()) + if _, err := c.InstallOperatorObjToCluster(&testO, installNamespace); err != nil { + t.Errorf("%s: failed to install operator: %v", tt.name, err) + } + + if tt.instanceExists { + if _, err := c.InstallInstanceObjToCluster(&testInstance, installNamespace); err != nil { + t.Errorf("%s: failed to install instance: %v", tt.name, err) + } + } + if tt.ovExists { + if _, err := c.InstallOperatorVersionObjToCluster(&testOv, installNamespace); err != nil { + t.Errorf("%s: failed to install operator version: %v", tt.name, err) + } + } + newOv := testOv + newOv.Spec.Version = tt.newVersion + newOv.SetNamespace(installNamespace) + + err := OperatorVersion(c, &newOv, "test", nil, nil) + if err != nil { + if !strings.Contains(err.Error(), tt.errMessageContains) { + t.Errorf("%s: expected error '%s' but got '%v'", tt.name, tt.errMessageContains, err) + } + } else if tt.errMessageContains != "" { + t.Errorf("%s: expected no error but got %v", tt.name, err) + } else { + // the upgrade should have passed without error + instance, err := c.GetInstance(testInstance.Name, installNamespace) + if err != nil { + t.Errorf("%s: error when getting instance to verify the test: %v", tt.name, err) + } + expectedVersion := fmt.Sprintf("test-%s", tt.newVersion) + if instance.Spec.OperatorVersion.Name != expectedVersion { + t.Errorf("%s: instance has wrong version '%s', expected '%s'", tt.name, instance.Spec.OperatorVersion.Name, expectedVersion) + } + } + } +} + +type testResolver struct { + pkg packages.Package +} + +func (r *testResolver) Resolve(name string, appVersion string, operatorVersion string) (*packages.Package, error) { + return &r.pkg, nil +} + +func Test_UpgradeOperatorVersionWithDependency(t *testing.T) { + testO := v1beta1.Operator{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "kudo.dev/v1beta1", + Kind: "Operator", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + }, + } + + testOv := v1beta1.OperatorVersion{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "kudo.dev/v1beta1", + Kind: "OperatorVersion", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-1.0", + }, + Spec: v1beta1.OperatorVersionSpec{ + Version: "1.0", + Operator: v1.ObjectReference{ + Name: "test", + }, + }, + } + + testInstance := v1beta1.Instance{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "kudo.dev/v1beta1", + Kind: "Instance", + }, + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + util.OperatorLabel: "test", + }, + Name: "test", + }, + Spec: v1beta1.InstanceSpec{ + OperatorVersion: v1.ObjectReference{ + Name: "test-1.0", + }, + }, + } + + testDependency := packages.Package{ + Resources: &packages.Resources{ + Operator: &v1beta1.Operator{ + ObjectMeta: metav1.ObjectMeta{ + Name: "dependency", + }, + }, + OperatorVersion: &v1beta1.OperatorVersion{ + ObjectMeta: metav1.ObjectMeta{ + Name: "dependency-1.0", + }, + }, + }, + } + + c := kudo.NewClientFromK8s(fake.NewSimpleClientset(), kubefake.NewSimpleClientset()) + + _, err := c.InstallInstanceObjToCluster(&testInstance, installNamespace) + assert.NoError(t, err) + + _, err = c.InstallOperatorVersionObjToCluster(&testOv, installNamespace) + assert.NoError(t, err) + + _, err = c.InstallOperatorObjToCluster(&testO, installNamespace) + assert.NoError(t, err) + + newOv := testOv + newOv.Name = "test-1.1" + newOv.Spec.Version = "1.1" + newOv.Spec.Tasks = append(newOv.Spec.Tasks, v1beta1.Task{ + Name: "dependency", + Kind: engtask.KudoOperatorTaskKind, + Spec: v1beta1.TaskSpec{ + KudoOperatorTaskSpec: v1beta1.KudoOperatorTaskSpec{ + Package: "dependency", + }, + }, + }) + newOv.SetNamespace(installNamespace) + + resolver := &testResolver{testDependency} + + err = OperatorVersion(c, &newOv, "test", nil, resolver) + assert.NoError(t, err) + + assert.True(t, c.OperatorExistsInCluster("dependency", "default")) + + depOv, err := c.GetOperatorVersion("dependency-1.0", "default") + assert.NoError(t, err) + assert.NotNil(t, depOv) +} diff --git a/pkg/kudoctl/util/kudo/upgrade.go b/pkg/kudoctl/util/kudo/upgrade.go deleted file mode 100644 index abe310b7f..000000000 --- a/pkg/kudoctl/util/kudo/upgrade.go +++ /dev/null @@ -1,63 +0,0 @@ -package kudo - -import ( - "fmt" - - "github.com/Masterminds/semver" - "github.com/thoas/go-funk" - - "github.com/kudobuilder/kudo/pkg/apis/kudo/v1beta1" - "github.com/kudobuilder/kudo/pkg/kudoctl/clog" - "github.com/kudobuilder/kudo/pkg/util/convert" -) - -// UpgradeOperatorVersion upgrades an OperatorVersion and its Instance. -// For the updated Instance, new parameters can be provided. -func UpgradeOperatorVersion(kc *Client, newOv *v1beta1.OperatorVersion, instanceName, namespace string, parameters map[string]string) error { - operatorName := newOv.Spec.Operator.Name - - instance, err := kc.GetInstance(instanceName, namespace) - if err != nil { - return fmt.Errorf("failed to get instance: %v", err) - } - if instance == nil { - return fmt.Errorf("instance %s in namespace %s does not exist in the cluster", instanceName, namespace) - } - - ov, err := kc.GetOperatorVersion(instance.Spec.OperatorVersion.Name, namespace) - if err != nil { - return fmt.Errorf("failed to retrieve existing operator version: %v", err) - } - if ov == nil { - return fmt.Errorf("no operator version for this operator installed yet for %s in namespace %s. Please use install command if you want to install new operator into cluster", operatorName, namespace) - } - oldVersion, err := semver.NewVersion(ov.Spec.Version) - if err != nil { - return fmt.Errorf("failed to parse %s as semver: %v", ov.Spec.Version, err) - } - newVersion, err := semver.NewVersion(newOv.Spec.Version) - if err != nil { - return fmt.Errorf("failed to parse %s as semver: %v", newOv.Spec.Version, err) - } - if !oldVersion.LessThan(newVersion) { - return fmt.Errorf("upgraded version %s is the same or smaller as current version %s -> not upgrading", newOv.Spec.Version, ov.Spec.Version) - } - - versionsInstalled, err := kc.OperatorVersionsInstalled(operatorName, namespace) - if err != nil { - return fmt.Errorf("failed to retrieve operator versions: %v", err) - } - if !funk.ContainsString(versionsInstalled, newOv.Spec.Version) { - if _, err := kc.InstallOperatorVersionObjToCluster(newOv, namespace); err != nil { - return fmt.Errorf("failed to update %s-operatorversion.yaml to version %s: %v", operatorName, newOv.Spec.Version, err) - } - clog.Printf("operatorversion.%s/%s created", newOv.APIVersion, newOv.Name) - } - - if err = kc.UpdateInstance(instanceName, namespace, convert.StringPtr(newOv.Name), parameters, nil, false, 0); err != nil { - return fmt.Errorf("failed to update instance for new OperatorVersion %s", newOv.Name) - } - clog.Printf("instance.%s/%s updated", instance.APIVersion, instanceName) - - return nil -} diff --git a/pkg/kudoctl/util/kudo/upgrade_test.go b/pkg/kudoctl/util/kudo/upgrade_test.go deleted file mode 100644 index edb396969..000000000 --- a/pkg/kudoctl/util/kudo/upgrade_test.go +++ /dev/null @@ -1,101 +0,0 @@ -package kudo - -import ( - "fmt" - "strings" - "testing" - - v1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - kubefake "k8s.io/client-go/kubernetes/fake" - - "github.com/kudobuilder/kudo/pkg/apis/kudo/v1beta1" - "github.com/kudobuilder/kudo/pkg/client/clientset/versioned/fake" - util "github.com/kudobuilder/kudo/pkg/util/kudo" -) - -func Test_UpgradeOperatorVersion(t *testing.T) { - testOv := v1beta1.OperatorVersion{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "kudo.dev/v1beta1", - Kind: "OperatorVersion", - }, - ObjectMeta: metav1.ObjectMeta{ - Name: "test-1.0", - }, - Spec: v1beta1.OperatorVersionSpec{ - Version: "1.0", - Operator: v1.ObjectReference{ - Name: "test", - }, - }, - } - - testInstance := v1beta1.Instance{ - TypeMeta: metav1.TypeMeta{ - APIVersion: "kudo.dev/v1beta1", - Kind: "Instance", - }, - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{ - util.OperatorLabel: "test", - }, - Name: "test", - }, - Spec: v1beta1.InstanceSpec{ - OperatorVersion: v1.ObjectReference{ - Name: "test-1.0", - }, - }, - } - - installNamespace := "default" - tests := []struct { - name string - newVersion string - instanceExists bool - ovExists bool - errMessageContains string - }{ - {"instance does not exist", "1.1.1", false, true, "instance test in namespace default does not exist in the cluster"}, - {"operatorversion does not exist", "1.1.1", true, false, "no operator version for this operator installed yet"}, - {"upgrade to same version", "1.0", true, true, "upgraded version 1.0 is the same or smaller"}, - {"upgrade to smaller version", "0.1", true, true, "upgraded version 0.1 is the same or smaller"}, - {"upgrade to smaller version", "1.1.1", true, true, ""}, - } - - for _, tt := range tests { - c := NewClientFromK8s(fake.NewSimpleClientset(), kubefake.NewSimpleClientset()) - if tt.instanceExists { - if _, err := c.InstallInstanceObjToCluster(&testInstance, installNamespace); err != nil { - t.Errorf("%s: failed to install instance: %v", tt.name, err) - } - } - if tt.ovExists { - if _, err := c.InstallOperatorVersionObjToCluster(&testOv, installNamespace); err != nil { - t.Errorf("%s: failed to install operator version: %v", tt.name, err) - } - } - newOv := testOv - newOv.Spec.Version = tt.newVersion - - err := UpgradeOperatorVersion(c, &newOv, "test", "default", nil) - if err != nil { - if !strings.Contains(err.Error(), tt.errMessageContains) { - t.Errorf("%s: expected error '%s' but got '%v'", tt.name, tt.errMessageContains, err) - } - } else if tt.errMessageContains != "" { - t.Errorf("%s: expected no error but got %v", tt.name, err) - } else { - // the upgrade should have passed without error - instance, err := c.GetInstance(testInstance.Name, installNamespace) - if err != nil { - t.Errorf("%s: error when getting instance to verify the test: %v", tt.name, err) - } - expectedVersion := fmt.Sprintf("test-%s", tt.newVersion) - if instance.Spec.OperatorVersion.Name != expectedVersion { - t.Errorf("%s: instance has wrong version '%s', expected '%s'", tt.name, instance.Spec.OperatorVersion.Name, expectedVersion) - } - } - } -} diff --git a/test/integration/upgrade-with-dependencies/00-assert.yaml b/test/integration/upgrade-with-dependencies/00-assert.yaml new file mode 100644 index 000000000..18e68f8f3 --- /dev/null +++ b/test/integration/upgrade-with-dependencies/00-assert.yaml @@ -0,0 +1,11 @@ +apiVersion: kudo.dev/v1beta1 +kind: Instance +metadata: + name: operator-instance +spec: + operatorVersion: + name: operator-1.0 +status: + planStatus: + deploy: + status: COMPLETE \ No newline at end of file diff --git a/test/integration/upgrade-with-dependencies/00-errors.yaml b/test/integration/upgrade-with-dependencies/00-errors.yaml new file mode 100644 index 000000000..c8b1429b4 --- /dev/null +++ b/test/integration/upgrade-with-dependencies/00-errors.yaml @@ -0,0 +1,9 @@ +apiVersion: kudo.dev/v1beta1 +kind: Operator +metadata: + name: dependency +--- +apiVersion: kudo.dev/v1beta1 +kind: OperatorVersion +metadata: + name: dependency-1.0 diff --git a/test/integration/upgrade-with-dependencies/00-install.yaml b/test/integration/upgrade-with-dependencies/00-install.yaml new file mode 100644 index 000000000..dac351289 --- /dev/null +++ b/test/integration/upgrade-with-dependencies/00-install.yaml @@ -0,0 +1,5 @@ +apiVersion: kudo.dev/v1beta1 +kind: TestStep +commands: + - command: kubectl kudo install --instance operator-instance ./operator-1.0 + namespaced: true diff --git a/test/integration/upgrade-with-dependencies/01-assert.yaml b/test/integration/upgrade-with-dependencies/01-assert.yaml new file mode 100644 index 000000000..6567cc1ac --- /dev/null +++ b/test/integration/upgrade-with-dependencies/01-assert.yaml @@ -0,0 +1,21 @@ +apiVersion: kudo.dev/v1beta1 +kind: Instance +metadata: + name: operator-instance +spec: + operatorVersion: + name: operator-2.0 +status: + planStatus: + upgrade: + status: COMPLETE +--- +apiVersion: kudo.dev/v1beta1 +kind: Operator +metadata: + name: dependency +--- +apiVersion: kudo.dev/v1beta1 +kind: OperatorVersion +metadata: + name: dependency-1.0 diff --git a/test/integration/upgrade-with-dependencies/01-upgrade.yaml b/test/integration/upgrade-with-dependencies/01-upgrade.yaml new file mode 100644 index 000000000..f11c15375 --- /dev/null +++ b/test/integration/upgrade-with-dependencies/01-upgrade.yaml @@ -0,0 +1,5 @@ +apiVersion: kudo.dev/v1beta1 +kind: TestStep +commands: + - command: kubectl kudo upgrade --instance operator-instance ./operator-2.0 + namespaced: true diff --git a/test/integration/upgrade-with-dependencies/dependency/operator.yaml b/test/integration/upgrade-with-dependencies/dependency/operator.yaml new file mode 100644 index 000000000..d1eedf8df --- /dev/null +++ b/test/integration/upgrade-with-dependencies/dependency/operator.yaml @@ -0,0 +1,24 @@ +apiVersion: kudo.dev/v1beta1 +name: dependency +operatorVersion: "1.0" +kubernetesVersion: 1.15.0 +maintainers: + - name: nfnt + email: +url: https://kudo.dev +tasks: + - name: deploy + kind: Dummy + spec: + done: true + wantErr: false +plans: + deploy: + strategy: serial + phases: + - name: main + strategy: parallel + steps: + - name: deploy + tasks: + - deploy diff --git a/test/integration/upgrade-with-dependencies/dependency/params.yaml b/test/integration/upgrade-with-dependencies/dependency/params.yaml new file mode 100644 index 000000000..ea343c6c9 --- /dev/null +++ b/test/integration/upgrade-with-dependencies/dependency/params.yaml @@ -0,0 +1,6 @@ +apiVersion: kudo.dev/v1beta1 +parameters: + - name: deploy + description: Triggers the deploy plan + default: 1 + trigger: deploy diff --git a/test/integration/upgrade-with-dependencies/operator-1.0/operator.yaml b/test/integration/upgrade-with-dependencies/operator-1.0/operator.yaml new file mode 100644 index 000000000..e81e84f1d --- /dev/null +++ b/test/integration/upgrade-with-dependencies/operator-1.0/operator.yaml @@ -0,0 +1,24 @@ +apiVersion: kudo.dev/v1beta1 +name: operator +operatorVersion: "1.0" +kubernetesVersion: 1.15.0 +maintainers: + - name: nfnt + email: +url: https://kudo.dev +tasks: + - name: deploy + kind: Dummy + spec: + done: true + wantErr: false +plans: + deploy: + strategy: serial + phases: + - name: main + strategy: parallel + steps: + - name: deploy + tasks: + - deploy diff --git a/test/integration/upgrade-with-dependencies/operator-1.0/params.yaml b/test/integration/upgrade-with-dependencies/operator-1.0/params.yaml new file mode 100644 index 000000000..ea343c6c9 --- /dev/null +++ b/test/integration/upgrade-with-dependencies/operator-1.0/params.yaml @@ -0,0 +1,6 @@ +apiVersion: kudo.dev/v1beta1 +parameters: + - name: deploy + description: Triggers the deploy plan + default: 1 + trigger: deploy diff --git a/test/integration/upgrade-with-dependencies/operator-2.0/operator.yaml b/test/integration/upgrade-with-dependencies/operator-2.0/operator.yaml new file mode 100644 index 000000000..73b91facc --- /dev/null +++ b/test/integration/upgrade-with-dependencies/operator-2.0/operator.yaml @@ -0,0 +1,25 @@ +apiVersion: kudo.dev/v1beta1 +name: operator +operatorVersion: "2.0" +kubernetesVersion: 1.15.0 +maintainers: + - name: nfnt + email: +url: https://kudo.dev +tasks: + - name: dependency + kind: KudoOperator + spec: + package: "./dependency" + operatorVersion: "1.0" + +plans: + upgrade: + strategy: serial + phases: + - name: main + strategy: parallel + steps: + - name: deploy + tasks: + - dependency \ No newline at end of file diff --git a/test/integration/upgrade-with-dependencies/operator-2.0/params.yaml b/test/integration/upgrade-with-dependencies/operator-2.0/params.yaml new file mode 100644 index 000000000..ea343c6c9 --- /dev/null +++ b/test/integration/upgrade-with-dependencies/operator-2.0/params.yaml @@ -0,0 +1,6 @@ +apiVersion: kudo.dev/v1beta1 +parameters: + - name: deploy + description: Triggers the deploy plan + default: 1 + trigger: deploy