Merge pull request #145 from dmvolod/add-observed-generation

✨ Add support for the ObservedGeneration in HelmChartProxy Status field
kubernetes-sigs · Feb 5, 2024 · 338306a · 338306a
2 parents 7c62fad + 0d6b80f
commit 338306a
Show file tree

Hide file tree

Showing 7 changed files with 31 additions and 2 deletions.
diff --git a/api/v1alpha1/condition_consts.go b/api/v1alpha1/condition_consts.go
@@ -24,6 +24,9 @@ const (
 	// meaning that the HelmReleaseProxies are created/updated, value template parsing succeeded, and the orphaned HelmReleaseProxies are deleted.
 	HelmReleaseProxySpecsUpToDateCondition clusterv1.ConditionType = "HelmReleaseProxySpecsUpToDate"
 
+	// HelmReleaseProxySpecsUpdatingReason indicates that the HelmReleaseProxy entity is not yet updated by the corresponding controller.
+	HelmReleaseProxySpecsUpdatingReason = "HelmReleaseProxySpecsUpdating"
+
 	// HelmReleaseProxyCreationFailedReason indicates that the HelmChartProxy controller failed to create a HelmReleaseProxy.
 	HelmReleaseProxyCreationFailedReason = "HelmReleaseProxyCreationFailed"
 

diff --git a/api/v1alpha1/helmchartproxy_types.go b/api/v1alpha1/helmchartproxy_types.go
@@ -205,6 +205,10 @@ type HelmChartProxyStatus struct {
 	// MatchingClusters is the list of references to Clusters selected by the ClusterSelector.
 	// +optional
 	MatchingClusters []corev1.ObjectReference `json:"matchingClusters"`
+
+	// ObservedGeneration is the latest generation observed by the controller.
+	// +optional
+	ObservedGeneration int64 `json:"observedGeneration,omitempty"`
 }
 
 // +kubebuilder:object:root=true

diff --git a/api/v1alpha1/helmreleaseproxy_types.go b/api/v1alpha1/helmreleaseproxy_types.go
@@ -89,6 +89,10 @@ type HelmReleaseProxyStatus struct {
 	// Revision is the current revision of the Helm release.
 	// +optional
 	Revision int `json:"revision,omitempty"`
+
+	// ObservedGeneration is the latest generation observed by the controller.
+	// +optional
+	ObservedGeneration int64 `json:"observedGeneration,omitempty"`
 }
 
 // +kubebuilder:object:root=true

diff --git a/config/crd/bases/addons.cluster.x-k8s.io_helmchartproxies.yaml b/config/crd/bases/addons.cluster.x-k8s.io_helmchartproxies.yaml
@@ -386,6 +386,11 @@ spec:
                   type: object
                   x-kubernetes-map-type: atomic
                 type: array
+              observedGeneration:
+                description: ObservedGeneration is the latest generation observed
+                  by the controller.
+                format: int64
+                type: integer
             type: object
         type: object
     served: true

diff --git a/config/crd/bases/addons.cluster.x-k8s.io_helmreleaseproxies.yaml b/config/crd/bases/addons.cluster.x-k8s.io_helmreleaseproxies.yaml
@@ -330,6 +330,11 @@ spec:
                   - type
                   type: object
                 type: array
+              observedGeneration:
+                description: ObservedGeneration is the latest generation observed
+                  by the controller.
+                format: int64
+                type: integer
               revision:
                 description: Revision is the current revision of the Helm release.
                 type: integer

diff --git a/controllers/helmchartproxy/helmchartproxy_controller.go b/controllers/helmchartproxy/helmchartproxy_controller.go
@@ -18,6 +18,7 @@ package helmchartproxy
 
 import (
 	"context"
+	"fmt"
 
 	"github.com/pkg/errors"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
@@ -297,7 +298,12 @@ func (r *HelmChartProxyReconciler) aggregateHelmReleaseProxyReadyCondition(ctx c
 
 	getters := make([]conditions.Getter, 0, len(releaseList.Items))
 	for i := range releaseList.Items {
-		getters = append(getters, &releaseList.Items[i])
+		helmReleaseProxy := &releaseList.Items[i]
+		if helmReleaseProxy.Generation != helmReleaseProxy.Status.ObservedGeneration {
+			conditions.MarkFalse(helmChartProxy, addonsv1alpha1.HelmReleaseProxySpecsUpToDateCondition, addonsv1alpha1.HelmReleaseProxySpecsUpdatingReason, clusterv1.ConditionSeverityInfo, fmt.Sprintf("Helm release proxy '%s' is not updated yet", helmReleaseProxy.Name))
+			return nil
+		}
+		getters = append(getters, helmReleaseProxy)
 	}
 
 	conditions.SetAggregate(helmChartProxy, addonsv1alpha1.HelmReleaseProxiesReadyCondition, getters, conditions.AddSourceRef(), conditions.WithStepCounterIf(false))
@@ -306,7 +312,7 @@ func (r *HelmChartProxyReconciler) aggregateHelmReleaseProxyReadyCondition(ctx c
 }
 
 // patchHelmChartProxy patches the HelmChartProxy object and sets the ReadyCondition as an aggregate of the other condition set.
-// TODO: Is this preferrable to client.Update() calls? Based on testing it seems like it avoids race conditions.
+// TODO: Is this preferable to client.Update() calls? Based on testing it seems like it avoids race conditions.
 func patchHelmChartProxy(ctx context.Context, patchHelper *patch.Helper, helmChartProxy *addonsv1alpha1.HelmChartProxy) error {
 	conditions.SetSummary(helmChartProxy,
 		conditions.WithConditions(

diff --git a/controllers/helmchartproxy/helmchartproxy_controller_test.go b/controllers/helmchartproxy/helmchartproxy_controller_test.go
@@ -285,6 +285,7 @@ func TestReconcileNormal(t *testing.T) {
 				g.Expect(conditions.IsTrue(hcp, addonsv1alpha1.HelmReleaseProxiesReadyCondition)).To(BeTrue())
 				g.Expect(conditions.Has(hcp, clusterv1.ReadyCondition)).To(BeTrue())
 				g.Expect(conditions.IsTrue(hcp, clusterv1.ReadyCondition)).To(BeTrue())
+				g.Expect(hcp.Status.ObservedGeneration).To(Equal(hcp.Generation))
 			},
 			expectedError: "",
 		},
@@ -304,6 +305,7 @@ func TestReconcileNormal(t *testing.T) {
 				g.Expect(conditions.IsTrue(hcp, addonsv1alpha1.HelmReleaseProxiesReadyCondition)).To(BeTrue())
 				g.Expect(conditions.Has(hcp, clusterv1.ReadyCondition)).To(BeTrue())
 				g.Expect(conditions.IsTrue(hcp, clusterv1.ReadyCondition)).To(BeTrue())
+				g.Expect(hcp.Status.ObservedGeneration).To(Equal(hcp.Generation))
 			},
 			expectedError: "",
 		},