Skip to content

Commit 764cc13

Browse files
fix(controller): Skip no-op infra resource updates
1 parent 5c07d99 commit 764cc13

2 files changed

Lines changed: 104 additions & 1 deletion

File tree

internal/controller/common/resource.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"reflect"
66

77
"github.com/wandb/operator/internal/logx"
8+
"k8s.io/apimachinery/pkg/api/equality"
89
apierrors "k8s.io/apimachinery/pkg/api/errors"
910
"k8s.io/apimachinery/pkg/types"
1011
"sigs.k8s.io/controller-runtime/pkg/client"
@@ -64,8 +65,13 @@ func CrudResource[T client.Object](ctx context.Context, c client.Client, desired
6465
actualExists := !IsNil(actual) && actual.GetName() != ""
6566

6667
if actualExists && desiredExists {
67-
action = UpdateAction
68+
// Avoid no-op updates. DeepDerivative treats desired as a semantic subset of actual,
69+
// so controller-added/defaulted fields on actual do not trigger unnecessary updates.
70+
if equality.Semantic.DeepDerivative(desired, actual) {
71+
return NoAction, nil
72+
}
6873

74+
action = UpdateAction
6975
desired.SetResourceVersion(actual.GetResourceVersion())
7076
err = c.Update(ctx, desired)
7177
}
Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
package common
2+
3+
import (
4+
"context"
5+
"testing"
6+
7+
corev1 "k8s.io/api/core/v1"
8+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
9+
"k8s.io/apimachinery/pkg/runtime"
10+
"k8s.io/apimachinery/pkg/types"
11+
"sigs.k8s.io/controller-runtime/pkg/client/fake"
12+
)
13+
14+
func TestCrudResourceNoActionForSubsetDesired(t *testing.T) {
15+
scheme := runtime.NewScheme()
16+
if err := corev1.AddToScheme(scheme); err != nil {
17+
t.Fatalf("failed to add corev1 scheme: %v", err)
18+
}
19+
20+
existing := &corev1.ConfigMap{
21+
ObjectMeta: metav1.ObjectMeta{
22+
Name: "cm",
23+
Namespace: "default",
24+
Annotations: map[string]string{"controller-added": "true"},
25+
},
26+
Data: map[string]string{"key": "value"},
27+
}
28+
29+
client := fake.NewClientBuilder().WithScheme(scheme).WithObjects(existing).Build()
30+
31+
actual := &corev1.ConfigMap{}
32+
if err := client.Get(context.Background(), types.NamespacedName{Name: "cm", Namespace: "default"}, actual); err != nil {
33+
t.Fatalf("failed to get configmap: %v", err)
34+
}
35+
36+
desired := &corev1.ConfigMap{
37+
ObjectMeta: metav1.ObjectMeta{
38+
Name: "cm",
39+
Namespace: "default",
40+
},
41+
Data: map[string]string{"key": "value"},
42+
}
43+
44+
action, err := CrudResource(context.Background(), client, desired, actual)
45+
if err != nil {
46+
t.Fatalf("CrudResource returned error: %v", err)
47+
}
48+
if action != NoAction {
49+
t.Fatalf("expected no action, got %s", action)
50+
}
51+
}
52+
53+
func TestCrudResourceUpdateWhenDesiredDiffers(t *testing.T) {
54+
scheme := runtime.NewScheme()
55+
if err := corev1.AddToScheme(scheme); err != nil {
56+
t.Fatalf("failed to add corev1 scheme: %v", err)
57+
}
58+
59+
existing := &corev1.ConfigMap{
60+
ObjectMeta: metav1.ObjectMeta{
61+
Name: "cm",
62+
Namespace: "default",
63+
},
64+
Data: map[string]string{"key": "old"},
65+
}
66+
67+
client := fake.NewClientBuilder().WithScheme(scheme).WithObjects(existing).Build()
68+
69+
actual := &corev1.ConfigMap{}
70+
if err := client.Get(context.Background(), types.NamespacedName{Name: "cm", Namespace: "default"}, actual); err != nil {
71+
t.Fatalf("failed to get configmap: %v", err)
72+
}
73+
74+
desired := &corev1.ConfigMap{
75+
ObjectMeta: metav1.ObjectMeta{
76+
Name: "cm",
77+
Namespace: "default",
78+
},
79+
Data: map[string]string{"key": "new"},
80+
}
81+
82+
action, err := CrudResource(context.Background(), client, desired, actual)
83+
if err != nil {
84+
t.Fatalf("CrudResource returned error: %v", err)
85+
}
86+
if action != UpdateAction {
87+
t.Fatalf("expected update action, got %s", action)
88+
}
89+
90+
updated := &corev1.ConfigMap{}
91+
if err := client.Get(context.Background(), types.NamespacedName{Name: "cm", Namespace: "default"}, updated); err != nil {
92+
t.Fatalf("failed to get updated configmap: %v", err)
93+
}
94+
if updated.Data["key"] != "new" {
95+
t.Fatalf("expected updated data, got %q", updated.Data["key"])
96+
}
97+
}

0 commit comments

Comments
 (0)