From c3559f2b8e9201562d5caaf403871f1a6cb25cf3 Mon Sep 17 00:00:00 2001 From: shuting Date: Fri, 10 Feb 2023 18:22:11 +0800 Subject: [PATCH] chore: generate controller cleanups (#6281) * deepcopy ur before processing Signed-off-by: ShutingZhao * refactor retry annotation updates Signed-off-by: ShutingZhao * set pending status on UR creation Signed-off-by: ShutingZhao * clean up UR on completion Signed-off-by: ShutingZhao * unset Signed-off-by: ShutingZhao * linter fixes Signed-off-by: ShutingZhao * revert Signed-off-by: ShutingZhao --------- Signed-off-by: ShutingZhao --- pkg/background/generate/generate.go | 61 ++++----------------- pkg/background/generate/utils.go | 34 ++++++++++++ pkg/background/update_request_controller.go | 9 +-- 3 files changed, 51 insertions(+), 53 deletions(-) create mode 100644 pkg/background/generate/utils.go diff --git a/pkg/background/generate/generate.go b/pkg/background/generate/generate.go index 3ef417b2d6..dd72a84d8d 100644 --- a/pkg/background/generate/generate.go +++ b/pkg/background/generate/generate.go @@ -6,7 +6,6 @@ import ( "errors" "fmt" "reflect" - "strconv" "strings" "time" @@ -100,49 +99,25 @@ func (c *GenerateController) ProcessUR(ur *kyvernov1beta1.UpdateRequest) error { // Don't update status // re-queueing the UR by updating the annotation // retry - 5 times - logger.V(3).Info("resource does not exist or is pending creation, re-queueing", "details", err.Error(), "retry") - urAnnotations := ur.Annotations - - if len(urAnnotations) == 0 { - urAnnotations = map[string]string{ - urAnnotations[kyvernov1beta1.URGenerateRetryCountAnnotation]: "1", - } - } else { - if val, ok := urAnnotations[kyvernov1beta1.URGenerateRetryCountAnnotation]; ok { - sleepCountInt64, err := strconv.ParseUint(val, 10, 32) - if err != nil { - logger.Error(err, "unable to convert retry-count") - return err - } - - sleepCountInt := int(sleepCountInt64) + 1 - if sleepCountInt > 5 { - if err := deleteGeneratedResources(logger, c.client, *ur); err != nil { - return err - } - // - trigger-resource is deleted - // - generated-resources are deleted - // - > Now delete the UpdateRequest CR - return c.kyvernoClient.KyvernoV1beta1().UpdateRequests(config.KyvernoNamespace()).Delete(context.TODO(), ur.Name, metav1.DeleteOptions{}) - } else { - time.Sleep(time.Second * time.Duration(sleepCountInt)) - incrementedCountString := strconv.Itoa(sleepCountInt) - urAnnotations[kyvernov1beta1.URGenerateRetryCountAnnotation] = incrementedCountString - } - } else { - time.Sleep(time.Second * 1) - urAnnotations[kyvernov1beta1.URGenerateRetryCountAnnotation] = "1" + logger.V(3).Info("resource does not exist or is pending creation, re-queueing", "details", err.Error()) + retry, urAnnotations, err := increaseRetryAnnotation(ur) + if 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 } } ur.SetAnnotations(urAnnotations) - _, err := c.kyvernoClient.KyvernoV1beta1().UpdateRequests(config.KyvernoNamespace()).Update(context.TODO(), ur, metav1.UpdateOptions{}) + _, 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()) + 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 } - - return err } // trigger resource is being terminated @@ -841,15 +816,3 @@ func (c *GenerateController) GetUnstrResource(genResourceSpec kyvernov1.Resource } return resource, nil } - -func deleteGeneratedResources(log logr.Logger, client dclient.Interface, ur kyvernov1beta1.UpdateRequest) error { - for _, genResource := range ur.Status.GeneratedResources { - err := client.DeleteResource(context.TODO(), genResource.APIVersion, genResource.Kind, genResource.Namespace, genResource.Name, false) - if err != nil && !apierrors.IsNotFound(err) { - return err - } - - log.V(3).Info("generated resource deleted", "genKind", ur.Spec.Resource.Kind, "genNamespace", ur.Spec.Resource.Namespace, "genName", ur.Spec.Resource.Name) - } - return nil -} diff --git a/pkg/background/generate/utils.go b/pkg/background/generate/utils.go new file mode 100644 index 0000000000..08b385ea76 --- /dev/null +++ b/pkg/background/generate/utils.go @@ -0,0 +1,34 @@ +package generate + +import ( + "fmt" + "strconv" + + kyvernov1beta1 "github.com/kyverno/kyverno/api/kyverno/v1beta1" +) + +func increaseRetryAnnotation(ur *kyvernov1beta1.UpdateRequest) (int, map[string]string, error) { + urAnnotations := ur.Annotations + if len(urAnnotations) == 0 { + urAnnotations = map[string]string{ + urAnnotations[kyvernov1beta1.URGenerateRetryCountAnnotation]: "1", + } + } + + retry := 1 + val, ok := urAnnotations[kyvernov1beta1.URGenerateRetryCountAnnotation] + if !ok { + urAnnotations[kyvernov1beta1.URGenerateRetryCountAnnotation] = "1" + } else { + retryUint, err := strconv.ParseUint(val, 10, 64) + if err != nil { + return retry, urAnnotations, fmt.Errorf("unable to convert retry-count %v: %w", val, err) + } + retry = int(retryUint) + retry += 1 + incrementedRetryString := strconv.Itoa(retry) + urAnnotations[kyvernov1beta1.URGenerateRetryCountAnnotation] = incrementedRetryString + } + + return retry, urAnnotations, nil +} diff --git a/pkg/background/update_request_controller.go b/pkg/background/update_request_controller.go index 30bd6b4484..76c6e9250b 100644 --- a/pkg/background/update_request_controller.go +++ b/pkg/background/update_request_controller.go @@ -184,13 +184,14 @@ func (c *controller) syncUpdateRequest(key string) error { return err } + // Deep-copy otherwise we are mutating our cache. + ur = ur.DeepCopy() + // if not in any state, try to set it to pending if ur.Status.State == "" { - ur = ur.DeepCopy() ur.Status.State = kyvernov1beta1.Pending - if _, err := c.kyvernoClient.KyvernoV1beta1().UpdateRequests(config.KyvernoNamespace()).UpdateStatus(context.TODO(), ur, metav1.UpdateOptions{}); err != nil { - return err - } + _, err := c.kyvernoClient.KyvernoV1beta1().UpdateRequests(config.KyvernoNamespace()).UpdateStatus(context.TODO(), ur, metav1.UpdateOptions{}) + return err } // try to get the linked policy if _, err := c.getPolicy(ur.Spec.Policy); err != nil {