KEP-29: Handle changed dependencies when upgrading operators

pkg/controller/instance/instance_controller.go

@@ -50,7 +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/dependencies"
+	"github.com/kudobuilder/kudo/pkg/kudoctl/resources/dependencies"
 	"github.com/kudobuilder/kudo/pkg/util/convert"
 )
 

pkg/kudoctl/cmd/upgrade.go

@@ -109,5 +109,7 @@ func runUpgrade(args []string, options *options, fs afero.Fs, settings *env.Sett
 
 	resources := pkg.Resources
 
-	return upgrade.OperatorVersion(kc, resources.OperatorVersion, options.InstanceName, settings.Namespace, options.Parameters, resolver)
+	resources.OperatorVersion.SetNamespace(settings.Namespace)
+
+	return upgrade.OperatorVersion(kc, resources.OperatorVersion, options.InstanceName, options.Parameters, resolver)
 }

pkg/kudoctl/packages/install/package.go

@@ -5,10 +5,8 @@ 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/dependencies"
 	"github.com/kudobuilder/kudo/pkg/kudoctl/packages/resolver"
 	"github.com/kudobuilder/kudo/pkg/kudoctl/resources/install"
 	"github.com/kudobuilder/kudo/pkg/kudoctl/util/kudo"
@@ -62,37 +60,8 @@ func Package(
 		}
 	}
 
-	dependencies, err := dependencies.Resolve(resources.OperatorVersion, 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 := install.OperatorAndOperatorVersion(
-			client, dependency.Resources.Operator, dependency.Resources.OperatorVersion); err != nil {
-			return err
-		}
-	}
-
 	if err := install.OperatorAndOperatorVersion(
-		client, resources.Operator, resources.OperatorVersion); err != nil {
+		client, resources.Operator, resources.OperatorVersion, resolver); err != nil {
 		return err
 	}
 
@@ -158,24 +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(
-	dependencies []dependencies.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
-}

pkg/kudoctl/resources/dependencies/operatorversion.go

@@ -0,0 +1,27 @@
+package dependencies
+
+import (
+	"github.com/kudobuilder/kudo/pkg/apis/kudo/v1beta1"
+	engtask "github.com/kudobuilder/kudo/pkg/engine/task"
+)
+
+// 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 []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
+}

pkg/kudoctl/resources/install/operator.go

@@ -7,14 +7,19 @@ import (
 
 	"github.com/kudobuilder/kudo/pkg/apis/kudo/v1beta1"
 	"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) error {
+	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(
@@ -36,6 +41,14 @@ func OperatorAndOperatorVersion(
 	}
 
 	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 {
@@ -52,3 +65,67 @@ func OperatorAndOperatorVersion(
 
 	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.
+
+	deps.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) {
+			deps.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
+}

pkg/kudoctl/resources/upgrade/operatorversion.go

@@ -4,11 +4,9 @@ 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/kudoctl/packages/dependencies"
 	"github.com/kudobuilder/kudo/pkg/kudoctl/packages/resolver"
 	"github.com/kudobuilder/kudo/pkg/kudoctl/resources/install"
 	"github.com/kudobuilder/kudo/pkg/kudoctl/util/kudo"
@@ -21,12 +19,10 @@ func OperatorVersion(
 	kc *kudo.Client,
 	newOv *v1beta1.OperatorVersion,
 	instanceName string,
-	namespace string,
 	parameters map[string]string,
 	resolver resolver.Resolver) error {
-	operatorName := newOv.Spec.Operator.Name
 
-	ov, err := operatorVersionFromInstance(kc, instanceName, namespace)
+	ov, err := operatorVersionFromInstance(kc, instanceName, newOv.Namespace)
 	if err != nil {
 		return err
 	}
@@ -35,29 +31,20 @@ func OperatorVersion(
 		return err
 	}
 
-	versionsInstalled, err := kc.OperatorVersionsInstalled(operatorName, namespace)
+	o, err := kc.GetOperator(newOv.Spec.Operator.Name, newOv.Namespace)
 	if err != nil {
-		return fmt.Errorf("failed to retrieve operatorversions: %v", err)
+		return fmt.Errorf("failed to retrieve operator %s/%s: %v", newOv.Namespace, newOv.Spec.Operator.Name, err)
 	}
 
-	if !funk.ContainsString(versionsInstalled, newOv.Spec.Version) {
-		if err := installDependencies(kc, newOv, namespace, resolver); err != nil {
-			return fmt.Errorf("failed to install dependencies of operatorversion %s/%s: %v", namespace, newOv.Name, err)
-		}
-
-		if _, err := kc.InstallOperatorVersionObjToCluster(newOv, namespace); err != nil {
-			return fmt.Errorf(
-				"failed to update operatorversion %s/%s to version %s: %v", namespace, newOv.Name, newOv.Spec.Version, err)
-		}
-
-		clog.Printf("operatorversion %s/%s created", namespace, newOv.Name)
+	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, namespace, convert.StringPtr(newOv.Name), parameters, nil, false, 0); err != nil {
-		return fmt.Errorf("failed to update instance for new operatorversion %s/%s", namespace, newOv.Name)
+	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", namespace, instanceName)
+	clog.Printf("instance %s/%s updated", newOv.Namespace, instanceName)
 	return nil
 }
 
@@ -106,36 +93,3 @@ func compareVersions(old string, new string) error {
 
 	return nil
 }
-
-func installDependencies(
-	kc *kudo.Client,
-	ov *v1beta1.OperatorVersion,
-	namespace string,
-	resolver resolver.Resolver) error {
-	dependencies, err := dependencies.Resolve(ov, resolver)
-	if err != nil {
-		return err
-	}
-
-	for _, dependency := range dependencies {
-		installed, err := kc.OperatorVersionsInstalled(dependency.Operator.Name, namespace)
-		if err != nil {
-			return fmt.Errorf(
-				"failed to retrieve operatorversion of dependency %s/%s: %v",
-				namespace,
-				dependency.OperatorVersion.Name,
-				err)
-		}
-
-		if !funk.ContainsString(installed, dependency.OperatorVersion.Spec.Version) {
-			dependency.Operator.SetNamespace(namespace)
-			dependency.OperatorVersion.SetNamespace(namespace)
-
-			if err := install.OperatorAndOperatorVersion(kc, dependency.Operator, dependency.OperatorVersion); err != nil {
-				return err
-			}
-		}
-	}
-
-	return nil
-}