Skip to content

Commit a42701e

Browse files
authored
[v0.14] - Fixes race condition when BD secrets are transiently unavailable (#4726)
* Fix BundleDeployents race condition --------- Signed-off-by: Xavi Garcia <xavi.garcia@suse.com>
1 parent 0ace6e3 commit a42701e

File tree

8 files changed

+408
-33
lines changed

8 files changed

+408
-33
lines changed

charts/fleet-crd/templates/crds.yaml

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -962,6 +962,18 @@ spec:
962962
the bundle.
963963
nullable: true
964964
type: string
965+
waitingForValues:
966+
description: 'WaitingForValues is set to true by the bundle controller
967+
when the
968+
969+
options secret for this BundleDeployment could not be found (e.g.
970+
971+
due to transient API server pressure). While true, the agent skips
972+
973+
reconciliation to avoid deploying with missing Helm values. The
974+
975+
controller clears this flag once the secret is successfully loaded.'
976+
type: boolean
965977
type: object
966978
status:
967979
properties:

internal/cmd/agent/controller/bundledeployment_controller.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,10 @@ func (r *BundleDeploymentReconciler) Reconcile(ctx context.Context, req ctrl.Req
141141

142142
return ctrl.Result{}, err
143143
}
144+
if bd.Spec.WaitingForValues {
145+
logger.V(1).Info("BundleDeployment waiting for options secret to become available, skipping deployment")
146+
return ctrl.Result{}, nil
147+
}
144148

145149
// load the bundledeployment options from the secret, if present
146150
if bd.Spec.ValuesHash != "" {

internal/cmd/cli/target.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ func (t *Target) Run(cmd *cobra.Command, args []string) error {
101101
}
102102

103103
builder := target.New(client, client)
104-
matchedTargets, err := builder.Targets(ctx, bundle, manifestID)
104+
matchedTargets, _, err := builder.Targets(ctx, bundle, manifestID)
105105
if err != nil {
106106
return err
107107
}

internal/cmd/controller/reconciler/bundle_controller.go

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import (
2525
"github.com/rancher/fleet/internal/metrics"
2626
"github.com/rancher/fleet/internal/ocistorage"
2727
fleet "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1"
28+
"github.com/rancher/fleet/pkg/durations"
2829
fleetevent "github.com/rancher/fleet/pkg/event"
2930
"github.com/rancher/fleet/pkg/sharding"
3031
corev1 "k8s.io/api/core/v1"
@@ -62,7 +63,7 @@ type Store interface {
6263
}
6364

6465
type TargetBuilder interface {
65-
Targets(ctx context.Context, bundle *fleet.Bundle, manifestID string) ([]*target.Target, error)
66+
Targets(ctx context.Context, bundle *fleet.Bundle, manifestID string) ([]*target.Target, bool, error)
6667
}
6768

6869
// BundleReconciler reconciles a Bundle object
@@ -243,7 +244,7 @@ func (r *BundleReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
243244
}
244245
}
245246

246-
matchedTargets, err := r.Builder.Targets(ctx, bundle, manifestID)
247+
matchedTargets, secretsMissing, err := r.Builder.Targets(ctx, bundle, manifestID)
247248
if err != nil {
248249
return ctrl.Result{},
249250
r.updateErrorStatus(
@@ -328,13 +329,20 @@ func (r *BundleReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
328329
bd.Spec.OCIContents = contentsInOCI
329330
bd.Spec.HelmChartOptions = bundle.Spec.HelmOpOptions
330331

331-
valuesHash, optionsSecret, err := r.manageOptionsSecret(ctx, bd)
332-
if err != nil {
333-
return r.computeResult(ctx, logger, bundleOrig, bundle, "failed to initialize options secret", err)
334-
}
332+
var optionsSecret *corev1.Secret
333+
// If the options secret was unavailable during Targets(). Preserve the
334+
// existing ValuesHash (already present in bd.Spec from BundleDeployment())
335+
// and skip writing the secret so we don't overwrite it with empty values.
336+
if !bd.Spec.WaitingForValues {
337+
var valuesHash string
338+
valuesHash, optionsSecret, err = r.manageOptionsSecret(ctx, bd)
339+
if err != nil {
340+
return r.computeResult(ctx, logger, bundleOrig, bundle, "failed to initialize options secret", err)
341+
}
335342

336-
// Changes in the values hash trigger a bundle deployment reconcile.
337-
bd.Spec.ValuesHash = valuesHash
343+
// Changes in the values hash trigger a bundle deployment reconcile.
344+
bd.Spec.ValuesHash = valuesHash
345+
}
338346

339347
// When content resources are stored in etcd, we need to keep track of the content resource so they
340348
// are properly gargabe-collected by the content controller.
@@ -402,6 +410,13 @@ func (r *BundleReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr
402410
return ctrl.Result{}, errutil.NewAggregate(merr)
403411
}
404412

413+
if secretsMissing {
414+
// At least one BundleDeployment had its options secret transiently
415+
// unavailable. Requeue so we can retry loading those values; we cannot
416+
// rely on an external event to re-trigger the reconcile.
417+
return ctrl.Result{RequeueAfter: durations.DefaultRequeueAfter}, errutil.NewAggregate(merr)
418+
}
419+
405420
return ctrl.Result{}, errutil.NewAggregate(merr)
406421
}
407422

0 commit comments

Comments
 (0)