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.

Sign up for GitHub

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

Jump to bottom

Implement adopted cluster e2e tests #855

Merged
merged 6 commits into from
Jan 15, 2025
Merged

Implement adopted cluster e2e tests #855

merged 6 commits into from
Jan 15, 2025

Conversation

kylewuolle
Copy link
Contributor

@kylewuolle kylewuolle commented Jan 2, 2025 •
edited
Loading

This adds adopted cluster e2e tests to the suite. Also, I've ported in the various fixes around timing issues like waiting for the cluster templates to be valid and the credentials to be ready before creating deployments. Resolves #828, #339 and #871

@kylewuolle kylewuolle force-pushed the adopted-cluster-e2e branch 3 times, most recently from 58ea636 to 4d0a763 Compare January 7, 2025 18:50
zerospiel
zerospiel previously approved these changes Jan 8, 2025
View reviewed changes
@zerospiel
Copy link
Contributor

LGTM, the PR needs to be rebased and wait for the e2e-CI POW after the renaming to execute the tests

@DinaBelova DinaBelova mentioned this pull request Jan 9, 2025
Closed
@kylewuolle kylewuolle force-pushed the adopted-cluster-e2e branch from fed227b to 0a94ef0 Compare January 9, 2025 21:37
@kylewuolle kylewuolle requested a review from zerospiel January 9, 2025 21:38
@kylewuolle kylewuolle force-pushed the adopted-cluster-e2e branch 2 times, most recently from c551da8 to 88ed748 Compare January 9, 2025 23:40
This was linked to issues Jan 10, 2025
Re-enable aws-standalone-cp template deletion test #339
Closed
e2e tests fail on admission check #871
Closed
eromanova
eromanova reviewed Jan 14, 2025
View reviewed changes
test/e2e/provider_adopted_test.go Outdated Show resolved Hide resolved
test/e2e/provider_adopted_test.go Show resolved Hide resolved
@kylewuolle kylewuolle requested a review from eromanova January 14, 2025 16:27
eromanova
eromanova previously approved these changes Jan 15, 2025
View reviewed changes
@eromanova eromanova added the test e2e Runs the entire provider E2E test suite, controller E2E tests are always ran label Jan 15, 2025
@eromanova eromanova self-requested a review January 15, 2025 13:44
@eromanova eromanova dismissed their stale review January 15, 2025 13:45

Failing tests

eromanova
eromanova reviewed Jan 15, 2025
View reviewed changes
Copy link
Contributor

@eromanova eromanova left a comment

Choose a reason for hiding this comment

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

E2E tests are failing due to a “not ready” clustertemplate error.
@kylewuolle, could you please double-check if the cluster templates readiness verification is functioning as expected? Based on the E2E testing logs, it seems that either the readiness check for the templates wasn’t executed, or it completed too early before the templates were actually ready.

@kylewuolle
Copy link
Contributor Author

E2E tests are failing due to a “not ready” clustertemplate error. @kylewuolle, could you please double-check if the cluster templates readiness verification is functioning as expected? Based on the E2E testing logs, it seems that either the readiness check for the templates wasn’t executed, or it completed too early before the templates were actually ready.

It's confusing but the e2e test process for now is:

  1. The PR code is checked out, built, unit tested and then the controller image is pushed to github package repo
  2. e2e tests run but they are run using the code from the main branch to test the PR code essentially

So basically we have to wait for this to merge to see the results. I believe this is done so you can't steal secrets by creating a PR and changing the e2e test code since the PR would be reviewed first.

a13x5
a13x5 approved these changes Jan 15, 2025
View reviewed changes
Copy link
Collaborator

@a13x5 a13x5 left a comment

Choose a reason for hiding this comment

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

LGTM.
Will merge without waiting for the tests, because of the checkout issue

@a13x5 a13x5 merged commit 23d65fd into k0rdent:main Jan 15, 2025
3 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eromanova eromanova eromanova left review comments

@a13x5 a13x5 a13x5 approved these changes

@Kshatrix Kshatrix Awaiting requested review from Kshatrix Kshatrix is a code owner

@zerospiel zerospiel Awaiting requested review from zerospiel zerospiel is a code owner

Assignees
No one assigned
Labels
test e2e Runs the entire provider E2E test suite, controller E2E tests are always ran
Projects
k0rdent
Status: Done
Milestone
No milestone
4 participants
@kylewuolle @zerospiel @eromanova @a13x5