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: cleanup enhancements-1 #5796

Merged
merged 12 commits into from
Jan 4, 2023

Conversation

realshuting
Copy link
Member

Explanation

This is an enhancement PR to address comments in #5778. Details in Proposed Changes below.

Related issue

#5778

Milestone of this PR

/1.9.0

What type of PR is this

/enhancements

Proposed Changes

This PR addresses following comments in the original issue:

    1. Change ClusterRole name to kyverno:cleanup-controller
    1. Update debug log level to 4 (from 5), here's the log (level 4) messages for a successful cleanup
I1228 09:10:00.103493       1 run.go:153] cleanup-controller/worker "msg"="reconciling ..." "id"=1 "key"="foo/myclustercleanpolicy" "name"="myclustercleanpolicy" "namespace"="foo"
I1228 09:10:00.112534       1 run.go:155] cleanup-controller/worker "msg"="done" "duration"="62.202µs" "id"=1 "key"="foo/myclustercleanpolicy" "name"="myclustercleanpolicy" "namespace"="foo"
I1228 09:10:00.639231       1 handlers.go:42] cleanup "msg"="cleaning up..." "policy"="foo/myclustercleanpolicy"
I1228 09:10:00.639250       1 handlers.go:70] cleanup "msg"="processing..." "kind"="Deployment" "policy"="foo/myclustercleanpolicy"
I1228 09:10:00.646222       1 context.go:278]  "msg"="updated image info" "images"={"containers":{"nginx":{"registry":"docker.io","name":"nginx","path":"nginx","tag":"latest"}}}
I1228 09:10:00.647399       1 vars.go:402] cleanup "msg"="variable substituted" "path"="" "policy"="foo/myclustercleanpolicy" "value"=1 "variable"="{{ request.object.spec.replicas }}"
I1228 09:10:00.648862       1 vars.go:402] cleanup "msg"="variable substituted" "path"="" "policy"="foo/myclustercleanpolicy" "value"="foo" "variable"="{{request.namespace}}"
I1228 09:10:00.649177       1 handlers.go:139] cleanup "msg"="resource matched, it will be deleted..." "kind"="Deployment" "name"="nginx" "namespace"="foo" "policy"="foo/myclustercleanpolicy"
I1228 09:10:00.653937       1 handlers.go:144] cleanup "msg"="deleted" "kind"="Deployment" "name"="nginx" "namespace"="foo" "policy"="foo/myclustercleanpolicy"
I1228 09:10:00.653970       1 handlers.go:52] cleanup "msg"="done" "policy"="foo/myclustercleanpolicy"
I1228 09:10:02.963653       1 run.go:153] cleanup-controller/worker "msg"="reconciling ..." "id"=0 "key"="foo/myclustercleanpolicy" "name"="myclustercleanpolicy" "namespace"="foo"
I1228 09:10:02.972271       1 run.go:155] cleanup-controller/worker "msg"="done" "duration"="53.772µs" "id"=0 "key"="foo/myclustercleanpolicy" "name"="myclustercleanpolicy" "namespace"="foo"
    1. Update fields descriptions
    1. Add variables validations, supported variables are request. and images..
      For an allowed but invalid variable request.object.spec.replica, the following messages are printed for the error:
I1228 09:18:00.743170       1 vars.go:72] cleanup "msg"="Variable substitution failed in preconditions, therefore nil value assigned to variable,  \"request.object.spec.replica\" " "policy"="foo/myclustercleanpolicy"
E1228 09:18:00.743295       1 handlers.go:130] cleanup "msg"="failed to check condition" "error"="failed to substitute variables in condition key: failed to resolve request.object.spec.replica at path : JMESPath query failed: Unknown key \"replica\" in path" "kind"="Deployment" "name"="nginx" "namespace"="foo" "policy"="foo/myclustercleanpolicy"

Proof Manifests

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.
    • I have added or changed the documentation myself in an existing PR and the link is:
    • I have raised an issue in kyverno/website to track the documentation update and the link is:

Further Comments

Signed-off-by: ShutingZhao <shuting@nirmata.com>
Signed-off-by: ShutingZhao <shuting@nirmata.com>
…date debug log level to 4

Signed-off-by: ShutingZhao <shuting@nirmata.com>
Signed-off-by: ShutingZhao <shuting@nirmata.com>
@realshuting realshuting enabled auto-merge (squash) December 28, 2022 09:26
@codecov
Copy link

codecov bot commented Dec 28, 2022

Codecov Report

Merging #5796 (e2a444b) into main (3c0b785) will decrease coverage by 0.02%.
The diff coverage is 8.33%.

@@            Coverage Diff             @@
##             main    #5796      +/-   ##
==========================================
- Coverage   35.95%   35.92%   -0.03%     
==========================================
  Files         186      187       +1     
  Lines       20700    20700              
==========================================
- Hits         7442     7436       -6     
- Misses      12441    12447       +6     
  Partials      817      817              
Impacted Files Coverage Δ
api/kyverno/v2alpha1/cleanup_policy_types.go 53.84% <ø> (ø)
api/kyverno/v2beta1/common_types.go 0.00% <ø> (ø)
...md/cleanup-controller/handlers/cleanup/handlers.go 0.00% <0.00%> (ø)
pkg/engine/variables/errs.go 0.00% <0.00%> (ø)
pkg/policy/validate.go 51.19% <100.00%> (-0.10%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

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

Thanks, @realshuting. Can you also address 11? We need to be accurate in the permissions we tell users they need to add.

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

Thanks, @realshuting. Can you also address 11? We need to be accurate in the permissions we tell users they need to add.

Added the check for list permission.

pkg/validation/cleanuppolicy/validate.go Outdated Show resolved Hide resolved
pkg/validation/cleanuppolicy/validate.go Outdated Show resolved Hide resolved
Signed-off-by: shuting <shutting06@gmail.com>
Signed-off-by: shuting <shutting06@gmail.com>
@realshuting realshuting merged commit 18455b4 into kyverno:main Jan 4, 2023
@realshuting
Copy link
Member Author

/cherry-pick release-1.9

gcp-cherry-pick-bot bot pushed a commit that referenced this pull request Jan 4, 2023
* update fields description

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

* update cleanup controller clusterrole name

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

* - add variables validations to support "request." and "images."; - update debug log level to 4

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

* add missing files

Signed-off-by: ShutingZhao <shuting@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 Jan 4, 2023
realshuting added a commit to realshuting/kyverno that referenced this pull request Jan 4, 2023
* update fields description

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

* update cleanup controller clusterrole name

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

* - add variables validations to support "request." and "images."; - update debug log level to 4

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

* add missing files

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

Signed-off-by: ShutingZhao <shuting@nirmata.com>
realshuting added a commit to realshuting/kyverno that referenced this pull request Jan 4, 2023
Signed-off-by: ShutingZhao <shuting@nirmata.com>
realshuting added a commit that referenced this pull request Jan 4, 2023
* feat: cleanup enhancements-1 (#5796)

* update fields description

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

* update cleanup controller clusterrole name

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

* - add variables validations to support "request." and "images."; - update debug log level to 4

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

* add missing files

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

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

* cherry-pick #5796

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

Signed-off-by: ShutingZhao <shuting@nirmata.com>
@realshuting realshuting deleted the cleanup_enhancements branch January 9, 2023 12:55
MdSahil-oss pushed a commit to MdSahil-oss/kyverno that referenced this pull request Jan 11, 2023
* update fields description

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

* update cleanup controller clusterrole name

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

* - add variables validations to support "request." and "images."; - update debug log level to 4

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

* add missing files

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

Signed-off-by: ShutingZhao <shuting@nirmata.com>
Signed-off-by: MdSahil-oss <Mohdssahil1@gmail.com>
MdSahil-oss pushed a commit to MdSahil-oss/kyverno that referenced this pull request Jan 11, 2023
* update fields description

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

* update cleanup controller clusterrole name

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

* - add variables validations to support "request." and "images."; - update debug log level to 4

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

* add missing files

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

Signed-off-by: ShutingZhao <shuting@nirmata.com>
Signed-off-by: MdSahil-oss <Mohdssahil1@gmail.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 milestone 1.9.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants