Introduce common controller

To remove duplicate code across the test-operator controllers,
this patch introduces a common controller. This change should make
fixing controller bugs easier and prepare the test-operator for
new resource addition, if it is needed in the future.

Author: kstrenkova

api/v1beta1/ansibletest_types.go

@@ -220,3 +220,8 @@ func (instance AnsibleTest) RbacNamespace() string {
 func (instance AnsibleTest) RbacResourceName() string {
 	return instance.Name
 }
+
+// GetConditions - return the conditions from the status
+func (instance *AnsibleTest) GetConditions() *condition.Conditions {
+	return &instance.Status.Conditions
+}

api/v1beta1/horizontest_types.go

@@ -191,3 +191,8 @@ func (instance HorizonTest) RbacNamespace() string {
 func (instance HorizonTest) RbacResourceName() string {
 	return instance.Name
 }
+
+// GetConditions - return the conditions from the status
+func (instance *HorizonTest) GetConditions() *condition.Conditions {
+	return &instance.Status.Conditions
+}

api/v1beta1/tempest_types.go

@@ -522,3 +522,8 @@ func (instance Tempest) RbacNamespace() string {
 func (instance Tempest) RbacResourceName() string {
 	return instance.Name
 }
+
+// GetConditions - return the conditions from the status
+func (instance *Tempest) GetConditions() *condition.Conditions {
+	return &instance.Status.Conditions
+}

api/v1beta1/tobiko_types.go

@@ -246,3 +246,8 @@ func (instance Tobiko) RbacNamespace() string {
 func (instance Tobiko) RbacResourceName() string {
 	return instance.Name
 }
+
+// GetConditions - return the conditions from the status
+func (instance *Tobiko) GetConditions() *condition.Conditions {
+	return &instance.Status.Conditions
+}

internal/controller/ansibletest_controller.go

@@ -19,18 +19,13 @@ package controller
 
 import (
 	"context"
-	"fmt"
-	"strconv"
 
 	"github.com/go-logr/logr"
-	"github.com/openstack-k8s-operators/lib-common/modules/common"
 	"github.com/openstack-k8s-operators/lib-common/modules/common/condition"
 	"github.com/openstack-k8s-operators/lib-common/modules/common/env"
-	"github.com/openstack-k8s-operators/lib-common/modules/common/helper"
 	testv1beta1 "github.com/openstack-k8s-operators/test-operator/api/v1beta1"
 	"github.com/openstack-k8s-operators/test-operator/internal/ansibletest"
 	corev1 "k8s.io/api/core/v1"
-	k8s_errors "k8s.io/apimachinery/pkg/api/errors"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/log"
 )
@@ -56,222 +51,84 @@ func (r *AnsibleTestReconciler) GetLogger(ctx context.Context) logr.Logger {
 // +kubebuilder:rbac:groups="",resources=persistentvolumeclaims,verbs=get;list;create;update;watch;patch;delete
 
 // Reconcile - AnsibleTest
-func (r *AnsibleTestReconciler) Reconcile(ctx context.Context, req ctrl.Request) (result ctrl.Result, _err error) {
-	Log := r.GetLogger(ctx)
-
-	// Fetch the ansible instance
+func (r *AnsibleTestReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
 	instance := &testv1beta1.AnsibleTest{}
-	err := r.Client.Get(ctx, req.NamespacedName, instance)
-	if err != nil {
-		if k8s_errors.IsNotFound(err) {
-			return ctrl.Result{}, nil
-		}
-		return ctrl.Result{}, err
-	}
-
-	// Create a helper
-	helper, err := helper.NewHelper(
-		instance,
-		r.Client,
-		r.Kclient,
-		r.Scheme,
-		r.Log,
-	)
-	if err != nil {
-		return ctrl.Result{}, err
-	}
-
-	// initialize status
-	isNewInstance := instance.Status.Conditions == nil
-	if isNewInstance {
-		instance.Status.Conditions = condition.Conditions{}
-	}
-
-	// Save a copy of the conditions so that we can restore the LastTransitionTime
-	// when a condition's state doesn't change.
-	savedConditions := instance.Status.Conditions.DeepCopy()
-
-	// Always patch the instance status when exiting this function so we
-	// can persist any changes.
-	defer func() {
-		// Don't update the status, if reconciler Panics
-		if r := recover(); r != nil {
-			Log.Info(fmt.Sprintf("panic during reconcile %v\n", r))
-			panic(r)
-		}
-		condition.RestoreLastTransitionTimes(&instance.Status.Conditions, savedConditions)
-		if instance.Status.Conditions.IsUnknown(condition.ReadyCondition) {
-			instance.Status.Conditions.Set(
-				instance.Status.Conditions.Mirror(condition.ReadyCondition))
-		}
-		err := helper.PatchInstance(ctx, instance)
-		if err != nil {
-			_err = err
-			return
-		}
-	}()
-
-	if isNewInstance {
-		// Initialize conditions used later as Status=Unknown
-		cl := condition.CreateList(
-			condition.UnknownCondition(condition.ReadyCondition, condition.InitReason, condition.ReadyInitMessage),
-			condition.UnknownCondition(condition.InputReadyCondition, condition.InitReason, condition.InputReadyInitMessage),
-			condition.UnknownCondition(condition.DeploymentReadyCondition, condition.InitReason, condition.DeploymentReadyInitMessage),
-		)
-		instance.Status.Conditions.Init(&cl)
 
-		// Register overall status immediately to have an early feedback
-		// e.g. in the cli
-		return ctrl.Result{}, nil
+	config := FrameworkConfig[*testv1beta1.AnsibleTest]{
+		ServiceName:             ansibletest.ServiceName,
+		NeedsNetworkAttachments: false,
+		NeedsConfigMaps:         false,
+		NeedsFinalizer:          false,
+		SupportsWorkflow:        true,
+
+		BuildPod: func(ctx context.Context, instance *testv1beta1.AnsibleTest, labels, annotations map[string]string, workflowStepNum int, pvcIndex int) (*corev1.Pod, error) {
+			return r.buildAnsibleTestPod(ctx, instance, labels, annotations, workflowStepNum, pvcIndex)
+		},
+
+		GetInitialConditions: func() []*condition.Condition {
+			return []*condition.Condition{
+				condition.UnknownCondition(condition.ReadyCondition, condition.InitReason, condition.ReadyInitMessage),
+				condition.UnknownCondition(condition.InputReadyCondition, condition.InitReason, condition.InputReadyInitMessage),
+				condition.UnknownCondition(condition.DeploymentReadyCondition, condition.InitReason, condition.DeploymentReadyInitMessage),
+			}
+		},
+
+		ValidateInputs: func(ctx context.Context, instance *testv1beta1.AnsibleTest) error {
+			return r.ValidateOpenstackInputs(ctx, instance, instance.Spec.OpenStackConfigMap, instance.Spec.OpenStackConfigSecret)
+		},
+
+		GetSpec: func(instance *testv1beta1.AnsibleTest) interface{} {
+			return &instance.Spec
+		},
+
+		GetWorkflowStep: func(instance *testv1beta1.AnsibleTest, step int) interface{} {
+			return instance.Spec.Workflow[step]
+		},
+
+		GetWorkflowLength: func(instance *testv1beta1.AnsibleTest) int {
+			return len(instance.Spec.Workflow)
+		},
+
+		GetStorageClass: func(instance *testv1beta1.AnsibleTest) string {
+			return instance.Spec.StorageClass
+		},
+
+		SetObservedGeneration: func(instance *testv1beta1.AnsibleTest) {
+			instance.Status.ObservedGeneration = instance.Generation
+		},
 	}
-	instance.Status.ObservedGeneration = instance.Generation
-
-	workflowLength := len(instance.Spec.Workflow)
-	nextAction, nextWorkflowStep, err := r.NextAction(ctx, instance, workflowLength)
-	if nextWorkflowStep < workflowLength {
-		MergeSections(&instance.Spec, instance.Spec.Workflow[nextWorkflowStep])
-	}
-
-	switch nextAction {
-	case Failure:
-		return ctrl.Result{}, err
-
-	case Wait:
-		Log.Info(InfoWaitingOnPod)
-		return ctrl.Result{RequeueAfter: RequeueAfterValue}, nil
-
-	case EndTesting:
-		// All pods created by the instance were completed. Release the lock
-		// so that other instances can spawn their pods.
-		if lockReleased, err := r.ReleaseLock(ctx, instance); !lockReleased {
-			Log.Info(fmt.Sprintf(InfoCanNotReleaseLock, testOperatorLockName))
-			return ctrl.Result{RequeueAfter: RequeueAfterValue}, err
-		}
-
-		instance.Status.Conditions.MarkTrue(
-			condition.DeploymentReadyCondition,
-			condition.DeploymentReadyMessage)
-
-		if instance.Status.Conditions.AllSubConditionIsTrue() {
-			instance.Status.Conditions.MarkTrue(condition.ReadyCondition, condition.ReadyMessage)
-		}
-
-		Log.Info(InfoTestingCompleted)
-		return ctrl.Result{}, nil
-
-	case CreateFirstPod:
-		lockAcquired, err := r.AcquireLock(ctx, instance, helper, false)
-		if !lockAcquired {
-			Log.Info(fmt.Sprintf(InfoCanNotAcquireLock, testOperatorLockName))
-			return ctrl.Result{RequeueAfter: RequeueAfterValue}, err
-		}
-
-		Log.Info(fmt.Sprintf(InfoCreatingFirstPod, nextWorkflowStep))
-
-	case CreateNextPod:
-		// Confirm that we still hold the lock. This is useful to check if for
-		// example somebody / something deleted the lock and it got claimed by
-		// another instance. This is considered to be an error state.
-		lockAcquired, err := r.AcquireLock(ctx, instance, helper, false)
-		if !lockAcquired {
-			Log.Error(err, ErrConfirmLockOwnership, testOperatorLockName)
-			return ctrl.Result{RequeueAfter: RequeueAfterValue}, err
-		}
 
-		Log.Info(fmt.Sprintf(InfoCreatingNextPod, nextWorkflowStep))
-
-	default:
-		return ctrl.Result{}, ErrReceivedUnexpectedAction
-	}
-
-	serviceLabels := map[string]string{
-		common.AppSelector: ansibletest.ServiceName,
-		workflowStepLabel:  strconv.Itoa(nextWorkflowStep),
-		instanceNameLabel:  instance.Name,
-		operatorNameLabel:  "test-operator",
-	}
-
-	err = r.ValidateOpenstackInputs(ctx, instance, instance.Spec.OpenStackConfigMap, instance.Spec.OpenStackConfigSecret)
-	if err != nil {
-		instance.Status.Conditions.Set(condition.FalseCondition(
-			condition.InputReadyCondition,
-			condition.ErrorReason,
-			condition.SeverityError,
-			condition.InputReadyErrorMessage,
-			err.Error()))
-		return ctrl.Result{RequeueAfter: RequeueAfterValue}, err
-	}
-	instance.Status.Conditions.MarkTrue(condition.InputReadyCondition, condition.InputReadyMessage)
-
-	// Create PersistentVolumeClaim
-	ctrlResult, err := r.EnsureLogsPVCExists(
-		ctx,
-		instance,
-		helper,
-		serviceLabels,
-		instance.Spec.StorageClass,
-		0,
-	)
-	if err != nil {
-		return ctrlResult, err
-	} else if (ctrlResult != ctrl.Result{}) {
-		return ctrlResult, nil
-	}
-	// Create PersistentVolumeClaim - end
+	return CommonReconcile(ctx, &r.Reconciler, req, instance, config, r.GetLogger(ctx))
+}
 
-	// Create a new pod
+func (r *AnsibleTestReconciler) buildAnsibleTestPod(
+	ctx context.Context,
+	instance *testv1beta1.AnsibleTest,
+	labels, _ map[string]string,
+	workflowStepNum int,
+	pvcIndex int,
+) (*corev1.Pod, error) {
 	mountCerts := r.CheckSecretExists(ctx, instance, "combined-ca-bundle")
-	podName := r.GetPodName(instance, nextWorkflowStep)
 	envVars := r.PrepareAnsibleEnv(instance)
-	logsPVCName := r.GetPVCLogsName(instance, 0)
+
+	podName := r.GetPodName(instance, workflowStepNum)
+	logsPVCName := r.GetPVCLogsName(instance, pvcIndex)
+
 	containerImage, err := r.GetContainerImage(ctx, instance)
 	if err != nil {
-		return ctrl.Result{}, err
+		return nil, err
 	}
 
-	podDef := ansibletest.Pod(
+	return ansibletest.Pod(
 		instance,
-		serviceLabels,
+		labels,
 		podName,
 		logsPVCName,
 		mountCerts,
 		envVars,
-		nextWorkflowStep,
+		workflowStepNum,
 		containerImage,
-	)
-
-	ctrlResult, err = r.CreatePod(ctx, *helper, podDef)
-	if err != nil {
-		// Creation of the ansibleTests pod was not successful.
-		// Release the lock and allow other controllers to spawn
-		// a pod.
-		if lockReleased, err := r.ReleaseLock(ctx, instance); !lockReleased {
-			return ctrl.Result{}, err
-		}
-
-		instance.Status.Conditions.Set(condition.FalseCondition(
-			condition.DeploymentReadyCondition,
-			condition.ErrorReason,
-			condition.SeverityWarning,
-			condition.DeploymentReadyErrorMessage,
-			err.Error()))
-		return ctrlResult, err
-	} else if (ctrlResult != ctrl.Result{}) {
-		instance.Status.Conditions.Set(condition.FalseCondition(
-			condition.DeploymentReadyCondition,
-			condition.RequestedReason,
-			condition.SeverityInfo,
-			condition.DeploymentReadyRunningMessage))
-		return ctrlResult, nil
-	}
-	// Create a new pod - end
-
-	if instance.Status.Conditions.AllSubConditionIsTrue() {
-		instance.Status.Conditions.MarkTrue(condition.ReadyCondition, condition.ReadyMessage)
-	}
-
-	Log.Info("Reconciled Service successfully")
-	return ctrl.Result{}, nil
+	), nil
 }
 
 // SetupWithManager sets up the controller with the Manager.