Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(audit): use a worker pool for Audit policies #10048

Merged
merged 38 commits into from
Apr 17, 2024

Conversation

KhaledEmaraDev
Copy link
Collaborator

@KhaledEmaraDev KhaledEmaraDev commented Apr 12, 2024

Explanation

This PR limits the number of goroutines used by Audit policies to 8 to limit contention and scheduling overhead. This should net a nice performance improvement.

Related issue

Milestone of this PR

Documentation (required for features)

My PR contains new or altered behavior to Kyverno.

What type of PR is this

Proposed Changes

Proof Manifests

The following results were tested with settings:

  1. event generation off
  2. CPU limits set to 2 for the k8s node docker update --cpus=2 <node>
  3. ./start.sh tests/kyverno-pods-dry-run.js 100 1000
  • audit PSS policies with this change
http_req_duration: avg=409.45ms min=99.54ms med=398.78ms max=1.29s    p(90)=698.04ms p(95)=798.72ms
  • audit PSS policies with v1.12.0-rc.4
http_req_duration: avg=846.89ms min=2.98ms  med=601.92ms max=4.29s    p(90)=1.8s     p(95)=2.1s
  • enforce PSS policies with this change
http_req_duration: avg=1.21s   min=102.67ms med=1.19s   max=2.89s    p(90)=1.79s    p(95)=1.99s
  • enforce PSS policies with v1.12.0-rc.4
http_req_duration: avg=1.25s    min=94.74ms  med=1.19s   max=3.19s    p(90)=1.8s     p(95)=2.19s

Checklist

  • I have read the contributing guidelines.
  • I have read the PR documentation guide and followed the process including adding proof manifests to this PR.
  • This is a bug fix and I have added unit tests that prove my fix is effective.
  • This is a feature and I have added CLI tests that are applicable.
  • My PR needs to be cherry picked to a specific release branch which is .
  • My PR contains new or altered behavior to Kyverno and
    • CLI support should be added and my PR doesn't contain that functionality.

Further Comments

realshuting and others added 12 commits April 10, 2024 23:55
… return admission response earlier

Signed-off-by: ShutingZhao <shuting@nirmata.com>
Signed-off-by: ShutingZhao <shuting@nirmata.com>
Signed-off-by: ShutingZhao <shuting@nirmata.com>
Signed-off-by: ShutingZhao <shuting@nirmata.com>
Signed-off-by: ShutingZhao <shuting@nirmata.com>
Signed-off-by: ShutingZhao <shuting@nirmata.com>
Signed-off-by: ShutingZhao <shuting@nirmata.com>
Signed-off-by: ShutingZhao <shuting@nirmata.com>
Signed-off-by: ShutingZhao <shuting@nirmata.com>
Signed-off-by: ShutingZhao <shuting@nirmata.com>
Signed-off-by: Khaled Emara <khaled.emara@nirmata.com>
@realshuting
Copy link
Member

I added stress test results comparing this change to rc.4.

Signed-off-by: ShutingZhao <shuting@nirmata.com>
@realshuting
Copy link
Member

Fixed unit tests.

@vishal-chdhry vishal-chdhry added this to the Kyverno Release 1.12.0 milestone Apr 12, 2024
Copy link

codecov bot commented Apr 12, 2024

Codecov Report

Attention: Patch coverage is 41.86047% with 50 lines in your changes are missing coverage. Please review.

Project coverage is 10.10%. Comparing base (90d1440) to head (af94a53).

❗ Current head af94a53 differs from pull request most recent head b630257. Consider uploading reports for the commit b630257 to get more accurate results

Files Patch % Lines
pkg/webhooks/resource/validation/validation.go 22.91% 32 Missing and 5 partials ⚠️
cmd/kyverno/main.go 0.00% 6 Missing ⚠️
pkg/webhooks/resource/validation/utils.go 45.45% 3 Missing and 3 partials ⚠️
pkg/webhooks/resource/handlers.go 90.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##            main   #10048      +/-   ##
=========================================
+ Coverage   9.96%   10.10%   +0.14%     
=========================================
  Files       1029     1030       +1     
  Lines      91686    91722      +36     
=========================================
+ Hits        9134     9266     +132     
+ Misses     81554    81437     -117     
- Partials     998     1019      +21     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@realshuting
Copy link
Member

3/4 of PSS load tests failed, did they exceed the threshold?

https://github.com/kyverno/kyverno/actions/runs/8660137654/job/23747565640?pr=10048

Signed-off-by: ShutingZhao <shuting@nirmata.com>
Signed-off-by: ShutingZhao <shuting@nirmata.com>
Signed-off-by: ShutingZhao <shuting@nirmata.com>
@vishal-chdhry vishal-chdhry added the Documentation Update Documentation label Apr 16, 2024
@vishal-chdhry
Copy link
Member

@realshuting we need to add documentation for the new flags

@realshuting
Copy link
Member

realshuting commented Apr 17, 2024

@realshuting we need to add documentation for the new flags

Yes, opened kyverno/website#1210.

@realshuting
Copy link
Member

/cherry-pick release-1.12

@realshuting realshuting merged commit fb40aa5 into kyverno:main Apr 17, 2024
245 of 248 checks passed
Copy link

