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

e2e profiles #10634

Merged
merged 4 commits into from
Feb 17, 2025
Merged

e2e profiles #10634

merged 4 commits into from
Feb 17, 2025

Conversation

jenshu
Copy link
Contributor

@jenshu jenshu commented Feb 17, 2025

Previously, our e2e tests applied 3 levels of helm values:

  • a common values file with production recommendations
  • a "profile" values file which indicated whether the installation was for edge, kube gateway, or both
  • a test-specific helm values file

The latter 2 files would be specified by each test as part of the kgateway.Context, and then when installing the chart, the test helper would apply the production recommendations manifest, along with the profile manifest and the test-specific manifest.

We no longer need the profile manifest (since only kube gateway is supported), so removed the references to those profiles. In addition, to make the use of the "production recommendations" manifest more explicit, repurposed the ProfileValuesManifestFile in the existing tests to point to the production recommendations manifest (instead of the test helper making an assumption that it should always be used).

Signed-off-by: Jenny Shu <[email protected]>
Signed-off-by: Jenny Shu <[email protected]>
# This file defines the common recommendations for a user who wants to run Gloo Gateway.
# As we call out in the [profiles README](./profiles/README.md), these should be reviewed and tested
# before being adopted.
# This file defines the common recommendations for a user who wants to run kgateway.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this file is currently mostly commented out; need to figure out what the new helm values are. the current chart might not support all of them yet. filed #10635 to track

Copy link
Contributor

Choose a reason for hiding this comment

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

in general think having a set of "production recommendation" is generally overkill for the current state of the project/helm chart, but keeping this file around mostly commented out makes sense to me in the short term

Copy link
Member

Choose a reason for hiding this comment

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

+1 on profiles being a bit premature right now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah i don't know if there's much to add to this file for now, but eventually we might have some recommended values documented (like we did for edge in production) and probably want to test them here

@jenshu jenshu mentioned this pull request Feb 11, 2025
3 tasks
@jenshu jenshu mentioned this pull request Feb 17, 2025
Copy link
Contributor

Choose a reason for hiding this comment

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

fine leaving as-is for now but pretty much all values in here other than global are no longer relevant right?

# This file defines the common recommendations for a user who wants to run Gloo Gateway.
# As we call out in the [profiles README](./profiles/README.md), these should be reviewed and tested
# before being adopted.
# This file defines the common recommendations for a user who wants to run kgateway.
Copy link
Contributor

Choose a reason for hiding this comment

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

in general think having a set of "production recommendation" is generally overkill for the current state of the project/helm chart, but keeping this file around mostly commented out makes sense to me in the short term

@jenshu jenshu added this pull request to the merge queue Feb 17, 2025
Merged via the queue into kgateway-dev:main with commit 4dd80fd Feb 17, 2025
9 checks passed
@jenshu jenshu deleted the e2e-profiles branch February 17, 2025 15:36
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.

3 participants