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 extraLabels to connectInject and serverACLInit pods #1380

Closed
wants to merge 3 commits into from

Conversation

s-andresen
Copy link

Changes proposed in this PR:
Added extraLabels to connectInject and serverACLInit in values.yaml so if declared in the values file then these additional labels will be added to the pods

How I've tested this PR:
Tested the helm chart on my kubernetes cluster with and without declaring the extraLabels to ensure everything works as expected.

How I expect reviewers to test this PR:
Same as how I tested

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...)

Add extraLabels to connectInject pods and to serverACLInit pod to values
Add extraLabels if extraLabels are defined in the values file
Add extraLabels to serverACLInit pod if declared in values
@hashicorp-cla
Copy link

hashicorp-cla commented Jul 28, 2022

CLA assistant check
All committers have signed the CLA.

@s-andresen s-andresen changed the title Patch 1 Add extraLabels to connectInject and serverACLInit pods Aug 10, 2022
@david-yu
Copy link
Contributor

@s-andresen Could you tell us whether you expect to apply the same labels for both components? We had a similar PR previously but opted for an approach to create a label for all components for a global label instead.

Copy link
Member

@lkysow lkysow left a comment

Choose a reason for hiding this comment

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

Looking good, but can you please add bats tests? You should be able to follow the pattern for other extraLabels tests.

@@ -2934,3 +2947,17 @@ prometheus:
# is only useful when running helm template.
tests:
enabled: true

serverACLInit:
Copy link
Member

Choose a reason for hiding this comment

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

Let's put this under global.acls.serverACLInit?

@david-yu
Copy link
Contributor

Closing in favor of #1678, could you open a separate PR That enables ServerACLInit with bats test included?

@david-yu david-yu closed this Nov 12, 2022
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.

4 participants