KEP-29: Add KudoOperatorTask implementation (#1541)

Summary:
implemented `KudoTaskOperator` which, given a `KudoOperator` task in the operator will create the `Instance` object and wait for it to become healthy. Additionally added `paramsFile` to the `KudoOperatorTaskSpec`.

Fixes: #1509
Signed-off-by: Aleksey Dukhovniy <alex.dukhovniy@googlemail.com>

config/crds/kudo.dev_operatorversions.yaml

@@ -183,6 +183,10 @@ spec:
                         type: boolean
                       instanceName:
                         type: string
+                      operatorName:
+                        description: name of installed operator. this field is set
+                          by the CLI and should not be set directly by the user
+                        type: string
                       operatorVersion:
                         description: a specific operator version in the official repo,
                           defaults to the most recent one
@@ -193,6 +197,11 @@ spec:
                         type: string
                       parameter:
                         type: string
+                      parameterFile:
+                        description: name of the template file (located in the `templates`
+                          folder) from which the *parent* instance generates a parameter
+                          file used to populate the *child* Instance.Spec.Parameters
+                        type: string
                       pipe:
                         items:
                           description: PipeSpec describes how a file generated by

keps/0029-operator-dependencies.md

@@ -23,7 +23,7 @@ status: provisional
   * [Non-Goals](#non-goals)
 * [Proposal](#proposal)
   * [Implementation Details](#implementation-details)
-    * [Operator Task](#operator-task)
+    * [KudoOperator Task](#kudooperator-task)
     * [Deployment](#deployment)
       * [Client-Side](#client-side)
       * [Server-Side](#server-side)
@@ -61,9 +61,9 @@ KUDO operators **already have** a mechanism to deal with installation dependenci
 
 ### Implementation details
 
-#### Operator Task
+#### KudoOperator Task
 
-A new task kind `Operator` is introduced which extends `operator.yaml` with the ability to install dependencies. Let's take a look at the Kafka+Zookeeper example:
+A new task kind `KudoOperator` is introduced which extends `operator.yaml` with the ability to install dependencies. Let's take a look at the Kafka+Zookeeper example:
 
 ```yaml
 apiVersion: kudo.dev/v1beta1
@@ -178,7 +178,7 @@ Operator `BB` has a required and empty parameter `PASSWORD`. To provide a way fo
 ```yaml
 tasks:
 - name: deploy-bb
-  kind: Operator
+  kind: KudoOperator
   spec:
     parameterFile: bb-params.yaml # optional, defines the parameter that will be set on the bb-instance  
 ```
@@ -224,7 +224,7 @@ spec:
     required: true
   tasks:
   - name: deploy-bb
-    kind: Operator
+    kind: KudoOperator
     spec:
       parameterFile: bb-params.yaml
   templates:
@@ -270,11 +270,11 @@ Current `kudo uninstall` CLI command only removes instances (with required `--in
 
 #### Other Plans
 
-It can be meaningful to allow [operator-tasks](#operator-task) outside of `deploy`, `update` and `upgrade` plans. A `monitoring` plan might install a monitoring operator package. We could even allow installation from a local disk by doing the same client-side steps for the `monitoring` plan when it is triggered. While the foundation provided by this KEP would make it easy, this KEP focuses on the installation dependencies, so we would probably forbid operator-tasks outside of `deploy`, `update` and `upgrade` in the beginning.
+It can be meaningful to allow [operator-tasks](#kudooperator-task) outside of `deploy`, `update` and `upgrade` plans. A `monitoring` plan might install a monitoring operator package. We could even allow installation from a local disk by doing the same client-side steps for the `monitoring` plan when it is triggered. While the foundation provided by this KEP would make it easy, this KEP focuses on the installation dependencies, so we would probably forbid operator-tasks outside of `deploy`, `update` and `upgrade` in the beginning.
 
 ### Risks and Mitigation
 
-The biggest risk is the increased complexity of the instance controller and the workflow engine. With the above approach, we can reuse much of the code and UX we have currently: plans and phases for flow control, local operators and custom operator repositories for easier development and deployment, and usual status reporting for debugging. The API footprint remains small as the only new API element is the [operator-task](#operator-task). Dependency graph building and traversal will require a graph library and there are a [few](https://github.com/yourbasic/graph) [out](https://godoc.org/github.com/twmb/algoimpl/go/graph) [there](https://godoc.org/gonum.org/v1/gonum/graph) so this will help mitigate some complexity.
+The biggest risk is the increased complexity of the instance controller and the workflow engine. With the above approach, we can reuse much of the code and UX we have currently: plans and phases for flow control, local operators and custom operator repositories for easier development and deployment, and usual status reporting for debugging. The API footprint remains small as the only new API element is the [operator-task](#kudooperator-task). Dependency graph building and traversal will require a graph library and there are a [few](https://github.com/yourbasic/graph) [out](https://godoc.org/github.com/twmb/algoimpl/go/graph) [there](https://godoc.org/gonum.org/v1/gonum/graph) so this will help mitigate some complexity.
 
 ## Alternatives
 

pkg/apis/kudo/v1beta1/instance_types_helpers.go

@@ -1,13 +1,16 @@
 package v1beta1
 
 import (
+	"context"
 	"encoding/json"
 	"fmt"
 	"log"
 
 	"github.com/thoas/go-funk"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/uuid"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
 const (
@@ -22,8 +25,9 @@ const (
 //   webhook
 // 2. GetPlanInProgress goes through i.Spec.PlanStatus map and returns the first found plan that is running
 //
-// (1) is set directly when the user updates the instance and reset **after** the plan is terminal
-// (2) is updated **after** each time the instance controller executes the plan
+// In (1), i.Spec.PlanExecution.PlanName is set directly when the user updates the instance and reset **after** the plan
+// is terminal
+// In (2) i.Spec.PlanStatus is updated **AFTER** the instance controller is done with the reconciliation call
 func (i *Instance) GetScheduledPlan() *PlanStatus {
 	return i.PlanStatus(i.Spec.PlanExecution.PlanName)
 }
@@ -222,6 +226,25 @@ func remove(values []string, s string) []string {
 	})
 }
 
+// GetOperatorVersion retrieves OperatorVersion belonging to the given instance
+func (i *Instance) GetOperatorVersion(c client.Reader) (ov *OperatorVersion, err error) {
+	return GetOperatorVersionByName(i.Spec.OperatorVersion.Name, i.OperatorVersionNamespace(), c)
+}
+
+func GetOperatorVersionByName(ovn, ns string, c client.Reader) (ov *OperatorVersion, err error) {
+	ov = &OperatorVersion{}
+	err = c.Get(context.TODO(),
+		types.NamespacedName{
+			Name:      ovn,
+			Namespace: ns,
+		},
+		ov)
+	if err != nil {
+		return nil, err
+	}
+	return ov, nil
+}
+
 // wasRunAfter returns true if p1 was run after p2
 func wasRunAfter(p1 PlanStatus, p2 PlanStatus) bool {
 	if p1.Status == ExecutionNeverRun || p2.Status == ExecutionNeverRun || p1.LastUpdatedTimestamp == nil || p2.LastUpdatedTimestamp == nil {

pkg/apis/kudo/v1beta1/operatorversion_types.go

@@ -202,6 +202,9 @@ type KudoOperatorTaskSpec struct {
 	// either repo package name, local package folder or an URL to package tarball
 	// +optional
 	Package string `json:"package,omitempty"`
+	// name of installed operator. this field is set by the CLI and should not be set directly by the user
+	// +optional
+	OperatorName string `json:"operatorName,omitempty"`
 	// +optional
 	InstanceName string `json:"instanceName,omitempty"`
 	// a specific app version in the official repo, defaults to the most recent
@@ -210,6 +213,10 @@ type KudoOperatorTaskSpec struct {
 	// a specific operator version in the official repo, defaults to the most recent one
 	// +optional
 	OperatorVersion string `json:"operatorVersion,omitempty"`
+	// name of the template file (located in the `templates` folder) from which the *parent* instance
+	// generates a parameter file used to populate the *child* Instance.Spec.Parameters
+	// +optional
+	ParameterFile string `json:"parameterFile,omitempty"`
 }
 
 // OperatorVersionStatus defines the observed state of OperatorVersion.

pkg/apis/kudo/v1beta1/operatorversion_types_helpers.go

@@ -0,0 +1,13 @@
+package v1beta1
+
+import (
+	"fmt"
+)
+
+func OperatorInstanceName(operatorName string) string {
+	return fmt.Sprintf("%s-instance", operatorName)
+}
+
+func OperatorVersionName(operatorName, version string) string {
+	return fmt.Sprintf("%s-%s", operatorName, version)
+}

pkg/controller/instance/instance_controller.go

@@ -177,7 +177,7 @@ func (r *Reconciler) Reconcile(request ctrl.Request) (ctrl.Result, error) {
 	}
 	oldInstance := instance.DeepCopy()
 
-	ov, err := GetOperatorVersion(instance, r.Client)
+	ov, err := instance.GetOperatorVersion(r.Client)
 	if err != nil {
 		err = fmt.Errorf("InstanceController: Error getting operatorVersion %s for instance %s/%s: %v",
 			instance.Spec.OperatorVersion.Name, instance.Namespace, instance.Name, err)
@@ -366,21 +366,6 @@ func (r *Reconciler) getInstance(request ctrl.Request) (instance *kudov1beta1.In
 	return instance, nil
 }
 
-// GetOperatorVersion retrieves OperatorVersion belonging to the given instance
-func GetOperatorVersion(instance *kudov1beta1.Instance, c client.Reader) (ov *kudov1beta1.OperatorVersion, err error) {
-	ov = &kudov1beta1.OperatorVersion{}
-	err = c.Get(context.TODO(),
-		types.NamespacedName{
-			Name:      instance.Spec.OperatorVersion.Name,
-			Namespace: instance.OperatorVersionNamespace(),
-		},
-		ov)
-	if err != nil {
-		return nil, err
-	}
-	return ov, nil
-}
-
 // ParamsMap generates {{ Params.* }} map of keys and values which is later used during template rendering.
 func ParamsMap(instance *kudov1beta1.Instance, operatorVersion *kudov1beta1.OperatorVersion) (map[string]interface{}, error) {
 	params := make(map[string]interface{}, len(operatorVersion.Spec.Parameters))

pkg/engine/health/health.go

@@ -76,15 +76,12 @@ func IsHealthy(obj runtime.Object) error {
 
 		return fmt.Errorf("job %q still running or failed", obj.Name)
 	case *kudov1beta1.Instance:
-		ps := obj.GetLastExecutedPlanStatus()
-		if ps == nil {
-			return fmt.Errorf("no plan has been executed for Instance %v", obj.Name)
-		}
-
-		if ps.Status.IsFinished() {
+		// if there is no scheduled plan, then we're done
+		if obj.Spec.PlanExecution.PlanName == "" {
 			return nil
 		}
-		return fmt.Errorf("instance's active plan is in state %v", ps.Status)
+
+		return fmt.Errorf("instance %s/%s active plan is in state %v", obj.Namespace, obj.Name, obj.Spec.PlanExecution.Status)
 
 	case *corev1.Pod:
 		if obj.Status.Phase == corev1.PodRunning {

pkg/engine/task/task.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"regexp"
 
+	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/client-go/discovery"
 	"k8s.io/client-go/rest"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -19,6 +20,7 @@ type Context struct {
 	Client     client.Client
 	Discovery  discovery.CachedDiscoveryInterface
 	Config     *rest.Config
+	Scheme     *runtime.Scheme
 	Enhancer   renderer.Enhancer
 	Meta       renderer.Metadata
 	Templates  map[string]string      // Raw templates
@@ -139,7 +141,7 @@ func newToggle(task *v1beta1.Task) (Tasker, error) {
 		return nil, errors.New("task validation error: toggle task has an empty resource list. if that's what you need, use a Dummy task instead")
 	}
 	// validate if the parameter is present
-	if len(task.Spec.ToggleTaskSpec.Parameter) == 0 {
+	if task.Spec.ToggleTaskSpec.Parameter == "" {
 		return nil, errors.New("task validation error: Missing parameter to evaluate the Toggle Task")
 	}
 	return ToggleTask{
@@ -187,15 +189,20 @@ func fatalExecutionError(cause error, eventName string, meta renderer.Metadata)
 
 func newKudoOperator(task *v1beta1.Task) (Tasker, error) {
 	// validate KudoOperatorTask
-	if len(task.Spec.KudoOperatorTaskSpec.Package) == 0 {
-		return nil, fmt.Errorf("task validation error: kudo operator task '%s' has an empty package name", task.Name)
+	if task.Spec.KudoOperatorTaskSpec.OperatorName == "" {
+		return nil, fmt.Errorf("task validation error: kudo operator task '%s' has an empty operator name", task.Name)
+	}
+
+	if task.Spec.KudoOperatorTaskSpec.OperatorVersion == "" {
+		return nil, fmt.Errorf("task validation error: kudo operator task '%s' has an empty operatorVersion", task.Name)
 	}
 
 	return KudoOperatorTask{
 		Name:            task.Name,
-		Package:         task.Spec.KudoOperatorTaskSpec.Package,
+		OperatorName:    task.Spec.KudoOperatorTaskSpec.OperatorName,
 		InstanceName:    task.Spec.KudoOperatorTaskSpec.InstanceName,
 		AppVersion:      task.Spec.KudoOperatorTaskSpec.AppVersion,
 		OperatorVersion: task.Spec.KudoOperatorTaskSpec.OperatorVersion,
+		ParameterFile:   task.Spec.KudoOperatorTaskSpec.ParameterFile,
 	}, nil
 }