Skip to content

Commit fac60a9

Browse files
kvapsclaude
andcommitted
fix(controller): retry setQuorum on conflict
The RD reconciler races against itself under heavy fan-out (a Watches event from every Resource creation enqueues the RD): two reconciles read the same RD spec, one writes, the second hits 'object has been modified' on Update. Stand-side run showed every e2e after the first one regressing because the conflict bubbled up as Reconcile error and the tiebreaker / quorum stamp never landed. Refetch + retry up to 3 times on apierrors.IsConflict; bubble any other error. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
1 parent 6fe4903 commit fac60a9

1 file changed

Lines changed: 36 additions & 11 deletions

File tree

internal/controller/resourcedefinition_controller.go

Lines changed: 36 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,11 @@ package controller
1818

1919
import (
2020
"context"
21-
"errors"
21+
stderrors "errors"
2222
"slices"
2323
"strings"
2424

25+
apierrors "k8s.io/apimachinery/pkg/api/errors"
2526
"k8s.io/apimachinery/pkg/runtime"
2627
"k8s.io/apimachinery/pkg/types"
2728
ctrl "sigs.k8s.io/controller-runtime"
@@ -213,7 +214,7 @@ func (r *ResourceDefinitionReconciler) createWitness(ctx context.Context, rd *bl
213214
}
214215

215216
err = r.Store.Resources().Create(ctx, &newWitness)
216-
if err != nil && !errors.Is(err, store.ErrAlreadyExists) && !alreadyExists(err) {
217+
if err != nil && !stderrors.Is(err, store.ErrAlreadyExists) && !alreadyExists(err) {
217218
return err
218219
}
219220

@@ -238,20 +239,44 @@ func filterTieBreaker(diskless []apiv1.Resource) []apiv1.Resource {
238239
// Idempotent: returns early if the value is already what we want.
239240
// The satellite picks up the change on next dispatch and re-renders
240241
// the .res file with the new quorum policy.
242+
//
243+
// Retries on conflict because the RD reconciler races against the
244+
// resource reconciler — both can write the RD spec under heavy
245+
// reconcile pressure (e.g. fan-out from a Watches event), and a
246+
// stale local copy hits "object has been modified" on Update.
241247
func (r *ResourceDefinitionReconciler) setQuorum(ctx context.Context, rd *blockstoriov1alpha1.ResourceDefinition, value string) error {
242248
const propKey = "DrbdOptions/Resource/quorum"
243249

244-
if rd.Spec.Props != nil && rd.Spec.Props[propKey] == value {
245-
return nil
246-
}
250+
for range 3 {
251+
if rd.Spec.Props != nil && rd.Spec.Props[propKey] == value {
252+
return nil
253+
}
247254

248-
if rd.Spec.Props == nil {
249-
rd.Spec.Props = map[string]string{}
250-
}
255+
if rd.Spec.Props == nil {
256+
rd.Spec.Props = map[string]string{}
257+
}
258+
259+
rd.Spec.Props[propKey] = value
260+
261+
err := r.Update(ctx, rd)
262+
if err == nil {
263+
return nil
264+
}
265+
266+
if !apierrors.IsConflict(err) {
267+
return err
268+
}
251269

252-
rd.Spec.Props[propKey] = value
270+
// Refetch and retry.
271+
err = r.Get(ctx, client.ObjectKey{Name: rd.Name}, rd)
272+
if err != nil {
273+
return err
274+
}
275+
}
253276

254-
return r.Update(ctx, rd)
277+
return apierrors.NewConflict(
278+
blockstoriov1alpha1.GroupVersion.WithResource("resourcedefinitions").GroupResource(),
279+
rd.Name, nil)
255280
}
256281

257282
// removeWitnesses deletes every TIE_BREAKER replica of the named RD.
@@ -260,7 +285,7 @@ func (r *ResourceDefinitionReconciler) setQuorum(ctx context.Context, rd *blocks
260285
func (r *ResourceDefinitionReconciler) removeWitnesses(ctx context.Context, rdName string, witnesses []apiv1.Resource) error {
261286
for i := range witnesses {
262287
err := r.Store.Resources().Delete(ctx, rdName, witnesses[i].NodeName)
263-
if err != nil && !errors.Is(err, store.ErrNotFound) {
288+
if err != nil && !stderrors.Is(err, store.ErrNotFound) {
264289
return err
265290
}
266291
}

0 commit comments

Comments
 (0)