fix(controller): retry the auto-snapshot RD bookkeeping stamp on update conflict

internal/controller/autosnapshot_controller.go

@@ -28,6 +28,7 @@ import (
 	"github.com/cockroachdb/errors"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/client-go/util/retry"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/manager"
@@ -338,25 +339,44 @@ func (r *AutoSnapshotRunnable) createAutoSnapshot(
 // uncreated snapshot (bumped without snapshot). The Snapshot CRD is
 // the source of truth so id-only inconsistency is recoverable on
 // retry.
+// The rd argument comes from Tick's List and is stale by the time we
+// write: the Snapshot create right before this wakes reconcilers that
+// update the RD concurrently, so a bare Update on the listed object
+// 409s (observed deterministically in the L-integration stack). The
+// stamp re-reads the RD fresh and retries the conflict away — losing
+// the stamp is worse than the extra read: the next tick re-derives
+// the SAME id and only createAutoSnapshot's AlreadyExists guard keeps
+// the loop idempotent.
 func (r *AutoSnapshotRunnable) stampRDAfterCreate(
 	ctx context.Context,
 	rd *blockstoriov1alpha1.ResourceDefinition,
 	nextID int64,
 	now time.Time,
 ) error {
-	if rd.Spec.Props == nil {
-		rd.Spec.Props = make(map[string]string)
-	}
+	err := retry.RetryOnConflict(retry.DefaultRetry, func() error {
+		var fresh blockstoriov1alpha1.ResourceDefinition
 
-	rd.Spec.Props[PropAutoSnapshotNextID] = strconv.FormatInt(nextID, 10)
+		getErr := r.Client.Get(ctx, client.ObjectKeyFromObject(rd), &fresh)
+		if getErr != nil {
+			// Bare error: RetryOnConflict matches on the apierrors type.
+			return getErr
+		}
 
-	if rd.Annotations == nil {
-		rd.Annotations = make(map[string]string)
-	}
+		if fresh.Spec.Props == nil {
+			fresh.Spec.Props = make(map[string]string)
+		}
 
-	rd.Annotations[AnnotationAutoSnapshotLastAt] = now.UTC().Format(time.RFC3339Nano)
+		fresh.Spec.Props[PropAutoSnapshotNextID] = strconv.FormatInt(nextID, 10)
 
-	err := r.Client.Update(ctx, rd)
+		if fresh.Annotations == nil {
+			fresh.Annotations = make(map[string]string)
+		}
+
+		fresh.Annotations[AnnotationAutoSnapshotLastAt] = now.UTC().Format(time.RFC3339Nano)
+
+		// Bare error: RetryOnConflict matches on the apierrors type.
+		return r.Client.Update(ctx, &fresh)
+	})
 	if err != nil {
 		return errors.Wrap(err, "update RD")
 	}

internal/controller/autosnapshot_stamp_conflict_test.go

@@ -0,0 +1,110 @@
+// SPDX-License-Identifier: Apache-2.0
+
+/*
+Copyright 2026 Cozystack contributors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package controller
+
+import (
+	"context"
+	"testing"
+
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apimachinery/pkg/types"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/client/fake"
+	"sigs.k8s.io/controller-runtime/pkg/client/interceptor"
+
+	blockstoriov1alpha1 "github.com/cozystack/blockstor/api/v1alpha1"
+)
+
+// TestAutoSnapshotStampSurvivesRDUpdateConflict pins the bookkeeping
+// half of the auto-snapshot tick against the reconciler race observed
+// in the L-integration stack (TestGroupG/AutoSnapshotPeriodicTick):
+// the RD object the runnable stamps comes from Tick's List, and the
+// Snapshot create wakes reconcilers that update the RD concurrently —
+// so the stamp's bare Update hits a 409 Conflict. Pre-fix the conflict
+// aborted the stamp (Tick swallows per-RD errors), leaving NextAutoId
+// and the last-at annotation unset: the next tick re-derived the SAME
+// id every interval and only the createAutoSnapshot AlreadyExists
+// short-circuit kept the loop from duplicating snapshots. The stamp
+// must instead re-read the RD fresh and retry the conflict away.
+func TestAutoSnapshotStampSurvivesRDUpdateConflict(t *testing.T) {
+	t.Parallel()
+
+	scheme := runtime.NewScheme()
+	if err := blockstoriov1alpha1.AddToScheme(scheme); err != nil {
+		t.Fatalf("AddToScheme: %v", err)
+	}
+
+	rd := &blockstoriov1alpha1.ResourceDefinition{
+		ObjectMeta: metav1.ObjectMeta{Name: "rd-stamp-conflict"},
+		Spec: blockstoriov1alpha1.ResourceDefinitionSpec{
+			Props: map[string]string{PropAutoSnapshotRunEvery: "1"},
+		},
+	}
+
+	// Model the racing reconciler: the FIRST RD Update lands on a
+	// stale resourceVersion and 409s; the retry (against a fresh
+	// read) succeeds.
+	conflicted := false
+	cli := fake.NewClientBuilder().
+		WithScheme(scheme).
+		WithObjects(rd).
+		WithInterceptorFuncs(interceptor.Funcs{
+			Update: func(ctx context.Context, c client.WithWatch, obj client.Object, opts ...client.UpdateOption) error {
+				_, isRD := obj.(*blockstoriov1alpha1.ResourceDefinition)
+				if isRD && !conflicted {
+					conflicted = true
+
+					return apierrors.NewConflict(
+						schema.GroupResource{Group: "blockstor.cozystack.io", Resource: "resourcedefinitions"},
+						obj.GetName(),
+						nil,
+					)
+				}
+
+				return c.Update(ctx, obj, opts...)
+			},
+		}).
+		Build()
+
+	runnable := &AutoSnapshotRunnable{Client: cli}
+
+	if err := runnable.Tick(context.Background()); err != nil {
+		t.Fatalf("Tick: %v", err)
+	}
+
+	if !conflicted {
+		t.Fatalf("interceptor never fired — the test no longer models the stamp Update")
+	}
+
+	var updated blockstoriov1alpha1.ResourceDefinition
+	if err := cli.Get(context.Background(), types.NamespacedName{Name: "rd-stamp-conflict"}, &updated); err != nil {
+		t.Fatalf("re-fetch RD: %v", err)
+	}
+
+	if got := updated.Spec.Props[PropAutoSnapshotNextID]; got != "2" {
+		t.Errorf("NextAutoId after conflicted stamp: got %q, want \"2\"", got)
+	}
+
+	if updated.Annotations[AnnotationAutoSnapshotLastAt] == "" {
+		t.Errorf("AnnotationAutoSnapshotLastAt unset after conflicted stamp; the tick would re-fire and re-derive the same id")
+	}
+}

tests/integration/harness/asserts.go

@@ -22,6 +22,7 @@ package harness
 
 import (
 	"context"
+	"os"
 	"testing"
 	"time"
 
@@ -35,26 +36,48 @@ import (
 // elapses. Fails the test via t.Fatalf on timeout. The default
 // poll interval is 100ms — short enough that a 30s budget gives
 // 300 attempts, long enough that the apiserver isn't hammered.
+//
+// The budget is stretched by scaledTimeout on CI — see its comment.
 func Eventually(t *testing.T, timeout time.Duration, predicate func() bool, msg string) {
 	t.Helper()
 
 	const pollInterval = 100 * time.Millisecond
 
-	deadline := time.Now().Add(timeout)
+	effective := scaledTimeout(timeout)
+	deadline := time.Now().Add(effective)
 
 	for {
 		if predicate() {
 			return
 		}
 
 		if time.Now().After(deadline) {
-			t.Fatalf("Eventually timed out after %s: %s", timeout, msg)
+			t.Fatalf("Eventually timed out after %s: %s", effective, msg)
 		}
 
 		time.Sleep(pollInterval)
 	}
 }
 
+// scaledTimeout stretches every Eventually budget on CI. The per-group
+// convergence constants (30s) were tuned on dev machines; GitHub-hosted
+// runners under a full suite run are several times slower, so a random
+// group blows its budget and the Integration lane rotate-flakes
+// (GroupI's ResourceConnectionPathCreate one run, GroupJ's
+// CSICreateVolumeFromEmpty the next). Eventually carries only
+// positive-convergence asserts — it returns the moment the predicate
+// passes — so green runs pay nothing for the stretch; only genuinely
+// failing runs report slower, still capped by the job-level -timeout.
+func scaledTimeout(timeout time.Duration) time.Duration {
+	const ciScale = 3
+
+	if os.Getenv("CI") == "" {
+		return timeout
+	}
+
+	return timeout * ciScale
+}
+
 // MustList returns the .Items slice of the given list-type. The
 // caller passes a pointer to an empty list (e.g. &blockstoriov1alpha1.NodeList{})
 // and a function that extracts the Items. We keep the interface

tests/integration/harness/asserts_ci_scale_test.go

@@ -0,0 +1,44 @@
+// SPDX-License-Identifier: Apache-2.0
+
+//go:build integration
+
+/*
+Copyright 2026 Cozystack contributors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package harness
+
+import (
+	"testing"
+	"time"
+)
+
+// TestScaledTimeoutStretchesOnCI pins the CI budget stretch: the
+// per-group convergence constants are tuned for dev machines, and the
+// Integration lane rotate-flaked on GitHub runners until Eventually
+// budgets were scaled there (GroupI / GroupJ 30s timeouts).
+func TestScaledTimeoutStretchesOnCI(t *testing.T) {
+	t.Setenv("CI", "true")
+
+	if got := scaledTimeout(30 * time.Second); got != 90*time.Second {
+		t.Fatalf("scaledTimeout on CI: got %s, want 90s", got)
+	}
+
+	t.Setenv("CI", "")
+
+	if got := scaledTimeout(30 * time.Second); got != 30*time.Second {
+		t.Fatalf("scaledTimeout off CI: got %s, want 30s", got)
+	}
+}