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

Actually run E2E tests when we should #2751

Closed
rainest opened this issue Jul 29, 2022 · 4 comments
Closed

Actually run E2E tests when we should #2751

rainest opened this issue Jul 29, 2022 · 4 comments

Comments

@rainest
Copy link
Contributor

rainest commented Jul 29, 2022

We apparently wrote a fake E2E test way back when that just ran integration tests and then never fixed that TODO when we actually made a separate E2E suite. This is still used in the release clearance and nightly image tests, but shouldn't be, as those are intended to test images, not just code:


(inexplicably just explicitly calls integration tests)
run: KONG_CLUSTER_VERSION=${{ matrix.kubernetes_version }} make test.integration.${{ matrix.dbmode }}

The integration tests already run elsewhere, when code is pushed to main. We need to:

  • Change these to running the actual E2E tests.
  • For nightly, set the E2E environment image override, TEST_KONG_CONTROLLER_IMAGE_OVERRIDE="kong/nightly-ingress-controller:nightly-redhat"
  • Preserve the matrix behavior that runs these on different Kubernetes versions. I don't think this should need any changes, but we'll want to confirm that it does work on a fork (GKE tests will fail for lack of credentials, but as long as they attempt to run on multiple versions that should be proof enough.
@randmonkey
Copy link
Contributor

randmonkey commented Aug 1, 2022

The integration tests already run elsewhere, when code is pushed to main. We need to:

Change these to running the actual E2E tests.
For nightly, set the E2E environment image override, TEST_KONG_CONTROLLER_IMAGE_OVERRIDE="kong/nightly-ingress-controller:nightly-redhat"
Preserve the matrix behavior that runs these on different Kubernetes versions. I don't think this should need any changes, but we'll want to confirm that it does work on a fork (GKE tests will fail for lack of credentials, but as long as they attempt to run on multiple versions that should be proof enough.
For these TODOs:

  • currently we have daily e2e tests with image "kong/nightly-ingress-controller:nightly, we should move the building of nightly image and running e2e tests into one action. (what to do with current nightly? run integration tests with kong nightly?)
  • e2e tests uses kind as test env now, if we want to move the env to GKE, we should not use the addons only supported in kind. For example, metallb is used in every case because we wanted to create LoadBalancer services, but the addon is not supported on GKE.

@rainest
Copy link
Contributor Author

rainest commented Aug 1, 2022

You could make the nightly image build and nightly E2E test two jobs in the same workflow. AFAIK the only reason they're not is that they were just originally created at different times (we started making nightly images before we had the ability to override images in E2E tests).

There's no reason to run integration tests with the nightly image (in KIND, anyway, see the next bit). Effectively you're already getting the same info from whichever merge to main happens to run before that day's nightly test.

Yeah, I missed that, we would indeed need to add GKE support (loading the correct addon set similar to how integration does it). I also forgot that image loading doesn't work on GKE either (and unlike metallb, we actually need it); we would need to add new KTF plugin code to support that via uploading images to GCR and deleting them after.

With that additional work in mind, we probably want to keep and rename the existing test-previous-kubernetes jobs to reflect that they're actually running integration on GKE, add the current Kubernetes version to their matrix, and create an additional job that runs E2E on previous versions of KIND until we can support GKE E2E. We may want to actually keep that GKE integration run indefinitely, actually, since it's currently the only place we do run those on GKE and I don't think we want to run them in the normal integration workflows (it costs more and isn't likely to find anything KIND runs wouldn't in most cases).

@pmalek
Copy link
Member

pmalek commented Aug 2, 2022

... we probably want to keep and rename the existing test-previous-kubernetes jobs to reflect that they're actually running integration on GKE

Definitely! We should be more explicit in such cases so that the future reader knows which tests are run where.

@mflendrich
Copy link
Contributor

Given relative overlap between #1605 and this issue, I've taken a stab at rewriting these two into 3 smaller issues with (hopefully) clearer scope boundaries:

@mflendrich mflendrich closed this as not planned Won't fix, can't repro, duplicate, stale Aug 8, 2022
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

No branches or pull requests

4 participants