From 6a062523a47b9211b337ae3aea80d7a0349ee71b Mon Sep 17 00:00:00 2001 From: Shuting Zhao Date: Fri, 20 Nov 2020 14:14:59 -0800 Subject: [PATCH] properly handle errors in generate controller --- pkg/generate/cleanup/cleanup.go | 7 ++----- pkg/generate/cleanup/controller.go | 9 +++++---- pkg/generate/controller.go | 9 +++++---- pkg/generate/generate.go | 14 ++++++++------ pkg/generate/resource.go | 11 ++++++++++- 5 files changed, 30 insertions(+), 20 deletions(-) diff --git a/pkg/generate/cleanup/cleanup.go b/pkg/generate/cleanup/cleanup.go index 9bc7145e2f..cfc7071002 100644 --- a/pkg/generate/cleanup/cleanup.go +++ b/pkg/generate/cleanup/cleanup.go @@ -42,14 +42,11 @@ func ownerResourceExists(log logr.Logger, client *dclient.Client, gr kyverno.Gen func deleteGeneratedResources(log logr.Logger, client *dclient.Client, gr kyverno.GenerateRequest) error { for _, genResource := range gr.Status.GeneratedResources { err := client.DeleteResource("", genResource.Kind, genResource.Namespace, genResource.Name, false) - if apierrors.IsNotFound(err) { - log.Error(err, "resource not found will not delete", "genKind", gr.Spec.Resource.Kind, "genNamespace", gr.Spec.Resource.Namespace, "genName", gr.Spec.Resource.Name) - continue - } - if err != nil { + if err != nil && !apierrors.IsNotFound(err) { return err } + log.V(3).Info("generated resource deleted", "genKind", gr.Spec.Resource.Kind, "genNamespace", gr.Spec.Resource.Namespace, "genName", gr.Spec.Resource.Name) } return nil } diff --git a/pkg/generate/cleanup/controller.go b/pkg/generate/cleanup/controller.go index a7b6d42ee8..e48641bbe6 100644 --- a/pkg/generate/cleanup/controller.go +++ b/pkg/generate/cleanup/controller.go @@ -12,6 +12,7 @@ import ( "github.com/kyverno/kyverno/pkg/constant" dclient "github.com/kyverno/kyverno/pkg/dclient" "k8s.io/apimachinery/pkg/api/errors" + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/apimachinery/pkg/util/wait" @@ -174,13 +175,13 @@ func (c *Controller) deleteGR(obj interface{}) { } for _, resource := range gr.Status.GeneratedResources { r, err := c.client.GetResource(resource.APIVersion, resource.Kind, resource.Namespace, resource.Name) - if err != nil { + if err != nil && !apierrors.IsNotFound(err) { logger.Error(err, "Generated resource is not deleted", "Resource", resource.Name) return } - labels := r.GetLabels() - if labels["policy.kyverno.io/synchronize"] == "enable" { - if err := c.client.DeleteResource(r.GetAPIVersion(), r.GetKind(), r.GetNamespace(), r.GetName(), false); err != nil { + + if r != nil && r.GetLabels()["policy.kyverno.io/synchronize"] == "enable" { + if err := c.client.DeleteResource(r.GetAPIVersion(), r.GetKind(), r.GetNamespace(), r.GetName(), false); err != nil && !apierrors.IsNotFound(err) { logger.Error(err, "Generated resource is not deleted", "Resource", r.GetName()) return } diff --git a/pkg/generate/controller.go b/pkg/generate/controller.go index 42789ecce0..f1d9754e5f 100644 --- a/pkg/generate/controller.go +++ b/pkg/generate/controller.go @@ -15,6 +15,7 @@ import ( "github.com/kyverno/kyverno/pkg/policystatus" "github.com/kyverno/kyverno/pkg/resourcecache" "k8s.io/apimachinery/pkg/api/errors" + apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/apimachinery/pkg/util/wait" @@ -220,13 +221,13 @@ func (c *Controller) deleteGR(obj interface{}) { } for _, resource := range gr.Status.GeneratedResources { r, err := c.client.GetResource(resource.APIVersion, resource.Kind, resource.Namespace, resource.Name) - if err != nil { + if err != nil && !apierrors.IsNotFound(err) { logger.Error(err, "Generated resource is not deleted", "Resource", resource.Name) continue } - labels := r.GetLabels() - if labels["policy.kyverno.io/synchronize"] == "enable" { - if err := c.client.DeleteResource(r.GetAPIVersion(), r.GetKind(), r.GetNamespace(), r.GetName(), false); err != nil { + + if r != nil && r.GetLabels()["policy.kyverno.io/synchronize"] == "enable" { + if err := c.client.DeleteResource(r.GetAPIVersion(), r.GetKind(), r.GetNamespace(), r.GetName(), false); err != nil && !apierrors.IsNotFound(err) { logger.Error(err, "Generated resource is not deleted", "Resource", r.GetName()) } } diff --git a/pkg/generate/generate.go b/pkg/generate/generate.go index 4b33462e30..30964857b2 100644 --- a/pkg/generate/generate.go +++ b/pkg/generate/generate.go @@ -34,6 +34,11 @@ func (c *Controller) processGR(gr *kyverno.GenerateRequest) error { return err } + // trigger resource is being terminated + if resource == nil { + return nil + } + // 2 - Apply the generate policy on the resource genResources, err = c.applyGenerate(*resource, *gr) @@ -57,13 +62,12 @@ func (c *Controller) applyGenerate(resource unstructured.Unstructured, gr kyvern if apierrors.IsNotFound(err) { for _, e := range gr.Status.GeneratedResources { resp, err := c.client.GetResource(e.APIVersion, e.Kind, e.Namespace, e.Name) - if err != nil { + if err != nil && !apierrors.IsNotFound(err) { logger.Error(err, "failed to find generated resource", "name", e.Name) continue } - labels := resp.GetLabels() - if labels["policy.kyverno.io/synchronize"] == "enable" { + 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) } @@ -348,8 +352,6 @@ func applyRule(log logr.Logger, client *dclient.Client, rule kyverno.Rule, resou logger.V(4).Info("creating new resource") _, err = client.CreateResource(genAPIVersion, genKind, genNamespace, newResource, false) if err != nil { - logger.Error(err, "failed to create resource", "resource", newResource.GetName()) - // Failed to create resource return noGenResource, err } logger.V(2).Info("created generated resource") @@ -403,7 +405,7 @@ func manageData(log logr.Logger, apiVersion, kind, namespace, name string, data obj, err := client.GetResource(apiVersion, kind, namespace, name) if err != nil { if apierrors.IsNotFound(err) { - log.Error(err, "resource does not exist, will try to create", "genKind", kind, "genAPIVersion", apiVersion, "genNamespace", namespace, "genName", name) + log.V(3).Info("resource does not exist, will try to create", "genKind", kind, "genAPIVersion", apiVersion, "genNamespace", namespace, "genName", name) return data, Create, nil } //something wrong while fetching resource diff --git a/pkg/generate/resource.go b/pkg/generate/resource.go index b0f0b8b8dc..63d068d98e 100644 --- a/pkg/generate/resource.go +++ b/pkg/generate/resource.go @@ -7,5 +7,14 @@ import ( ) func getResource(client *dclient.Client, resourceSpec kyverno.ResourceSpec) (*unstructured.Unstructured, error) { - return client.GetResource(resourceSpec.APIVersion, resourceSpec.Kind, resourceSpec.Namespace, resourceSpec.Name) + resource, err := client.GetResource(resourceSpec.APIVersion, resourceSpec.Kind, resourceSpec.Namespace, resourceSpec.Name) + if err != nil { + return nil, err + } + + if resource.GetDeletionTimestamp() != nil { + return nil, nil + } + + return resource, nil }