Skip to content

Commit 9f768c4

Browse files
committed
Revert the upgrade to controller-runtime 0.22
and the associated changes to the SSA handling in the test client.
1 parent fbd4bba commit 9f768c4

4 files changed

Lines changed: 43 additions & 83 deletions

File tree

go.mod

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ require (
2222
k8s.io/api v0.34.1
2323
k8s.io/apimachinery v0.34.1
2424
k8s.io/client-go v0.34.1
25-
sigs.k8s.io/controller-runtime v0.22.1
25+
sigs.k8s.io/controller-runtime v0.21.0
2626
)
2727

2828
require (
@@ -124,4 +124,4 @@ require (
124124
sigs.k8s.io/yaml v1.6.0 // indirect
125125
)
126126

127-
replace github.com/codeready-toolchain/api => github.com/metlos/api v0.0.0-20260318102359-237e681a639a
127+
replace github.com/codeready-toolchain/api => github.com/metlos/api v0.0.0-20260327105121-b79d7632f264

go.sum

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -136,8 +136,8 @@ github.com/magiconair/properties v1.8.5 h1:b6kJs+EmPFMYGkow9GiUyCyOvIwYetYJ3fSaW
136136
github.com/magiconair/properties v1.8.5/go.mod h1:y3VJvCyxH9uVvJTWEGAELF3aiYNyPKd5NZ3oSwXrF60=
137137
github.com/mailru/easyjson v0.7.7 h1:UGYAvKxe3sBsEDzO8ZeWOSlIQfWFlxbzLZe7hwFURr0=
138138
github.com/mailru/easyjson v0.7.7/go.mod h1:xzfreul335JAWq5oZzymOObrkdz5UnU4kGfJJLY9Nlc=
139-
github.com/metlos/api v0.0.0-20260318102359-237e681a639a h1:623z9TxSwryuiPqdsDGmho3OFHYTXwP68gqMwkIL4EA=
140-
github.com/metlos/api v0.0.0-20260318102359-237e681a639a/go.mod h1:AGEED+QcaXFa32pulan2/QBB+hzQXzPy/BEUCvqy4EE=
139+
github.com/metlos/api v0.0.0-20260327105121-b79d7632f264 h1:2QmcyE60xfkdoLdedDHlURak+NYgRxXnxblJeI78Lmk=
140+
github.com/metlos/api v0.0.0-20260327105121-b79d7632f264/go.mod h1:O/a3wnFwg05mWTcbPe5yFr6HrvaxLyEvwUxL56VSW2c=
141141
github.com/migueleliasweb/go-github-mock v0.0.18 h1:0lWt9MYmZQGnQE2rFtjlft/YtD6hzxuN6JJRFpujzEI=
142142
github.com/migueleliasweb/go-github-mock v0.0.18/go.mod h1:CcgXcbMoRnf3rRVHqGssuBquZDIcaplxL2W6G+xs7kM=
143143
github.com/mitchellh/copystructure v1.0.0 h1:Laisrj+bAB6b/yJwB5Bt3ITZhGJdqmxquMKeZ+mmkFQ=
@@ -363,8 +363,8 @@ k8s.io/kubectl v0.34.1 h1:1qP1oqT5Xc93K+H8J7ecpBjaz511gan89KO9Vbsh/OI=
363363
k8s.io/kubectl v0.34.1/go.mod h1:JRYlhJpGPyk3dEmJ+BuBiOB9/dAvnrALJEiY/C5qa6A=
364364
k8s.io/utils v0.0.0-20251002143259-bc988d571ff4 h1:SjGebBtkBqHFOli+05xYbK8YF1Dzkbzn+gDM4X9T4Ck=
365365
k8s.io/utils v0.0.0-20251002143259-bc988d571ff4/go.mod h1:OLgZIPagt7ERELqWJFomSt595RzquPNLL48iOWgYOg0=
366-
sigs.k8s.io/controller-runtime v0.22.1 h1:Ah1T7I+0A7ize291nJZdS1CabF/lB4E++WizgV24Eqg=
367-
sigs.k8s.io/controller-runtime v0.22.1/go.mod h1:FwiwRjkRPbiN+zp2QRp7wlTCzbUXxZ/D4OzuQUDwBHY=
366+
sigs.k8s.io/controller-runtime v0.21.0 h1:CYfjpEuicjUecRk+KAeyYh+ouUBn4llGyDYytIGcJS8=
367+
sigs.k8s.io/controller-runtime v0.21.0/go.mod h1:OSg14+F65eWqIu4DceX7k/+QRAbTTvxeQSNSOQpukWM=
368368
sigs.k8s.io/json v0.0.0-20250730193827-2d320260d730 h1:IpInykpT6ceI+QxKBbEflcR5EXP7sU1kvOlxwZh5txg=
369369
sigs.k8s.io/json v0.0.0-20250730193827-2d320260d730/go.mod h1:mdzfpAEoE6DHQEN0uh9ZbOCuHbLK5wOm7dK4ctXE9Tg=
370370
sigs.k8s.io/kustomize/api v0.20.1 h1:iWP1Ydh3/lmldBnH/S5RXgT98vWYMaTUL1ADcr+Sv7I=

pkg/client/ssa_client_test.go

Lines changed: 8 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -203,17 +203,14 @@ func TestSsaClient(t *testing.T) {
203203
Namespace: "default",
204204
ManagedFields: []metav1.ManagedFieldsEntry{
205205
{
206-
APIVersion: "v1",
207206
FieldsType: "FieldsV1",
208207
FieldsV1: &metav1.FieldsV1{Raw: []byte(`{"f:spec": {"f:selector": {}}}`)},
209208
Manager: strings.Split(rest.DefaultKubernetesUserAgent(), "/")[0],
210209
Operation: metav1.ManagedFieldsOperationUpdate,
211210
},
212211
},
213212
},
214-
Spec: corev1.ServiceSpec{
215-
Selector: map[string]string{"app": "test"},
216-
},
213+
Spec: corev1.ServiceSpec{},
217214
}
218215
toApply := obj.DeepCopy()
219216
toApply.SetManagedFields(nil)
@@ -233,28 +230,14 @@ func TestSsaClient(t *testing.T) {
233230
// then
234231
inCluster = &corev1.Service{}
235232
require.NoError(t, cl.Get(context.TODO(), runtimeclient.ObjectKeyFromObject(obj), inCluster))
236-
237-
// The SSA Apply always creates a managed field entry for the field owner.
238-
// When migration is expected, the old Update entry is converted to Apply
239-
// with the new owner, and the subsequent SSA Apply merges with it.
240-
// When migration is NOT expected, the old Update entry persists alongside
241-
// the new Apply entry.
242-
hasApplyEntry := false
243-
hasOldUpdateEntry := false
244-
oldOwner := strings.Split(rest.DefaultKubernetesUserAgent(), "/")[0]
245-
for _, mf := range inCluster.ManagedFields {
246-
if mf.Manager == "test-field-owner" && mf.Operation == metav1.ManagedFieldsOperationApply {
247-
hasApplyEntry = true
248-
}
249-
if mf.Manager == oldOwner && mf.Operation == metav1.ManagedFieldsOperationUpdate {
250-
hasOldUpdateEntry = true
251-
}
252-
}
253-
assert.True(t, hasApplyEntry, "expected Apply entry for test-field-owner")
254233
if setup.migrationExpected {
255-
assert.False(t, hasOldUpdateEntry, "expected old Update entry to be migrated away")
234+
assert.Len(t, inCluster.ManagedFields, 1)
235+
assert.Equal(t, "test-field-owner", inCluster.ManagedFields[0].Manager)
236+
assert.Equal(t, metav1.ManagedFieldsOperationApply, inCluster.ManagedFields[0].Operation)
256237
} else {
257-
assert.True(t, hasOldUpdateEntry, "expected old Update entry to still exist")
238+
assert.Len(t, inCluster.ManagedFields, 1)
239+
assert.NotEqual(t, "test-field-owner", inCluster.ManagedFields[0].Manager)
240+
assert.Equal(t, metav1.ManagedFieldsOperationUpdate, inCluster.ManagedFields[0].Operation)
258241
}
259242
})
260243
}
@@ -434,7 +417,7 @@ func TestEnsureGVK(t *testing.T) {
434417
}
435418

436419
func NewTestSsaApplyClient(t *testing.T, initObjs ...runtimeclient.Object) (*test.FakeClient, *client.SSAApplyClient) {
437-
cl := test.NewFakeClientWithManagedFields(t, initObjs...)
420+
cl := test.NewFakeClient(t, initObjs...)
438421

439422
return cl, &client.SSAApplyClient{
440423
Client: cl,

pkg/test/client.go

Lines changed: 29 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -19,30 +19,17 @@ import (
1919

2020
// NewFakeClient creates a fake K8s client with ability to override specific Get/List/Create/Update/StatusUpdate/Delete functions
2121
func NewFakeClient(t T, initObjs ...client.Object) *FakeClient {
22-
return newFakeClient(t, false, initObjs...)
23-
}
24-
25-
// NewFakeClientWithManagedFields is like NewFakeClient but configures the fake client to return managed fields.
26-
// This is needed for tests that verify SSA managed fields behavior.
27-
func NewFakeClientWithManagedFields(t T, initObjs ...client.Object) *FakeClient {
28-
return newFakeClient(t, true, initObjs...)
29-
}
30-
31-
func newFakeClient(t T, returnManagedFields bool, initObjs ...client.Object) *FakeClient {
3222
s := scheme.Scheme
3323
err := toolchainv1alpha1.AddToScheme(s)
3424
require.NoError(t, err)
3525

3626
toolchainObjs := getAllToolchainResources(s)
3727

38-
builder := fake.NewClientBuilder().
28+
cl := fake.NewClientBuilder().
3929
WithScheme(s).
4030
WithObjects(initObjs...).
41-
WithStatusSubresource(toolchainObjs...)
42-
if returnManagedFields {
43-
builder = builder.WithReturnManagedFields()
44-
}
45-
cl := builder.Build()
31+
WithStatusSubresource(toolchainObjs...).
32+
Build()
4633
return &FakeClient{Client: cl, T: t}
4734
}
4835

@@ -150,35 +137,7 @@ func Update(ctx context.Context, cl *FakeClient, obj client.Object, opts ...clie
150137
obj.SetGeneration(current.GetGeneration())
151138
}
152139

153-
// Work around a controller-runtime fake client bug where Update unconditionally
154-
// replaces managed fields with the stored version, discarding caller-set values.
155-
// A real API server preserves managed fields set by the caller, which is needed
156-
// for csaupgrade.UpgradeManagedFields to work correctly.
157-
managedFields := obj.GetManagedFields()
158-
159-
if err := cl.Client.Update(ctx, obj, opts...); err != nil {
160-
return err
161-
}
162-
163-
if managedFields != nil {
164-
// Fix the stored managed fields in the tracker using a MergePatch.
165-
// The fake client's Update unconditionally replaces managed fields with
166-
// the stored version, so we use Patch to correct them.
167-
mfJSON, err := json.Marshal(map[string]any{
168-
"metadata": map[string]any{
169-
"managedFields": managedFields,
170-
},
171-
})
172-
if err != nil {
173-
return err
174-
}
175-
if err := cl.Client.Patch(ctx, obj, client.RawPatch(types.MergePatchType, mfJSON)); err != nil {
176-
return err
177-
}
178-
obj.SetManagedFields(managedFields)
179-
}
180-
181-
return nil
140+
return cl.Client.Update(ctx, obj, opts...)
182141
}
183142

184143
func isGenerationChangeNeeded(currentObj, updatedObj client.Object) (bool, error) {
@@ -283,9 +242,19 @@ func (c *FakeClient) Patch(ctx context.Context, obj client.Object, patch client.
283242

284243
func Patch(ctx context.Context, fakeClient *FakeClient, obj client.Object, patch client.Patch, opts ...client.PatchOption) error {
285244
// Our tests assume that an update to the spec increases the Generation - this is what we do by default in the Create and Update
286-
// methods, too. We need to replicate this behavior in Patch, too.
287-
288-
// SSA patches are fully supported by the fake client now, so we just need to handle the generation bump.
245+
// methods, too. We need replicate this behavior in Patch, too.
246+
247+
// Fake client doesn't support SSA yet, so we have to be creative here and try to mock it out. Hopefully, SSA will be merged soon and we will
248+
// be able to remove this. See https://github.com/kubernetes-sigs/controller-runtime/issues/2341 and
249+
// https://github.com/kubernetes-sigs/controller-runtime/pull/2981.
250+
//
251+
// The above PR is merged but the SSA implementation in the fake client is still problematic. There is an issue and PR
252+
// that we created to fix the problem we have with controller-runtime 0.22 and 0.23:
253+
//
254+
// https://github.com/kubernetes-sigs/controller-runtime/issues/3484
255+
// https://github.com/kubernetes-sigs/controller-runtime/pull/3485
256+
//
257+
// NOTE: this doesn't really implement SSA in any sense. It is just here so that the existing tests pass.
289258

290259
found := true
291260
orig := obj.DeepCopyObject().(client.Object)
@@ -296,6 +265,18 @@ func Patch(ctx context.Context, fakeClient *FakeClient, obj client.Object, patch
296265
found = false
297266
}
298267

268+
// A non-SSA patch assumes the object must already exist and should break if it doesn't. The SSA patch, on the other hand, creates the object
269+
// if it doesn't exist.
270+
if patch == client.Apply {
271+
if !found {
272+
if err := Create(ctx, fakeClient, obj); err != nil {
273+
return err
274+
}
275+
}
276+
// the fake client actively complains if it sees an SSA patch...
277+
patch = client.Merge
278+
}
279+
299280
if found {
300281
// we need to figure out whether we should update the generation or not.
301282
// We do that by applying the patch in a dry-run and comparing the changes it made
@@ -321,10 +302,6 @@ func Patch(ctx context.Context, fakeClient *FakeClient, obj client.Object, patch
321302
if shouldUpdateGeneration {
322303
obj.SetGeneration(orig.GetGeneration() + 1)
323304
}
324-
} else if patch == client.Apply {
325-
// SSA Apply creates the object if it doesn't exist. Set Generation to 1
326-
// since the fake client doesn't do this automatically.
327-
obj.SetGeneration(1)
328305
}
329306

330307
return fakeClient.Client.Patch(ctx, obj, patch, opts...)

0 commit comments

Comments
 (0)