From bf381e678cc9b8b10f9c5cc1237a64bbf18a6e8e Mon Sep 17 00:00:00 2001 From: Chaitanya Kolluru Date: Tue, 30 Sep 2025 20:12:06 -0500 Subject: [PATCH 01/13] adding rollingStepSize to helm chart proxy spec Signed-off-by: Chaitanya Kolluru --- api/v1alpha1/condition_consts.go | 11 ++++++++++ api/v1alpha1/helmchartproxy_types.go | 20 +++++++++++++++++++ api/v1alpha1/zz_generated.deepcopy.go | 6 ++++++ ...ons.cluster.x-k8s.io_helmchartproxies.yaml | 12 +++++++++++ 4 files changed, 49 insertions(+) diff --git a/api/v1alpha1/condition_consts.go b/api/v1alpha1/condition_consts.go index d7a337bd..271656da 100644 --- a/api/v1alpha1/condition_consts.go +++ b/api/v1alpha1/condition_consts.go @@ -42,9 +42,20 @@ const ( // ClusterSelectionFailedReason indicates that the HelmChartProxy controller failed to select the workload Clusters. ClusterSelectionFailedReason = "ClusterSelectionFailed" + // HelmReleaseProxiesRolloutNotReady indicates that the initial rollout + // of HelmReleaseProxies has not been completed. + HelmReleaseProxiesRolloutNotReady = "HelmReleaseProxiesRolloutNotReady" + + // HelmReleaseProxiesRolloutUndefined indicates that HelmChartProxy doesn't + // use Rollout Step Size to reconcile HelmReleaseProxies. + HelmReleaseProxiesRolloutUndefined = "HelmReleaseProxiesRolloutUndefined" + // HelmReleaseProxiesReadyCondition indicates that the HelmReleaseProxies are ready, meaning that the Helm installation, upgrade // or deletion is complete. HelmReleaseProxiesReadyCondition clusterv1.ConditionType = "HelmReleaseProxiesReady" + + // HelmReleaseProxiesRolloutCompletedCondition indicates if the initial rollout of HelmReleaseProxies is complete. + HelmReleaseProxiesRolloutCompletedCondition clusterv1.ConditionType = "HelmReleaseProxiesRolloutCompleted" ) // HelmReleaseProxy Conditions and Reasons. diff --git a/api/v1alpha1/helmchartproxy_types.go b/api/v1alpha1/helmchartproxy_types.go index 6ecfda69..d962d4a7 100644 --- a/api/v1alpha1/helmchartproxy_types.go +++ b/api/v1alpha1/helmchartproxy_types.go @@ -19,6 +19,7 @@ package v1alpha1 import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" ) @@ -84,6 +85,14 @@ type HelmChartProxySpec struct { // +optional ReconcileStrategy string `json:"reconcileStrategy,omitempty"` + // RolloutStepSize is an opt-in feature that defines the step size during + // initial rollout of HelmReleaseProxy resources on matching clusters. + // Once all existing HelmReleaseProxy resources are ready=true, the next + // batch of HelmReleaseProxy resources are reconciled. + // If undefined, will default to creating HelmReleaseProxy resources for all + // matching clusters. + RolloutStepSize *intstr.IntOrString `json:"rolloutStepSize,omitempty"` + // Options represents CLI flags passed to Helm operations (i.e. install, upgrade, delete) and // include options such as wait, skipCRDs, timeout, waitForJobs, etc. // +optional @@ -288,6 +297,17 @@ func (c *HelmChartProxy) SetConditions(conditions clusterv1.Conditions) { c.Status.Conditions = conditions } +func (c *HelmChartProxy) GetHelmReleaseProxyReadyCondition() *clusterv1.Condition { + cnds := c.GetConditions() + for _, cnd := range cnds { + if cnd.Type == HelmReleaseProxiesReadyCondition { + return &cnd + } + } + + return nil +} + // SetMatchingClusters will set the given list of matching clusters on an HelmChartProxy object. func (c *HelmChartProxy) SetMatchingClusters(clusterList []clusterv1.Cluster) { matchingClusters := make([]corev1.ObjectReference, 0, len(clusterList)) diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 19fb7024..c2e03f16 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -24,6 +24,7 @@ import ( corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/intstr" "sigs.k8s.io/cluster-api/api/v1beta1" ) @@ -106,6 +107,11 @@ func (in *HelmChartProxyList) DeepCopyObject() runtime.Object { func (in *HelmChartProxySpec) DeepCopyInto(out *HelmChartProxySpec) { *out = *in in.ClusterSelector.DeepCopyInto(&out.ClusterSelector) + if in.RolloutStepSize != nil { + in, out := &in.RolloutStepSize, &out.RolloutStepSize + *out = new(intstr.IntOrString) + **out = **in + } in.Options.DeepCopyInto(&out.Options) if in.Credentials != nil { in, out := &in.Credentials, &out.Credentials diff --git a/config/crd/bases/addons.cluster.x-k8s.io_helmchartproxies.yaml b/config/crd/bases/addons.cluster.x-k8s.io_helmchartproxies.yaml index f4fc2562..38ebf413 100644 --- a/config/crd/bases/addons.cluster.x-k8s.io_helmchartproxies.yaml +++ b/config/crd/bases/addons.cluster.x-k8s.io_helmchartproxies.yaml @@ -279,6 +279,18 @@ spec: RepoURL is the URL of the Helm chart repository. e.g. chart-path oci://repo-url/chart-name as repoURL: oci://repo-url and https://repo-url/chart-name as repoURL: https://repo-url type: string + rolloutStepSize: + anyOf: + - type: integer + - type: string + description: |- + RolloutStepSize is an opt-in feature that defines the step size during + initial rollout of HelmReleaseProxy resources on matching clusters. + Once all existing HelmReleaseProxy resources are ready=true, the next + batch of HelmReleaseProxy resources are reconciled. + If undefined, will default to creating HelmReleaseProxy resources for all + matching clusters. + x-kubernetes-int-or-string: true tlsConfig: description: TLSConfig contains the TLS configuration for a HelmChartProxy. properties: From 95c6dcf978dde8149fc498f538d449e41dfec107 Mon Sep 17 00:00:00 2001 From: Chaitanya Kolluru Date: Tue, 30 Sep 2025 20:12:44 -0500 Subject: [PATCH 02/13] implementation and unit tests Signed-off-by: Chaitanya Kolluru --- .../helmchartproxy_controller.go | 109 +++++++ .../helmchartproxy_controller_test.go | 285 ++++++++++++++++++ 2 files changed, 394 insertions(+) diff --git a/controllers/helmchartproxy/helmchartproxy_controller.go b/controllers/helmchartproxy/helmchartproxy_controller.go index fc970451..c1ec1f65 100644 --- a/controllers/helmchartproxy/helmchartproxy_controller.go +++ b/controllers/helmchartproxy/helmchartproxy_controller.go @@ -20,10 +20,13 @@ import ( "context" "github.com/pkg/errors" + v1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/intstr" addonsv1alpha1 "sigs.k8s.io/cluster-api-addon-provider-helm/api/v1alpha1" clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1" "sigs.k8s.io/cluster-api/util/conditions" @@ -45,6 +48,15 @@ type HelmChartProxyReconciler struct { WatchFilterValue string } +// helmReleaseProxyRolloutMeta is used to gather HelmReleaseProxy rollout +// metadata for matching clusters. +type helmReleaseProxyRolloutMeta struct { + cluster clusterv1.Cluster + + // Identifies whether HelmReleaseProxy exists for the cluster. + hrpExists bool +} + // SetupWithManager sets up the controller with the Manager. func (r *HelmChartProxyReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, options controller.Options) error { log := ctrl.LoggerFrom(ctx) @@ -197,6 +209,101 @@ func (r *HelmChartProxyReconciler) reconcileNormal(ctx context.Context, helmChar } } + // RolloutStepSize is defined; check if count of HelmReleaseProxies matches + // the count of matching clusters. + if helmChartProxy.Spec.RolloutStepSize != nil { + if len(clusters) != len(helmReleaseProxies) { + // Set HelmReleaseProxiesRolloutCompletedCondition to false as + // HelmReleaseProxies are being rolled out. + conditions.MarkFalse( + helmChartProxy, + addonsv1alpha1.HelmReleaseProxiesRolloutCompletedCondition, + addonsv1alpha1.HelmReleaseProxiesRolloutNotReady, + clusterv1.ConditionSeverityInfo, + "%d Helm release proxies not yet rolled out", + len(clusters)-len(helmReleaseProxies), + ) + + // Identifies clusters by their NamespacedName and gathers their + // helmReleaseProxyRolloutMeta. + clusterRolloutMeta := map[string]*helmReleaseProxyRolloutMeta{} + for _, cls := range clusters { + clusterRolloutMeta[types.NamespacedName{Namespace: cls.Namespace, Name: cls.Name}.String()] = &helmReleaseProxyRolloutMeta{cluster: cls} + } + for _, hrp := range helmReleaseProxies { + hrpClsRef := hrp.Spec.ClusterRef + rltMeta := clusterRolloutMeta[types.NamespacedName{Namespace: hrpClsRef.Namespace, Name: hrpClsRef.Name}.String()] + rltMeta.hrpExists = true + } + + hrpReadyCond := helmChartProxy.GetHelmReleaseProxyReadyCondition() + // If HelmReleaseProxiesReadyCondition is false, reconcile existing + // HelmReleaseProxies and exit. + if hrpReadyCond != nil && (hrpReadyCond.Status == v1.ConditionFalse) { + for _, hrpRltMeta := range clusterRolloutMeta { + if hrpRltMeta.hrpExists { + // Don't reconcile if the Cluster is being deleted + if !hrpRltMeta.cluster.DeletionTimestamp.IsZero() { + continue + } + + err := r.reconcileForCluster(ctx, helmChartProxy, hrpRltMeta.cluster) + if err != nil { + return err + } + } + } + return nil + } + + // HelmReleaseProxyReadyCondition is True; continue with reconciling the + // next batch of HelmReleaseProxies. + count := 0 + stepSize, err := intstr.GetScaledValueFromIntOrPercent(helmChartProxy.Spec.RolloutStepSize, len(clusters), true) + if err != nil { + return err + } + for _, hrpRltMeta := range clusterRolloutMeta { + // The next batch of helmReleaseProxies have been reconciled. + if count >= stepSize { + return nil + } + + // Skip reconciling the cluster if its HelmReleaseProxy already exists. + if hrpRltMeta.hrpExists { + continue + } + + // Don't reconcile if the Cluster is being deleted + if !hrpRltMeta.cluster.DeletionTimestamp.IsZero() { + continue + } + + err := r.reconcileForCluster(ctx, helmChartProxy, hrpRltMeta.cluster) + if err != nil { + return err + } + count++ + } + + // In cases where the count of remaining HelmReleaseProxies to be rolled + // out is less than rollout step size. + return nil + } + // RolloutStepSize is defined and all HelmReleaseProxies have been rolled out. + conditions.MarkTrue(helmChartProxy, addonsv1alpha1.HelmReleaseProxiesRolloutCompletedCondition) + } else { + // RolloutStepSize is undefined. Set HelmReleaseProxiesRolloutCompletedCondition to True with reason. + conditions.MarkTrueWithNegativePolarity( + helmChartProxy, + addonsv1alpha1.HelmReleaseProxiesRolloutCompletedCondition, + addonsv1alpha1.HelmReleaseProxiesRolloutUndefined, + clusterv1.ConditionSeverityInfo, + "HelmChartProxy does not use Rollout Step Size", + ) + } + + // Continue with reconciling for all clusters after initial rollout. for _, cluster := range clusters { // Don't reconcile if the Cluster is being deleted if !cluster.DeletionTimestamp.IsZero() { @@ -321,6 +428,7 @@ func patchHelmChartProxy(ctx context.Context, patchHelper *patch.Helper, helmCha conditions.WithConditions( addonsv1alpha1.HelmReleaseProxySpecsUpToDateCondition, addonsv1alpha1.HelmReleaseProxiesReadyCondition, + addonsv1alpha1.HelmReleaseProxiesRolloutCompletedCondition, ), ) @@ -332,6 +440,7 @@ func patchHelmChartProxy(ctx context.Context, patchHelper *patch.Helper, helmCha clusterv1.ReadyCondition, addonsv1alpha1.HelmReleaseProxySpecsUpToDateCondition, addonsv1alpha1.HelmReleaseProxiesReadyCondition, + addonsv1alpha1.HelmReleaseProxiesRolloutCompletedCondition, }}, patch.WithStatusObservedGeneration{}, ) diff --git a/controllers/helmchartproxy/helmchartproxy_controller_test.go b/controllers/helmchartproxy/helmchartproxy_controller_test.go index 852f514d..8bb543b1 100644 --- a/controllers/helmchartproxy/helmchartproxy_controller_test.go +++ b/controllers/helmchartproxy/helmchartproxy_controller_test.go @@ -24,6 +24,7 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/client-go/kubernetes/scheme" "k8s.io/utils/ptr" addonsv1alpha1 "sigs.k8s.io/cluster-api-addon-provider-helm/api/v1alpha1" @@ -68,6 +69,69 @@ var ( }, } + rolloutStepSizeProxy = &addonsv1alpha1.HelmChartProxy{ + TypeMeta: metav1.TypeMeta{ + APIVersion: addonsv1alpha1.GroupVersion.String(), + Kind: "HelmChartProxy", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-hcp", + Namespace: "test-namespace", + }, + Spec: addonsv1alpha1.HelmChartProxySpec{ + ClusterSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{ + "test-label": "test-value", + }, + }, + RolloutStepSize: &intstr.IntOrString{Type: intstr.String, StrVal: "25%"}, + ReleaseName: "test-release-name", + ChartName: "test-chart-name", + RepoURL: "https://test-repo-url", + ReleaseNamespace: "test-release-namespace", + Version: "test-version", + ValuesTemplate: "apiServerPort: {{ .Cluster.spec.clusterNetwork.apiServerPort }}", + ReconcileStrategy: string(addonsv1alpha1.ReconcileStrategyContinuous), + Options: addonsv1alpha1.HelmOptions{}, + }, + } + + rolloutStepSizeProxyWithReleaseProxyReadyFalse = &addonsv1alpha1.HelmChartProxy{ + TypeMeta: metav1.TypeMeta{ + APIVersion: addonsv1alpha1.GroupVersion.String(), + Kind: "HelmChartProxy", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-hcp", + Namespace: "test-namespace", + }, + Spec: addonsv1alpha1.HelmChartProxySpec{ + ClusterSelector: metav1.LabelSelector{ + MatchLabels: map[string]string{ + "test-label": "test-value", + }, + }, + RolloutStepSize: &intstr.IntOrString{Type: intstr.String, StrVal: "25%"}, + ReleaseName: "test-release-name", + ChartName: "test-chart-name", + RepoURL: "https://test-repo-url", + ReleaseNamespace: "test-release-namespace", + Version: "test-version", + ValuesTemplate: "apiServerPort: {{ .Cluster.spec.clusterNetwork.apiServerPort }}", + ReconcileStrategy: string(addonsv1alpha1.ReconcileStrategyContinuous), + Options: addonsv1alpha1.HelmOptions{}, + }, + Status: addonsv1alpha1.HelmChartProxyStatus{ + Conditions: []clusterv1.Condition{ + { + Type: addonsv1alpha1.HelmReleaseProxiesReadyCondition, + Status: corev1.ConditionFalse, + Severity: clusterv1.ConditionSeverityInfo, + }, + }, + }, + } + unsetProxy = &addonsv1alpha1.HelmChartProxy{ TypeMeta: metav1.TypeMeta{ APIVersion: addonsv1alpha1.GroupVersion.String(), @@ -289,6 +353,53 @@ var ( }, } + hrpNotReady1 = &addonsv1alpha1.HelmReleaseProxy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-hrp-1", + Namespace: "test-namespace", + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: addonsv1alpha1.GroupVersion.String(), + Kind: "HelmChartProxy", + Name: "test-hcp", + Controller: ptr.To(true), + BlockOwnerDeletion: ptr.To(true), + }, + }, + Labels: map[string]string{ + clusterv1.ClusterNameLabel: "test-cluster-1", + addonsv1alpha1.HelmChartProxyLabelName: "test-hcp", + }, + Annotations: map[string]string{ + addonsv1alpha1.ReleaseSuccessfullyInstalledAnnotation: "true", + }, + }, + Spec: addonsv1alpha1.HelmReleaseProxySpec{ + ClusterRef: corev1.ObjectReference{ + APIVersion: clusterv1.GroupVersion.String(), + Kind: "Cluster", + Name: "test-cluster-1", + Namespace: "test-namespace", + }, + ReleaseName: "test-release-name", + ChartName: "test-chart-name", + RepoURL: "https://test-repo-url", + ReleaseNamespace: "test-release-namespace", + Version: "test-version", + Values: "apiServerPort: 1234", + Options: addonsv1alpha1.HelmOptions{}, + }, + Status: addonsv1alpha1.HelmReleaseProxyStatus{ + Conditions: []clusterv1.Condition{ + { + Type: clusterv1.ReadyCondition, + Status: corev1.ConditionFalse, + Severity: clusterv1.ConditionSeverityInfo, + }, + }, + }, + } + hrpReady2 = &addonsv1alpha1.HelmReleaseProxy{ ObjectMeta: metav1.ObjectMeta{ Name: "test-hrp-2", @@ -334,6 +445,53 @@ var ( }, }, } + + hrpNotReady2 = &addonsv1alpha1.HelmReleaseProxy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-hrp-2", + Namespace: "test-namespace", + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: addonsv1alpha1.GroupVersion.String(), + Kind: "HelmChartProxy", + Name: "test-hcp", + Controller: ptr.To(true), + BlockOwnerDeletion: ptr.To(true), + }, + }, + Labels: map[string]string{ + clusterv1.ClusterNameLabel: "test-cluster-2", + addonsv1alpha1.HelmChartProxyLabelName: "test-hcp", + }, + Annotations: map[string]string{ + addonsv1alpha1.ReleaseSuccessfullyInstalledAnnotation: "true", + }, + }, + Spec: addonsv1alpha1.HelmReleaseProxySpec{ + ClusterRef: corev1.ObjectReference{ + APIVersion: clusterv1.GroupVersion.String(), + Kind: "Cluster", + Name: "test-cluster-2", + Namespace: "test-namespace", + }, + ReleaseName: "test-release-name", + ChartName: "test-chart-name", + RepoURL: "https://test-repo-url", + ReleaseNamespace: "test-release-namespace", + Version: "test-version", + Values: "apiServerPort: 5678", + Options: addonsv1alpha1.HelmOptions{}, + }, + Status: addonsv1alpha1.HelmReleaseProxyStatus{ + Conditions: []clusterv1.Condition{ + { + Type: clusterv1.ReadyCondition, + Status: corev1.ConditionFalse, + Severity: clusterv1.ConditionSeverityInfo, + }, + }, + }, + } ) func TestReconcileNormal(t *testing.T) { @@ -372,6 +530,34 @@ func TestReconcileNormal(t *testing.T) { }, expectedError: "", }, + { + name: "successfully select clusters and set Rollout Step Ready Condition as False", + helmChartProxy: rolloutStepSizeProxy, + objects: []client.Object{cluster1, cluster2, cluster3, cluster4}, + expect: func(g *WithT, c client.Client, hcp *addonsv1alpha1.HelmChartProxy) { + g.Expect(hcp.Status.MatchingClusters).To(BeEquivalentTo([]corev1.ObjectReference{ + { + APIVersion: clusterv1.GroupVersion.String(), + Kind: "Cluster", + Name: "test-cluster-1", + Namespace: "test-namespace", + }, + { + APIVersion: clusterv1.GroupVersion.String(), + Kind: "Cluster", + Name: "test-cluster-2", + Namespace: "test-namespace", + }, + })) + g.Expect(conditions.Has(hcp, addonsv1alpha1.HelmReleaseProxySpecsUpToDateCondition)).To(BeTrue()) + g.Expect(conditions.IsTrue(hcp, addonsv1alpha1.HelmReleaseProxySpecsUpToDateCondition)).To(BeTrue()) + g.Expect(conditions.Has(hcp, addonsv1alpha1.HelmReleaseProxiesRolloutCompletedCondition)).To(BeTrue()) + g.Expect(conditions.IsTrue(hcp, addonsv1alpha1.HelmReleaseProxiesRolloutCompletedCondition)).To(BeFalse()) + // This is false as the HelmReleaseProxies won't be ready until the HelmReleaseProxy controller runs. + g.Expect(conditions.Has(hcp, addonsv1alpha1.HelmReleaseProxiesReadyCondition)).To(BeFalse()) + }, + expectedError: "", + }, { name: "successfully select clusters and install HelmReleaseProxies for unset strategy", helmChartProxy: unsetProxy, @@ -447,6 +633,8 @@ func TestReconcileNormal(t *testing.T) { g.Expect(conditions.IsTrue(hcp, addonsv1alpha1.HelmReleaseProxySpecsUpToDateCondition)).To(BeTrue()) g.Expect(conditions.Has(hcp, addonsv1alpha1.HelmReleaseProxiesReadyCondition)).To(BeTrue()) g.Expect(conditions.IsTrue(hcp, addonsv1alpha1.HelmReleaseProxiesReadyCondition)).To(BeTrue()) + g.Expect(conditions.Has(hcp, addonsv1alpha1.HelmReleaseProxiesRolloutCompletedCondition)).To(BeTrue()) + g.Expect(conditions.IsTrue(hcp, addonsv1alpha1.HelmReleaseProxiesRolloutCompletedCondition)).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)) @@ -476,6 +664,8 @@ func TestReconcileNormal(t *testing.T) { g.Expect(conditions.IsTrue(hcp, addonsv1alpha1.HelmReleaseProxySpecsUpToDateCondition)).To(BeTrue()) g.Expect(conditions.Has(hcp, addonsv1alpha1.HelmReleaseProxiesReadyCondition)).To(BeTrue()) g.Expect(conditions.IsTrue(hcp, addonsv1alpha1.HelmReleaseProxiesReadyCondition)).To(BeTrue()) + g.Expect(conditions.Has(hcp, addonsv1alpha1.HelmReleaseProxiesRolloutCompletedCondition)).To(BeTrue()) + g.Expect(conditions.IsTrue(hcp, addonsv1alpha1.HelmReleaseProxiesRolloutCompletedCondition)).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)) @@ -506,12 +696,107 @@ func TestReconcileNormal(t *testing.T) { g.Expect(conditions.IsTrue(hcp, addonsv1alpha1.HelmReleaseProxySpecsUpToDateCondition)).To(BeTrue()) g.Expect(conditions.Has(hcp, addonsv1alpha1.HelmReleaseProxiesReadyCondition)).To(BeTrue()) g.Expect(conditions.IsTrue(hcp, addonsv1alpha1.HelmReleaseProxiesReadyCondition)).To(BeTrue()) + g.Expect(conditions.Has(hcp, addonsv1alpha1.HelmReleaseProxiesRolloutCompletedCondition)).To(BeTrue()) + g.Expect(conditions.IsTrue(hcp, addonsv1alpha1.HelmReleaseProxiesRolloutCompletedCondition)).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: "", + }, + { + name: "mark HelmChartProxy as ready once HelmReleaseProxies are rolled out and ready", + helmChartProxy: rolloutStepSizeProxy, + objects: []client.Object{cluster1, cluster2, hrpReady1, hrpReady2}, + expect: func(g *WithT, c client.Client, hcp *addonsv1alpha1.HelmChartProxy) { + g.Expect(hcp.Status.MatchingClusters).To(BeEquivalentTo([]corev1.ObjectReference{ + { + APIVersion: clusterv1.GroupVersion.String(), + Kind: "Cluster", + Name: "test-cluster-1", + Namespace: "test-namespace", + }, + { + APIVersion: clusterv1.GroupVersion.String(), + Kind: "Cluster", + Name: "test-cluster-2", + Namespace: "test-namespace", + }, + })) + g.Expect(conditions.Has(hcp, addonsv1alpha1.HelmReleaseProxySpecsUpToDateCondition)).To(BeTrue()) + g.Expect(conditions.IsTrue(hcp, addonsv1alpha1.HelmReleaseProxySpecsUpToDateCondition)).To(BeTrue()) + g.Expect(conditions.Has(hcp, addonsv1alpha1.HelmReleaseProxiesReadyCondition)).To(BeTrue()) + g.Expect(conditions.IsTrue(hcp, addonsv1alpha1.HelmReleaseProxiesReadyCondition)).To(BeTrue()) + g.Expect(conditions.Has(hcp, addonsv1alpha1.HelmReleaseProxiesRolloutCompletedCondition)).To(BeTrue()) + g.Expect(conditions.IsTrue(hcp, addonsv1alpha1.HelmReleaseProxiesRolloutCompletedCondition)).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: "", }, + { + name: "mark HelmChartProxy as not-ready when not all helm release proxies are rolled out", + helmChartProxy: rolloutStepSizeProxyWithReleaseProxyReadyFalse, + objects: []client.Object{cluster1, cluster2, hrpNotReady1}, + expect: func(g *WithT, c client.Client, hcp *addonsv1alpha1.HelmChartProxy) { + g.Expect(hcp.Status.MatchingClusters).To(BeEquivalentTo([]corev1.ObjectReference{ + { + APIVersion: clusterv1.GroupVersion.String(), + Kind: "Cluster", + Name: "test-cluster-1", + Namespace: "test-namespace", + }, + { + APIVersion: clusterv1.GroupVersion.String(), + Kind: "Cluster", + Name: "test-cluster-2", + Namespace: "test-namespace", + }, + })) + g.Expect(conditions.Has(hcp, addonsv1alpha1.HelmReleaseProxySpecsUpToDateCondition)).To(BeTrue()) + g.Expect(conditions.IsTrue(hcp, addonsv1alpha1.HelmReleaseProxySpecsUpToDateCondition)).To(BeTrue()) + g.Expect(conditions.Has(hcp, addonsv1alpha1.HelmReleaseProxiesReadyCondition)).To(BeTrue()) + g.Expect(conditions.IsTrue(hcp, addonsv1alpha1.HelmReleaseProxiesReadyCondition)).To(BeFalse()) + g.Expect(conditions.Has(hcp, addonsv1alpha1.HelmReleaseProxiesRolloutCompletedCondition)).To(BeTrue()) + g.Expect(conditions.IsTrue(hcp, addonsv1alpha1.HelmReleaseProxiesRolloutCompletedCondition)).To(BeFalse()) + g.Expect(conditions.Has(hcp, clusterv1.ReadyCondition)).To(BeTrue()) + g.Expect(conditions.IsTrue(hcp, clusterv1.ReadyCondition)).To(BeFalse()) + g.Expect(hcp.Status.ObservedGeneration).To(Equal(hcp.Generation)) + }, + expectedError: "", + }, + { + name: "mark HelmChartProxy as not-ready when all helm release proxies are rolled out and not ready", + helmChartProxy: rolloutStepSizeProxyWithReleaseProxyReadyFalse, + objects: []client.Object{cluster1, cluster2, hrpNotReady1, hrpNotReady2}, + expect: func(g *WithT, c client.Client, hcp *addonsv1alpha1.HelmChartProxy) { + g.Expect(hcp.Status.MatchingClusters).To(BeEquivalentTo([]corev1.ObjectReference{ + { + APIVersion: clusterv1.GroupVersion.String(), + Kind: "Cluster", + Name: "test-cluster-1", + Namespace: "test-namespace", + }, + { + APIVersion: clusterv1.GroupVersion.String(), + Kind: "Cluster", + Name: "test-cluster-2", + Namespace: "test-namespace", + }, + })) + g.Expect(conditions.Has(hcp, addonsv1alpha1.HelmReleaseProxySpecsUpToDateCondition)).To(BeTrue()) + g.Expect(conditions.IsTrue(hcp, addonsv1alpha1.HelmReleaseProxySpecsUpToDateCondition)).To(BeTrue()) + g.Expect(conditions.Has(hcp, addonsv1alpha1.HelmReleaseProxiesReadyCondition)).To(BeTrue()) + g.Expect(conditions.IsTrue(hcp, addonsv1alpha1.HelmReleaseProxiesReadyCondition)).To(BeFalse()) + g.Expect(conditions.Has(hcp, addonsv1alpha1.HelmReleaseProxiesRolloutCompletedCondition)).To(BeTrue()) + g.Expect(conditions.IsTrue(hcp, addonsv1alpha1.HelmReleaseProxiesRolloutCompletedCondition)).To(BeTrue()) + g.Expect(conditions.Has(hcp, clusterv1.ReadyCondition)).To(BeTrue()) + g.Expect(conditions.IsTrue(hcp, clusterv1.ReadyCondition)).To(BeFalse()) + g.Expect(hcp.Status.ObservedGeneration).To(Equal(hcp.Generation)) + }, + expectedError: "", + }, { name: "successfully delete orphaned HelmReleaseProxies for Continuous strategy", helmChartProxy: continuousProxy, From 725e3cb04c1ee9446269ec1369eab482ac17d073 Mon Sep 17 00:00:00 2001 From: Chaitanya Kolluru Date: Tue, 30 Sep 2025 21:04:15 -0500 Subject: [PATCH 03/13] fixing reason names. updating condition message Signed-off-by: Chaitanya Kolluru --- api/v1alpha1/condition_consts.go | 8 ++++---- controllers/helmchartproxy/helmchartproxy_controller.go | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/api/v1alpha1/condition_consts.go b/api/v1alpha1/condition_consts.go index 271656da..4cc53de5 100644 --- a/api/v1alpha1/condition_consts.go +++ b/api/v1alpha1/condition_consts.go @@ -42,13 +42,13 @@ const ( // ClusterSelectionFailedReason indicates that the HelmChartProxy controller failed to select the workload Clusters. ClusterSelectionFailedReason = "ClusterSelectionFailed" - // HelmReleaseProxiesRolloutNotReady indicates that the initial rollout + // HelmReleaseProxiesRolloutNotCompleteReason indicates that the initial rollout // of HelmReleaseProxies has not been completed. - HelmReleaseProxiesRolloutNotReady = "HelmReleaseProxiesRolloutNotReady" + HelmReleaseProxiesRolloutNotCompleteReason = "HelmReleaseProxiesRolloutNotComplete" - // HelmReleaseProxiesRolloutUndefined indicates that HelmChartProxy doesn't + // HelmReleaseProxiesRolloutUndefinedReason indicates that HelmChartProxy doesn't // use Rollout Step Size to reconcile HelmReleaseProxies. - HelmReleaseProxiesRolloutUndefined = "HelmReleaseProxiesRolloutUndefined" + HelmReleaseProxiesRolloutUndefinedReason = "HelmReleaseProxiesRolloutUndefined" // HelmReleaseProxiesReadyCondition indicates that the HelmReleaseProxies are ready, meaning that the Helm installation, upgrade // or deletion is complete. diff --git a/controllers/helmchartproxy/helmchartproxy_controller.go b/controllers/helmchartproxy/helmchartproxy_controller.go index c1ec1f65..3241b3c5 100644 --- a/controllers/helmchartproxy/helmchartproxy_controller.go +++ b/controllers/helmchartproxy/helmchartproxy_controller.go @@ -218,7 +218,7 @@ func (r *HelmChartProxyReconciler) reconcileNormal(ctx context.Context, helmChar conditions.MarkFalse( helmChartProxy, addonsv1alpha1.HelmReleaseProxiesRolloutCompletedCondition, - addonsv1alpha1.HelmReleaseProxiesRolloutNotReady, + addonsv1alpha1.HelmReleaseProxiesRolloutNotCompleteReason, clusterv1.ConditionSeverityInfo, "%d Helm release proxies not yet rolled out", len(clusters)-len(helmReleaseProxies), @@ -297,9 +297,9 @@ func (r *HelmChartProxyReconciler) reconcileNormal(ctx context.Context, helmChar conditions.MarkTrueWithNegativePolarity( helmChartProxy, addonsv1alpha1.HelmReleaseProxiesRolloutCompletedCondition, - addonsv1alpha1.HelmReleaseProxiesRolloutUndefined, + addonsv1alpha1.HelmReleaseProxiesRolloutUndefinedReason, clusterv1.ConditionSeverityInfo, - "HelmChartProxy does not use Rollout Step Size", + "HelmChartProxy does not use rollout step", ) } From d72d7f147ee158d28573ea47209853bbf75ac488 Mon Sep 17 00:00:00 2001 From: Chaitanya Kolluru Date: Wed, 1 Oct 2025 10:56:37 -0500 Subject: [PATCH 04/13] fixing lint Signed-off-by: Chaitanya Kolluru --- controllers/helmchartproxy/helmchartproxy_controller.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/controllers/helmchartproxy/helmchartproxy_controller.go b/controllers/helmchartproxy/helmchartproxy_controller.go index 3241b3c5..e17bf47b 100644 --- a/controllers/helmchartproxy/helmchartproxy_controller.go +++ b/controllers/helmchartproxy/helmchartproxy_controller.go @@ -20,7 +20,7 @@ import ( "context" "github.com/pkg/errors" - v1 "k8s.io/api/core/v1" + corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" @@ -239,7 +239,7 @@ func (r *HelmChartProxyReconciler) reconcileNormal(ctx context.Context, helmChar hrpReadyCond := helmChartProxy.GetHelmReleaseProxyReadyCondition() // If HelmReleaseProxiesReadyCondition is false, reconcile existing // HelmReleaseProxies and exit. - if hrpReadyCond != nil && (hrpReadyCond.Status == v1.ConditionFalse) { + if hrpReadyCond != nil && (hrpReadyCond.Status == corev1.ConditionFalse) { for _, hrpRltMeta := range clusterRolloutMeta { if hrpRltMeta.hrpExists { // Don't reconcile if the Cluster is being deleted @@ -253,6 +253,7 @@ func (r *HelmChartProxyReconciler) reconcileNormal(ctx context.Context, helmChar } } } + return nil } From 5cfd2d226eb0635616b4b0de7cc20ce4c8404f3c Mon Sep 17 00:00:00 2001 From: Chaitanya Kolluru Date: Wed, 1 Oct 2025 15:24:53 -0500 Subject: [PATCH 05/13] removes unneeded condition getter fn. replaces it with get from cluster-api's condition package. Signed-off-by: Chaitanya Kolluru --- api/v1alpha1/helmchartproxy_types.go | 11 ----------- .../helmchartproxy/helmchartproxy_controller.go | 2 +- 2 files changed, 1 insertion(+), 12 deletions(-) diff --git a/api/v1alpha1/helmchartproxy_types.go b/api/v1alpha1/helmchartproxy_types.go index d962d4a7..20a0a467 100644 --- a/api/v1alpha1/helmchartproxy_types.go +++ b/api/v1alpha1/helmchartproxy_types.go @@ -297,17 +297,6 @@ func (c *HelmChartProxy) SetConditions(conditions clusterv1.Conditions) { c.Status.Conditions = conditions } -func (c *HelmChartProxy) GetHelmReleaseProxyReadyCondition() *clusterv1.Condition { - cnds := c.GetConditions() - for _, cnd := range cnds { - if cnd.Type == HelmReleaseProxiesReadyCondition { - return &cnd - } - } - - return nil -} - // SetMatchingClusters will set the given list of matching clusters on an HelmChartProxy object. func (c *HelmChartProxy) SetMatchingClusters(clusterList []clusterv1.Cluster) { matchingClusters := make([]corev1.ObjectReference, 0, len(clusterList)) diff --git a/controllers/helmchartproxy/helmchartproxy_controller.go b/controllers/helmchartproxy/helmchartproxy_controller.go index e17bf47b..109ad63b 100644 --- a/controllers/helmchartproxy/helmchartproxy_controller.go +++ b/controllers/helmchartproxy/helmchartproxy_controller.go @@ -236,7 +236,7 @@ func (r *HelmChartProxyReconciler) reconcileNormal(ctx context.Context, helmChar rltMeta.hrpExists = true } - hrpReadyCond := helmChartProxy.GetHelmReleaseProxyReadyCondition() + hrpReadyCond := conditions.Get(helmChartProxy, addonsv1alpha1.HelmReleaseProxiesReadyCondition) // If HelmReleaseProxiesReadyCondition is false, reconcile existing // HelmReleaseProxies and exit. if hrpReadyCond != nil && (hrpReadyCond.Status == corev1.ConditionFalse) { From f34e26f70139768b7cc7de42154355ced4be1cac Mon Sep 17 00:00:00 2001 From: Chaitanya Kolluru Date: Wed, 1 Oct 2025 17:55:39 -0500 Subject: [PATCH 06/13] using conditions.IsFalse to determine condition status Signed-off-by: Chaitanya Kolluru --- controllers/helmchartproxy/helmchartproxy_controller.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/controllers/helmchartproxy/helmchartproxy_controller.go b/controllers/helmchartproxy/helmchartproxy_controller.go index 109ad63b..50e58195 100644 --- a/controllers/helmchartproxy/helmchartproxy_controller.go +++ b/controllers/helmchartproxy/helmchartproxy_controller.go @@ -20,7 +20,6 @@ import ( "context" "github.com/pkg/errors" - corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" @@ -236,10 +235,9 @@ func (r *HelmChartProxyReconciler) reconcileNormal(ctx context.Context, helmChar rltMeta.hrpExists = true } - hrpReadyCond := conditions.Get(helmChartProxy, addonsv1alpha1.HelmReleaseProxiesReadyCondition) // If HelmReleaseProxiesReadyCondition is false, reconcile existing // HelmReleaseProxies and exit. - if hrpReadyCond != nil && (hrpReadyCond.Status == corev1.ConditionFalse) { + if conditions.IsFalse(helmChartProxy, addonsv1alpha1.HelmReleaseProxiesReadyCondition) { for _, hrpRltMeta := range clusterRolloutMeta { if hrpRltMeta.hrpExists { // Don't reconcile if the Cluster is being deleted From 98072730637cfd2dffee4f9da724ab717182f096 Mon Sep 17 00:00:00 2001 From: Chaitanya Kolluru Date: Wed, 1 Oct 2025 17:57:50 -0500 Subject: [PATCH 07/13] moved cluster detetion check to within the fn to reconcile for cluster Signed-off-by: Chaitanya Kolluru --- .../helmchartproxy/helmchartproxy_controller.go | 16 ---------------- .../helmchartproxy_controller_phases.go | 5 +++++ 2 files changed, 5 insertions(+), 16 deletions(-) diff --git a/controllers/helmchartproxy/helmchartproxy_controller.go b/controllers/helmchartproxy/helmchartproxy_controller.go index 50e58195..8e66e62c 100644 --- a/controllers/helmchartproxy/helmchartproxy_controller.go +++ b/controllers/helmchartproxy/helmchartproxy_controller.go @@ -240,11 +240,6 @@ func (r *HelmChartProxyReconciler) reconcileNormal(ctx context.Context, helmChar if conditions.IsFalse(helmChartProxy, addonsv1alpha1.HelmReleaseProxiesReadyCondition) { for _, hrpRltMeta := range clusterRolloutMeta { if hrpRltMeta.hrpExists { - // Don't reconcile if the Cluster is being deleted - if !hrpRltMeta.cluster.DeletionTimestamp.IsZero() { - continue - } - err := r.reconcileForCluster(ctx, helmChartProxy, hrpRltMeta.cluster) if err != nil { return err @@ -272,12 +267,6 @@ func (r *HelmChartProxyReconciler) reconcileNormal(ctx context.Context, helmChar if hrpRltMeta.hrpExists { continue } - - // Don't reconcile if the Cluster is being deleted - if !hrpRltMeta.cluster.DeletionTimestamp.IsZero() { - continue - } - err := r.reconcileForCluster(ctx, helmChartProxy, hrpRltMeta.cluster) if err != nil { return err @@ -304,11 +293,6 @@ func (r *HelmChartProxyReconciler) reconcileNormal(ctx context.Context, helmChar // Continue with reconciling for all clusters after initial rollout. for _, cluster := range clusters { - // Don't reconcile if the Cluster is being deleted - if !cluster.DeletionTimestamp.IsZero() { - continue - } - err := r.reconcileForCluster(ctx, helmChartProxy, cluster) if err != nil { return err diff --git a/controllers/helmchartproxy/helmchartproxy_controller_phases.go b/controllers/helmchartproxy/helmchartproxy_controller_phases.go index c5f9eb3f..b19520e7 100644 --- a/controllers/helmchartproxy/helmchartproxy_controller_phases.go +++ b/controllers/helmchartproxy/helmchartproxy_controller_phases.go @@ -57,6 +57,11 @@ func (r *HelmChartProxyReconciler) deleteOrphanedHelmReleaseProxies(ctx context. // reconcileForCluster will create or update a HelmReleaseProxy for the given cluster. func (r *HelmChartProxyReconciler) reconcileForCluster(ctx context.Context, helmChartProxy *addonsv1alpha1.HelmChartProxy, cluster clusterv1.Cluster) error { + // Don't reconcile if the Cluster is being deleted + if !cluster.DeletionTimestamp.IsZero() { + return nil + } + log := ctrl.LoggerFrom(ctx) // Don't reconcile if the Cluster or the helmChartProxy is paused. From 1f96a25200d23a54dabdb27662342e7b41ddf02a Mon Sep 17 00:00:00 2001 From: Chaitanya Kolluru Date: Thu, 2 Oct 2025 08:58:11 -0500 Subject: [PATCH 08/13] sorts cluster meta by namespaced name before rolling out HelmReleaseProxies Signed-off-by: Chaitanya Kolluru --- .../helmchartproxy_controller.go | 62 +++++++++++++++---- 1 file changed, 49 insertions(+), 13 deletions(-) diff --git a/controllers/helmchartproxy/helmchartproxy_controller.go b/controllers/helmchartproxy/helmchartproxy_controller.go index 8e66e62c..a697967d 100644 --- a/controllers/helmchartproxy/helmchartproxy_controller.go +++ b/controllers/helmchartproxy/helmchartproxy_controller.go @@ -18,6 +18,7 @@ package helmchartproxy import ( "context" + "slices" "github.com/pkg/errors" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -225,22 +226,52 @@ func (r *HelmChartProxyReconciler) reconcileNormal(ctx context.Context, helmChar // Identifies clusters by their NamespacedName and gathers their // helmReleaseProxyRolloutMeta. - clusterRolloutMeta := map[string]*helmReleaseProxyRolloutMeta{} - for _, cls := range clusters { - clusterRolloutMeta[types.NamespacedName{Namespace: cls.Namespace, Name: cls.Name}.String()] = &helmReleaseProxyRolloutMeta{cluster: cls} + clusterNnRolloutMeta := map[string]*helmReleaseProxyRolloutMeta{} + for _, c := range clusters { + nn := getNnStringFor(c.Namespace, c.Name) + clusterNnRolloutMeta[nn] = &helmReleaseProxyRolloutMeta{ + cluster: c, + } + } + for _, h := range helmReleaseProxies { + ref := h.Spec.ClusterRef + nn := getNnStringFor(ref.Namespace, ref.Name) + meta := clusterNnRolloutMeta[nn] + meta.hrpExists = true + } + + // Sort helmReleaseProxy rollout metadata by cluster namespaced name to + // ensure orderliness. + rolloutMetaSorted := make([]*helmReleaseProxyRolloutMeta, len(clusterNnRolloutMeta)) + i := 0 + for _, m := range clusterNnRolloutMeta { + rolloutMetaSorted[i] = m + i++ } - for _, hrp := range helmReleaseProxies { - hrpClsRef := hrp.Spec.ClusterRef - rltMeta := clusterRolloutMeta[types.NamespacedName{Namespace: hrpClsRef.Namespace, Name: hrpClsRef.Name}.String()] - rltMeta.hrpExists = true + for m := range clusterNnRolloutMeta { + delete(clusterNnRolloutMeta, m) } + slices.SortStableFunc(rolloutMetaSorted, func(a, b *helmReleaseProxyRolloutMeta) int { + nnA := getNnStringFor(a.cluster.Namespace, a.cluster.Name) + nnB := getNnStringFor(b.cluster.Namespace, b.cluster.Name) + if nnA < nnB { + return -1 + } + + if nnA > nnB { + return 1 + } + + return 0 + }) + // If HelmReleaseProxiesReadyCondition is false, reconcile existing // HelmReleaseProxies and exit. if conditions.IsFalse(helmChartProxy, addonsv1alpha1.HelmReleaseProxiesReadyCondition) { - for _, hrpRltMeta := range clusterRolloutMeta { - if hrpRltMeta.hrpExists { - err := r.reconcileForCluster(ctx, helmChartProxy, hrpRltMeta.cluster) + for _, meta := range rolloutMetaSorted { + if meta.hrpExists { + err := r.reconcileForCluster(ctx, helmChartProxy, meta.cluster) if err != nil { return err } @@ -257,17 +288,17 @@ func (r *HelmChartProxyReconciler) reconcileNormal(ctx context.Context, helmChar if err != nil { return err } - for _, hrpRltMeta := range clusterRolloutMeta { + for _, meta := range rolloutMetaSorted { // The next batch of helmReleaseProxies have been reconciled. if count >= stepSize { return nil } // Skip reconciling the cluster if its HelmReleaseProxy already exists. - if hrpRltMeta.hrpExists { + if meta.hrpExists { continue } - err := r.reconcileForCluster(ctx, helmChartProxy, hrpRltMeta.cluster) + err := r.reconcileForCluster(ctx, helmChartProxy, meta.cluster) if err != nil { return err } @@ -499,3 +530,8 @@ func HelmReleaseProxyToHelmChartProxyMapper(ctx context.Context, o client.Object return nil } + +// getNnStringFor to retrieve the namespaced name as a string. +func getNnStringFor(namespace, name string) string { + return types.NamespacedName{Namespace: namespace, Name: name}.String() +} From 16d93839ecd2bc2bd5151fa435f350aa5e47a671 Mon Sep 17 00:00:00 2001 From: Chaitanya Kolluru Date: Thu, 2 Oct 2025 08:59:33 -0500 Subject: [PATCH 09/13] rolloutStepSize description update Signed-off-by: Chaitanya Kolluru --- api/v1alpha1/helmchartproxy_types.go | 13 +++++++------ .../addons.cluster.x-k8s.io_helmchartproxies.yaml | 12 ++++++------ 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/api/v1alpha1/helmchartproxy_types.go b/api/v1alpha1/helmchartproxy_types.go index 20a0a467..9f3e5231 100644 --- a/api/v1alpha1/helmchartproxy_types.go +++ b/api/v1alpha1/helmchartproxy_types.go @@ -85,12 +85,13 @@ type HelmChartProxySpec struct { // +optional ReconcileStrategy string `json:"reconcileStrategy,omitempty"` - // RolloutStepSize is an opt-in feature that defines the step size during - // initial rollout of HelmReleaseProxy resources on matching clusters. - // Once all existing HelmReleaseProxy resources are ready=true, the next - // batch of HelmReleaseProxy resources are reconciled. - // If undefined, will default to creating HelmReleaseProxy resources for all - // matching clusters. + // RolloutStepSize is an opt-in feature that defines a step size during the + // initial rollout of HelmReleaseProxies on matching clusters. Once all + // rolled out HelmReleaseProxy resources are ready=true, the next batch of + // HelmReleaseProxy resources are reconciled. If undefined, will default to + // creating HelmReleaseProxy resources for all matching clusters. + // e.g. an int (5) or percentage of count of total matching clusters (25%) + // +optional RolloutStepSize *intstr.IntOrString `json:"rolloutStepSize,omitempty"` // Options represents CLI flags passed to Helm operations (i.e. install, upgrade, delete) and diff --git a/config/crd/bases/addons.cluster.x-k8s.io_helmchartproxies.yaml b/config/crd/bases/addons.cluster.x-k8s.io_helmchartproxies.yaml index 38ebf413..76abb096 100644 --- a/config/crd/bases/addons.cluster.x-k8s.io_helmchartproxies.yaml +++ b/config/crd/bases/addons.cluster.x-k8s.io_helmchartproxies.yaml @@ -284,12 +284,12 @@ spec: - type: integer - type: string description: |- - RolloutStepSize is an opt-in feature that defines the step size during - initial rollout of HelmReleaseProxy resources on matching clusters. - Once all existing HelmReleaseProxy resources are ready=true, the next - batch of HelmReleaseProxy resources are reconciled. - If undefined, will default to creating HelmReleaseProxy resources for all - matching clusters. + RolloutStepSize is an opt-in feature that defines a step size during the + initial rollout of HelmReleaseProxies on matching clusters. Once all + rolled out HelmReleaseProxy resources are ready=true, the next batch of + HelmReleaseProxy resources are reconciled. If undefined, will default to + creating HelmReleaseProxy resources for all matching clusters. + e.g. an int (5) or percentage of count of total matching clusters (25%) x-kubernetes-int-or-string: true tlsConfig: description: TLSConfig contains the TLS configuration for a HelmChartProxy. From 0d755be7ea5cc6ac963c84b68bc8b00483f851d2 Mon Sep 17 00:00:00 2001 From: Chaitanya Kolluru Date: Mon, 13 Oct 2025 10:10:11 -0500 Subject: [PATCH 10/13] verbose naming for getNamespacedNameStringFor Signed-off-by: Chaitanya Kolluru --- .../helmchartproxy/helmchartproxy_controller.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/controllers/helmchartproxy/helmchartproxy_controller.go b/controllers/helmchartproxy/helmchartproxy_controller.go index a697967d..00e61eb5 100644 --- a/controllers/helmchartproxy/helmchartproxy_controller.go +++ b/controllers/helmchartproxy/helmchartproxy_controller.go @@ -228,14 +228,14 @@ func (r *HelmChartProxyReconciler) reconcileNormal(ctx context.Context, helmChar // helmReleaseProxyRolloutMeta. clusterNnRolloutMeta := map[string]*helmReleaseProxyRolloutMeta{} for _, c := range clusters { - nn := getNnStringFor(c.Namespace, c.Name) + nn := getNamespacedNameStringFor(c.Namespace, c.Name) clusterNnRolloutMeta[nn] = &helmReleaseProxyRolloutMeta{ cluster: c, } } for _, h := range helmReleaseProxies { ref := h.Spec.ClusterRef - nn := getNnStringFor(ref.Namespace, ref.Name) + nn := getNamespacedNameStringFor(ref.Namespace, ref.Name) meta := clusterNnRolloutMeta[nn] meta.hrpExists = true } @@ -253,8 +253,8 @@ func (r *HelmChartProxyReconciler) reconcileNormal(ctx context.Context, helmChar } slices.SortStableFunc(rolloutMetaSorted, func(a, b *helmReleaseProxyRolloutMeta) int { - nnA := getNnStringFor(a.cluster.Namespace, a.cluster.Name) - nnB := getNnStringFor(b.cluster.Namespace, b.cluster.Name) + nnA := getNamespacedNameStringFor(a.cluster.Namespace, a.cluster.Name) + nnB := getNamespacedNameStringFor(b.cluster.Namespace, b.cluster.Name) if nnA < nnB { return -1 } @@ -531,7 +531,7 @@ func HelmReleaseProxyToHelmChartProxyMapper(ctx context.Context, o client.Object return nil } -// getNnStringFor to retrieve the namespaced name as a string. -func getNnStringFor(namespace, name string) string { +// getNamespacedNameStringFor to retrieve the namespaced name as a string. +func getNamespacedNameStringFor(namespace, name string) string { return types.NamespacedName{Namespace: namespace, Name: name}.String() } From daeeb40935b955f411d9f4cbf5cd22543d86c67a Mon Sep 17 00:00:00 2001 From: Chaitanya Kolluru Date: Wed, 15 Oct 2025 19:07:52 -0500 Subject: [PATCH 11/13] adding hrp ready condition check Signed-off-by: Chaitanya Kolluru --- .../helmchartproxy/helmchartproxy_controller.go | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/controllers/helmchartproxy/helmchartproxy_controller.go b/controllers/helmchartproxy/helmchartproxy_controller.go index 00e61eb5..1f608833 100644 --- a/controllers/helmchartproxy/helmchartproxy_controller.go +++ b/controllers/helmchartproxy/helmchartproxy_controller.go @@ -55,6 +55,9 @@ type helmReleaseProxyRolloutMeta struct { // Identifies whether HelmReleaseProxy exists for the cluster. hrpExists bool + + // Identifies whether HelmReleaseProxy's ready condition is True. + hrpReady bool } // SetupWithManager sets up the controller with the Manager. @@ -238,6 +241,7 @@ func (r *HelmChartProxyReconciler) reconcileNormal(ctx context.Context, helmChar nn := getNamespacedNameStringFor(ref.Namespace, ref.Name) meta := clusterNnRolloutMeta[nn] meta.hrpExists = true + meta.hrpReady = conditions.IsTrue(&h, addonsv1alpha1.HelmReleaseReadyCondition) } // Sort helmReleaseProxy rollout metadata by cluster namespaced name to @@ -288,7 +292,14 @@ func (r *HelmChartProxyReconciler) reconcileNormal(ctx context.Context, helmChar if err != nil { return err } + for _, meta := range rolloutMetaSorted { + // Exit if HelmReleaseProxyReadyCondition has not caught up to existing + // HelmReleaseProxies status. + if meta.hrpExists && !meta.hrpReady { + return nil + } + // The next batch of helmReleaseProxies have been reconciled. if count >= stepSize { return nil @@ -304,7 +315,6 @@ func (r *HelmChartProxyReconciler) reconcileNormal(ctx context.Context, helmChar } count++ } - // In cases where the count of remaining HelmReleaseProxies to be rolled // out is less than rollout step size. return nil From 4eb543894dc2bc9255528411dbcf8d987a4533a2 Mon Sep 17 00:00:00 2001 From: Chaitanya Kolluru Date: Wed, 15 Oct 2025 19:10:24 -0500 Subject: [PATCH 12/13] ensure rollout completed condition is setup for rolled out hrp Signed-off-by: Chaitanya Kolluru --- controllers/helmchartproxy/helmchartproxy_controller.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/controllers/helmchartproxy/helmchartproxy_controller.go b/controllers/helmchartproxy/helmchartproxy_controller.go index 1f608833..36da4efe 100644 --- a/controllers/helmchartproxy/helmchartproxy_controller.go +++ b/controllers/helmchartproxy/helmchartproxy_controller.go @@ -318,9 +318,10 @@ func (r *HelmChartProxyReconciler) reconcileNormal(ctx context.Context, helmChar // In cases where the count of remaining HelmReleaseProxies to be rolled // out is less than rollout step size. return nil + } else { + // RolloutStepSize is defined and all HelmReleaseProxies have been rolled out. + conditions.MarkTrue(helmChartProxy, addonsv1alpha1.HelmReleaseProxiesRolloutCompletedCondition) } - // RolloutStepSize is defined and all HelmReleaseProxies have been rolled out. - conditions.MarkTrue(helmChartProxy, addonsv1alpha1.HelmReleaseProxiesRolloutCompletedCondition) } else { // RolloutStepSize is undefined. Set HelmReleaseProxiesRolloutCompletedCondition to True with reason. conditions.MarkTrueWithNegativePolarity( From 2d32f4d9be4432546ceb950527b6facfd028588e Mon Sep 17 00:00:00 2001 From: Chaitanya Kolluru Date: Wed, 15 Oct 2025 19:20:55 -0500 Subject: [PATCH 13/13] adding a check for hrp ready condition being unknown Signed-off-by: Chaitanya Kolluru --- .../helmchartproxy_controller.go | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/controllers/helmchartproxy/helmchartproxy_controller.go b/controllers/helmchartproxy/helmchartproxy_controller.go index 36da4efe..105a8eaf 100644 --- a/controllers/helmchartproxy/helmchartproxy_controller.go +++ b/controllers/helmchartproxy/helmchartproxy_controller.go @@ -270,6 +270,38 @@ func (r *HelmChartProxyReconciler) reconcileNormal(ctx context.Context, helmChar return 0 }) + // If HelmReleaseProxiesReadyCondition is Unknown, create the first batch + // of HelmReleaseProxies and exit. + if conditions.IsUnknown(helmChartProxy, addonsv1alpha1.HelmReleaseProxiesReadyCondition) { + count := 0 + stepSize, err := intstr.GetScaledValueFromIntOrPercent(helmChartProxy.Spec.RolloutStepSize, len(clusters), true) + if err != nil { + return err + } + + // If HelmReleaseProxiesReadyCondition is Unknown and the first batch of HelmReleaseProxies have + // been created, then exit early. + if stepSize == len(helmReleaseProxies) { + return nil + } + + for _, meta := range rolloutMetaSorted { + // The first batch of helmReleaseProxies have been reconciled. + if count >= stepSize { + return nil + } + + err := r.reconcileForCluster(ctx, helmChartProxy, meta.cluster) + if err != nil { + return err + } + count++ + } + // In cases where the count of remaining HelmReleaseProxies to be rolled + // out is less than rollout step size. + return nil + } + // If HelmReleaseProxiesReadyCondition is false, reconcile existing // HelmReleaseProxies and exit. if conditions.IsFalse(helmChartProxy, addonsv1alpha1.HelmReleaseProxiesReadyCondition) {