1319 fix throttling (kyverno#1348)

* fix policy status and generate controller issues * shorten ACTION column name * update logs * improve naming * add temp logs for troubleshooting * cleanup logs * apply generate policy to old & new resource in webhook * cleanup log messages * cleanup log messages * cleanup log messages * fix clean up of policy report in init container Co-authored-by: Jim Bugwadia <[email protected]>
realshuting · Dec 1, 2020 · 2ec5a0f · 2ec5a0f
1 parent 1c73dd9
commit 2ec5a0f
Show file tree

Hide file tree

Showing 13 changed files with 119 additions and 97 deletions.
diff --git a/cmd/initContainer/main.go b/cmd/initContainer/main.go
@@ -259,6 +259,8 @@ func removeClusterPolicyReport(client *client.Client, kind string) error {
 	for _, cpolr := range cpolrs.Items {
 		if err := client.DeleteResource(cpolr.GetAPIVersion(), cpolr.GetKind(), "", cpolr.GetName(), false); err != nil {
 			logger.Error(err, "failed to delete clusterPolicyReport", "name", cpolr.GetName())
+		} else {
+			logger.Info("successfully cleaned up ClusterPolicyReport")
 		}
 	}
 	return nil
@@ -276,10 +278,12 @@ func removePolicyReport(client *client.Client, kind string) error {
 	// name of namespace policy report follows the name convention
 	// policyreport-ns-<namespace name>
 	for _, ns := range namespaces.Items {
-		reportName := fmt.Sprintf("policyreport-ns-%s", ns.GetName())
+		reportName := fmt.Sprintf("pr-ns-%s", ns.GetName())
 		err := client.DeleteResource("", kind, ns.GetName(), reportName, false)
 		if err != nil && !errors.IsNotFound(err) {
 			logger.Error(err, "failed to delete policyReport", "name", reportName)
+		} else {
+			logger.Info("successfully cleaned up PolicyReport", "name", reportName)
 		}
 	}
 

diff --git a/pkg/dclient/client.go b/pkg/dclient/client.go
@@ -276,6 +276,10 @@ func (c ServerPreferredResources) OpenAPISchema() (*openapi_v2.Document, error)
 
 // GetGVRFromKind get the Group Version Resource from kind
 func (c ServerPreferredResources) GetGVRFromKind(kind string) schema.GroupVersionResource {
+	if kind == "" {
+		return schema.GroupVersionResource{}
+	}
+
 	_, gvr, err := c.FindResource("", kind)
 	if err != nil {
 		c.log.Info("schema not found", "kind", kind)
@@ -289,7 +293,7 @@ func (c ServerPreferredResources) GetGVRFromKind(kind string) schema.GroupVersio
 func (c ServerPreferredResources) GetGVRFromAPIVersionKind(apiVersion string, kind string) schema.GroupVersionResource {
 	_, gvr, err := c.FindResource(apiVersion, kind)
 	if err != nil {
-		c.log.Info("schema not found", "kind", kind, "apiVersion", apiVersion, "Error : ", err)
+		c.log.Info("schema not found", "kind", kind, "apiVersion", apiVersion, "error : ", err)
 		return schema.GroupVersionResource{}
 	}
 

diff --git a/pkg/engine/generation.go b/pkg/engine/generation.go
@@ -20,33 +20,37 @@ import (
 // 2. returns the list of rules that are applicable on this policy and resource, if 1 succeed
 func Generate(policyContext PolicyContext) (resp response.EngineResponse) {
 	policy := policyContext.Policy
-	resource := policyContext.NewResource
+	new := policyContext.NewResource
+	old := policyContext.OldResource
 	admissionInfo := policyContext.AdmissionInfo
 	ctx := policyContext.Context
 
 	resCache := policyContext.ResourceCache
 	jsonContext := policyContext.JSONContext
-	logger := log.Log.WithName("Generate").WithValues("policy", policy.Name, "kind", resource.GetKind(), "namespace", resource.GetNamespace(), "name", resource.GetName())
+	logger := log.Log.WithName("Generate").WithValues("policy", policy.Name, "kind", new.GetKind(), "namespace", new.GetNamespace(), "name", new.GetName())
 
-	return filterRules(policy, resource, admissionInfo, ctx, logger, policyContext.ExcludeGroupRole, resCache, jsonContext)
+	return filterRules(policy, new, old, admissionInfo, ctx, logger, policyContext.ExcludeGroupRole, resCache, jsonContext)
 }
 
-func filterRule(rule kyverno.Rule, resource unstructured.Unstructured, admissionInfo kyverno.RequestInfo, ctx context.EvalInterface, log logr.Logger, excludeGroupRole []string, resCache resourcecache.ResourceCacheIface, jsonContext *context.Context) *response.RuleResponse {
+func filterRule(rule kyverno.Rule, new, old unstructured.Unstructured, admissionInfo kyverno.RequestInfo, ctx context.EvalInterface, log logr.Logger, excludeGroupRole []string, resCache resourcecache.ResourceCacheIface, jsonContext *context.Context) *response.RuleResponse {
 	if !rule.HasGenerate() {
 		return nil
 	}
 
 	startTime := time.Now()
 
-	if err := MatchesResourceDescription(resource, rule, admissionInfo, excludeGroupRole); err != nil {
-		return &response.RuleResponse{
-			Name:    rule.Name,
-			Type:    "Generation",
-			Success: false,
-			RuleStats: response.RuleStats{
-				ProcessingTime: time.Since(startTime),
-			},
+	if err := MatchesResourceDescription(new, rule, admissionInfo, excludeGroupRole); err != nil {
+		if err := MatchesResourceDescription(old, rule, admissionInfo, excludeGroupRole); err == nil {
+			return &response.RuleResponse{
+				Name:    rule.Name,
+				Type:    "Generation",
+				Success: false,
+				RuleStats: response.RuleStats{
+					ProcessingTime: time.Since(startTime),
+				},
+			}
 		}
+		return nil
 	}
 
 	// add configmap json data to context
@@ -74,19 +78,19 @@ func filterRule(rule kyverno.Rule, resource unstructured.Unstructured, admission
 	}
 }
 
-func filterRules(policy kyverno.ClusterPolicy, resource unstructured.Unstructured, admissionInfo kyverno.RequestInfo, ctx context.EvalInterface, log logr.Logger, excludeGroupRole []string, resCache resourcecache.ResourceCacheIface, jsonContext *context.Context) response.EngineResponse {
+func filterRules(policy kyverno.ClusterPolicy, new, old unstructured.Unstructured, admissionInfo kyverno.RequestInfo, ctx context.EvalInterface, log logr.Logger, excludeGroupRole []string, resCache resourcecache.ResourceCacheIface, jsonContext *context.Context) response.EngineResponse {
 	resp := response.EngineResponse{
 		PolicyResponse: response.PolicyResponse{
 			Policy: policy.Name,
 			Resource: response.ResourceSpec{
-				Kind:      resource.GetKind(),
-				Name:      resource.GetName(),
-				Namespace: resource.GetNamespace(),
+				Kind:      new.GetKind(),
+				Name:      new.GetName(),
+				Namespace: new.GetNamespace(),
 			},
 		},
 	}
 	for _, rule := range policy.Spec.Rules {
-		if ruleResp := filterRule(rule, resource, admissionInfo, ctx, log, excludeGroupRole, resCache, jsonContext); ruleResp != nil {
+		if ruleResp := filterRule(rule, new, old, admissionInfo, ctx, log, excludeGroupRole, resCache, jsonContext); ruleResp != nil {
 			resp.PolicyResponse.Rules = append(resp.PolicyResponse.Rules, *ruleResp)
 		}
 	}

diff --git a/pkg/generate/cleanup/controller.go b/pkg/generate/cleanup/controller.go
@@ -23,7 +23,7 @@ import (
 )
 
 const (
-	maxRetries = 5
+	maxRetries = 10
 )
 
 //Controller manages life-cycle of generate-requests
@@ -204,7 +204,8 @@ func (c *Controller) enqueue(gr *kyverno.GenerateRequest) {
 		logger.Error(err, "failed to extract key")
 		return
 	}
-	logger.V(4).Info("eneque generate request", "name", gr.Name)
+
+	logger.V(5).Info("enqueue generate request", "name", gr.Name)
 	c.queue.Add(key)
 }
 
@@ -253,18 +254,18 @@ func (c *Controller) handleErr(err error, key interface{}) {
 	}
 
 	if errors.IsNotFound(err) {
+		logger.V(4).Info("dropping generate request", "key", key, "error", err.Error())
 		c.queue.Forget(key)
-		logger.V(4).Info("Dropping generate request from the queue", "key", key, "error", err.Error())
 		return
 	}
 
 	if c.queue.NumRequeues(key) < maxRetries {
-		logger.Error(err, "failed to sync generate request", "key", key)
+		logger.V(3).Info("retrying generate request", "key", key, "error", err.Error())
 		c.queue.AddRateLimited(key)
 		return
 	}
-	utilruntime.HandleError(err)
-	logger.Error(err, "dropping generate request out of the queue", "key", key)
+
+	logger.Error(err, "failed to cleanup generate request", "key", key)
 	c.queue.Forget(key)
 }
 

diff --git a/pkg/generate/generate.go b/pkg/generate/generate.go
@@ -57,7 +57,9 @@ func (c *Controller) applyGenerate(resource unstructured.Unstructured, gr kyvern
 	// build context
 	ctx := context.NewContext()
 
-	policyObj, err := c.pLister.Get(gr.Spec.Policy)
+	logger.V(3).Info("applying generate policy rule")
+
+	policyObj, err := c.policyLister.Get(gr.Spec.Policy)
 	if err != nil {
 		if apierrors.IsNotFound(err) {
 			for _, e := range gr.Status.GeneratedResources {
@@ -69,7 +71,7 @@ func (c *Controller) applyGenerate(resource unstructured.Unstructured, gr kyvern
 
 				if resp != nil && resp.GetLabels()["policy.kyverno.io/synchronize"] == "enable" {
 					if err := c.client.DeleteResource(resp.GetAPIVersion(), resp.GetKind(), resp.GetNamespace(), resp.GetName(), false); err != nil {
-						logger.Error(err, "Generated resource is not deleted", "Resource", e.Name)
+						logger.Error(err, "generated resource is not deleted", "Resource", e.Name)
 					}
 				}
 			}
@@ -86,16 +88,19 @@ func (c *Controller) applyGenerate(resource unstructured.Unstructured, gr kyvern
 		logger.Error(err, "failed to marshal resource")
 		return nil, err
 	}
+
 	err = ctx.AddResource(resourceRaw)
 	if err != nil {
 		logger.Error(err, "failed to load resource in context")
 		return nil, err
 	}
+
 	err = ctx.AddUserInfo(gr.Spec.Context.UserRequestInfo)
 	if err != nil {
 		logger.Error(err, "failed to load SA in context")
 		return nil, err
 	}
+
 	err = ctx.AddSA(gr.Spec.Context.UserRequestInfo.AdmissionUserInfo.Username)
 	if err != nil {
 		logger.Error(err, "failed to load UserInfo in context")
@@ -116,12 +121,14 @@ func (c *Controller) applyGenerate(resource unstructured.Unstructured, gr kyvern
 	engineResponse := engine.Generate(policyContext)
 	if len(engineResponse.PolicyResponse.Rules) == 0 {
 		logger.V(4).Info("policy does not apply to resource")
-		return nil, fmt.Errorf("policy %s, dont not apply to resource %v", gr.Spec.Policy, gr.Spec.Resource)
+		return nil, fmt.Errorf("policy %s, does not apply to resource %v", gr.Spec.Policy, gr.Spec.Resource)
 	}
 
 	// Removing GR if rule is failed. Used when the generate condition failed but gr exist
 	for _, r := range engineResponse.PolicyResponse.Rules {
 		if !r.Success {
+
+			logger.V(4).Info("querying all generate requests")
 			grList, err := c.kyvernoClient.KyvernoV1().GenerateRequests(config.KyvernoNamespace).List(contextdefault.TODO(), metav1.ListOptions{})
 			if err != nil {
 				logger.Error(err, "failed to list generate requests")
@@ -350,12 +357,12 @@ func applyRule(log logr.Logger, client *dclient.Client, rule kyverno.Rule, resou
 		// Reset resource version
 		newResource.SetResourceVersion("")
 		// Create the resource
-		logger.V(4).Info("creating new resource")
 		_, err = client.CreateResource(genAPIVersion, genKind, genNamespace, newResource, false)
 		if err != nil {
 			return noGenResource, err
 		}
-		logger.V(2).Info("created generated resource")
+
+		logger.V(2).Info("created resource")
 
 	} else if mode == Update {
 		var isUpdate bool