Skip to content

Commit 3a53a11

Browse files
committed
Use optimistic concurrency and log retries
The Kubernetes clients provided by controller-runtime Manager fetch from a cache. When fetching then writing back a single object, one should use the object's resourceVersion to avoid races and lost updates. Issue: [sc-16285] See: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#concurrency-control-and-consistency
1 parent 99cde4f commit 3a53a11

File tree

2 files changed

+25
-0
lines changed

2 files changed

+25
-0
lines changed

internal/bridge/installation.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
"time"
2323

2424
corev1 "k8s.io/api/core/v1"
25+
apierrors "k8s.io/apimachinery/pkg/api/errors"
2526
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2627
corev1apply "k8s.io/client-go/applyconfigurations/core/v1"
2728
"sigs.k8s.io/controller-runtime/pkg/builder"
@@ -34,6 +35,7 @@ import (
3435
"sigs.k8s.io/yaml"
3536

3637
"github.com/crunchydata/postgres-operator/internal/controller/runtime"
38+
"github.com/crunchydata/postgres-operator/internal/logging"
3739
"github.com/crunchydata/postgres-operator/internal/naming"
3840
)
3941

@@ -123,6 +125,12 @@ func (r *InstallationReconciler) Reconcile(
123125
// TODO: Check for corev1.NamespaceTerminatingCause after
124126
// k8s.io/apimachinery@v0.25; see https://issue.k8s.io/108528.
125127

128+
// Write conflicts are returned as errors; log and retry with backoff.
129+
if err != nil && apierrors.IsConflict(err) {
130+
logging.FromContext(ctx).Info("Requeue", "reason", err)
131+
err, result.Requeue, result.RequeueAfter = nil, true, 0
132+
}
133+
126134
return result, err
127135
}
128136

@@ -132,6 +140,12 @@ func (r *InstallationReconciler) reconcile(ctx context.Context, read *corev1.Sec
132140
return err
133141
}
134142

143+
// We GET-extract-PATCH the Secret and do not build it up from scratch.
144+
// Send the ResourceVersion from the GET in the body of every PATCH.
145+
if len(read.ResourceVersion) != 0 {
146+
write.WithResourceVersion(read.ResourceVersion)
147+
}
148+
135149
// Read the Installation from the Secret, if any.
136150
var installation Installation
137151
if yaml.Unmarshal(read.Data[KeyBridgeToken], &installation) != nil {

internal/bridge/installation_test.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,17 @@ func TestExtractSecretContract(t *testing.T) {
6464
assert.Equal(t, *extracted.Name, "s2")
6565
}
6666
})
67+
68+
t.Run("ResourceVersion", func(t *testing.T) {
69+
versioned := &corev1.Secret{}
70+
versioned.ResourceVersion = "asdf"
71+
72+
extracted, err := corev1apply.ExtractSecret(versioned, "")
73+
assert.NilError(t, err)
74+
75+
// ResourceVersion is not copied from the original.
76+
assert.Assert(t, extracted.ResourceVersion == nil)
77+
})
6778
}
6879

6980
func TestInstallationReconcile(t *testing.T) {

0 commit comments

Comments
 (0)