fix: take into account chart version when determine the apply strategy (#232)

Signed-off-by: Luca Burgazzoli <[email protected]>

Author: lburgazzoli

api/operator/v1alpha1/zz_generated.deepcopy.go

@@ -8,7 +8,7 @@ require (
 	github.com/gorilla/mux v1.8.1
 	github.com/hashicorp/go-cleanhttp v0.5.2
 	github.com/lburgazzoli/gomega-matchers v0.1.0
-	github.com/lburgazzoli/k8s-manifests-renderer-helm v0.1.1
+	github.com/lburgazzoli/k8s-manifests-renderer-helm v0.1.3
 	github.com/onsi/gomega v1.34.2
 	github.com/operator-framework/api v0.27.0
 	github.com/operator-framework/operator-lifecycle-manager v0.22.0

go.mod

@@ -245,8 +245,8 @@ github.com/lann/ps v0.0.0-20150810152359-62de8c46ede0 h1:P6pPBnrTSX3DEVR4fDembhR
 github.com/lann/ps v0.0.0-20150810152359-62de8c46ede0/go.mod h1:vmVJ0l/dxyfGW6FmdpVm2joNMFikkuWg0EoCKLGUMNw=
 github.com/lburgazzoli/gomega-matchers v0.1.0 h1:jkW45zWWGz6CP9EaUCKWcWgXiogiuHEsba28BMWv3IY=
 github.com/lburgazzoli/gomega-matchers v0.1.0/go.mod h1:MwRD1wEQrYBrON1pBYI/bwohSd3xeqflZa9nxXeSd9Q=
-github.com/lburgazzoli/k8s-manifests-renderer-helm v0.1.1 h1:vFpZ2tLT56bVNCtlzfI9z/FXAiVS1SRRUyPkogFhwlI=
-github.com/lburgazzoli/k8s-manifests-renderer-helm v0.1.1/go.mod h1:eqarsYWia91+fzg1Nzhol5rUO3WOEy6Z1acXr+1lCMs=
+github.com/lburgazzoli/k8s-manifests-renderer-helm v0.1.3 h1:P3VLDIraHPjJeD6tim0ueJu8nZslE171IB3ldRGDsQM=
+github.com/lburgazzoli/k8s-manifests-renderer-helm v0.1.3/go.mod h1:m3GWnxJgaLSTqc6k3EZZpjdDSIE+HqyjQicjgxn4xAo=
 github.com/lib/pq v1.10.9 h1:YXG7RB+JIjhP29X+OtkiDnYaXQwpS4JEWq7dtCCRUEw=
 github.com/lib/pq v1.10.9/go.mod h1:AlVN5x4E4T544tWzH6hKfbfQvm3HdbOxrmggDNAPY9o=
 github.com/liggitt/tabwriter v0.0.0-20181228230101-89fcab3d43de h1:9TO3cAIGXtEhnIaL+V+BEER86oLrvS+kWobKpbJuye0=

go.sum

@@ -51,6 +51,9 @@ func (a *ApplyCRDsAction) Run(ctx context.Context, rc *ReconciliationRequest) er
 		return fmt.Errorf("cannot load CRDs: %w", err)
 	}
 
+	invalidate := false
+	force := rc.Resource.Generation != rc.Resource.Status.ObservedGeneration || !helm.IsSameChart(c, rc.Resource.Status.Chart)
+
 	for _, crd := range crds {
 		resources.Labels(&crd, map[string]string{
 			helm.ReleaseGeneration: strconv.FormatInt(rc.Resource.Generation, 10),
@@ -59,14 +62,20 @@ func (a *ApplyCRDsAction) Run(ctx context.Context, rc *ReconciliationRequest) er
 			helm.ReleaseVersion:    c.Version(),
 		})
 
-		err = a.apply(ctx, rc, &crd)
+		applied, err := a.apply(ctx, rc, &crd, force)
 		if err != nil {
 			return err
 		}
+
+		if applied {
+			invalidate = true
+		}
 	}
 
-	// invalidate the client so it gets aware of the new CRDs
-	rc.Client.Invalidate()
+	if invalidate {
+		// invalidate the client so it gets aware of the new CRDs
+		rc.Client.Invalidate()
+	}
 
 	return nil
 }
@@ -75,32 +84,28 @@ func (a *ApplyCRDsAction) Cleanup(_ context.Context, _ *ReconciliationRequest) e
 	return nil
 }
 
