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

[kyverno helm chart] make webhook pod annotations configurable #9875

Merged
merged 5 commits into from
Apr 18, 2024

Conversation

monotek
Copy link
Contributor

@monotek monotek commented Mar 8, 2024

Explanation

  • make webhook pod annotations configurable

Related issue

Milestone of this PR

/milestone 1.12

What type of PR is this

Proposed Changes

Proof Manifests

  • [x ] I have read the contributing guidelines.
  • [x ] 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.
  • [x ] 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

Copy link

codecov bot commented Mar 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 10.11%. Comparing base (31905eb) to head (dbbae0c).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #9875      +/-   ##
==========================================
- Coverage   10.11%   10.11%   -0.01%     
==========================================
  Files        1030     1030              
  Lines       91736    91736              
==========================================
- Hits         9279     9277       -2     
- Misses      81440    81441       +1     
- Partials     1017     1018       +1     

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

Signed-off-by: André Bauer <andre.bauer@staffbase.com>
@realshuting
Copy link
Member

@realshuting
Copy link
Member

@monotek - can you verify the upgrade process and attach the test result?

Signed-off-by: André Bauer <andre.bauer@staffbase.com>
@monotek
Copy link
Contributor Author

monotek commented Mar 11, 2024

Updated codegen by running "make codegen-helm-all".

How is the upgrade testet?

Usualy the ci for helm charts does that by using "--upgrade" in the "ct install" command but it seems you're only using "ct lint"?

@realshuting
Copy link
Member

Updated codegen by running "make codegen-helm-all".

How is the upgrade testet?

Usualy the ci for helm charts does that by using "--upgrade" in the "ct install" command but it seems you're only using "ct lint"?

helm upgrade --install kyverno ./charts/kyverno --namespace kyverno?

@monotek
Copy link
Contributor Author

monotek commented Mar 11, 2024

Ah, ok. I thought you have some dedicated test i'm not aware of :D

Helm install:

helm install kyverno kyverno/kyverno
NAME: kyverno
LAST DEPLOYED: Mon Mar 11 10:43:29 2024
NAMESPACE: default
STATUS: deployed
REVISION: 1
NOTES:
Chart version: 3.1.4
Kyverno version: v1.11.4

Thank you for installing kyverno! Your release is named kyverno.

The following components have been installed in your cluster:
- CRDs
- Admission controller
- Reports controller
- Cleanup controller
- Background controller


⚠️  WARNING: Setting the admission controller replica count below 3 means Kyverno is not running in high availability mode.

💡 Note: There is a trade-off when deciding which approach to take regarding Namespace exclusions. Please see the documentation at https://kyverno.io/docs/installation/#security-vs-operability to understand the risks.
 k get po -n default
NAMESPACE            NAME                                             READY   STATUS    RESTARTS   AGE
default              kyverno-admission-controller-8468c8d5df-rqggn    1/1     Running   0          99s
default              kyverno-background-controller-7b6c6c8574-ms45j   1/1     Running   0          99s
default              kyverno-cleanup-controller-5b8548c6f-ffpxg       1/1     Running   0          99s
default              kyverno-reports-controller-66d88fc55b-gt9ct      1/1     Running   0          99s

Helm upgrade:

 helm upgrade kyverno .
Release "kyverno" has been upgraded. Happy Helming!
NAME: kyverno
LAST DEPLOYED: Mon Mar 11 10:45:36 2024
NAMESPACE: default
STATUS: deployed
REVISION: 2
NOTES:
Chart version: v0.0.0
Kyverno version: latest

Thank you for installing kyverno! Your release is named kyverno.

The following components have been installed in your cluster:
- CRDs
- Admission controller
- Reports controller
- Cleanup controller
- Background controller


⚠️  WARNING: Setting the admission controller replica count below 2 means Kyverno is not running in high availability mode.

💡 Note: If Kyverno has been installed on AKS, it is likely you will need to disable the Admission Enforcer. Please see the Kyverno documentation at https://kyverno.io/docs/installation/platform-notes/#notes-for-aks-users for more details.

💡 Note: There is a trade-off when deciding which approach to take regarding Namespace exclusions. Please see the documentation at https://kyverno.io/docs/installation/#security-vs-operability to understand the risks.

k get po -n default
NAME                                             READY   STATUS    RESTARTS   AGE
kyverno-admission-controller-699f7c6f6f-97rq8    1/1     Running   0          101s
kyverno-background-controller-84bc79c84b-vjsct   1/1     Running   0          101s
kyverno-cleanup-controller-587f7f7487-zq6r2      1/1     Running   0          101s
kyverno-reports-controller-7995f8f659-ws5xs      1/1     Running   0          101s

@realshuting
Copy link
Member

Ah, ok. I thought you have some dedicated test i'm not aware of :D

We should really test the Helm upgrade in our CI pipelines 🤔

Copy link
Member

@realshuting realshuting left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs review from @eddycharly @treydock 🙏

@monotek
Copy link
Contributor Author

monotek commented Mar 26, 2024

Can i do anything to get this merged?

@realshuting realshuting enabled auto-merge (squash) April 18, 2024 09:02
Copy link
Member

@realshuting realshuting left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@realshuting realshuting merged commit 6930105 into kyverno:main Apr 18, 2024
248 of 252 checks passed
@monotek
Copy link
Contributor Author

monotek commented Apr 26, 2024

Seems that this somehow did not get added to v1.12.0 :(

Edit: Oh, i see. Milestone 1.13.

@realshuting
Copy link
Member

/cherry-pick release-1.12

@realshuting
Copy link
Member

@monotek - the bot doesn't respond, would you like to cherry-pick it to release-1.12 branch?

@monotek
Copy link
Contributor Author

monotek commented May 6, 2024

This would be realy nice :)

realshuting added a commit to realshuting/kyverno that referenced this pull request May 6, 2024
…no#9875)

* make webhook pod annotations configurable

Signed-off-by: André Bauer <andre.bauer@staffbase.com>

* run make codegen-helm-all

Signed-off-by: André Bauer <andre.bauer@staffbase.com>

---------

Signed-off-by: André Bauer <andre.bauer@staffbase.com>
Co-authored-by: shuting <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 May 6, 2024
realshuting added a commit that referenced this pull request May 7, 2024
#10185)

* make webhook pod annotations configurable



* run make codegen-helm-all



---------

Signed-off-by: André Bauer <andre.bauer@staffbase.com>
Signed-off-by: ShutingZhao <shuting@nirmata.com>
Co-authored-by: André Bauer <monotek@users.noreply.github.com>
anushkamittal2001 pushed a commit to nirmata/kyverno that referenced this pull request May 24, 2024
…no#9875) (kyverno#10185)

* make webhook pod annotations configurable



* run make codegen-helm-all



---------

Signed-off-by: André Bauer <andre.bauer@staffbase.com>
Signed-off-by: ShutingZhao <shuting@nirmata.com>
Co-authored-by: André Bauer <monotek@users.noreply.github.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.12.2 milestone 1.13.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] helm chart - kyverno-hook-post-upgrade becomes unready
2 participants