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

Run VSphere tests in self-hosted, refactor test suite into spec per provider, support label filtering, get all tests green #360

Merged
merged 4 commits into from
Oct 17, 2024

Conversation

squizzi
Copy link
Contributor

@squizzi squizzi commented Sep 20, 2024

This PR refactors our existing e2e test spec into a more conventional e2e test run. We now have a BeforeSuite that runs and installs/validates the controller. Prior to this we had all of our e2e tests under a single Controller spec, but this breaks it into:

  • Controller (Label(controller))
  • Providers (Label(provider))
    • AWS (Label(provider:aws, provider:cloud)) - test/e2e/provider_aws_test.go
    • Azure (Label(provider:azure, provider:cloud)) - test/e2e/provider_azure_test.go
    • Vsphere (Label(provider:vsphere, provider:onprem)) - test/e2e/provider_vsphere_test.go

The labels for these tests have been documented here: https://github.com/Mirantis/hmc/pull/360/files#diff-3a6cab76ecce19612a704fe0ef89dc83f1ed2df44f7bfa28d47d2529a7949fafR139

Each of the umbrella labels correspond to a GitHub job that is dependent on the Build and Unit Test job which provides setup. The Controller tests always run, no matter the label, right now the Controller test is disabled because there aren't any tests other than Before/AfterSuite, but we can enable it because that sort of is a test of the controller in itself.

The provider tests are still attached to the test e2e label, but I think we can break these out further with provider specific GitHub labels at some point.

provider:onprem tests run on a self-hosted GitHub runner that has access to Mirantis' network, this is to support providers that do not necessarily have cloud infrastructure we can get to from a GitHub hosted runner -- like VSphere.

This PR also adds support for creating ClusterIdentity, Credential and their associated Secret resources with a new clusteridentity package.

And of course, with testing comes bug fixing, so this PR fixes several different issues discovered along the way.

Closes: #210, Closes: #323


Note: I've opened two other issues to further simplify these tests, we could make these a lot easier to write in the future with:

@squizzi squizzi marked this pull request as draft September 20, 2024 17:30
@squizzi squizzi added the test e2e Runs the entire provider E2E test suite, controller E2E tests are always ran label Sep 20, 2024
@squizzi squizzi force-pushed the vsphere-tests-on-self-hosted branch 2 times, most recently from dc5f8d2 to 605513d Compare September 20, 2024 21:03
@squizzi squizzi added the github actions Pull requests that update GitHub Actions code label Sep 20, 2024
@squizzi

This comment was marked as resolved.

@squizzi squizzi force-pushed the vsphere-tests-on-self-hosted branch 5 times, most recently from 92cc4f5 to d4f0344 Compare September 23, 2024 21:10
@squizzi squizzi force-pushed the vsphere-tests-on-self-hosted branch from d4f0344 to fea83d7 Compare September 24, 2024 18:39
@squizzi squizzi changed the title Try running VSphere tests in self-hosted runner Run VSphere tests in self-hosted, refactor test suite into spec per provider, support label filtering Sep 24, 2024
@squizzi squizzi force-pushed the vsphere-tests-on-self-hosted branch 4 times, most recently from f76cfe0 to e19f75f Compare September 26, 2024 23:23
@squizzi squizzi changed the title Run VSphere tests in self-hosted, refactor test suite into spec per provider, support label filtering Run VSphere tests in self-hosted, refactor test suite into spec per provider, support label filtering, get all tests green Sep 26, 2024
@squizzi squizzi force-pushed the vsphere-tests-on-self-hosted branch 8 times, most recently from 7560f52 to 00088a6 Compare October 2, 2024 19:17
@squizzi
Copy link
Contributor Author

squizzi commented Oct 2, 2024

  • All standalone-cp tests and AWS hosted is green 🎉
  • Azure hosted is red, working on debugging this one.

kylewuolle
kylewuolle previously approved these changes Oct 10, 2024
a13x5
a13x5 previously requested changes Oct 10, 2024
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.

Some minor notes here and there.

I don't have major concerns regarding actual tests, apart from the fact that standalone and hosted CP tests are squashed together.

