Skip to content

Commit

Permalink
Merge pull request #2483 from k8s-infra-cherrypick-robot/cherry-pick-…
Browse files Browse the repository at this point in the history
…2479-to-release-0.16

[release-0.16] 🐛 Do not update anything but status when using subresource client
  • Loading branch information
k8s-ci-robot authored Sep 9, 2023
2 parents 94cefd3 + 339df9a commit 580f203
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 40 deletions.
42 changes: 2 additions & 40 deletions pkg/client/fake/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -400,9 +400,10 @@ func (t versionedTracker) update(gvr schema.GroupVersionResource, obj runtime.Ob

if t.withStatusSubresource.Has(gvk) {
if isStatus { // copy everything but status and metadata.ResourceVersion from original object
if err := copyNonStatusFrom(oldObject, obj); err != nil {
if err := copyStatusFrom(obj, oldObject); err != nil {
return fmt.Errorf("failed to copy non-status field for object with status subresouce: %w", err)
}
obj = oldObject.DeepCopyObject().(client.Object)
} else { // copy status from original object
if err := copyStatusFrom(oldObject, obj); err != nil {
return fmt.Errorf("failed to copy the status for object with status subresource: %w", err)
Expand Down Expand Up @@ -949,45 +950,6 @@ func dryPatch(action testing.PatchActionImpl, tracker testing.ObjectTracker) (ru
return obj, nil
}

func copyNonStatusFrom(old, new runtime.Object) error {
newClientObject, ok := new.(client.Object)
if !ok {
return fmt.Errorf("%T is not a client.Object", new)
}
// The only thing other than status we have to retain
rv := newClientObject.GetResourceVersion()

oldMapStringAny, err := toMapStringAny(old)
if err != nil {
return fmt.Errorf("failed to convert old to *unstructured.Unstructured: %w", err)
}
newMapStringAny, err := toMapStringAny(new)
if err != nil {
return fmt.Errorf("failed to convert new to *unststructured.Unstructured: %w", err)
}

// delete everything other than status in case it has fields that were not present in
// the old object
for k := range newMapStringAny {
if k != "status" {
delete(newMapStringAny, k)
}
}
// copy everything other than status from the old object
for k := range oldMapStringAny {
if k != "status" {
newMapStringAny[k] = oldMapStringAny[k]
}
}

if err := fromMapStringAny(newMapStringAny, new); err != nil {
return fmt.Errorf("failed to convert back from map[string]any: %w", err)
}
newClientObject.SetResourceVersion(rv)

return nil
}

// copyStatusFrom copies the status from old into new
func copyStatusFrom(old, new runtime.Object) error {
oldMapStringAny, err := toMapStringAny(old)
Expand Down
37 changes: 37 additions & 0 deletions pkg/client/fake/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1460,6 +1460,13 @@ var _ = Describe("Fake client", func() {
objOriginal := obj.DeepCopy()

obj.Spec.PodCIDR = "cidr-from-status-update"
obj.Annotations = map[string]string{
"some-annotation-key": "some-annotation-value",
}
obj.Labels = map[string]string{
"some-label-key": "some-label-value",
}

obj.Status.NodeInfo.MachineID = "machine-id-from-status-update"
Expect(cl.Status().Update(context.Background(), obj)).NotTo(HaveOccurred())

Expand All @@ -1472,6 +1479,36 @@ var _ = Describe("Fake client", func() {
objOriginal.Status.NodeInfo.MachineID = "machine-id-from-status-update"
Expect(cmp.Diff(objOriginal, actual)).To(BeEmpty())
})
It("Should only override status fields of typed objects that have a status subresource on status update", func() {
obj := &corev1.Node{
ObjectMeta: metav1.ObjectMeta{
Name: "node",
},
Spec: corev1.NodeSpec{
PodCIDR: "old-cidr",
},
Status: corev1.NodeStatus{
NodeInfo: corev1.NodeSystemInfo{
MachineID: "machine-id",
},
},
}
cl := NewClientBuilder().WithStatusSubresource(obj).WithObjects(obj).Build()
objOriginal := obj.DeepCopy()

obj.Status.Phase = corev1.NodeRunning
Expect(cl.Status().Update(context.Background(), obj)).NotTo(HaveOccurred())

actual := &corev1.Node{ObjectMeta: metav1.ObjectMeta{Name: obj.Name}}
Expect(cl.Get(context.Background(), client.ObjectKeyFromObject(actual), actual)).NotTo(HaveOccurred())

objOriginal.APIVersion = actual.APIVersion
objOriginal.Kind = actual.Kind
objOriginal.ResourceVersion = actual.ResourceVersion
Expect(cmp.Diff(objOriginal, actual)).ToNot(BeEmpty())
Expect(objOriginal.Status.NodeInfo.MachineID).To(Equal(actual.Status.NodeInfo.MachineID))
Expect(objOriginal.Status.Phase).ToNot(Equal(actual.Status.Phase))
})

It("should not change the status of typed objects that have a status subresource on patch", func() {
obj := &corev1.Node{
Expand Down

0 comments on commit 580f203

Please sign in to comment.