-func (a *ApplyCRDsAction) apply(ctx context.Context, rc *ReconciliationRequest, crd *unstructured.Unstructured) error {
+func (a *ApplyCRDsAction) apply(ctx context.Context, rc *ReconciliationRequest, crd *unstructured.Unstructured, apply bool) (bool, error) {
 	dc, err := rc.Client.Dynamic(rc.Resource.Namespace, crd)
 	if err != nil {
-		return fmt.Errorf("cannot create dynamic client: %w", err)
+		return false, fmt.Errorf("cannot create dynamic client: %w", err)
 	}
 
-	apply := rc.Resource.Generation != rc.Resource.Status.ObservedGeneration
-
 	_, err = dc.Get(ctx, crd.GetName(), metav1.GetOptions{})
 	if err != nil && !k8serrors.IsNotFound(err) {
-		return fmt.Errorf("cannot determine if CRD %s exists: %w", resources.Ref(crd), err)
+		return false, fmt.Errorf("cannot determine if CRD %s exists: %w", resources.Ref(crd), err)
 	}
 
-	if err != nil && k8serrors.IsNotFound(err) {
+	if k8serrors.IsNotFound(err) {
 		apply = true
 	}
 
 	if !apply {
 		a.l.Info("run",
 			"apply", "false",
 			"gen", rc.Resource.Generation,
-			"ref", resources.Ref(crd),
-			"generation-changed", rc.Resource.Generation != rc.Resource.Status.ObservedGeneration,
-			"not-found", k8serrors.IsNotFound(err))
+			"ref", resources.Ref(crd))
 
-		return nil
+		return false, nil
 	}
 
 	_, err = dc.Apply(ctx, crd.GetName(), crd, metav1.ApplyOptions{
@@ -109,13 +114,13 @@ func (a *ApplyCRDsAction) apply(ctx context.Context, rc *ReconciliationRequest,
 	})
 
 	if err != nil {
-		return fmt.Errorf("cannot apply CRD %s: %w", resources.Ref(crd), err)
+		return false, fmt.Errorf("cannot apply CRD %s: %w", resources.Ref(crd), err)
 	}
 
 	a.l.Info("run",
 		"apply", "true",
 		"gen", rc.Resource.Generation,
 		"ref", resources.Ref(crd))
 
-	return nil
+	return true, nil
 }

internal/controller/operator/instance/dapr_instance_controller_action_apply_crds.go

@@ -63,23 +63,18 @@ func (a *ApplyResourcesAction) Run(ctx context.Context, rc *ReconciliationReques
 		return istr < jstr
 	})
 
-	installedVersion := ""
-	if rc.Resource.Status.Chart != nil {
-		installedVersion = rc.Resource.Status.Chart.Version
-	}
-
-	force := rc.Resource.Generation != rc.Resource.Status.ObservedGeneration || c.Version() != installedVersion
+	force := rc.Resource.Generation != rc.Resource.Status.ObservedGeneration || !helm.IsSameChart(c, rc.Resource.Status.Chart)
 
 	if force {
 		rc.Reconciler.Event(
 			rc.Resource,
 			corev1.EventTypeNormal,
 			"RenderFullHelmTemplate",
-			fmt.Sprintf("Render full Helm template (observedGeneration: %d, generation: %d, installedChartVersion: %s, chartVersion: %s)",
+			fmt.Sprintf("Render full Helm template (observedGeneration: %d, generation: %d, installedChart: %v, chart: %v)",
 				rc.Resource.Status.ObservedGeneration,
 				rc.Resource.Generation,
-				installedVersion,
-				c.Version()),
+				rc.Resource.Status.Chart,
+				c.Spec()),
 		)
 	}
 

internal/controller/operator/instance/dapr_instance_controller_action_apply_resources.go

@@ -1,6 +1,8 @@
 package helm
 
 import (
+	daprApi "github.com/dapr/kubernetes-operator/api/operator/v1alpha1"
+	helme "github.com/lburgazzoli/k8s-manifests-renderer-helm/engine"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/selection"
 )
@@ -36,3 +38,13 @@ func ReleaseSelector() (labels.Selector, error) {
 
 	return selector, nil
 }
+
+func IsSameChart(c *helme.Chart, meta *daprApi.ChartMeta) bool {
+	if c == nil || meta == nil {
+		return false
+	}
+
+	return c.Name() == meta.Name &&
+		c.Version() == meta.Version &&
+		c.Repo() == meta.Repo
+}