Cherry-pick failed with Merge error fb40aa5f38500ebf739b7efd7f6f3d4f74d9c8a1 into temp-cherry-pick-4a89e6-release-1.12

realshuting added a commit to realshuting/kyverno that referenced this pull request Apr 17, 2024
* enhancement: split validation logic for enforce and audit policies to return admission response earlier

Signed-off-by: ShutingZhao <shuting@nirmata.com>

* chore: add missing file

Signed-off-by: ShutingZhao <shuting@nirmata.com>

* fix: unit tests

Signed-off-by: ShutingZhao <shuting@nirmata.com>

* fix: linter issues

Signed-off-by: ShutingZhao <shuting@nirmata.com>

* fix: unit tests

Signed-off-by: ShutingZhao <shuting@nirmata.com>

* fix: get latest policy object before updating status

Signed-off-by: ShutingZhao <shuting@nirmata.com>

* chore: remove debug code

Signed-off-by: ShutingZhao <shuting@nirmata.com>

* fix: compare before updates

Signed-off-by: ShutingZhao <shuting@nirmata.com>

* fix: initial reconcile

Signed-off-by: ShutingZhao <shuting@nirmata.com>

* fix: updates

Signed-off-by: ShutingZhao <shuting@nirmata.com>

* feat(audit): use a worker pool for Audit policies

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

* fix: unit test

Signed-off-by: ShutingZhao <shuting@nirmata.com>

* fix(attempt): spin up go routine

Signed-off-by: ShutingZhao <shuting@nirmata.com>

* feat: add flags maxAuditWorkers, maxAuditCapacity

Signed-off-by: ShutingZhao <shuting@nirmata.com>

* fix: enable debug log on failure

Signed-off-by: ShutingZhao <shuting@nirmata.com>

* fix: wait group panic

Signed-off-by: ShutingZhao <shuting@nirmata.com>

* load-tests: add stess tests configurations

Signed-off-by: ShutingZhao <shuting@nirmata.com>

* load-tests: disable admissionreports

Signed-off-by: ShutingZhao <shuting@nirmata.com>

* fix: build policy contexts syncronously

Signed-off-by: Vishal Choudhary <vishal.choudhary@nirmata.com>

* fix: only run generate and mutate existing go routines when policies are present

Signed-off-by: Vishal Choudhary <vishal.choudhary@nirmata.com>

* fix: mutate and verify tests

Signed-off-by: Vishal Choudhary <vishal.choudhary@nirmata.com>

* fix: return early if no audit policy

Signed-off-by: ShutingZhao <shuting@nirmata.com>

* fix: run handlegenerate and mutate existing in all cases

Signed-off-by: Vishal Choudhary <vishal.choudhary@nirmata.com>

* fix: only test bgapplies in generate test

Signed-off-by: Vishal Choudhary <vishal.choudhary@nirmata.com>

* fix: defer wait in tests

Signed-off-by: Vishal Choudhary <vishal.choudhary@nirmata.com>

* enhancement: process validate enforce in a go routine

Signed-off-by: ShutingZhao <shuting@nirmata.com>

---------

Signed-off-by: ShutingZhao <shuting@nirmata.com>
Signed-off-by: Khaled Emara <khaled.emara@nirmata.com>
Signed-off-by: Vishal Choudhary <vishal.choudhary@nirmata.com>
Co-authored-by: ShutingZhao <shuting@nirmata.com>
Co-authored-by: Vishal Choudhary <vishal.choudhary@nirmata.com>
Signed-off-by: ShutingZhao <shuting@nirmata.com>
@realshuting realshuting added the cherry-pick-completed The PR was cherry-picked (or merged) to required release branches label Apr 17, 2024
realshuting added a commit that referenced this pull request Apr 17, 2024
* enhancement: split validation logic for enforce and audit policies to return admission response earlier



* chore: add missing file



* fix: unit tests



* fix: linter issues



* fix: unit tests



* fix: get latest policy object before updating status



* chore: remove debug code



* fix: compare before updates



* fix: initial reconcile



* fix: updates



* feat(audit): use a worker pool for Audit policies



* fix: unit test



* fix(attempt): spin up go routine



* feat: add flags maxAuditWorkers, maxAuditCapacity



* fix: enable debug log on failure



* fix: wait group panic



* load-tests: add stess tests configurations



* load-tests: disable admissionreports



* fix: build policy contexts syncronously



* fix: only run generate and mutate existing go routines when policies are present



* fix: mutate and verify tests



* fix: return early if no audit policy



* fix: run handlegenerate and mutate existing in all cases



* fix: only test bgapplies in generate test



* fix: defer wait in tests



* enhancement: process validate enforce in a go routine



---------

Signed-off-by: ShutingZhao <shuting@nirmata.com>
Signed-off-by: Khaled Emara <khaled.emara@nirmata.com>
Signed-off-by: Vishal Choudhary <vishal.choudhary@nirmata.com>
Co-authored-by: Khaled Emara <khaled.emara@nirmata.com>
Co-authored-by: Vishal Choudhary <vishal.choudhary@nirmata.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherry-pick-completed The PR was cherry-picked (or merged) to required release branches cherry-pick-required Documentation Update Documentation milestone 1.12.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants