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-6812 Allow meshGateway.enabled when resource-apis experiment is enabled #3285

Merged
merged 4 commits into from
Dec 6, 2023

Conversation

nathancoleman
Copy link
Member

@nathancoleman nathancoleman commented Nov 29, 2023

Changes proposed in this PR:
Currently, attempting to install with meshGateway.enabled returns the following error today:

$ helm upgrade --install consul ~/workspaces/consul-k8s/charts/consul --values ./values.yaml --namespace consul --create-namespace
Error: UPGRADE FAILED: execution error at (consul/templates/_helpers.tpl:510:2): When the value global.experiments.resourceAPIs is set, meshGateway.enabled is currently unsupported.

This made sense for the 1.17.0 release of Consul when Mesh Gateway was not implemented for v2; however, we're currently building the v2 mesh gateway and need to be able to helm install with it enabled. The implementation will be sufficient enough for Consul 1.18 to ship without this guard in place.

This PR does three things:

  1. Allows meshGateway.enabled to be set at the same time as experiments[].resource-apis (which is what enables v2 things)
  2. Only has the gateway-resources-job create v2 mesh gateway things if meshGateway.enabled
  3. Prevents the v1 mesh gateway things from being created if experiments[].resource-apis is set (meaning v2 is enabled)

How I've tested this PR:
See helm install command above, verify it does not return error and obeys the 3 items above

How I expect reviewers to test this PR:
See above

Checklist:

@nathancoleman nathancoleman 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 Nov 29, 2023
@nathancoleman nathancoleman force-pushed the enable-mesh-gateways-with-experiment branch from 57db171 to 8ed455a Compare December 1, 2023 16:15
@nathancoleman nathancoleman requested review from a team, andrewstucki and NiniOak and removed request for a team December 1, 2023 16:15
@nathancoleman nathancoleman marked this pull request as ready for review December 1, 2023 16:15
@nathancoleman nathancoleman force-pushed the enable-mesh-gateways-with-experiment branch from 8ed455a to dda37eb Compare December 5, 2023 15:23
Copy link
Collaborator

@NiniOak NiniOak left a comment

Choose a reason for hiding this comment

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

Code looks good, I'm going to manually test the change manually before smashing the ✅ button

Copy link
Collaborator

@NiniOak NiniOak left a comment

Choose a reason for hiding this comment

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

non blocking comment - looks good otherwise.

@nathancoleman nathancoleman enabled auto-merge (squash) December 6, 2023 16:18
@nathancoleman nathancoleman changed the title Allow meshGateway.enabled when resource-apis experiment is enabled NET-6812 Allow meshGateway.enabled when resource-apis experiment is enabled Dec 6, 2023
@nathancoleman nathancoleman merged commit 8e9d8fa into main Dec 6, 2023
3 checks passed
@nathancoleman nathancoleman deleted the enable-mesh-gateways-with-experiment branch December 6, 2023 16:52
sarahalsmiller pushed a commit that referenced this pull request Jan 5, 2024
…3285)

* Allow meshGateway.enabled when resource-apis experiment is enabled

* Exclude v1 mesh gateway templates when resource-apis experiment enabled

* Enable mesh gateway for bats test where new gate added

* Combine if statements for gating v2 configmap component
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.

3 participants