From 6a60a3da2bb3dcd804695bb792d88a554c9972f5 Mon Sep 17 00:00:00 2001 From: shuting Date: Mon, 27 Mar 2023 16:44:12 +0800 Subject: [PATCH] fix: failed to update UR status (#6676) * reconcile failed URs Signed-off-by: ShutingZhao * - getting URs by client; - lose ur status update; Signed-off-by: ShutingZhao * retry if the trigger is in termination Signed-off-by: ShutingZhao * set retry limit to 3 Signed-off-by: ShutingZhao --------- Signed-off-by: ShutingZhao --- pkg/background/common/resource.go | 2 +- pkg/background/common/util.go | 67 ++++++--------------- pkg/background/generate/generate.go | 19 +----- pkg/background/generate/utils.go | 23 +++++++ pkg/background/update_request_controller.go | 56 ++++++++--------- 5 files changed, 69 insertions(+), 98 deletions(-) diff --git a/pkg/background/common/resource.go b/pkg/background/common/resource.go index 44345c2f3f..f77bcd9f1f 100644 --- a/pkg/background/common/resource.go +++ b/pkg/background/common/resource.go @@ -34,7 +34,7 @@ func GetResource(client dclient.Interface, urSpec kyvernov1beta1.UpdateRequestSp if resource.GetDeletionTimestamp() != nil { log.V(4).Info("trigger resource is in termination", "operation", urSpec.Context.AdmissionRequestInfo.Operation) - return nil, nil + return nil, fmt.Errorf("trigger resource is in termination") } return resource, nil diff --git a/pkg/background/common/util.go b/pkg/background/common/util.go index be72bd8a85..22709c4170 100644 --- a/pkg/background/common/util.go +++ b/pkg/background/common/util.go @@ -9,62 +9,31 @@ import ( kyvernov1beta1listers "github.com/kyverno/kyverno/pkg/client/listers/kyverno/v1beta1" "github.com/kyverno/kyverno/pkg/config" "github.com/kyverno/kyverno/pkg/logging" + errors "github.com/pkg/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" - "k8s.io/client-go/util/retry" ) -func Update(client versioned.Interface, urLister kyvernov1beta1listers.UpdateRequestNamespaceLister, name string, mutator func(*kyvernov1beta1.UpdateRequest)) (*kyvernov1beta1.UpdateRequest, error) { - var ur *kyvernov1beta1.UpdateRequest - err := retry.RetryOnConflict(retry.DefaultRetry, func() error { - ur, err := urLister.Get(name) - if err != nil { - logging.Error(err, "[ATTEMPT] failed to fetch update request", "name", name) - return err - } - ur = ur.DeepCopy() - mutator(ur) - _, err = client.KyvernoV1beta1().UpdateRequests(config.KyvernoNamespace()).Update(context.TODO(), ur, metav1.UpdateOptions{}) - if err != nil { - logging.Error(err, "[ATTEMPT] failed to update update request", "name", name) - } - return err - }) - if err != nil { - logging.Error(err, "failed to update update request", "name", name) - } else { - logging.V(3).Info("updated update request", "name", name) - } - return ur, err -} - func UpdateStatus(client versioned.Interface, urLister kyvernov1beta1listers.UpdateRequestNamespaceLister, name string, state kyvernov1beta1.UpdateRequestState, message string, genResources []kyvernov1.ResourceSpec) (*kyvernov1beta1.UpdateRequest, error) { - var ur *kyvernov1beta1.UpdateRequest - err := retry.RetryOnConflict(retry.DefaultRetry, func() error { - ur, err := urLister.Get(name) - if err != nil { - logging.Error(err, "[ATTEMPT] failed to fetch update request", "name", name) - return err - } - ur = ur.DeepCopy() - ur.Status.State = state - ur.Status.Message = message - if genResources != nil { - ur.Status.GeneratedResources = genResources - } - _, err = client.KyvernoV1beta1().UpdateRequests(config.KyvernoNamespace()).UpdateStatus(context.TODO(), ur, metav1.UpdateOptions{}) - if err != nil { - logging.Error(err, "[ATTEMPT] failed to update update request status", "name", name) - return err - } - return err - }) + var latest *kyvernov1beta1.UpdateRequest + ur, err := client.KyvernoV1beta1().UpdateRequests(config.KyvernoNamespace()).Get(context.TODO(), name, metav1.GetOptions{}) if err != nil { - logging.Error(err, "failed to update update request status", "name", name) - } else { - logging.V(3).Info("updated update request status", "name", name, "status", string(state)) + return ur, errors.Wrapf(err, "failed to fetch update request") } - return ur, err + latest = ur.DeepCopy() + latest.Status.State = state + latest.Status.Message = message + if genResources != nil { + latest.Status.GeneratedResources = genResources + } + + new, err := client.KyvernoV1beta1().UpdateRequests(config.KyvernoNamespace()).UpdateStatus(context.TODO(), latest, metav1.UpdateOptions{}) + if err != nil { + return ur, errors.Wrapf(err, "failed to update ur status to %s", string(state)) + } + + logging.V(3).Info("updated update request status", "name", name, "status", string(state), "state", new.Status.State) + return ur, nil } func PolicyKey(namespace, name string) string { diff --git a/pkg/background/generate/generate.go b/pkg/background/generate/generate.go index 012eee4668..0780e50a3e 100644 --- a/pkg/background/generate/generate.go +++ b/pkg/background/generate/generate.go @@ -3,7 +3,6 @@ package generate import ( "context" "encoding/json" - "errors" "fmt" "strings" "time" @@ -27,6 +26,7 @@ import ( datautils "github.com/kyverno/kyverno/pkg/utils/data" engineutils "github.com/kyverno/kyverno/pkg/utils/engine" kubeutils "github.com/kyverno/kyverno/pkg/utils/kube" + "github.com/pkg/errors" "golang.org/x/exp/slices" admissionv1 "k8s.io/api/admission/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" @@ -95,24 +95,9 @@ func (c *GenerateController) ProcessUR(ur *kyvernov1beta1.UpdateRequest) error { trigger, err := c.getTrigger(ur.Spec) if err != nil { logger.V(3).Info("the trigger resource does not exist or is pending creation, re-queueing", "details", err.Error()) - retry, urAnnotations, err := increaseRetryAnnotation(ur) - if err != nil { + if err := updateRetryAnnotation(c.kyvernoClient, ur); err != nil { return err } - if retry > 5 { - err = c.kyvernoClient.KyvernoV1beta1().UpdateRequests(config.KyvernoNamespace()).Delete(context.TODO(), ur.GetName(), metav1.DeleteOptions{}) - if err != nil { - logger.Error(err, "exceeds retry limit, failed to delete the UR", "update request", ur.Name, "retry", retry, "resourceVersion", ur.GetResourceVersion()) - return err - } - } else { - ur.SetAnnotations(urAnnotations) - _, err = c.kyvernoClient.KyvernoV1beta1().UpdateRequests(config.KyvernoNamespace()).Update(context.TODO(), ur, metav1.UpdateOptions{}) - if err != nil { - logger.Error(err, "failed to update annotation in update request for the resource", "update request", ur.Name, "resourceVersion", ur.GetResourceVersion(), "annotations", urAnnotations, "retry", retry) - return err - } - } } if trigger == nil { diff --git a/pkg/background/generate/utils.go b/pkg/background/generate/utils.go index ca00daad18..fb838a7916 100644 --- a/pkg/background/generate/utils.go +++ b/pkg/background/generate/utils.go @@ -8,7 +8,10 @@ import ( kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1" kyvernov1beta1 "github.com/kyverno/kyverno/api/kyverno/v1beta1" "github.com/kyverno/kyverno/pkg/background/common" + "github.com/kyverno/kyverno/pkg/client/clientset/versioned" "github.com/kyverno/kyverno/pkg/clients/dclient" + "github.com/kyverno/kyverno/pkg/config" + "github.com/pkg/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" ) @@ -39,6 +42,26 @@ func increaseRetryAnnotation(ur *kyvernov1beta1.UpdateRequest) (int, map[string] return retry, urAnnotations, nil } +func updateRetryAnnotation(kyvernoClient versioned.Interface, ur *kyvernov1beta1.UpdateRequest) error { + retry, urAnnotations, err := increaseRetryAnnotation(ur) + if err != nil { + return err + } + if retry > 3 { + err = kyvernoClient.KyvernoV1beta1().UpdateRequests(config.KyvernoNamespace()).Delete(context.TODO(), ur.GetName(), metav1.DeleteOptions{}) + if err != nil { + return errors.Wrapf(err, "exceeds retry limit, failed to delete the UR: %s, retry: %v, resourceVersion: %s", ur.Name, retry, ur.GetResourceVersion()) + } + } else { + ur.SetAnnotations(urAnnotations) + _, err = kyvernoClient.KyvernoV1beta1().UpdateRequests(config.KyvernoNamespace()).Update(context.TODO(), ur, metav1.UpdateOptions{}) + if err != nil { + return errors.Wrapf(err, "failed to update annotation in update request: %s for the resource, retry: %v, resourceVersion %s, annotations: %v", ur.Name, retry, ur.GetResourceVersion(), urAnnotations) + } + } + return nil +} + func TriggerFromLabels(labels map[string]string) kyvernov1.ResourceSpec { return kyvernov1.ResourceSpec{ Kind: labels[common.GenerateTriggerKindLabel], diff --git a/pkg/background/update_request_controller.go b/pkg/background/update_request_controller.go index 27e5438d38..6c07aeb5fd 100644 --- a/pkg/background/update_request_controller.go +++ b/pkg/background/update_request_controller.go @@ -91,7 +91,6 @@ func NewController( _, _ = urInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{ AddFunc: c.addUR, UpdateFunc: c.updateUR, - DeleteFunc: c.deleteUR, }) c.informersSynced = []cache.InformerSynced{cpolInformer.Informer().HasSynced, polInformer.Informer().HasSynced, urInformer.Informer().HasSynced, namespaceInformer.Informer().HasSynced} @@ -150,7 +149,7 @@ func (c *controller) handleErr(err error, key interface{}) { if c.queue.NumRequeues(key) < maxRetries { logger.V(3).Info("retrying update request", "key", key, "error", err.Error()) - c.queue.AddRateLimited(key) + c.queue.AddAfter(key, time.Second) return } @@ -161,9 +160,6 @@ func (c *controller) handleErr(err error, key interface{}) { func (c *controller) syncUpdateRequest(key string) error { startTime := time.Now() logger.V(4).Info("started sync", "key", key, "startTime", startTime) - defer func() { - logger.V(4).Info("completed sync update request", "key", key, "processingTime", time.Since(startTime).String()) - }() _, urName, err := cache.SplitMetaNamespaceKey(key) if err != nil { return err @@ -187,8 +183,13 @@ func (c *controller) syncUpdateRequest(key string) error { } } - err = c.cleanUR(ur) - return err + urStatus, err := c.reconcileURStatus(ur) + if err != nil { + return err + } + + logger.V(4).Info("synced update request", "key", key, "processingTime", time.Since(startTime).String(), "ur status", urStatus) + return nil } func (c *controller) enqueueUpdateRequest(obj interface{}) { @@ -208,28 +209,10 @@ func (c *controller) addUR(obj interface{}) { func (c *controller) updateUR(_, cur interface{}) { curUr := cur.(*kyvernov1beta1.UpdateRequest) - c.enqueueUpdateRequest(curUr) -} - -func (c *controller) deleteUR(obj interface{}) { - ur, ok := obj.(*kyvernov1beta1.UpdateRequest) - if !ok { - tombstone, ok := obj.(cache.DeletedFinalStateUnknown) - if !ok { - logger.Info("Couldn't get object from tombstone", "obj", obj) - return - } - ur, ok = tombstone.Obj.(*kyvernov1beta1.UpdateRequest) - if !ok { - logger.Info("tombstone contained object that is not a Update Request CR", "obj", obj) - return - } - } - if ur.Status.Handler != "" { + if curUr.Status.State == kyvernov1beta1.Skip || curUr.Status.State == kyvernov1beta1.Completed { return } - // sync Handler will remove it from the queue - c.enqueueUpdateRequest(ur) + c.enqueueUpdateRequest(curUr) } func (c *controller) processUR(ur *kyvernov1beta1.UpdateRequest) error { @@ -245,11 +228,22 @@ func (c *controller) processUR(ur *kyvernov1beta1.UpdateRequest) error { return nil } -func (c *controller) cleanUR(ur *kyvernov1beta1.UpdateRequest) error { - if ur.Status.State == kyvernov1beta1.Completed { - return c.kyvernoClient.KyvernoV1beta1().UpdateRequests(config.KyvernoNamespace()).Delete(context.TODO(), ur.GetName(), metav1.DeleteOptions{}) +func (c *controller) reconcileURStatus(ur *kyvernov1beta1.UpdateRequest) (kyvernov1beta1.UpdateRequestState, error) { + new, err := c.kyvernoClient.KyvernoV1beta1().UpdateRequests(config.KyvernoNamespace()).Get(context.TODO(), ur.GetName(), metav1.GetOptions{}) + if err != nil { + logger.V(2).Info("cannot fetch latest UR, fallback to the existing one", "reason", err.Error()) + new = ur } - return nil + + var errUpdate error + switch new.Status.State { + case kyvernov1beta1.Completed: + errUpdate = c.kyvernoClient.KyvernoV1beta1().UpdateRequests(config.KyvernoNamespace()).Delete(context.TODO(), ur.GetName(), metav1.DeleteOptions{}) + case kyvernov1beta1.Failed: + new.Status.State = kyvernov1beta1.Pending + _, errUpdate = c.kyvernoClient.KyvernoV1beta1().UpdateRequests(config.KyvernoNamespace()).UpdateStatus(context.TODO(), new, metav1.UpdateOptions{}) + } + return new.Status.State, errUpdate } func (c *controller) getPolicy(key string) (kyvernov1.PolicyInterface, error) {