Merge pull request dana-team#151 from mzeevi/bug/fixCappRevision

fixed buggy behavior on CappRevision creation

api/v1alpha1/capprevision_types.go

@@ -44,7 +44,7 @@ type CappTemplate struct {
 
 	// Annotations is a map of string keys and values which are the actual annotations of the related Capp
 	// +optional
-	Annotations map[string]string `json:"Annotations,omitempty"`
+	Annotations map[string]string `json:"annotations,omitempty"`
 }
 
 //+kubebuilder:object:root=true

config/crd/bases/rcs.dana.io_capprevisions.yaml

@@ -43,7 +43,7 @@ spec:
                 description: CappTemplate holds the manifest of a specific Capp corresponding
                   to a particular RevisionNumber
                 properties:
-                  Annotations:
+                  annotations:
                     additionalProperties:
                       type: string
                     description: Annotations is a map of string keys and values which

internal/kinds/capprevision/actionmanagers/update.go

@@ -4,6 +4,8 @@ import (
 	"context"
 	"sort"
 
+	"k8s.io/apimachinery/pkg/api/equality"
+
 	cappv1alpha1 "github.com/dana-team/container-app-operator/api/v1alpha1"
 	"github.com/dana-team/container-app-operator/internal/kinds/capprevision/adapters"
 	"github.com/go-logr/logr"
@@ -33,11 +35,40 @@ func sortByCreationTime(cappRevisions []cappv1alpha1.CappRevision) {
 	})
 }
 
+// equalAnnotations returns a boolean indicating whether two Capp specs are equal.
+func equalSpec(cappSpec, revisionCappSpec cappv1alpha1.CappSpec) bool {
+	return equality.Semantic.DeepEqual(cappSpec, revisionCappSpec)
+}
+
+// equalAnnotations returns a boolean indicating whether two annotation maps are equal.
+func equalAnnotations(cappAnnotations, revisionCappAnnotations map[string]string) bool {
+	return equality.Semantic.DeepEqual(cappAnnotations, revisionCappAnnotations)
+}
+
+// equalLabels returns a boolean indicating whether two label maps are equal.
+func equalLabels(cappLabels, revisionCappLabels map[string]string) bool {
+	return equality.Semantic.DeepEqual(cappLabels, revisionCappLabels)
+}
+
+// isEqual returns a boolean indicating whether a Capp instance is equal to a Capp Revision instance.
+// The comparison concerts the Capp Spec of the two instances, the Capp annotations and Capp labels.
+func isEqual(capp cappv1alpha1.Capp, revision cappv1alpha1.CappRevision) bool {
+	return equalSpec(capp.Spec, revision.Spec.CappTemplate.Spec) &&
+		equalAnnotations(capp.Annotations, revision.Spec.CappTemplate.Annotations) &&
+		equalLabels(capp.Labels, revision.Spec.CappTemplate.Labels)
+}
+
 // HandleCappUpdate manages the flow of CappRevision when a Capp is updated. It ensures that a CappRevision is created for every update.
 // It also maintains a limit of only revisionsToKeep CappRevisions in the same namespace as the Capp.
 func HandleCappUpdate(ctx context.Context, k8sClient client.Client, capp cappv1alpha1.Capp, logger logr.Logger, cappRevisions []cappv1alpha1.CappRevision) error {
 	sortByCreationTime(cappRevisions)
 	numOfRevisions := len(cappRevisions)
+
+	latestRevision := cappRevisions[0]
+	if isEqual(capp, latestRevision) {
+		return nil
+	}
+
 	if numOfRevisions < revisionsToKeep {
 		return adapters.CreateCappRevision(ctx, k8sClient, logger, capp, cappRevisions[0].Spec.RevisionNumber+1)
 	}

internal/kinds/capprevision/controllers/controller.go

@@ -3,7 +3,6 @@ package controllers
 import (
 	"context"
 	"fmt"
-	"reflect"
 	"time"
 
 	cappv1alpha1 "github.com/dana-team/container-app-operator/api/v1alpha1"
@@ -15,9 +14,7 @@ import (
 	"k8s.io/client-go/tools/record"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
-	"sigs.k8s.io/controller-runtime/pkg/event"
 	"sigs.k8s.io/controller-runtime/pkg/log"
-	"sigs.k8s.io/controller-runtime/pkg/predicate"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 )
 
@@ -37,29 +34,10 @@ type CappRevisionReconciler struct {
 // +kubebuilder:rbac:groups=rcs.dana.io,resources=capprevisions,verbs=get;list;watch;create;update;patch;delete
 // +kubebuilder:rbac:groups=rcs.dana.io,resources=capprevisions/status,verbs=get;update;patch
 
-type SpecChangedPredicate struct {
-	predicate.Funcs
-}
-
-// Update implements default UpdateEvent filter for checking label change.
-func (SpecChangedPredicate) Update(e event.UpdateEvent) bool {
-	if e.ObjectOld == nil {
-		return false
-	}
-	if e.ObjectNew == nil {
-		return false
-	}
-	newCapp := e.ObjectNew.(*cappv1alpha1.Capp)
-	oldCapp := e.ObjectOld.(*cappv1alpha1.Capp)
-	return !reflect.DeepEqual(oldCapp.Spec, newCapp.Spec)
-
-}
-
 // SetupWithManager sets up the controller with the Manager.
 func (r *CappRevisionReconciler) SetupWithManager(mgr ctrl.Manager) error {
 	return ctrl.NewControllerManagedBy(mgr).
 		For(&cappv1alpha1.Capp{}).
-		WithEventFilter(predicate.Or(predicate.AnnotationChangedPredicate{}, predicate.LabelChangedPredicate{}, SpecChangedPredicate{})).
 		Complete(r)
 }
 
@@ -69,7 +47,7 @@ func (r *CappRevisionReconciler) Reconcile(ctx context.Context, req reconcile.Re
 	capp := cappv1alpha1.Capp{}
 	if err := r.Client.Get(ctx, req.NamespacedName, &capp); err != nil {
 		if errors.IsNotFound(err) {
-			logger.Info("Didn't find Capp")
+			logger.Info("Capp does not exist")
 			return ctrl.Result{}, nil
 		}
 		return ctrl.Result{}, fmt.Errorf("failed to get Capp: %s", err.Error())