Skip to content

Commit efa41cc

Browse files
committed
🐛 Fakeclient: Fix status apply if existing object has managedFields set
The tracker we use in the fakeclient doesn't support status operations, so we implemented them by copying everything but status and ResourceVersion from the existing object. This meant that we copied the existing objects managedFields and that in turn made the `managedFieldsObjectTracker` error out on Apply with an `metadata.managedFields must be nil`. Stop copying the managedFields in status updates.
1 parent aebc15d commit efa41cc

2 files changed

Lines changed: 38 additions & 1 deletion

File tree

pkg/client/fake/client_test.go

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1834,6 +1834,41 @@ var _ = Describe("Fake client", func() {
18341834
Expect(initial).To(BeComparableTo(actual))
18351835
})
18361836

1837+
// https://github.com/kubernetes-sigs/controller-runtime/issues/3423
1838+
It("should be able to status apply existing objects that have managedFields set", func(ctx SpecContext) {
1839+
cl := NewClientBuilder().WithStatusSubresource(&corev1.Node{}).Build()
1840+
node := corev1applyconfigurations.Node("a-node").
1841+
WithSpec(corev1applyconfigurations.NodeSpec().WithPodCIDR("some-value"))
1842+
Expect(cl.Apply(ctx, node, client.FieldOwner("test-owner"))).To(Succeed())
1843+
1844+
node = node.
1845+
WithStatus(corev1applyconfigurations.NodeStatus().WithPhase(corev1.NodeRunning))
1846+
1847+
Expect(cl.Status().Apply(ctx, node, client.FieldOwner("test-owner"))).To(Succeed())
1848+
})
1849+
1850+
It("should not be able to manually update the fieldManager through a status update", func(ctx SpecContext) {
1851+
node := &corev1.Node{
1852+
ObjectMeta: metav1.ObjectMeta{
1853+
Name: "node",
1854+
},
1855+
Spec: corev1.NodeSpec{
1856+
PodCIDR: "old-cidr",
1857+
},
1858+
}
1859+
cl := NewClientBuilder().WithStatusSubresource(&corev1.Node{}).WithObjects(node).WithReturnManagedFields().Build()
1860+
node.Spec.PodCIDR = "new-cidr"
1861+
Expect(cl.Update(ctx, node, client.FieldOwner("spec-owner"))).To(Succeed())
1862+
1863+
node.ManagedFields = []metav1.ManagedFieldsEntry{{}}
1864+
node.Status.Phase = corev1.NodeRunning
1865+
1866+
Expect(cl.Status().Update(ctx, node, client.FieldOwner("status-owner"))).To(Succeed())
1867+
Expect(node.ManagedFields).To(HaveLen(2))
1868+
Expect(node.ManagedFields[0].Manager).To(Equal("spec-owner"))
1869+
Expect(node.ManagedFields[1].Manager).To(Equal("status-owner"))
1870+
})
1871+
18371872
It("should Unmarshal the schemaless object with int64 to preserve ints", func(ctx SpecContext) {
18381873
schemeBuilder := &scheme.Builder{GroupVersion: schema.GroupVersion{Group: "test", Version: "v1"}}
18391874
schemeBuilder.Register(&WithSchemalessSpec{})

pkg/client/fake/versioned_tracker.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -231,15 +231,17 @@ func (t versionedTracker) updateObject(
231231
}
232232

233233
if t.withStatusSubresource.Has(gvk) {
234-
if isStatus { // copy everything but status and metadata.ResourceVersion from original object
234+
if isStatus { // copy everything but status, managedFields and metadata.ResourceVersion from original object
235235
if err := copyStatusFrom(obj, oldObject); err != nil {
236236
return nil, false, fmt.Errorf("failed to copy non-status field for object with status subresouce: %w", err)
237237
}
238238
passedRV := accessor.GetResourceVersion()
239+
passedManagedFields := accessor.GetManagedFields()
239240
if err := copyFrom(oldObject, obj); err != nil {
240241
return nil, false, fmt.Errorf("failed to restore non-status fields: %w", err)
241242
}
242243
accessor.SetResourceVersion(passedRV)
244+
accessor.SetManagedFields(passedManagedFields)
243245
} else { // copy status from original object
244246
if err := copyStatusFrom(oldObject, obj); err != nil {
245247
return nil, false, fmt.Errorf("failed to copy the status for object with status subresource: %w", err)

0 commit comments

Comments
 (0)