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

Add custom labels for connectInject, acl init, and mesh gateway pods #773

Closed
wants to merge 2 commits into from

Conversation

jeanmorais
Copy link
Contributor

Changes proposed in this PR:

  • Add custom labels for connectInject, acl init, and mesh gateway pods via Helm.

How I've tested this PR:

  • Unit tests

Fixes #759

How I expect reviewers to test this PR:

  • bats connect-inject-deployment.bats
  • bats mesh-gateway-deployment.bats
  • bats server-acl-init-job.bats

Checklist:

  • Tests added
  • CHANGELOG entry added

    HashiCorp engineers only, community PRs should not add a changelog entry.
    Entries should use present tense (e.g. Add support for...)

@jeanmorais jeanmorais changed the title Add custom labels for connectInject, acl init, and mesh gateway pods … Add custom labels for connectInject, acl init, and mesh gateway pods Oct 10, 2021
@t-eckert t-eckert self-requested a review October 11, 2021 17:59
t-eckert
t-eckert previously approved these changes Oct 14, 2021
Copy link
Contributor

@t-eckert t-eckert left a comment

Choose a reason for hiding this comment

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

This looks great! You did a fantastic job of following the patterns set by the previous extra labels work. Thank you!

@t-eckert t-eckert dismissed their stale review October 14, 2021 21:21

I misunderstood how this connects into acl init.

@t-eckert
Copy link
Contributor

Hi @jeanmorais, I apologize for reviewing and then rescinding the review. Your PR definitely meets the mark for the issue it addresses, but there is now some question around the user experience as it relates to that issue and adding stanzas. We will discuss the UX as a team and get back to you soon. Once again, I apologize for the confusion.

@jeanmorais
Copy link
Contributor Author

Hi @jeanmorais, I apologize for reviewing and then rescinding the review. Your PR definitely meets the mark for the issue it addresses, but there is now some question around the user experience as it relates to that issue and adding stanzas. We will discuss the UX as a team and get back to you soon. Once again, I apologize for the confusion.

Hey @t-eckert, no problem! Let me know if any adaptations are needed in this PR, as soon as your team decides how to proceed.

@hashicorp-cla
Copy link

hashicorp-cla commented Mar 12, 2022

CLA assistant check
All committers have signed the CLA.

@david-yu
Copy link
Contributor

david-yu commented Jun 6, 2022

Hi @jeanmorais Sorry we did not get back to you since the last review. We've had some internal discussions on this recently, and it was decided that perhaps the better approach would be to re-do labeling globally as opposed to on a per stanza basis. I will close this PR and this issue for now but if you would like to move forward with a PR for global labels please open a new PR. We did accept this for hacktoberfest so hopefully you received credit for the work.

@david-yu david-yu closed this Jun 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Custom labels for connectInject, acl init, and mesh gateway pods via Helm
4 participants