Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🐛 Fix returning object after status update #2489

Merged
merged 3 commits into from Sep 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 3 additions & 1 deletion pkg/client/fake/client.go
Expand Up @@ -403,7 +403,9 @@ func (t versionedTracker) update(gvr schema.GroupVersionResource, obj runtime.Ob
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)
passedRV := accessor.GetResourceVersion()
reflect.ValueOf(obj).Elem().Set(reflect.ValueOf(oldObject.DeepCopyObject()).Elem())
accessor.SetResourceVersion(passedRV)
} 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
91 changes: 87 additions & 4 deletions pkg/client/fake/client_test.go
Expand Up @@ -45,6 +45,11 @@ import (
"sigs.k8s.io/controller-runtime/pkg/client/interceptor"
)

const (
machineIDFromStatusUpdate = "machine-id-from-status-update"
cidrFromStatusUpdate = "cidr-from-status-update"
)

var _ = Describe("Fake client", func() {
var dep *appsv1.Deployment
var dep2 *appsv1.Deployment
Expand Down Expand Up @@ -1456,15 +1461,15 @@ var _ = Describe("Fake client", func() {
cl := NewClientBuilder().WithStatusSubresource(obj).WithObjects(obj).Build()
objOriginal := obj.DeepCopy()

obj.Spec.PodCIDR = "cidr-from-status-update"
obj.Spec.PodCIDR = cidrFromStatusUpdate
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"
obj.Status.NodeInfo.MachineID = machineIDFromStatusUpdate
Expect(cl.Status().Update(context.Background(), obj)).NotTo(HaveOccurred())

actual := &corev1.Node{ObjectMeta: metav1.ObjectMeta{Name: obj.Name}}
Expand All @@ -1473,9 +1478,87 @@ var _ = Describe("Fake client", func() {
objOriginal.APIVersion = actual.APIVersion
objOriginal.Kind = actual.Kind
objOriginal.ResourceVersion = actual.ResourceVersion
objOriginal.Status.NodeInfo.MachineID = "machine-id-from-status-update"
objOriginal.Status.NodeInfo.MachineID = machineIDFromStatusUpdate
Expect(cmp.Diff(objOriginal, actual)).To(BeEmpty())
})

It("should be able to update an object after updating an object's status", 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()
expectedObj := obj.DeepCopy()

obj.Status.NodeInfo.MachineID = machineIDFromStatusUpdate
Expect(cl.Status().Update(context.Background(), obj)).NotTo(HaveOccurred())

obj.Annotations = map[string]string{
"some-annotation-key": "some",
}
expectedObj.Annotations = map[string]string{
"some-annotation-key": "some",
}
Expect(cl.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())

expectedObj.APIVersion = actual.APIVersion
expectedObj.Kind = actual.Kind
expectedObj.ResourceVersion = actual.ResourceVersion
expectedObj.Status.NodeInfo.MachineID = machineIDFromStatusUpdate
Expect(cmp.Diff(expectedObj, actual)).To(BeEmpty())
})

It("should be able to update an object's status after updating an object", 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()
expectedObj := obj.DeepCopy()

obj.Annotations = map[string]string{
"some-annotation-key": "some",
}
expectedObj.Annotations = map[string]string{
"some-annotation-key": "some",
}
Expect(cl.Update(context.Background(), obj)).NotTo(HaveOccurred())

obj.Spec.PodCIDR = cidrFromStatusUpdate
obj.Status.NodeInfo.MachineID = machineIDFromStatusUpdate
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())

expectedObj.APIVersion = actual.APIVersion
expectedObj.Kind = actual.Kind
expectedObj.ResourceVersion = actual.ResourceVersion
expectedObj.Status.NodeInfo.MachineID = machineIDFromStatusUpdate
Expect(cmp.Diff(expectedObj, 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{
Expand Down Expand Up @@ -1536,7 +1619,7 @@ var _ = Describe("Fake client", func() {
cl := NewClientBuilder().WithStatusSubresource(obj).WithObjects(obj).Build()
objOriginal := obj.DeepCopy()

obj.Spec.PodCIDR = "cidr-from-status-update"
obj.Spec.PodCIDR = cidrFromStatusUpdate
obj.Status.NodeInfo.MachineID = "machine-id"
Expect(cl.Status().Patch(context.Background(), obj, client.MergeFrom(objOriginal))).NotTo(HaveOccurred())

Expand Down
1 change: 1 addition & 0 deletions pkg/client/interfaces.go
Expand Up @@ -142,6 +142,7 @@ type SubResourceWriter interface {
// Create saves the subResource object in the Kubernetes cluster. obj must be a
// struct pointer so that obj can be updated with the content returned by the Server.
Create(ctx context.Context, obj Object, subResource Object, opts ...SubResourceCreateOption) error

// Update updates the fields corresponding to the status subresource for the
// given obj. obj must be a struct pointer so that obj can be updated
// with the content returned by the Server.
Expand Down