fix(backups): keep forcing objects that still lack the endpoint annotation

An object without backups.cozystack.io/endpoint stopped counting as
stale as soon as one forced render had been requested, whether or not
that render succeeded. A failed upgrade left the objects unannotated
and the gate silent until the controller restarted, which also hid the
climbing force counter meant to report exactly that failure. Such an
object now stays stale until a render stamps it; MinForceInterval
already bounds the cost.

Assisted-by: LLM
Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>
This commit is contained in:
Alexey Artamonov committed 2026-11-08 20:18:29 +03:00
1 parent dc909c778c
commit ee5e51ee6c
3 files changed
+19 -44

No files matched your search

+3 -3
View File
@@ -137,9 +137,9 @@ $tpl: $obj does not gate on crdEstablished for $crd"
# The controller's DefaultObjectsGate tells an object rendered with a stale S3
# endpoint (publishing.host changed after the render) by the
# backups.cozystack.io/endpoint annotation, because the endpoint itself sits at
# a different path in every kind. An object the chart renders without it is
# forced once and then never compared again, so a new template that forgets it
# would silently keep a stale endpoint.
# a different path in every kind. An object the chart renders without it
# counts as stale on every check, so a new template that forgets it would
# force this release every MinForceInterval forever.
@test "every rendered Strategy CR and the Velero BSL carry the rendered endpoint annotation" {
tmp=$(mktemp -d)
@@ -194,12 +194,6 @@ type DefaultObjectsGate struct {
// MinForceInterval for no reason.
lastForce time.Time
lastCredentialsForce time.Time
// renderForced records that a forced render of HelmRelease was requested
// (the annotations were patched; whether the upgrade succeeds is not
// observed). That render stamps endpointAnnotation on every object the
// chart renders, so from then on an object without it (one rendered by a
// chart that predates the annotation) is not counted as stale again.
renderForced bool
}
// errHelmReleaseSuspended reports that the target release has
@@ -423,10 +417,7 @@ func (g *DefaultObjectsGate) Check(ctx context.Context) ([]string, bool, error)
sort.Strings(objects)
_, objectsForced, err := g.force(ctx, g.HelmRelease, &g.lastForce, objects)
needed = append(needed, objects...)
if objectsForced {
forced = true
g.renderForced = true
}
forced = forced || objectsForced
errs = append(errs, err)
}
return needed, forced, errors.Join(errs...)
@@ -563,7 +554,9 @@ type defaultObjectsState struct {
}
// staleEndpoint reports an existing object rendered with another endpoint
// than want, formatted for logs. An empty want disables the comparison.
// than want, formatted for logs. An empty want disables the comparison. An
// object without the annotation (rendered by a chart that predates it) stays
// stale until a render stamps it: requesting one does not prove it landed.
func (g *DefaultObjectsGate) staleEndpoint(state *defaultObjectsState, key string, obj *unstructured.Unstructured, want string) {
if want == "" {
return
@@ -571,9 +564,7 @@ func (g *DefaultObjectsGate) staleEndpoint(state *defaultObjectsState, key strin
have, ok := obj.GetAnnotations()[endpointAnnotation]
switch {
case !ok:
if !g.renderForced {
state.stale = append(state.stale, key+" (no endpoint annotation)")
}
state.stale = append(state.stale, key+" (no endpoint annotation)")
case have != want:
state.stale = append(state.stale, fmt.Sprintf("%s (endpoint %q, want %q)", key, have, want))
}
@@ -1037,13 +1037,13 @@ func TestCheckIgnoresObjectEndpointWithoutSecretEndpoint(t *testing.T) {
}
}
// TestCheckForcesOnceForUnannotatedObjects covers objects rendered by a chart
// that predates the endpoint annotation. Their endpoint is unknown, so they
// are treated as stale until one forced render has gone through, which stamps
// the annotation. A force that did not go through (throttled) does not count,
// and once one has, an object still without the annotation (one the chart
// does not render) must not force again every MinForceInterval.
func TestCheckForcesOnceForUnannotatedObjects(t *testing.T) {
// TestCheckKeepsForcingForUnannotatedObjects covers objects rendered by a
// chart that predates the endpoint annotation. Their endpoint is unknown, so
// they count as stale until a render stamps the annotation. Having requested
// a render proves nothing: one that failed leaves them unannotated, and the
// next unthrottled check must force again rather than fall silent, so the
// force counter keeps reporting the render that does not land.
func TestCheckKeepsForcingForUnannotatedObjects(t *testing.T) {
base := time.Date(2026, 1, 2, 3, 4, 5, 0, time.UTC)
objs := []runtime.Object{helmReleaseObject(), credentialsHelmReleaseObject()}
for _, o := range allDefaultObjects() {
@@ -1065,27 +1065,11 @@ func TestCheckForcesOnceForUnannotatedObjects(t *testing.T) {
g.now = func() time.Time { return base.Add(time.Hour) }
missing, forced, err := g.Check(context.Background())
if err != nil || forced || len(missing) != 0 {
t.Fatalf("after the forced render: missing = %v, forced = %v, err = %v; want a no-op", missing, forced, err)
if err != nil || !forced || len(missing) != 7 {
t.Fatalf("after a render that left them unannotated: missing = %v, forced = %v, err = %v; want 7 reported and forced again", missing, forced, err)
}
if again := forceAnnotations(t, dyn)["reconcile.fluxcd.io/forceAt"]; again != first {
t.Errorf("forced again for unannotated objects: %q, then %q", first, again)
}
}
// TestCheckStillForcesStaleAfterUnannotatedForce pins that the once-only
// treatment covers the missing annotation alone: an annotation that names the
// wrong endpoint still forces after that.
func TestCheckStillForcesStaleAfterUnannotatedForce(t *testing.T) {
objs := []runtime.Object{helmReleaseObject(), credentialsHelmReleaseObject()}
for _, o := range allDefaultObjects() {
objs = append(objs, withEndpoint(o.(*unstructured.Unstructured), "https://s3.old.example.com"))
}
g, _ := newGate(t, []client.Object{sourceSecret("bucket-1a2b"), cozyDefaultBackupClass()}, objs...)
g.renderForced = true
if _, forced, err := g.Check(context.Background()); err != nil || !forced {
t.Fatalf("forced = %v, err = %v; want forced", forced, err)
if again := forceAnnotations(t, dyn)["reconcile.fluxcd.io/forceAt"]; again == first {
t.Errorf("not forced again for unannotated objects: forceAt still %q", first)
}
}