Skip to content

Commit

Permalink
Merge pull request #2523 from troy0820/troy0820/cherry-pick-to-releas…
Browse files Browse the repository at this point in the history
…e-0.16

[release-0.16] 🐛 Handle unstructured status update with fake client
  • Loading branch information
k8s-ci-robot authored Oct 5, 2023
2 parents 2d6b699 + f7b7450 commit 8361246
Show file tree
Hide file tree
Showing 2 changed files with 94 additions and 1 deletion.
17 changes: 16 additions & 1 deletion pkg/client/fake/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,9 @@ func (t versionedTracker) update(gvr schema.GroupVersionResource, obj runtime.Ob
return fmt.Errorf("failed to copy non-status field for object with status subresouce: %w", err)
}
passedRV := accessor.GetResourceVersion()
reflect.ValueOf(obj).Elem().Set(reflect.ValueOf(oldObject.DeepCopyObject()).Elem())
if err := copyFrom(oldObject, obj); err != nil {
return fmt.Errorf("failed to restore non-status fields: %w", err)
}
accessor.SetResourceVersion(passedRV)
} else { // copy status from original object
if err := copyStatusFrom(oldObject, obj); err != nil {
Expand Down Expand Up @@ -972,6 +974,19 @@ func copyStatusFrom(old, new runtime.Object) error {
return nil
}

// copyFrom copies from old into new
func copyFrom(old, new runtime.Object) error {
oldMapStringAny, err := toMapStringAny(old)
if err != nil {
return fmt.Errorf("failed to convert old to *unstructured.Unstructured: %w", err)
}
if err := fromMapStringAny(oldMapStringAny, new); err != nil {
return fmt.Errorf("failed to convert back from map[string]any: %w", err)
}

return nil
}

func toMapStringAny(obj runtime.Object) (map[string]any, error) {
if unstructured, isUnstructured := obj.(*unstructured.Unstructured); isUnstructured {
return unstructured.Object, nil
Expand Down
78 changes: 78 additions & 0 deletions pkg/client/fake/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1700,6 +1700,84 @@ var _ = Describe("Fake client", func() {
Expect(obj.Object["spec"]).To(BeEquivalentTo("original"))
})

It("should not change the status of known unstructured objects that have a status subresource on update", func() {
obj := &corev1.Pod{
ObjectMeta: metav1.ObjectMeta{
Name: "pod",
},
Spec: corev1.PodSpec{
RestartPolicy: corev1.RestartPolicyAlways,
},
Status: corev1.PodStatus{
Phase: corev1.PodPending,
},
}
cl := NewClientBuilder().WithStatusSubresource(obj).WithObjects(obj).Build()

// update using unstructured
u := &unstructured.Unstructured{}
u.SetAPIVersion("v1")
u.SetKind("Pod")
u.SetName(obj.Name)
err := cl.Get(context.Background(), client.ObjectKeyFromObject(u), u)
Expect(err).NotTo(HaveOccurred())

err = unstructured.SetNestedField(u.Object, string(corev1.RestartPolicyNever), "spec", "restartPolicy")
Expect(err).NotTo(HaveOccurred())
err = unstructured.SetNestedField(u.Object, string(corev1.PodRunning), "status", "phase")
Expect(err).NotTo(HaveOccurred())

Expect(cl.Update(context.Background(), u)).To(Succeed())

actual := &corev1.Pod{}
Expect(cl.Get(context.Background(), client.ObjectKeyFromObject(obj), actual)).To(Succeed())
obj.APIVersion = u.GetAPIVersion()
obj.Kind = u.GetKind()
obj.ResourceVersion = actual.ResourceVersion
// only the spec mutation should persist
obj.Spec.RestartPolicy = corev1.RestartPolicyNever
Expect(cmp.Diff(obj, actual)).To(BeEmpty())
})

It("should not change non-status field of known unstructured objects that have a status subresource on status update", func() {
obj := &corev1.Pod{
ObjectMeta: metav1.ObjectMeta{
Name: "pod",
},
Spec: corev1.PodSpec{
RestartPolicy: corev1.RestartPolicyAlways,
},
Status: corev1.PodStatus{
Phase: corev1.PodPending,
},
}
cl := NewClientBuilder().WithStatusSubresource(obj).WithObjects(obj).Build()

// status update using unstructured
u := &unstructured.Unstructured{}
u.SetAPIVersion("v1")
u.SetKind("Pod")
u.SetName(obj.Name)
err := cl.Get(context.Background(), client.ObjectKeyFromObject(u), u)
Expect(err).NotTo(HaveOccurred())

err = unstructured.SetNestedField(u.Object, string(corev1.RestartPolicyNever), "spec", "restartPolicy")
Expect(err).NotTo(HaveOccurred())
err = unstructured.SetNestedField(u.Object, string(corev1.PodRunning), "status", "phase")
Expect(err).NotTo(HaveOccurred())

Expect(cl.Status().Update(context.Background(), u)).To(Succeed())

actual := &corev1.Pod{}
Expect(cl.Get(context.Background(), client.ObjectKeyFromObject(obj), actual)).To(Succeed())
obj.APIVersion = "v1"
obj.Kind = "Pod"
obj.ResourceVersion = actual.ResourceVersion
// only the status mutation should persist
obj.Status.Phase = corev1.PodRunning
Expect(cmp.Diff(obj, actual)).To(BeEmpty())
})

It("should not change the status of unstructured objects that are configured to have a status subresource on patch", func() {
obj := &unstructured.Unstructured{}
obj.SetAPIVersion("foo/v1")
Expand Down

0 comments on commit 8361246

Please sign in to comment.