From a2f78ab79425b95a9cca156057913e4ba094dc7b Mon Sep 17 00:00:00 2001 From: pigletfly Date: Tue, 3 Aug 2021 15:05:29 +0800 Subject: [PATCH] Fix update rb status Signed-off-by: pigletfly --- pkg/controllers/binding/binding_controller.go | 1 + .../cluster_resource_binding_controller.go | 1 + pkg/util/helper/workstatus.go | 34 +++--- vendor/k8s.io/client-go/util/retry/OWNERS | 4 + vendor/k8s.io/client-go/util/retry/util.go | 105 ++++++++++++++++++ vendor/modules.txt | 1 + 6 files changed, 127 insertions(+), 19 deletions(-) create mode 100644 vendor/k8s.io/client-go/util/retry/OWNERS create mode 100644 vendor/k8s.io/client-go/util/retry/util.go diff --git a/pkg/controllers/binding/binding_controller.go b/pkg/controllers/binding/binding_controller.go index 853694df0..6f39b99d1 100644 --- a/pkg/controllers/binding/binding_controller.go +++ b/pkg/controllers/binding/binding_controller.go @@ -105,6 +105,7 @@ func (c *ResourceBindingController) syncBinding(binding *workv1alpha1.ResourceBi binding.GetNamespace(), binding.GetName(), err) return controllerruntime.Result{Requeue: true}, err } + klog.V(4).Infof("Update resourceBinding(%s/%s) with AggregatedStatus successfully.", binding.Namespace, binding.Name) return controllerruntime.Result{}, nil } diff --git a/pkg/controllers/binding/cluster_resource_binding_controller.go b/pkg/controllers/binding/cluster_resource_binding_controller.go index 2718495fb..5a2b2656a 100644 --- a/pkg/controllers/binding/cluster_resource_binding_controller.go +++ b/pkg/controllers/binding/cluster_resource_binding_controller.go @@ -99,6 +99,7 @@ func (c *ClusterResourceBindingController) syncBinding(binding *workv1alpha1.Clu klog.Errorf("Failed to aggregate workStatuses to clusterResourceBinding(%s). Error: %v.", binding.GetName(), err) return controllerruntime.Result{Requeue: true}, err } + klog.V(4).Infof("Update clusterResourceBinding(%s) with AggregatedStatus successfully.", binding.Name) return controllerruntime.Result{}, nil } diff --git a/pkg/util/helper/workstatus.go b/pkg/util/helper/workstatus.go index 44a62cb11..8d2cbaccf 100644 --- a/pkg/util/helper/workstatus.go +++ b/pkg/util/helper/workstatus.go @@ -9,6 +9,7 @@ import ( "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/client-go/util/retry" "k8s.io/klog/v2" "sigs.k8s.io/controller-runtime/pkg/client" @@ -33,16 +34,13 @@ func AggregateResourceBindingWorkStatus(c client.Client, binding *workv1alpha1.R binding.Namespace, binding.Name) return nil } - - binding.Status.AggregatedStatus = aggregatedStatuses - err = c.Status().Update(context.TODO(), binding) - if err != nil { - klog.Errorf("Failed update resourceBinding(%s/%s). Error: %v.", binding.Namespace, binding.Name, err) - return err - } - klog.Infof("Update resourceBinding(%s/%s) with AggregatedStatus successfully.", binding.Namespace, binding.Name) - - return nil + return retry.RetryOnConflict(retry.DefaultBackoff, func() (err error) { + if err = c.Get(context.TODO(), client.ObjectKey{Namespace: binding.Namespace, Name: binding.Name}, binding); err != nil { + return err + } + binding.Status.AggregatedStatus = aggregatedStatuses + return c.Status().Update(context.TODO(), binding) + }) } // AggregateClusterResourceBindingWorkStatus will collect all work statuses with current ClusterResourceBinding objects, @@ -60,15 +58,13 @@ func AggregateClusterResourceBindingWorkStatus(c client.Client, binding *workv1a return nil } - binding.Status.AggregatedStatus = aggregatedStatuses - err = c.Status().Update(context.TODO(), binding) - if err != nil { - klog.Errorf("Failed update clusterResourceBinding(%s). Error: %v.", binding.Name, err) - return err - } - klog.Infof("Update clusterResourceBinding(%s) with AggregatedStatus successfully.", binding.Name) - - return nil + return retry.RetryOnConflict(retry.DefaultBackoff, func() (err error) { + if err = c.Get(context.TODO(), client.ObjectKey{Name: binding.Name}, binding); err != nil { + return err + } + binding.Status.AggregatedStatus = aggregatedStatuses + return c.Status().Update(context.TODO(), binding) + }) } // assemble workStatuses from workList which list by selector and match with workload. diff --git a/vendor/k8s.io/client-go/util/retry/OWNERS b/vendor/k8s.io/client-go/util/retry/OWNERS new file mode 100644 index 000000000..dec3e88d6 --- /dev/null +++ b/vendor/k8s.io/client-go/util/retry/OWNERS @@ -0,0 +1,4 @@ +# See the OWNERS docs at https://go.k8s.io/owners + +reviewers: +- caesarxuchao diff --git a/vendor/k8s.io/client-go/util/retry/util.go b/vendor/k8s.io/client-go/util/retry/util.go new file mode 100644 index 000000000..15e2722f3 --- /dev/null +++ b/vendor/k8s.io/client-go/util/retry/util.go @@ -0,0 +1,105 @@ +/* +Copyright 2016 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package retry + +import ( + "time" + + "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/util/wait" +) + +// DefaultRetry is the recommended retry for a conflict where multiple clients +// are making changes to the same resource. +var DefaultRetry = wait.Backoff{ + Steps: 5, + Duration: 10 * time.Millisecond, + Factor: 1.0, + Jitter: 0.1, +} + +// DefaultBackoff is the recommended backoff for a conflict where a client +// may be attempting to make an unrelated modification to a resource under +// active management by one or more controllers. +var DefaultBackoff = wait.Backoff{ + Steps: 4, + Duration: 10 * time.Millisecond, + Factor: 5.0, + Jitter: 0.1, +} + +// OnError allows the caller to retry fn in case the error returned by fn is retriable +// according to the provided function. backoff defines the maximum retries and the wait +// interval between two retries. +func OnError(backoff wait.Backoff, retriable func(error) bool, fn func() error) error { + var lastErr error + err := wait.ExponentialBackoff(backoff, func() (bool, error) { + err := fn() + switch { + case err == nil: + return true, nil + case retriable(err): + lastErr = err + return false, nil + default: + return false, err + } + }) + if err == wait.ErrWaitTimeout { + err = lastErr + } + return err +} + +// RetryOnConflict is used to make an update to a resource when you have to worry about +// conflicts caused by other code making unrelated updates to the resource at the same +// time. fn should fetch the resource to be modified, make appropriate changes to it, try +// to update it, and return (unmodified) the error from the update function. On a +// successful update, RetryOnConflict will return nil. If the update function returns a +// "Conflict" error, RetryOnConflict will wait some amount of time as described by +// backoff, and then try again. On a non-"Conflict" error, or if it retries too many times +// and gives up, RetryOnConflict will return an error to the caller. +// +// err := retry.RetryOnConflict(retry.DefaultRetry, func() error { +// // Fetch the resource here; you need to refetch it on every try, since +// // if you got a conflict on the last update attempt then you need to get +// // the current version before making your own changes. +// pod, err := c.Pods("mynamespace").Get(name, metav1.GetOptions{}) +// if err ! nil { +// return err +// } +// +// // Make whatever updates to the resource are needed +// pod.Status.Phase = v1.PodFailed +// +// // Try to update +// _, err = c.Pods("mynamespace").UpdateStatus(pod) +// // You have to return err itself here (not wrapped inside another error) +// // so that RetryOnConflict can identify it correctly. +// return err +// }) +// if err != nil { +// // May be conflict if max retries were hit, or may be something unrelated +// // like permissions or a network error +// return err +// } +// ... +// +// TODO: Make Backoff an interface? +func RetryOnConflict(backoff wait.Backoff, fn func() error) error { + return OnError(backoff, errors.IsConflict, fn) +} diff --git a/vendor/modules.txt b/vendor/modules.txt index 4092a19c9..f88ac213f 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -763,6 +763,7 @@ k8s.io/client-go/util/connrotation k8s.io/client-go/util/flowcontrol k8s.io/client-go/util/homedir k8s.io/client-go/util/keyutil +k8s.io/client-go/util/retry k8s.io/client-go/util/workqueue # k8s.io/code-generator v0.20.6 => k8s.io/code-generator v0.20.6 ## explicit