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: Fix and add missing podLabels #14943

Merged
merged 3 commits into from
Feb 23, 2021
Merged

helm: Fix and add missing podLabels #14943

merged 3 commits into from
Feb 23, 2021

Conversation

Subreptivus
Copy link
Contributor

  • Pod Labels defined in values file were not propagated to actual manifests
  • Some of the Pods were missing podLabels definition in values file and in actual manifests (i.e. certgen jobs Pods)

Signed-off-by: Yurii Komar <IKOM@equinor.com>
@Subreptivus Subreptivus requested review from a team as code owners February 11, 2021 21:49
@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 Feb 11, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot added this to In progress in 1.10.0 Feb 11, 2021
@joestringer joestringer added needs-backport/1.9 release-note/minor This PR changes functionality that users may find relevant to operating Cilium. labels Feb 11, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot removed dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. labels Feb 11, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.9.5 Feb 11, 2021
@nathanjsweet
Copy link
Member

nathanjsweet commented Feb 11, 2021

@Subreptivus can you run the command make -C install/kubernetes all and commit the changes. We'll have test failures until you update the quick-install.yaml file. Thanks!

Signed-off-by: Yurii Komar <IKOM@equinor.com>
@maintainer-s-little-helper
Copy link

Commit 373e9e125b22fea9d12510a9ff76e87f832a7196 does not contain "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 the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Feb 11, 2021
Copy link
Member

@qmonnet qmonnet left a comment

Choose a reason for hiding this comment

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

Thank you! Can we get rid of the third commit (the merge, I suspect it was created because you tried to update the branch automatically in GitHub)? No need to update unless there are conflicts to solve, in which case it looks cleaner to rebase the PR on top on the current master.

install/kubernetes/cilium/values.yaml Outdated Show resolved Hide resolved
install/kubernetes/cilium/values.yaml Outdated Show resolved Hide resolved
@joestringer joestringer moved this from Done to In progress in 1.10.0 Feb 12, 2021
@maintainer-s-little-helper
Copy link

Commit 373e9e125b22fea9d12510a9ff76e87f832a7196 does not contain "Signed-off-by".

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

1 similar comment
@maintainer-s-little-helper
Copy link

Commit 373e9e125b22fea9d12510a9ff76e87f832a7196 does not contain "Signed-off-by".

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

Signed-off-by: Yurii Komar <IKOM@equinor.com>
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-sign-off The author needs to add signoff to their commits before merge. label Feb 15, 2021
@Subreptivus
Copy link
Contributor Author

@qmonnet Done, now it should be allright.
As for empty lines, I was just used few lines up to be consistent with a config that was there before (sometimes there were empty lines, sometimes not). So I've removed all empty lines. Hope you're happy now.

Copy link
Member

@qmonnet qmonnet left a comment

Choose a reason for hiding this comment

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

I'm happy now :). Thank you

@qmonnet
Copy link
Member

qmonnet commented Feb 15, 2021

test-me-please

@nebril nebril added ready-to-merge This PR has passed all tests and received consensus from code owners to merge. and removed ready-to-merge This PR has passed all tests and received consensus from code owners to merge. labels Feb 16, 2021
@qmonnet qmonnet added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Feb 22, 2021
@aanm aanm merged commit 40661dc into cilium:master Feb 23, 2021
1.10.0 automation moved this from In progress to Done Feb 23, 2021
This was referenced Feb 24, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from master to Backport pending to v1.9 in 1.9.5 Feb 24, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot removed the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Feb 24, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.9 to Backport done to v1.9 in 1.9.5 Mar 4, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.9 to Backport done to v1.9 in 1.9.5 Mar 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-note/minor This PR changes functionality that users may find relevant to operating Cilium.
Projects
No open projects
1.9.5
Backport done to v1.9
Development

Successfully merging this pull request may close these issues.

None yet

10 participants