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

Set k8sgateway api version when setting up e2e test cluster #10386

Merged
merged 8 commits into from
Nov 22, 2024

Conversation

sheidkamp
Copy link

@sheidkamp sheidkamp commented Nov 21, 2024

Description

When setting up our kind cluster for tests, we need to ensure that the correct version of the k8sgateway APIs are loaded, as we support multiple versions of Istio that use different versions of the rapidly changing k8sgateway APIs.

To support this, k8sgateway_api_version has been added to the the min/max environment variables for the test matrix and the value are set as the CONFORMANCE_VERSION environment variable when running the setup-kind script.

CI changes

The setup kind cluster action now has an optional parameter k8sgateway_api_version which is set as the CONFORMANCE_VERSION environment variable when running the setup-kind script.

k8sgateway_api_version has been set in the min/max_versions.env files

Context

Istio was failing to install when running the "min" tests because the expected CRDs were not present

Testing steps

Nightly test run without istio install failures: https://github.com/solo-io/gloo/actions/runs/11955940423

specifically istioctl install failed is not found in the "min" test log: https://github.com/solo-io/gloo/actions/runs/11955940423/job/33329519257

Interesting decsions

We should bump the minimum version of Istio, but will leave that to a separate/wholistic effort to determine minimum versions for the new release instead of cherry-picking this spot.

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

@solo-changelog-bot
Copy link

Issues linked to changelog:
#10365

@sheidkamp
Copy link
Author

/kick-ci

@sheidkamp
Copy link
Author

/kick-ci

@solo-changelog-bot
Copy link

Issues linked to changelog:
#10364

@jenshu
Copy link

jenshu commented Nov 21, 2024

@sheidkamp can you sync with @danehans on k8sgateway#10360
probably makes sense to have the same implementation in both repos

@@ -21,6 +21,10 @@ inputs:
istio-version:
required: true
description: The version of Istio
k8sgateway_api_version:

Choose a reason for hiding this comment

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

Suggested change
k8sgateway_api_version:
k8sgateway-api-version:

The existing inputs follow this convention. I can't find the github docs now, but I had tried to base it off them. If I was wrong and your convention is preferred, can we change the other inputs in this file so we're consistent?

Copy link
Author

Choose a reason for hiding this comment

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

Updated in the appropriate places. Left the underscores in the env files as that is the usual formatting for env variables, and the switch in conventions comes when the env variable is assigned to the action input.

Copy link

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

nice work!

@danehans
Copy link

xref #10390 is a downstream cherry-pick of k8sgateway#10360 for supporting Gateway API matrix testing.

@soloio-bulldozer soloio-bulldozer bot merged commit e290622 into main Nov 22, 2024
17 of 18 checks passed
@soloio-bulldozer soloio-bulldozer bot deleted the conformance-version-in-env branch November 22, 2024 00:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants