fix(status): status comparison is wrong (#11203)

* fix(status): status comparison is wrong

Signed-off-by: Khaled Emara <khaled.emara@nirmata.com>

* fix(status): retry status update after first failure

Signed-off-by: Khaled Emara <khaled.emara@nirmata.com>

---------

Signed-off-by: Khaled Emara <khaled.emara@nirmata.com>
Co-authored-by: shuting <shuting@nirmata.com>

pkg/autogen/autogen.go

@@ -214,8 +214,9 @@ func convertRule(rule kyvernoRule, kind string) (*kyvernov1.Rule, error) {
 	}
 
 	out := kyvernov1.Rule{
-		Name:         rule.Name,
+		Name:                   rule.Name,
-		VerifyImages: rule.VerifyImages,
+		VerifyImages:           rule.VerifyImages,
+		SkipBackgroundRequests: rule.SkipBackgroundRequests,
 	}
 	if rule.MatchResources != nil {
 		out.MatchResources = *rule.MatchResources

pkg/autogen/rule.go

@@ -21,14 +21,15 @@ import (
 // https://github.com/kyverno/kyverno/issues/568
 
 type kyvernoRule struct {
-	Name             string                        `json:"name"`
+	Name                   string                        `json:"name"`
-	MatchResources   *kyvernov1.MatchResources     `json:"match"`
+	MatchResources         *kyvernov1.MatchResources     `json:"match"`
-	ExcludeResources *kyvernov1.MatchResources     `json:"exclude,omitempty"`
+	ExcludeResources       *kyvernov1.MatchResources     `json:"exclude,omitempty"`
-	Context          *[]kyvernov1.ContextEntry     `json:"context,omitempty"`
+	Context                *[]kyvernov1.ContextEntry     `json:"context,omitempty"`
-	AnyAllConditions *kyvernov1.ConditionsWrapper  `json:"preconditions,omitempty"`
+	AnyAllConditions       *kyvernov1.ConditionsWrapper  `json:"preconditions,omitempty"`
-	Mutation         *kyvernov1.Mutation           `json:"mutate,omitempty"`
+	Mutation               *kyvernov1.Mutation           `json:"mutate,omitempty"`
-	Validation       *kyvernov1.Validation         `json:"validate,omitempty"`
+	Validation             *kyvernov1.Validation         `json:"validate,omitempty"`
-	VerifyImages     []kyvernov1.ImageVerification `json:"verifyImages,omitempty"`
+	VerifyImages           []kyvernov1.ImageVerification `json:"verifyImages,omitempty"`
+	SkipBackgroundRequests *bool                         `json:"skipBackgroundRequests,omitempty"`
 }
 
 func createRule(rule *kyvernov1.Rule) *kyvernoRule {
@@ -36,8 +37,9 @@ func createRule(rule *kyvernov1.Rule) *kyvernoRule {
 		return nil
 	}
 	jsonFriendlyStruct := kyvernoRule{
-		Name:         rule.Name,
+		Name:                   rule.Name,
-		VerifyImages: rule.VerifyImages,
+		VerifyImages:           rule.VerifyImages,
+		SkipBackgroundRequests: rule.SkipBackgroundRequests,
 	}
 	if !datautils.DeepEqual(rule.MatchResources, kyvernov1.MatchResources{}) {
 		jsonFriendlyStruct.MatchResources = rule.MatchResources.DeepCopy()
@@ -134,9 +136,10 @@ func generateRule(name string, rule *kyvernov1.Rule, tplKey, shift string, kinds
 	if rule.Validation != nil {
 		if target := rule.Validation.GetPattern(); target != nil {
 			newValidate := &kyvernov1.Validation{
-				Message:                variables.FindAndShiftReferences(logger, rule.Validation.Message, shift, "pattern"),
+				Message:                 variables.FindAndShiftReferences(logger, rule.Validation.Message, shift, "pattern"),
-				FailureAction:          rule.Validation.FailureAction,
+				FailureAction:           rule.Validation.FailureAction,
-				FailureActionOverrides: rule.Validation.FailureActionOverrides,
+				FailureActionOverrides:  rule.Validation.FailureActionOverrides,
+				AllowExistingViolations: rule.Validation.AllowExistingViolations,
 			}
 			newValidate.SetPattern(
 				map[string]interface{}{

pkg/controllers/webhook/controller.go

@@ -23,6 +23,7 @@ import (
 	"github.com/kyverno/kyverno/pkg/controllers"
 	"github.com/kyverno/kyverno/pkg/tls"
 	controllerutils "github.com/kyverno/kyverno/pkg/utils/controller"
+	datautils "github.com/kyverno/kyverno/pkg/utils/data"
 	kubeutils "github.com/kyverno/kyverno/pkg/utils/kube"
 	runtimeutils "github.com/kyverno/kyverno/pkg/utils/runtime"
 	admissionregistrationv1 "k8s.io/api/admissionregistration/v1"
@@ -45,6 +46,7 @@ import (
 	corev1listers "k8s.io/client-go/listers/core/v1"
 	rbacv1listers "k8s.io/client-go/listers/rbac/v1"
 	"k8s.io/client-go/tools/cache"
+	"k8s.io/client-go/util/retry"
 	"k8s.io/client-go/util/workqueue"
 )
 
@@ -576,30 +578,74 @@ func (c *controller) updatePolicyStatuses(ctx context.Context) error {
 	}
 	for _, policy := range policies {
 		if policy.GetNamespace() == "" {
-			_, err = controllerutils.UpdateStatus(
+			err := controllerutils.UpdateStatus(
 				ctx,
 				policy.(*kyvernov1.ClusterPolicy),
 				c.kyvernoClient.KyvernoV1().ClusterPolicies(),
 				func(policy *kyvernov1.ClusterPolicy) error {
 					return updateStatusFunc(policy)
 				},
+				func(a *kyvernov1.ClusterPolicy, b *kyvernov1.ClusterPolicy) bool {
+					return datautils.DeepEqual(a.Status, b.Status)
+				},
 			)
 			if err != nil {
-				logger.Error(err, "failed to update clusterpolicy status", "policy", policy.GetName())
+				retryErr := retry.RetryOnConflict(retry.DefaultRetry, func() error {
-				continue
+					objNew, err := c.kyvernoClient.KyvernoV1().ClusterPolicies().Get(ctx, policy.GetName(), metav1.GetOptions{})
+					if err != nil {
+						return err
+					}
+					return controllerutils.UpdateStatus(
+						ctx,
+						objNew,
+						c.kyvernoClient.KyvernoV1().ClusterPolicies(),
+						func(policy *kyvernov1.ClusterPolicy) error {
+							return updateStatusFunc(policy)
+						},
+						func(a *kyvernov1.ClusterPolicy, b *kyvernov1.ClusterPolicy) bool {
+							return datautils.DeepEqual(a.Status, b.Status)
+						},
+					)
+				})
+				if retryErr != nil {
+					logger.Error(err, "failed to update clusterpolicy status", "policy", policy.GetName())
+					continue
+				}
 			}
 		} else {
-			_, err = controllerutils.UpdateStatus(
+			err := controllerutils.UpdateStatus(
 				ctx,
 				policy.(*kyvernov1.Policy),
 				c.kyvernoClient.KyvernoV1().Policies(policy.GetNamespace()),
 				func(policy *kyvernov1.Policy) error {
 					return updateStatusFunc(policy)
 				},
+				func(a *kyvernov1.Policy, b *kyvernov1.Policy) bool {
+					return datautils.DeepEqual(a.Status, b.Status)
+				},
 			)
 			if err != nil {
-				logger.Error(err, "failed to update policy status", "namespace", policy.GetNamespace(), "policy", policy.GetName())
+				retryErr := retry.RetryOnConflict(retry.DefaultRetry, func() error {
-				continue
+					objNew, err := c.kyvernoClient.KyvernoV1().Policies(policy.GetNamespace()).Get(ctx, policy.GetName(), metav1.GetOptions{})
+					if err != nil {
+						return err
+					}
+					return controllerutils.UpdateStatus(
+						ctx,
+						objNew,
+						c.kyvernoClient.KyvernoV1().Policies(policy.GetNamespace()),
+						func(policy *kyvernov1.Policy) error {
+							return updateStatusFunc(policy)
+						},
+						func(a *kyvernov1.Policy, b *kyvernov1.Policy) bool {
+							return datautils.DeepEqual(a.Status, b.Status)
+						},
+					)
+				})
+				if retryErr != nil {
+					logger.Error(err, "failed to update policy status", "namespace", policy.GetNamespace(), "policy", policy.GetName())
+					continue
+				}
 			}
 		}
 	}

pkg/globalcontext/externalapi/entry.go

@@ -73,7 +73,7 @@ func New(
 				}, err))
 
 				if shouldUpdateStatus {
-					if updateErr := updateStatus(ctx, gce.Name, kyvernoClient, false, entryevent.ReasonAPICallFailure); updateErr != nil {
+					if updateErr := updateStatus(ctx, gce, kyvernoClient, false, entryevent.ReasonAPICallFailure); updateErr != nil {
 						logger.Error(updateErr, "failed to update status")
 					}
 				}
@@ -83,7 +83,7 @@ func New(
 				logger.V(4).Info("api call success", "data", data)
 
 				if shouldUpdateStatus {
-					if updateErr := updateStatus(ctx, gce.Name, kyvernoClient, true, "APICallSuccess"); updateErr != nil {
+					if updateErr := updateStatus(ctx, gce, kyvernoClient, true, "APICallSuccess"); updateErr != nil {
 						logger.Error(updateErr, "failed to update status")
 					}
 				}
@@ -147,25 +147,23 @@ func doCall(ctx context.Context, caller apicall.Executor, call kyvernov1.APICall
 	return result, retryError
 }
 
-func updateStatus(ctx context.Context, gceName string, kyvernoClient versioned.Interface, ready bool, reason string) error {
+func updateStatus(ctx context.Context, gce *kyvernov2alpha1.GlobalContextEntry, kyvernoClient versioned.Interface, ready bool, reason string) error {
 	retryErr := retry.RetryOnConflict(retry.DefaultRetry, func() error {
-		latestGCE, getErr := kyvernoClient.KyvernoV2alpha1().GlobalContextEntries().Get(ctx, gceName, metav1.GetOptions{})
+		latestGCE, getErr := kyvernoClient.KyvernoV2alpha1().GlobalContextEntries().Get(ctx, gce.GetName(), metav1.GetOptions{})
 		if getErr != nil {
 			return getErr
 		}
 
-		_, updateErr := controllerutils.UpdateStatus(ctx, latestGCE, kyvernoClient.KyvernoV2alpha1().GlobalContextEntries(), func(latest *kyvernov2alpha1.GlobalContextEntry) error {
+		return controllerutils.UpdateStatus(ctx, latestGCE, kyvernoClient.KyvernoV2alpha1().GlobalContextEntries(), func(latest *kyvernov2alpha1.GlobalContextEntry) error {
 			if latest == nil {
-				return fmt.Errorf("failed to update status: %s", latestGCE.Name)
+				return fmt.Errorf("failed to update status: %s", gce.GetName())
 			}
 			latest.Status.SetReady(ready, reason)
 			if ready {
 				latest.Status.UpdateRefreshTime()
 			}
 			return nil
-		})
+		}, nil)
-
-		return updateErr
 	})
 
 	return retryErr

pkg/globalcontext/k8sresource/entry.go

@@ -19,6 +19,7 @@ import (
 	"k8s.io/client-go/dynamic"
 	"k8s.io/client-go/dynamic/dynamicinformer"
 	"k8s.io/client-go/tools/cache"
+	"k8s.io/client-go/util/retry"
 )
 
 type entry struct {
@@ -135,12 +136,20 @@ func (e *entry) Stop() {
 }
 
 func updateStatus(ctx context.Context, gce *kyvernov2alpha1.GlobalContextEntry, kyvernoClient versioned.Interface, ready bool, reason string) error {
-	_, err := controllerutils.UpdateStatus(ctx, gce, kyvernoClient.KyvernoV2alpha1().GlobalContextEntries(), func(latest *kyvernov2alpha1.GlobalContextEntry) error {
+	retryErr := retry.RetryOnConflict(retry.DefaultRetry, func() error {
-		if latest == nil {
+		latestGCE, getErr := kyvernoClient.KyvernoV2alpha1().GlobalContextEntries().Get(ctx, gce.GetName(), metav1.GetOptions{})
-			return fmt.Errorf("failed to update status: %s", gce.Name)
+		if getErr != nil {
+			return getErr
 		}
-		latest.Status.SetReady(ready, reason)
+
-		return nil
+		updateErr := controllerutils.UpdateStatus(ctx, latestGCE, kyvernoClient.KyvernoV2alpha1().GlobalContextEntries(), func(latest *kyvernov2alpha1.GlobalContextEntry) error {
+			if latest == nil {
+				return fmt.Errorf("failed to update status: %s", gce.GetName())
+			}
+			latest.Status.SetReady(ready, reason)
+			return nil
+		}, nil)
+		return updateErr
 	})
-	return err
+	return retryErr
 }

pkg/utils/controller/utils.go

@@ -150,22 +150,25 @@ func Update[T interface {
 func UpdateStatus[T interface {
 	metav1.Object
 	DeepCopy[T]
-}, S ObjectStatusClient[T]](ctx context.Context, obj T, setter S, build func(T) error,
+}, S ObjectStatusClient[T]](
-) (T, error) {
+	ctx context.Context,
-	var objNew T
+	obj T,
-	objNew, err := setter.Get(ctx, obj.GetName(), metav1.GetOptions{})
+	setter S,
-	if err != nil {
+	build func(T) error,
-		return objNew, err
+	cmp func(T, T) bool,
-	}
+) error {
-	mutated := objNew.DeepCopy()
+	mutated := obj.DeepCopy()
 	if err := build(mutated); err != nil {
-		var d T
+		return err
-		return d, err
 	} else {
-		if datautils.DeepEqual(obj, mutated) {
+		if cmp == nil {
-			return mutated, nil
+			cmp = datautils.DeepEqual[T]
+		}
+		if cmp(obj, mutated) {
+			return nil
 		} else {
-			return setter.UpdateStatus(ctx, mutated, metav1.UpdateOptions{})
+			_, err := setter.UpdateStatus(ctx, mutated, metav1.UpdateOptions{})
+			return err
 		}
 	}
 }