From aaab55a0367166110f4a08c9cffc896114e6ab05 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Charles-Edouard=20Br=C3=A9t=C3=A9ch=C3=A9?= Date: Tue, 3 Jan 2023 14:51:37 +0100 Subject: [PATCH] feat: improve background scan reports enqueue logic (#5810) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat: improve background scan reports enqueue logic Signed-off-by: Charles-Edouard Brétéché * delay Signed-off-by: Charles-Edouard Brétéché * delay Signed-off-by: Charles-Edouard Brétéché * aggregation delay Signed-off-by: Charles-Edouard Brétéché * kuttl Signed-off-by: Charles-Edouard Brétéché * kuttl timeout Signed-off-by: Charles-Edouard Brétéché * delay Signed-off-by: Charles-Edouard Brétéché * kuttl timeout Signed-off-by: Charles-Edouard Brétéché Signed-off-by: Charles-Edouard Brétéché --- pkg/controllers/report/aggregate/controller.go | 12 ++++++------ pkg/controllers/report/background/controller.go | 14 ++++---------- pkg/controllers/report/utils/utils.go | 10 +--------- test/conformance/kuttl/kuttl-test.yaml | 2 +- .../test-report-background-mode/01-pod.yaml | 6 ++++++ .../test-report-background-mode/02-policy.yaml | 6 ++++++ .../test-report-background-mode/03-report.yaml | 4 ++++ .../test-report-background-mode/99-cleanup.yaml | 4 ---- .../test-report-background-mode/README.md | 9 ++++++++- .../{01-assert.yaml => pod-assert.yaml} | 0 .../{01-manifests.yaml => pod.yaml} | 0 .../{02-assert.yaml => policy-assert.yaml} | 0 .../{02-cpol.yaml => policy.yaml} | 0 .../{03-assert.yaml => report-assert.yaml} | 2 +- 14 files changed, 37 insertions(+), 32 deletions(-) create mode 100644 test/conformance/kuttl/reports/background/test-report-background-mode/01-pod.yaml create mode 100644 test/conformance/kuttl/reports/background/test-report-background-mode/02-policy.yaml create mode 100644 test/conformance/kuttl/reports/background/test-report-background-mode/03-report.yaml delete mode 100644 test/conformance/kuttl/reports/background/test-report-background-mode/99-cleanup.yaml rename test/conformance/kuttl/reports/background/test-report-background-mode/{01-assert.yaml => pod-assert.yaml} (100%) rename test/conformance/kuttl/reports/background/test-report-background-mode/{01-manifests.yaml => pod.yaml} (100%) rename test/conformance/kuttl/reports/background/test-report-background-mode/{02-assert.yaml => policy-assert.yaml} (100%) rename test/conformance/kuttl/reports/background/test-report-background-mode/{02-cpol.yaml => policy.yaml} (100%) rename test/conformance/kuttl/reports/background/test-report-background-mode/{03-assert.yaml => report-assert.yaml} (98%) diff --git a/pkg/controllers/report/aggregate/controller.go b/pkg/controllers/report/aggregate/controller.go index 4ac92ff951..c3512272a1 100644 --- a/pkg/controllers/report/aggregate/controller.go +++ b/pkg/controllers/report/aggregate/controller.go @@ -36,6 +36,7 @@ const ( ControllerName = "aggregate-report-controller" maxRetries = 10 mergeLimit = 1000 + enqueueDelay = 30 * time.Second ) type controller struct { @@ -94,15 +95,14 @@ func NewController( metadataCache: metadataCache, chunkSize: chunkSize, } - delay := 15 * time.Second - controllerutils.AddDelayedExplicitEventHandlers(logger, polrInformer.Informer(), c.queue, delay, keyFunc) - controllerutils.AddDelayedExplicitEventHandlers(logger, cpolrInformer.Informer(), c.queue, delay, keyFunc) - controllerutils.AddDelayedExplicitEventHandlers(logger, bgscanrInformer.Informer(), c.queue, delay, keyFunc) - controllerutils.AddDelayedExplicitEventHandlers(logger, cbgscanrInformer.Informer(), c.queue, delay, keyFunc) + controllerutils.AddDelayedExplicitEventHandlers(logger, polrInformer.Informer(), c.queue, enqueueDelay, keyFunc) + controllerutils.AddDelayedExplicitEventHandlers(logger, cpolrInformer.Informer(), c.queue, enqueueDelay, keyFunc) + controllerutils.AddDelayedExplicitEventHandlers(logger, bgscanrInformer.Informer(), c.queue, enqueueDelay, keyFunc) + controllerutils.AddDelayedExplicitEventHandlers(logger, cbgscanrInformer.Informer(), c.queue, enqueueDelay, keyFunc) enqueueFromAdmr := func(obj metav1.Object) { // no need to consider non aggregated reports if controllerutils.HasLabel(obj, reportutils.LabelAggregatedReport) { - c.queue.AddAfter(keyFunc(obj), delay) + c.queue.AddAfter(keyFunc(obj), enqueueDelay) } } controllerutils.AddEventHandlersT( diff --git a/pkg/controllers/report/background/controller.go b/pkg/controllers/report/background/controller.go index b3cd251468..436a49b156 100644 --- a/pkg/controllers/report/background/controller.go +++ b/pkg/controllers/report/background/controller.go @@ -39,6 +39,7 @@ const ( ControllerName = "background-scan-controller" maxRetries = 10 annotationLastScanTime = "audit.kyverno.io/last-scan-time" + enqueueDelay = 30 * time.Second ) type controller struct { @@ -107,17 +108,10 @@ func NewController( if eventType == resource.Deleted { return } - selector, err := reportutils.SelectorResourceUidEquals(uid) - if err != nil { - logger.Error(err, "failed to create label selector") - } - if err := c.enqueue(selector); err != nil { - logger.Error(err, "failed to enqueue") - } if res.Namespace == "" { - c.queue.Add(string(uid)) + c.queue.AddAfter(string(uid), enqueueDelay) } else { - c.queue.Add(res.Namespace + "/" + string(uid)) + c.queue.AddAfter(res.Namespace+"/"+string(uid), enqueueDelay) } }) return &c @@ -390,7 +384,7 @@ func (c *controller) getMeta(namespace, name string) (metav1.Object, error) { } } -func (c *controller) reconcile(ctx context.Context, logger logr.Logger, key, namespace, name string) error { +func (c *controller) reconcile(ctx context.Context, logger logr.Logger, _, namespace, name string) error { // try to find resource from the cache uid := types.UID(name) resource, gvk, exists := c.metadataCache.GetResourceHash(uid) diff --git a/pkg/controllers/report/utils/utils.go b/pkg/controllers/report/utils/utils.go index 5507cafc01..fc64595b80 100644 --- a/pkg/controllers/report/utils/utils.go +++ b/pkg/controllers/report/utils/utils.go @@ -59,15 +59,7 @@ func ReportsAreIdentical(before, after kyvernov1alpha2.ReportInterface) bool { if !reflect.DeepEqual(before.GetAnnotations(), after.GetAnnotations()) { return false } - bLabels := sets.New[string]() - aLabels := sets.New[string]() - for key := range before.GetLabels() { - bLabels.Insert(key) - } - for key := range after.GetLabels() { - aLabels.Insert(key) - } - if !aLabels.Equal(bLabels) { + if !reflect.DeepEqual(before.GetLabels(), after.GetLabels()) { return false } b := before.GetResults() diff --git a/test/conformance/kuttl/kuttl-test.yaml b/test/conformance/kuttl/kuttl-test.yaml index 6d7c0ba9c3..f33a3a1973 100644 --- a/test/conformance/kuttl/kuttl-test.yaml +++ b/test/conformance/kuttl/kuttl-test.yaml @@ -3,7 +3,7 @@ kind: TestSuite testDirs: - ./test/conformance/kuttl startKIND: false -# timeout: 15 +timeout: 90 parallel: 1 fullName: true skipTestRegex: '_.+' diff --git a/test/conformance/kuttl/reports/background/test-report-background-mode/01-pod.yaml b/test/conformance/kuttl/reports/background/test-report-background-mode/01-pod.yaml new file mode 100644 index 0000000000..3e1752d840 --- /dev/null +++ b/test/conformance/kuttl/reports/background/test-report-background-mode/01-pod.yaml @@ -0,0 +1,6 @@ +apiVersion: kuttl.dev/v1beta1 +kind: TestStep +apply: +- pod.yaml +assert: +- pod-assert.yaml diff --git a/test/conformance/kuttl/reports/background/test-report-background-mode/02-policy.yaml b/test/conformance/kuttl/reports/background/test-report-background-mode/02-policy.yaml new file mode 100644 index 0000000000..b088ed7601 --- /dev/null +++ b/test/conformance/kuttl/reports/background/test-report-background-mode/02-policy.yaml @@ -0,0 +1,6 @@ +apiVersion: kuttl.dev/v1beta1 +kind: TestStep +apply: +- policy.yaml +assert: +- policy-assert.yaml diff --git a/test/conformance/kuttl/reports/background/test-report-background-mode/03-report.yaml b/test/conformance/kuttl/reports/background/test-report-background-mode/03-report.yaml new file mode 100644 index 0000000000..db172beddc --- /dev/null +++ b/test/conformance/kuttl/reports/background/test-report-background-mode/03-report.yaml @@ -0,0 +1,4 @@ +apiVersion: kuttl.dev/v1beta1 +kind: TestStep +assert: +- report-assert.yaml diff --git a/test/conformance/kuttl/reports/background/test-report-background-mode/99-cleanup.yaml b/test/conformance/kuttl/reports/background/test-report-background-mode/99-cleanup.yaml deleted file mode 100644 index c5bcf59c45..0000000000 --- a/test/conformance/kuttl/reports/background/test-report-background-mode/99-cleanup.yaml +++ /dev/null @@ -1,4 +0,0 @@ -apiVersion: kuttl.dev/v1beta1 -kind: TestStep -commands: - - command: kubectl delete -f 01-manifests.yaml,02-cpol.yaml --force --wait=true --ignore-not-found=true \ No newline at end of file diff --git a/test/conformance/kuttl/reports/background/test-report-background-mode/README.md b/test/conformance/kuttl/reports/background/test-report-background-mode/README.md index 98bbddf4a3..a301efe0ec 100644 --- a/test/conformance/kuttl/reports/background/test-report-background-mode/README.md +++ b/test/conformance/kuttl/reports/background/test-report-background-mode/README.md @@ -1,3 +1,10 @@ # Title -This test checks that a Policy Report is created with an entry that is as expected. \ No newline at end of file +This test checks that a Policy Report is created with an entry that is as expected. + +## Steps + +1. - Create a pod +1. - Create a cluster policy + - Assert the policy becomes ready +1. - Assert a report is created for the pod/policy diff --git a/test/conformance/kuttl/reports/background/test-report-background-mode/01-assert.yaml b/test/conformance/kuttl/reports/background/test-report-background-mode/pod-assert.yaml similarity index 100% rename from test/conformance/kuttl/reports/background/test-report-background-mode/01-assert.yaml rename to test/conformance/kuttl/reports/background/test-report-background-mode/pod-assert.yaml diff --git a/test/conformance/kuttl/reports/background/test-report-background-mode/01-manifests.yaml b/test/conformance/kuttl/reports/background/test-report-background-mode/pod.yaml similarity index 100% rename from test/conformance/kuttl/reports/background/test-report-background-mode/01-manifests.yaml rename to test/conformance/kuttl/reports/background/test-report-background-mode/pod.yaml diff --git a/test/conformance/kuttl/reports/background/test-report-background-mode/02-assert.yaml b/test/conformance/kuttl/reports/background/test-report-background-mode/policy-assert.yaml similarity index 100% rename from test/conformance/kuttl/reports/background/test-report-background-mode/02-assert.yaml rename to test/conformance/kuttl/reports/background/test-report-background-mode/policy-assert.yaml diff --git a/test/conformance/kuttl/reports/background/test-report-background-mode/02-cpol.yaml b/test/conformance/kuttl/reports/background/test-report-background-mode/policy.yaml similarity index 100% rename from test/conformance/kuttl/reports/background/test-report-background-mode/02-cpol.yaml rename to test/conformance/kuttl/reports/background/test-report-background-mode/policy.yaml diff --git a/test/conformance/kuttl/reports/background/test-report-background-mode/03-assert.yaml b/test/conformance/kuttl/reports/background/test-report-background-mode/report-assert.yaml similarity index 98% rename from test/conformance/kuttl/reports/background/test-report-background-mode/03-assert.yaml rename to test/conformance/kuttl/reports/background/test-report-background-mode/report-assert.yaml index 73981fd078..a0dfcfc561 100644 --- a/test/conformance/kuttl/reports/background/test-report-background-mode/03-assert.yaml +++ b/test/conformance/kuttl/reports/background/test-report-background-mode/report-assert.yaml @@ -24,4 +24,4 @@ summary: fail: 1 pass: 0 skip: 0 - warn: 0 \ No newline at end of file + warn: 0