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.

Sign up for GitHub

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

Jump to bottom

CreateOrUpdate should only update status if it is specified #580

Merged
merged 1 commit into from
Mar 28, 2023
Merged

CreateOrUpdate should only update status if it is specified #580

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 1 addition & 15 deletions pkg/resource/kubernetes.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,6 @@ func ForDaemonSet(client kubernetes.Interface, namespace string) Interface {
UpdateFunc: func(ctx context.Context, obj runtime.Object, options metav1.UpdateOptions) (runtime.Object, error) {
return client.AppsV1().DaemonSets(namespace).Update(ctx, obj.(*appsv1.DaemonSet), options)
},
UpdateStatusFunc: func(ctx context.Context, obj runtime.Object, options metav1.UpdateOptions) (runtime.Object, error) {
return client.AppsV1().DaemonSets(namespace).UpdateStatus(ctx, obj.(*appsv1.DaemonSet), options)
},
DeleteFunc: func(ctx context.Context, name string, options metav1.DeleteOptions) error {
return client.AppsV1().DaemonSets(namespace).Delete(ctx, name, options)
},
Expand All @@ -69,9 +66,6 @@ func ForDeployment(client kubernetes.Interface, namespace string) Interface {
UpdateFunc: func(ctx context.Context, obj runtime.Object, options metav1.UpdateOptions) (runtime.Object, error) {
return client.AppsV1().Deployments(namespace).Update(ctx, obj.(*appsv1.Deployment), options)
},
UpdateStatusFunc: func(ctx context.Context, obj runtime.Object, options metav1.UpdateOptions) (runtime.Object, error) {
return client.AppsV1().Deployments(namespace).UpdateStatus(ctx, obj.(*appsv1.Deployment), options)
},
DeleteFunc: func(ctx context.Context, name string, options metav1.DeleteOptions) error {
return client.AppsV1().Deployments(namespace).Delete(ctx, name, options)
},
Expand All @@ -80,6 +74,7 @@ func ForDeployment(client kubernetes.Interface, namespace string) Interface {

// Core

//nolint:dupl //false positive - lines are similar but not duplicated
func ForNamespace(client kubernetes.Interface) Interface {
return &InterfaceFuncs{
GetFunc: func(ctx context.Context, name string, options metav1.GetOptions) (runtime.Object, error) {
Expand All @@ -91,9 +86,6 @@ func ForNamespace(client kubernetes.Interface) Interface {
UpdateFunc: func(ctx context.Context, obj runtime.Object, options metav1.UpdateOptions) (runtime.Object, error) {
return client.CoreV1().Namespaces().Update(ctx, obj.(*corev1.Namespace), options)
},
UpdateStatusFunc: func(ctx context.Context, obj runtime.Object, options metav1.UpdateOptions) (runtime.Object, error) {
return client.CoreV1().Namespaces().UpdateStatus(ctx, obj.(*corev1.Namespace), options)
},
DeleteFunc: func(ctx context.Context, name string, options metav1.DeleteOptions) error {
return client.CoreV1().Namespaces().Delete(ctx, name, options)
},
Expand All @@ -112,9 +104,6 @@ func ForPod(client kubernetes.Interface, namespace string) Interface {
UpdateFunc: func(ctx context.Context, obj runtime.Object, options metav1.UpdateOptions) (runtime.Object, error) {
return client.CoreV1().Pods(namespace).Update(ctx, obj.(*corev1.Pod), options)
},
UpdateStatusFunc: func(ctx context.Context, obj runtime.Object, options metav1.UpdateOptions) (runtime.Object, error) {
return client.CoreV1().Pods(namespace).UpdateStatus(ctx, obj.(*corev1.Pod), options)
},
DeleteFunc: func(ctx context.Context, name string, options metav1.DeleteOptions) error {
return client.CoreV1().Pods(namespace).Delete(ctx, name, options)
},
Expand All @@ -133,9 +122,6 @@ func ForService(client kubernetes.Interface, namespace string) Interface {
UpdateFunc: func(ctx context.Context, obj runtime.Object, options metav1.UpdateOptions) (runtime.Object, error) {
return client.CoreV1().Services(namespace).Update(ctx, obj.(*corev1.Service), options)
},
UpdateStatusFunc: func(ctx context.Context, obj runtime.Object, options metav1.UpdateOptions) (runtime.Object, error) {
return client.CoreV1().Services(namespace).UpdateStatus(ctx, obj.(*corev1.Service), options)
},
DeleteFunc: func(ctx context.Context, name string, options metav1.DeleteOptions) error {
return client.CoreV1().Services(namespace).Delete(ctx, name, options)
},
Expand Down
6 changes: 6 additions & 0 deletions pkg/resource/util.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ limitations under the License.
package resource

import (
"encoding/json"
"strings"
"unicode"

Expand Down Expand Up @@ -75,3 +76,8 @@ func EnsureValidName(name string) string {
return c
}, name)
}

func ToJSON(o any) string {
out, _ := json.MarshalIndent(o, "", " ")
return string(out)
}
22 changes: 13 additions & 9 deletions pkg/util/create_or_update.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,35 +119,39 @@ func maybeCreateOrUpdate(ctx context.Context, client resource.Interface, obj run
newObj := resource.MustToUnstructured(toUpdate)

origStatus := GetNestedField(origObj, StatusField)
newStatus := GetNestedField(newObj, StatusField)
if !equality.Semantic.DeepEqual(origStatus, newStatus) {
logger.V(log.LIBTRACE).Infof("Updating resource status: %#v", newStatus)
newStatus, ok := GetNestedField(newObj, StatusField).(map[string]interface{})

if !ok || len(newStatus) == 0 {
unstructured.RemoveNestedField(origObj.Object, StatusField)
unstructured.RemoveNestedField(newObj.Object, StatusField)
} else if !equality.Semantic.DeepEqual(origStatus, newStatus) {
logger.V(log.LIBTRACE).Infof("Updating resource status: %s", resource.ToJSON(newStatus))

result = OperationResultUpdated

unstructured.RemoveNestedField(origObj.Object, StatusField)
unstructured.RemoveNestedField(newObj.Object, StatusField)

// UpdateStatus for generic clients (eg dynamic client) will return NotFound error if the resource CRD
// doesn't have the status subresource so we'll ignore it.
updated, err := client.UpdateStatus(ctx, toUpdate, metav1.UpdateOptions{})
if err == nil {
resource.MustToMeta(toUpdate).SetResourceVersion(resource.MustToMeta(updated).GetResourceVersion())

unstructured.RemoveNestedField(origObj.Object, StatusField)
unstructured.RemoveNestedField(newObj.Object, StatusField)
} else if !apierrors.IsNotFound(err) {
return errors.Wrapf(err, "error updating status for %#v", toUpdate)
return errors.Wrapf(err, "error updating status %s", resource.ToJSON(toUpdate))
}
}

if equality.Semantic.DeepEqual(origObj, newObj) {
return nil
}

logger.V(log.LIBTRACE).Infof("Updating resource: %#v", obj)
logger.V(log.LIBTRACE).Infof("Updating resource: %s", resource.ToJSON(obj))

result = OperationResultUpdated
_, err = client.Update(ctx, toUpdate, metav1.UpdateOptions{})

return errors.Wrapf(err, "error updating %#v", toUpdate)
return errors.Wrapf(err, "error updating %s", resource.ToJSON(toUpdate))
})
if err != nil {
return OperationResultNone, errors.Wrap(err, "error creating or updating resource")
Expand Down
17 changes: 17 additions & 0 deletions pkg/util/create_or_update_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -376,6 +376,23 @@ func (t *createOrUpdateTestDriver) testUpdate(doUpdate func(util.OperationResult
})
})

Context("and the existing resource has a status but the status on update is empty", func() {
BeforeEach(func() {
t.pod.Status = corev1.PodStatus{Phase: corev1.PodPending}
})

JustBeforeEach(func() {
t.pod = test.GetPod(t.client, t.pod)
t.pod.Status = corev1.PodStatus{}
})

It("should not update the resource", func() {
Expect(doUpdate(util.OperationResultNone)).To(Succeed())
tests.EnsureNoActionsForResource(t.testingFake, "pods", "update")
tests.EnsureNoActionsForResource(t.testingFake, "pods/status", "update")
})
})

Context("and Update initially fails due to conflict", func() {
BeforeEach(func() {
t.client.FailOnUpdate = apierrors.NewConflict(schema.GroupResource{}, "", errors.New("conflict"))
Expand Down