I understand the reason for that, since we're reusing standalone cluster and it must be created anyway for hosted CP tests, but I see the following cons of this approach:

  • standalone and hosted tests can't be run separately
  • If hosted CP fails (and it will a lot I'm sure) it will fail the whole AWS test, which makes it slightly harder to interpret the test.

I know that it's e2e tests and we should run the whole suite, but I believe it makes sense to run only on the release or ad-hoc. Most changes could be tested by simply running tests on standalone cluster.

I will not block this PR because of that, but I a task for the future us would be nice.

.github/workflows/build_test.yml Show resolved Hide resolved
Makefile Show resolved Hide resolved
test/e2e/e2e_suite_test.go Outdated Show resolved Hide resolved
test/e2e/managedcluster/common.go Outdated Show resolved Hide resolved
test/e2e/provider_aws_test.go Outdated Show resolved Hide resolved
test/e2e/provider_vsphere_test.go Outdated Show resolved Hide resolved
.github/workflows/build_test.yml Show resolved Hide resolved
@squizzi squizzi dismissed stale reviews from kylewuolle and slysunkin via 5a7aad5 October 11, 2024 00:04
@squizzi squizzi requested a review from a13x5 October 11, 2024 00:04
@squizzi squizzi force-pushed the vsphere-tests-on-self-hosted branch 2 times, most recently from 80fc91c to 9c6a1b4 Compare October 11, 2024 00:06
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.

Minor notes

test/e2e/e2e_suite_test.go Outdated Show resolved Hide resolved
@squizzi squizzi force-pushed the vsphere-tests-on-self-hosted branch from 9c6a1b4 to 4ee982a Compare October 11, 2024 17:19
@squizzi squizzi requested a review from a13x5 October 11, 2024 17:19
@squizzi squizzi force-pushed the vsphere-tests-on-self-hosted branch 2 times, most recently from 9630f10 to 803e93c Compare October 15, 2024 18:08
slysunkin
slysunkin previously approved these changes Oct 15, 2024
@squizzi squizzi force-pushed the vsphere-tests-on-self-hosted branch from 803e93c to eb66984 Compare October 16, 2024 19:42
a13x5
a13x5 previously approved these changes Oct 17, 2024
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
PR needs rebasing

* Break provider tests into seperate files with labels
  representing either onprem or cloud providers.
* Add new jobs to CI workflow which dictate where tests
  will run, onprem provider tests like vSphere will run
  on self-hosted runners since they will use internal
  resources to test.  Cloud provider tests will use the
  existing workflow since they can access providers without
  network access and can take advantage of the much larger
  GitHub hosted pool.  Hosted/Self-hosted tests can run
  concurrently.
* Make Cleanup depend on the cloud-e2etest only.
* Use new GINKGO_LABEL_FILTER to manage what tests run
  where.
* Move controller validatation into BeforeSuite since the
  controller needs to be up and ready for each provider test,
  this will also enable us to add controller specific test
  cases later and make those run without the "test e2e" flag.
* Seperate self-hosted and hosted test concurrency groups
* Update docs with test filtering instructions
* Ensure a Release exists for the custom build.version we deploy
* Move all e2e related helpers into e2e dir
  * Add new clusteridentity package for creating ClusterIdentity kind's
  and associated Secret's.
* Merge PR workflows together
* Make sure VERSION gets passed across jobs
* Ensure uniqueness among deployed ManagedClusters, simplify
  MANAGED_CLUSTER_NAME in CI to prevent Azure availabilitySetName
  validation error.
* Default Azure test templates to uswest2 to prevent issues with
  AvailabilityZone.
* Use the same concurrency-group across all jobs, except Cleanup
  which intentionally does not belong to a concurrency-group.
* Use Setup Go across jobs for caching.
* Support patching other hosted clusters to status.ready with a
  common patching helper.
* Move VSphere delete into AfterEach to serve as cleanup.
* Add support for cleaning Azure resources.
* Prevent ginkgo from timing out tests.
* Use azure-disk CSI driver we deploy via templates.

Signed-off-by: Kyle Squizzato <[email protected]>
Signed-off-by: Kyle Squizzato <[email protected]>
@squizzi squizzi dismissed stale reviews from a13x5 and slysunkin via 24603f7 October 17, 2024 16:54
@squizzi squizzi force-pushed the vsphere-tests-on-self-hosted branch from eb66984 to 24603f7 Compare October 17, 2024 16:54
@squizzi
Copy link
Contributor Author

squizzi commented Oct 17, 2024

@a13x5 rebased

@DinaBelova
Copy link
Collaborator

Will be merging w/ admin rights to bypass the checks

Job will not run on this patch because of pull_request_target being the workflow, it needs the actions workflow to live in the base branch before it gets triggered, but this PR adds the actions.

@squizzi will make a follow-up after this PR merge to ensure no issues are present.

@DinaBelova DinaBelova merged commit e2ffd2b into main Oct 17, 2024
@squizzi squizzi deleted the vsphere-tests-on-self-hosted branch October 17, 2024 20:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
github actions Pull requests that update GitHub Actions code test e2e Runs the entire provider E2E test suite, controller E2E tests are always ran
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

E2E Test workflow should reuse the result of Build and Test E2e verification of Azure templates
5 participants