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

Adding helm tests for production recommendations #8768

Closed
wants to merge 13 commits into from
Closed

Conversation

davidjumani
Copy link
Contributor

@davidjumani davidjumani commented Oct 10, 2023

Description

Adds helm kube e2e tests to mimic the recommendations for production deployments mentioned here

Context

This is to ensure that we test gloo as users would deploy it in production and catch potential issues early

Interesting decisions

N/A

Testing steps

Run the helm kube2e tests

Notes for reviewers

The issue mentioned here needs to be fixed

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works

BOT NOTES:
resolves #8804

@github-actions github-actions bot added the keep pr updated signals bulldozer to keep pr up to date with base branch label Oct 10, 2023
@davidjumani davidjumani force-pushed the helm-prod-rec branch 2 times, most recently from c6339cc to 5942d35 Compare October 10, 2023 16:36
// The aim of the test is to ensure that the readiness probe is configured on the gateway proxy
// and the gateway-proxy deployment is ready before helm marks the upgrade as successful.
// Ref: https://github.com/solo-io/gloo/issues/8288
Context("Custom readiness probe", func() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Replacing the custom readiness probe test with the Production recommendations tests as the latter tests the former along with other prod settings

@github-actions
Copy link

github-actions bot commented Oct 10, 2023

Visit the preview URL for this PR (updated for commit b36cdca):

https://gloo-edge--pr8768-helm-prod-rec-xgga4pu8.web.app

(expires Fri, 27 Oct 2023 14:32:15 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 77c2b86e287749579b7ff9cadb81e099042ef677

@solo-changelog-bot
Copy link

Issues linked to changelog:
#8804

Copy link
Contributor

@sam-heilbron sam-heilbron left a comment

Choose a reason for hiding this comment

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

I like this and I think it's a nice step towards codifying our production recommendations! For each of these tests, we assert that the components are health (which is good) but not much beyond that. I had kind of envisioned we could also update the other suites (https://github.com/solo-io/gloo/blob/main/test/kube2e/gloo/artifacts/helm.yaml for example) so that when we ran the tests we configured as many production recommendations as well. That way we assert both that everything is healthy AND that the behavior matches what we would expect. What do you think about that?

test/helpers/kube_dump.go Show resolved Hide resolved
@davidjumani
Copy link
Contributor Author

Thanks @sam-heilbron ! I would like to use these same prod recommendations for all the tests but setting settings.disableKubernetesDestinations && global.glooRbac.namespaced (which is the standard in all our tests) leads to panic in gloo (#8801), which is why this was added as a separate test.
Most of the other recommendations such as pod disruption budgets, gzip, etc. have their own tests (albeit not e2e or helm tests). I can add those as well if you think it will be better to test them all together

sam-heilbron
sam-heilbron previously approved these changes Oct 18, 2023
@davidjumani
Copy link
Contributor Author

Will create a new PR along with the custom readiness probe fix

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
keep pr updated signals bulldozer to keep pr up to date with base branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Expose settings.endpointsWarmingTimeout in helm
2 participants