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.

Sign up for GitHub

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

Jump to bottom

Refactoring labels #176

Merged
merged 8 commits into from
Jan 3, 2023
Merged

Refactoring labels #176

merged 8 commits into from
Jan 3, 2023

Conversation

nlamirault
Copy link
Contributor

@nlamirault nlamirault commented Dec 23, 2022 •
edited
Loading

Description

Add recommended Kubernetes labels to all resources.

Test

Additional Information

Tradeoff

Potential improvement

@nlamirault
Update: Refactoring labels
384f426 
Signed-off-by: Nicolas Lamirault <nicolas.lamirault@gmail.com>
@nlamirault nlamirault requested a review from a team as a code owner December 23, 2022 16:04
@nlamirault
Update: bump chart version
5051e75 
Signed-off-by: Nicolas Lamirault <nicolas.lamirault@gmail.com>
viccuad
viccuad reviewed Jan 2, 2023
View reviewed changes
Copy link
Member

@viccuad viccuad left a comment •
edited
Loading

Choose a reason for hiding this comment

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

This is great, many thanks!
Indeed we need to provide a way to set labels on the charts. Added one tiny suggestion on the review, once we sort that out we can merge :).

If you have any other labels that you think would be nice to ship, don't hesitate to tell us or open a PR :).

I would welcome setting all the recommended labels and doing this for kubewarden-defaults, kubewarden-crds charts too. Opened #177, feel free to implement it on this PR or another. If not I will happily tackle it too.

@viccuad viccuad mentioned this pull request Jan 2, 2023
Closed
@nlamirault
Fix: remove additionalAnnotations
ff3405f 
Signed-off-by: Nicolas Lamirault <nicolas.lamirault@gmail.com>
@nlamirault
Update: rename component
4c2d3cd 
Signed-off-by: Nicolas Lamirault <nicolas.lamirault@gmail.com>
@nlamirault
Add: Recommanded labels for defaults chart
aef21e7 
Signed-off-by: Nicolas Lamirault <nicolas.lamirault@gmail.com>
@viccuad
Copy link
Member

viccuad commented Jan 3, 2023

Thanks for the changes!

Argh, the changes done to values.yaml for additionalLabels need to also be in chart-values.yaml, as explained for the additionalAnnotations case, for the CI to pass.

@nlamirault
Fix: missing default values
1c709a4 
Signed-off-by: Nicolas Lamirault <nicolas.lamirault@gmail.com>
viccuad and others added 2 commits January 3, 2023 15:26
@viccuad
Update charts/kubewarden-controller/Chart.yaml
22fdd0b 
Signed-off-by: Víctor Cuadrado Juan <2196685+viccuad@users.noreply.github.com>
@nlamirault
Fix: additional labels
be74935 
Signed-off-by: Nicolas Lamirault <nicolas.lamirault@gmail.com>
@viccuad
Copy link
Member

viccuad commented Jan 3, 2023

Merging, many thanks!

Should have pointed to the make generate-values and make check-generated-values earlier, to fix locally faster.

@viccuad viccuad merged commit b333ae7 into kubewarden:main Jan 3, 2023
@viccuad viccuad added the kind/enhancement New feature or request label Jan 3, 2023
@viccuad viccuad mentioned this pull request Jan 3, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@viccuad viccuad viccuad left review comments

Assignees
No one assigned
Labels
kind/enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@nlamirault @viccuad