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

helm: Correct spire labels indentation #28610

Merged
merged 1 commit into from Oct 18, 2023
Merged

helm: Correct spire labels indentation #28610

merged 1 commit into from Oct 18, 2023

Conversation

sayboras
Copy link
Member

@sayboras sayboras commented Oct 16, 2023

Fixes: #28599

fix: Correct spire labels identation in helm chart

@sayboras sayboras requested review from a team as code owners October 16, 2023 08:30
@maintainer-s-little-helper
Copy link

Commit 5b80406 does not match "(?m)^Signed-off-by:".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

@maintainer-s-little-helper maintainer-s-little-helper bot added dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. labels Oct 16, 2023
@maintainer-s-little-helper
Copy link

Commit 5b80406 does not match "(?m)^Signed-off-by:".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

@sayboras sayboras added needs-backport/1.14 This PR / issue needs backporting to the v1.14 branch and removed dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. labels Oct 16, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Oct 16, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from main in 1.14.3 Oct 16, 2023
@sayboras sayboras added area/helm Impacts helm charts and user deployment experience area/servicemesh GH issues or PRs regarding servicemesh release-note/bug This PR fixes an issue in a previous release of Cilium. labels Oct 16, 2023
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Oct 16, 2023
@sayboras
Copy link
Member Author

/test

@sayboras sayboras marked this pull request as draft October 16, 2023 09:21
Additionally, the labels are propagated to pod level as well.

Fixes: #28599
Signed-off-by: Tam Mach <tam.mach@cilium.io>
@sayboras
Copy link
Member Author

/test

@sayboras sayboras marked this pull request as ready for review October 16, 2023 10:35
Copy link
Member

@meyskens meyskens left a comment

Choose a reason for hiding this comment

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

Good catch! LGTM

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Oct 18, 2023
@joestringer
Copy link
Member

@sayboras Could you add a release note to the PR description? Glancing over the changes it looks like it's fixing the labels for SPIRE deployments, so it may be helpful to better highlight that in the release note.

@joestringer joestringer merged commit 55b3bb3 into main Oct 18, 2023
206 of 211 checks passed
@joestringer joestringer deleted the tam/spire-labels branch October 18, 2023 14:59
@jrajahalme jrajahalme added this to Needs backport from main in 1.14.4 Oct 18, 2023
@jrajahalme jrajahalme removed this from Needs backport from main in 1.14.3 Oct 18, 2023
@sayboras
Copy link
Member Author

Could you add a release note to the PR description? Glancing over the changes it looks like it's fixing the labels for SPIRE deployments, so it may be helpful to better highlight that in the release note.

Thanks and done 👍

@tklauser tklauser added backport-pending/1.14 The backport for Cilium 1.14.x for this PR is in progress. and removed needs-backport/1.14 This PR / issue needs backporting to the v1.14 branch labels Oct 24, 2023
@tklauser tklauser mentioned this pull request Oct 24, 2023
14 tasks
@tklauser tklauser added backport-done/1.14 The backport for Cilium 1.14.x for this PR is done. and removed backport-pending/1.14 The backport for Cilium 1.14.x for this PR is in progress. labels Oct 25, 2023
@joestringer
Copy link
Member

@sayboras ah, what I meant is that for a user "fixing the indentation" doesn't declare what the side effects are. If this is a cosmetic change then there's no need to backport and it's not a bugfix. If this ensures that when the user configures the feature with setting X or Y, it actually works, then we should document the user impact that way so that users can understand exactly which configurations are affected and decide if the bug is related to their problem or not. And in that case yeah it makes sense to backport this to 1.14.

@thorn3r thorn3r moved this from Needs backport from main to Backport done to v1.14 in 1.14.4 Nov 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/helm Impacts helm charts and user deployment experience area/servicemesh GH issues or PRs regarding servicemesh backport-done/1.14 The backport for Cilium 1.14.x for this PR is done. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/bug This PR fixes an issue in a previous release of Cilium.
Projects
No open projects
1.14.4
Backport done to v1.14
Development

Successfully merging this pull request may close these issues.

Helm chart: broken spire setup
5 participants