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

[NET-6809] Add chart related labels for mesh gateway deployments #3396

Merged
merged 2 commits into from
Dec 20, 2023

Conversation

jm96441n
Copy link
Member

Changes proposed in this PR

  • Adds chart related labels for mesh gateway deployments

How I've tested this PR

  • ran tests
  • ran local kind cluster and checked config

How I expect reviewers to test this PR

  • 👁️
  • run tests
  • run local kind cluster and checked config

Checklist

@jm96441n jm96441n added theme/mesh-gw pr/no-changelog PR does not need a corresponding .changelog entry pr/no-backport signals that a PR will not contain a backport label labels Dec 18, 2023
@jm96441n jm96441n requested review from a team, missylbytes and sarahalsmiller and removed request for a team December 18, 2023 21:29
@@ -196,6 +201,9 @@ func (c *Command) init() {
"K8s namespaces to explicitly deny. Takes precedence over allow. May be specified multiple times.")
c.flagSet.StringVar(&c.flagReleaseName, "release-name", "consul", "The Consul Helm installation release name, e.g 'helm install <RELEASE-NAME>'")
c.flagSet.StringVar(&c.flagReleaseNamespace, "release-namespace", "default", "The Consul Helm installation namespace, e.g 'helm install <RELEASE-NAME> --namespace <RELEASE-NAMESPACE>'")
c.flagSet.StringVar(&c.flagAppName, "app-name", "consul", "The Consul application name use when installing consul")
Copy link
Member Author

@jm96441n jm96441n Dec 18, 2023

Choose a reason for hiding this comment

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

I don't know how I feel about the descriptions for these flags, any feedback/suggestions would be appreciated

@nathancoleman
Copy link
Member

@jm96441n thoughts on instead adding these to GatewayClassConfig.spec.deployment.labels.set when creating the gateway-resources ConfigMap instead of adding all the plumbing and new flags to the command?

@jm96441n
Copy link
Member Author

that's a good idea! lemme update this and give that a shot

@jm96441n jm96441n force-pushed the NET-6809-chart-related-labels branch from 5c92ab1 to bb90713 Compare December 18, 2023 22:34
@nathancoleman
Copy link
Member

This PR will be further simplified by #3397

@jm96441n jm96441n force-pushed the NET-6809-chart-related-labels branch from bb90713 to 2a9a5fb Compare December 19, 2023 20:45
@jm96441n jm96441n enabled auto-merge (squash) December 19, 2023 20:56
@jm96441n jm96441n merged commit 6f293d5 into main Dec 20, 2023
3 checks passed
@jm96441n jm96441n deleted the NET-6809-chart-related-labels branch December 20, 2023 16:46
sarahalsmiller pushed a commit that referenced this pull request Jan 5, 2024
* use set labels for setting labels on deployments

* incorporate changes for setting labels
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr/no-backport signals that a PR will not contain a backport label pr/no-changelog PR does not need a corresponding .changelog entry theme/mesh-gw
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants