Skip to content
Closed
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions api/v1alpha1/condition_consts.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,20 @@ const (
// ClusterSelectionFailedReason indicates that the HelmChartProxy controller failed to select the workload Clusters.
ClusterSelectionFailedReason = "ClusterSelectionFailed"

// HelmReleaseProxiesRolloutNotCompleteReason indicates that the initial rollout
// of HelmReleaseProxies has not been completed.
HelmReleaseProxiesRolloutNotCompleteReason = "HelmReleaseProxiesRolloutNotComplete"

// HelmReleaseProxiesRolloutUndefinedReason indicates that HelmChartProxy doesn't
// use Rollout Step Size to reconcile HelmReleaseProxies.
HelmReleaseProxiesRolloutUndefinedReason = "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.
Expand Down
9 changes: 9 additions & 0 deletions api/v1alpha1/helmchartproxy_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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"`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible and currently processed changes to this field during recomcile, i.e. reset condition in case of reduce or increase it value?

Copy link
Author

@chaitanyakolluru chaitanyakolluru Oct 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It depends on the list of matched clusters per the clusterSelector field in the spec. Once all HelmReleaseProxies have been spawned during initial rollout, condition HelmReleaseProxiesRolloutCompletedCondition is marked True and upon changes to either the clusterSelector or the list of clusters being managed, the next reconcile will notice the difference in count of clusters matched to the HelmReleaseProxies, sets condition HelmReleaseProxiesRolloutCompletedCondition to False and rolls them out according to the rolloutStepSize defined. Hope that answers the question.


// Options represents CLI flags passed to Helm operations (i.e. install, upgrade, delete) and
// include options such as wait, skipCRDs, timeout, waitForJobs, etc.
// +optional
Expand Down
6 changes: 6 additions & 0 deletions api/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 12 additions & 0 deletions config/crd/bases/addons.cluster.x-k8s.io_helmchartproxies.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
110 changes: 110 additions & 0 deletions controllers/helmchartproxy/helmchartproxy_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,13 @@ 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"
"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"
Expand All @@ -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)
Expand Down Expand Up @@ -197,6 +209,102 @@ 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.HelmReleaseProxiesRolloutNotCompleteReason,
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 := conditions.Get(helmChartProxy, addonsv1alpha1.HelmReleaseProxiesReadyCondition)
// If HelmReleaseProxiesReadyCondition is false, reconcile existing
// HelmReleaseProxies and exit.
if hrpReadyCond != nil && (hrpReadyCond.Status == corev1.ConditionFalse) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you utilize conditions.IsTrue() or conditions.IsFalse() here and later in the code.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Used conditions.IsFalse() when checking for false status on the condition. I don't see other instance where conditions.IsTrue() can be used.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok

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 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please note that the objects in the list are in a non-deterministic order, and processing them in batches during different reconciliation cycles may lead to unpredictable results. Are you sure this will be handled correctly?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Introduced orderliness on rollout meta before processing to prevent inconsistencies with different reconciliations over a list of matching clusters. Thanks for bringing this up.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, good

// 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
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are very similar code to the code in lines 244-255, except condition in hrpRltMeta.hrpExists. You can wrap it with function (probably anonymous) and use it here and upper.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved cluster deletion timestamp check to within the reconcileForCluster() to make those sections look cleaner. Let me know if this works for you.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, good

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.HelmReleaseProxiesRolloutUndefinedReason,
clusterv1.ConditionSeverityInfo,
"HelmChartProxy does not use rollout step",
)
}

// 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() {
Expand Down Expand Up @@ -321,6 +429,7 @@ func patchHelmChartProxy(ctx context.Context, patchHelper *patch.Helper, helmCha
conditions.WithConditions(
addonsv1alpha1.HelmReleaseProxySpecsUpToDateCondition,
addonsv1alpha1.HelmReleaseProxiesReadyCondition,
addonsv1alpha1.HelmReleaseProxiesRolloutCompletedCondition,
),
)

Expand All @@ -332,6 +441,7 @@ func patchHelmChartProxy(ctx context.Context, patchHelper *patch.Helper, helmCha
clusterv1.ReadyCondition,
addonsv1alpha1.HelmReleaseProxySpecsUpToDateCondition,
addonsv1alpha1.HelmReleaseProxiesReadyCondition,
addonsv1alpha1.HelmReleaseProxiesRolloutCompletedCondition,
}},
patch.WithStatusObservedGeneration{},
)
Expand Down
Loading