Handle updates to the applicationgroup spec (#84)

* Reflect workflow phase in appgroup status Closes #82 Signed-off-by: nitishm <nitishm@microsoft.com> * Handle updates to the applicationgroup spec Handle updates to the spec by reapply Workflows eventually leading to a helmrelease version upgrade. Signed-off-by: nitishm <nitishm@microsoft.com> Co-authored-by: nitishm <nitishm@microsoft.com>
2021-03-02 11:40:43 -08:00 · 2021-03-02 11:40:43 -08:00 · 57e7335d44
--- a/api/v1alpha1/appgroup_types.go
+++ b/api/v1alpha1/appgroup_types.go
@ -72,6 +72,7 @@ type ApplicationGroupStatus struct {
 	Checksums    map[string]string   `json:"checksums,omitempty"`
 	Applications []ApplicationStatus `json:"status,omitempty"`
 	Phase        v1alpha12.NodePhase `json:"phase,omitempty"`
+	Update       bool                `json:"update,omitempty"`
 	Error        string              `json:"error,omitempty"`
 }

--- a/config/crd/bases/orkestra.azure.microsoft.com_applicationgroups.yaml
+++ b/config/crd/bases/orkestra.azure.microsoft.com_applicationgroups.yaml
@ -395,6 +395,8 @@ spec:
                  - name
                  type: object
                type: array
+              update:
+                type: boolean
            type: object
        type: object
    served: true
--- a/controllers/appgroup_controller.go
+++ b/controllers/appgroup_controller.go
@ -5,6 +5,7 @@ package controllers

 import (
 	"context"
+	"errors"
 	"fmt"
 	"net/url"
 	"strings"
@ -17,7 +18,7 @@ import (
 	v1alpha12 "github.com/argoproj/argo/pkg/apis/workflow/v1alpha1"
 	"github.com/go-logr/logr"
 	"helm.sh/helm/v3/pkg/chart"
-	"k8s.io/apimachinery/pkg/api/errors"
+	kerrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/client-go/tools/record"
 	ctrl "sigs.k8s.io/controller-runtime"
@ -71,7 +72,7 @@ func (r *ApplicationGroupReconciler) Reconcile(req ctrl.Request) (ctrl.Result, e
 	logr := r.Log.WithValues(appgroupNameKey, req.NamespacedName.Name)

 	if err := r.Get(ctx, req.NamespacedName, &appGroup); err != nil {
-		if errors.IsNotFound(err) {
+		if kerrors.IsNotFound(err) {
 			logr.V(3).Info("skip reconciliation since AppGroup instance not found on the cluster")
 			return ctrl.Result{}, nil
 		}
@ -85,6 +86,7 @@ func (r *ApplicationGroupReconciler) Reconcile(req ctrl.Request) (ctrl.Result, e
 		if appGroup.Finalizers != nil {
 			logr.Info("Cleaning up")
 			// TODO: Take remediation action
+			// Reverse the entire workflow to remove all the Helm Releases
 			appGroup.Finalizers = nil
 			_ = r.Update(ctx, &appGroup)
 			return ctrl.Result{Requeue: true}, nil
@ -93,29 +95,8 @@ func (r *ApplicationGroupReconciler) Reconcile(req ctrl.Request) (ctrl.Result, e
 		return ctrl.Result{Requeue: false}, nil
 	}

-	// Initialize the Status fields if not already setup
-	if len(appGroup.Status.Applications) == 0 {
-		appGroup.Status.Applications = make([]orkestrav1alpha1.ApplicationStatus, 0, len(appGroup.Spec.Applications))
-		for _, app := range appGroup.Spec.Applications {
-			status := orkestrav1alpha1.ApplicationStatus{
-				Name:        app.Name,
-				ChartStatus: orkestrav1alpha1.ChartStatus{Version: app.Spec.Version},
-				Subcharts:   make(map[string]orkestrav1alpha1.ChartStatus),
-			}
-			appGroup.Status.Applications = append(appGroup.Status.Applications, status)
-		}
-	}
-
-	// Initialize fields in the Application spec for every app in the appgroup
-	v := orkestrav1alpha1.ApplicationGroup{}
-	for _, app := range appGroup.Spec.Applications {
-		if app.Spec.Overlays.Data == nil {
-			app.Spec.Overlays.Data = make(map[string]interface{})
-		}
-		app.Spec.Values = app.Spec.Overlays
-		v.Spec.Applications = append(v.Spec.Applications, app)
-	}
-	appGroup.Spec.Applications = v.DeepCopy().Spec.Applications
+	// Initialize all the application specs and status fields embedded in the application group
+	initApplications(&appGroup)

 	// Add finalizer if it doesnt already exist
 	if appGroup.Finalizers == nil {
@ -125,19 +106,34 @@ func (r *ApplicationGroupReconciler) Reconcile(req ctrl.Request) (ctrl.Result, e
 	}

 	// handle UPDATE if checksum mismatched
-	_, checksums, err := pkg.Checksum(&appGroup)
+	checksums, err := pkg.Checksum(&appGroup)
 	if err != nil {
 		// TODO (nitishm) Handle different error types here to decide remediation action
-
-		if checksums != nil {
+		if errors.Is(err, pkg.ErrChecksumAppGroupSpecMismatch) {
 			appGroup.Status.Checksums = checksums
+			appGroup.Status.Update = true
+			requeue, err = r.reconcile(ctx, logr, r.WorkflowNS, &appGroup)
+			defer r.updateStatusAndEvent(ctx, appGroup, requeue, err)
+			if err != nil {
+				logr.Error(err, "failed to reconcile ApplicationGroup instance")
+				return ctrl.Result{Requeue: requeue}, err
+			}
+
+			if appGroup.Status.Phase != v1alpha12.NodeSucceeded {
+				return ctrl.Result{Requeue: true, RequeueAfter: requeueAfter}, nil
+			}
+
+			return ctrl.Result{Requeue: false}, nil
 		}
+
 		appGroup.Status.Error = err.Error()
 		_ = r.Status().Update(ctx, &appGroup)
 		logr.Error(err, "failed to calculate checksum annotations for application group specs")
 		return ctrl.Result{Requeue: false}, err
 	}

+	appGroup.Status.Checksums = checksums
+
 	// Lookup Workflow by ownership and heritage labels
 	wfs := v1alpha12.WorkflowList{}
 	listOption := client.MatchingLabels{
@ -158,8 +154,6 @@ func (r *ApplicationGroupReconciler) Reconcile(req ctrl.Request) (ctrl.Result, e

 	logr = logr.WithValues("phase", appGroup.Status.Phase, "status-error", appGroup.Status.Error)

-	appGroup.Status.Checksums = checksums
-
 	switch appGroup.Status.Phase {
 	case v1alpha12.NodeRunning, v1alpha12.NodePending:
 		logr.V(1).Info("workflow in pending/running state. requeue and reconcile after a short period")
@ -178,16 +172,6 @@ func (r *ApplicationGroupReconciler) Reconcile(req ctrl.Request) (ctrl.Result, e
 		return ctrl.Result{Requeue: false}, err
 	}

-	requeue, err = r.reconcile(ctx, logr, r.WorkflowNS, &appGroup)
-	defer r.updateStatusAndEvent(ctx, appGroup, requeue, err)
-	if err != nil {
-		logr.Error(err, "failed to reconcile ApplicationGroup instance")
-		return ctrl.Result{Requeue: requeue}, err
-	}
-
-	if appGroup.Status.Phase != v1alpha12.NodeSucceeded {
-		return ctrl.Result{Requeue: true, RequeueAfter: requeueAfter}, nil
-	}
 	return ctrl.Result{Requeue: false}, nil
 }

@ -223,7 +207,7 @@ func isDependenciesEmbedded(ch *chart.Chart) bool {
 	isURI := false
 	for _, d := range ch.Metadata.Dependencies {
 		if _, err := url.ParseRequestURI(d.Repository); err == nil {
-			// If this is an " assembled" chart (https://helm.sh/docs/chart_best_practices/dependencies/#versions) we must stage the embedded subchart
+			// If this is an "assembled" chart (https://helm.sh/docs/chart_best_practices/dependencies/#versions) we must stage the embedded subchart
 			if strings.Contains(d.Repository, "file://") {
 				isURI = false
 				break
@ -239,3 +223,29 @@ func isDependenciesEmbedded(ch *chart.Chart) bool {
 	}
 	return false
 }
+
+func initApplications(appGroup *orkestrav1alpha1.ApplicationGroup) {
+	// Initialize the Status fields if not already setup
+	if len(appGroup.Status.Applications) == 0 {
+		appGroup.Status.Applications = make([]orkestrav1alpha1.ApplicationStatus, 0, len(appGroup.Spec.Applications))
+		for _, app := range appGroup.Spec.Applications {
+			status := orkestrav1alpha1.ApplicationStatus{
+				Name:        app.Name,
+				ChartStatus: orkestrav1alpha1.ChartStatus{Version: app.Spec.Version},
+				Subcharts:   make(map[string]orkestrav1alpha1.ChartStatus),
+			}
+			appGroup.Status.Applications = append(appGroup.Status.Applications, status)
+		}
+	}
+
+	// Initialize fields in the Application spec for every app in the appgroup
+	v := orkestrav1alpha1.ApplicationGroup{}
+	for _, app := range appGroup.Spec.Applications {
+		if app.Spec.Overlays.Data == nil {
+			app.Spec.Overlays.Data = make(map[string]interface{})
+		}
+		app.Spec.Values = app.Spec.Overlays
+		v.Spec.Applications = append(v.Spec.Applications, app)
+	}
+	appGroup.Spec.Applications = v.DeepCopy().Spec.Applications
+}
--- a/pkg/checksum.go
+++ b/pkg/checksum.go
@ -4,26 +4,21 @@ import (
 	"errors"
 	"fmt"
 	"strconv"
-	"strings"

 	"github.com/Azure/Orkestra/api/v1alpha1"
 	"github.com/mitchellh/hashstructure/v2"
 )

 const (
-	appGroupCsumKey                = "checksum/application-group-spec"
-	applicationCsumKeyPrefix       = "checksum/application-spec-"
-	applicationValuesCsumKeyPrefix = "checksum/application-values-"
+	appGroupCsumKey = "application-group-spec"
 )

 var (
 	ErrChecksumGenerateFailure      = errors.New("checksum generate failure")
 	ErrChecksumAppGroupSpecMismatch = errors.New("application group spec checksum mismatch")
-	ErrChecksumAppSpecMismatch      = errors.New("application spec checksum mismatch")
-	ErrChecksumAppValuesMismatch    = errors.New("application values checksum mismatch")
 )

-func Checksum(ag *v1alpha1.ApplicationGroup) (bool, map[string]string, error) {
+func Checksum(ag *v1alpha1.ApplicationGroup) (map[string]string, error) {
 	var (
 		// reconcile bool = false
 		err  error
@ -32,46 +27,20 @@ func Checksum(ag *v1alpha1.ApplicationGroup) (bool, map[string]string, error) {

 	h, err := hash(ag.Spec)
 	if err != nil {
-		return false, nil, fmt.Errorf("%s : %w", err.Error(), ErrChecksumGenerateFailure)
+		return nil, fmt.Errorf("%s : %w", err.Error(), ErrChecksumGenerateFailure)
 	}

 	csum[appGroupCsumKey] = h

-	for _, application := range ag.Spec.Applications {
-		applicationHash, err2 := hash(application.Spec)
-		if err2 != nil {
-			return false, nil, ErrChecksumGenerateFailure
-		}
-
-		valuesHash, err2 := hash(application.Spec.Overlays)
-		if err2 != nil {
-			return false, nil, ErrChecksumGenerateFailure
-		}
-
-		csum[applicationCsumKeyPrefix+application.Name] = applicationHash
-		csum[applicationValuesCsumKeyPrefix+application.Name] = valuesHash
+	if ag.Status.Checksums == nil {
+		return csum, ErrChecksumAppGroupSpecMismatch
 	}

-	if ag.Status.Checksums != nil {
-		for k, v := range csum {
-			if strings.Contains(k, applicationCsumKeyPrefix) {
-				if v != ag.Status.Checksums[k] {
-					return true, csum, ErrChecksumAppSpecMismatch
-				}
-			}
-
-			if strings.Contains(k, applicationValuesCsumKeyPrefix) {
-				if v != ag.Status.Checksums[k] {
-					return true, csum, ErrChecksumAppValuesMismatch
-				}
-			}
-		}
-		if csum[appGroupCsumKey] != ag.Status.Checksums[appGroupCsumKey] {
-			return true, csum, ErrChecksumAppGroupSpecMismatch
-		}
+	if csum[appGroupCsumKey] != ag.Status.Checksums[appGroupCsumKey] {
+		return csum, ErrChecksumAppGroupSpecMismatch
 	}

-	return false, csum, nil
+	return csum, nil
 }

 func hash(v interface{}) (string, error) {
--- a/pkg/workflow/argo.go
+++ b/pkg/workflow/argo.go
@ -126,6 +126,37 @@ func (a *argo) Submit(ctx context.Context, l logr.Logger, g *v1alpha1.Applicatio
 			return fmt.Errorf("failed to GET workflow object with an unrecoverable error : %w", err)
 		}
 	}
+
+	// If the workflow needs an update, delete the previous workflow and apply the new one
+	// Argo Workflow does not rerun the workflow on UPDATE, so intead we cleanup and reapply
+	if g.Status.Update {
+		err = a.cli.Delete(ctx, obj)
+		if err != nil {
+			l.Error(err, "failed to DELETE argo workflow object")
+			return fmt.Errorf("failed to DELETE argo workflow object : %w", err)
+		}
+		// If the argo Workflow object is Found on the cluster
+		// update the workflow and submit it to the cluster
+		// Add OwnershipReference
+		err = controllerutil.SetControllerReference(g, a.wf, a.scheme)
+		if err != nil {
+			l.Error(err, "unable to set ApplicationGroup as owner of Argo Workflow object")
+			return fmt.Errorf("unable to set ApplicationGroup as owner of Argo Workflow: %w", err)
+		}
+
+		a.wf.Labels[OwnershipLabel] = g.Name
+
+		// If the argo Workflow object is NotFound and not AlreadyExists on the cluster
+		// create a new object and submit it to the cluster
+		err = a.cli.Create(ctx, a.wf)
+		if err != nil {
+			l.Error(err, "failed to CREATE argo workflow object")
+			return fmt.Errorf("failed to CREATE argo workflow object : %w", err)
+		}
+
+		g.Status.Update = false
+	}
+
 	return nil
 }

@ -449,7 +480,7 @@ func defaultExecutor() v1alpha12.Template {
 		Outputs: v1alpha12.Outputs{},
 		Resource: &v1alpha12.ResourceTemplate{
 			// SetOwnerReference: true,
-			Action:           "create",
+			Action:           "apply",
 			Manifest:         "{{inputs.parameters.helmrelease}}",
 			SuccessCondition: "status.phase == Succeeded",
 		},