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(helm): support annotations for certgen resources #1996

Conversation

chicco785
Copy link
Contributor

What type of PR is this?

feat(helm): support annotations for certgen resources

What this PR does / why we need it:

  • support annotations for certgen resources
  • support configuration for certgen job ttlSecondsAfterFinished

Which issue(s) this PR fixes:

Fixes #1954

@chicco785 chicco785 requested a review from a team as a code owner October 18, 2023 12:02
@chicco785 chicco785 force-pushed the 1-support-helm-chart-compatibility-with-argocd-via-annotations-for-jobs branch 2 times, most recently from 152aabd to e46a880 Compare October 18, 2023 12:03
@codecov
Copy link

codecov bot commented Oct 18, 2023

Codecov Report

Merging #1996 (753b466) into main (604a0fb) will increase coverage by 0.03%.
Report is 1 commits behind head on main.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #1996      +/-   ##
==========================================
+ Coverage   65.51%   65.55%   +0.03%     
==========================================
  Files          92       92              
  Lines       13528    13528              
==========================================
+ Hits         8863     8868       +5     
+ Misses       4116     4112       -4     
+ Partials      549      548       -1     

see 2 files with indirect coverage changes

@Xunzhuo
Copy link
Member

Xunzhuo commented Oct 18, 2023

You need to run make generate to update helm docs.

@chicco785 chicco785 force-pushed the 1-support-helm-chart-compatibility-with-argocd-via-annotations-for-jobs branch 4 times, most recently from a220d8c to 648b9c2 Compare October 18, 2023 15:13
@chicco785
Copy link
Contributor Author

thx @Xunzhuo, I applied changes as per your suggestion and re-generated helm chart documentation

* support annotations for certgen resources
* support configuration for certgen job ttlSecondsAfterFinished

Signed-off-by: Federico M. Facca <federico.facca@zaphiro.ch>
Signed-off-by: Federico M. Facca <federico.facca@zaphiro.ch>
Signed-off-by: Federico M. Facca <federico.facca@zaphiro.ch>
Signed-off-by: Federico M. Facca <federico.facca@zaphiro.ch>
@chicco785 chicco785 force-pushed the 1-support-helm-chart-compatibility-with-argocd-via-annotations-for-jobs branch from 1638458 to 753b466 Compare October 18, 2023 17:07
Copy link
Contributor

@arkodg arkodg left a comment

Choose a reason for hiding this comment

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

LGTM thanks !

Copy link
Contributor

@zirain zirain left a comment

Choose a reason for hiding this comment

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

it would be better if you can add an e2e test for this.

@chicco785
Copy link
Contributor Author

it would be better if you can add an e2e test for this.

if you point me to an example, I am happy to give it a try :)

@Xunzhuo Xunzhuo merged commit f203fe5 into envoyproxy:main Oct 19, 2023
18 checks passed
@chicco785 chicco785 deleted the 1-support-helm-chart-compatibility-with-argocd-via-annotations-for-jobs branch October 19, 2023 07:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Helm Chart compatibility with ArgoCD via supporting annotations for jobs
4 participants