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

Improve e2e testing #2995

Closed
16 tasks done
sbueringer opened this issue May 13, 2024 · 16 comments
Closed
16 tasks done

Improve e2e testing #2995

sbueringer opened this issue May 13, 2024 · 16 comments
Assignees
Labels
area/supervisor Issues or PRs related to the supervisor mode area/testing
Milestone

Comments

@sbueringer
Copy link
Member

sbueringer commented May 13, 2024

Improve e2e tests

We compared CAPV govmomi & supervisor e2e tests with core CAPI e2e tests.
As a result we want to make the following improvements.

Adding e2e tests:

After CAPV v1.11: Adding e2e tests:

Other improvements:

Reconsider if we need it later:

  • Add test: When using the autoscaler with Cluster API using ClusterClass (incl. scale from zero coverage?)

govmomi has the following additional jobs (intentionally):

  • accepted: it's okay if it's only tested with govmomi:
    • capv-e2e.[It] Cluster creation with [Ignition] bootstrap [PR-Blocking] Should create a workload cluster
  • accepted: not surfaced in CAPV supervisor CRDs:
    • capv-e2e.[It] Cluster creation with anti affined nodes should create a cluster with anti-affined nodes
    • capv-e2e.[It] Cluster creation with storage policy should create a cluster successfully
    • capv-e2e.[It] DHCPOverrides configuration test when Creating a cluster with DHCPOverrides configured Only configures the network with the provided nameservers
  • accepted: IPAM provider not supported in supervisor mode
    • capv-e2e.[It] ClusterClass Creation using Cluster API quick-start test and IPAM Provider [ClusterClass] Should create a workload cluster
@fabriziopandini
Copy link
Member

/area supervisor
Given that most of the improvements are for supervisor mode

@k8s-ci-robot k8s-ci-robot added the area/supervisor Issues or PRs related to the supervisor mode label May 13, 2024
@sbueringer sbueringer self-assigned this May 13, 2024
@zhanggbj
Copy link
Contributor

zhanggbj commented May 14, 2024

I would like to start with

@zhanggbj
Copy link
Contributor

Also would like to pick up a similar one :-)

  • P1: Add test "When testing ClusterClass rollouts [ClusterClass]" (supervisor)

@zhanggbj
Copy link
Contributor

zhanggbj commented May 24, 2024

It seems this one is more about testing efforts. Just raised a PR to test.

  • P1: Add clusterctl upgrade tests n-2, n-1 (supervisor)

@sbueringer
Copy link
Member Author

sbueringer commented May 24, 2024

Yeah good case is that most if it just works once added :)

A few require a bit more work

@chrischdi
Copy link
Member

Taking this one

  • P1: Let's check if we have unit test coverage for the following tests for supervisor

@chrischdi
Copy link
Member

chrischdi commented Jul 1, 2024

I created a PR for

  • P1: Let's check if we have unit test coverage for the following tests for supervisor @chrischdi
    • capv-e2e.[It] Label nodes with ESXi host info creates a workload cluster whose nodes have the ESXi host info

Regarding the other subpoint:

  • capv-e2e.[It] Hardware version upgrade creates a cluster with VM hardware versions upgraded

There is already unit-test coverage here:

We define the VSphereMachine here, the called function has a hardcoded value for vmx-17:

vsphereMachine = util.CreateVSphereMachine(machineName, clusterName, className, imageName, storageClass, controlPlaneLabelTrue)

Later we check the spec of the VirtualMachine to match the minHardwareVersion (again 17):

Expect(vmopVM.Spec.MinHardwareVersion).To(Equal(minHardwareVersion))

Plus an immutability check at the end:

By("Updates to immutable VMOp fields are dropped", func() {
vsphereMachine.Spec.ImageName = "new-image"
vsphereMachine.Spec.ClassName = "new-class"
vsphereMachine.Spec.StorageClass = "new-storageclass"
vsphereMachine.Spec.MinHardwareVersion = "vmx-9999"
vsphereCluster.Status.ResourcePolicyName = "new-resourcepolicy"
requeue, err = vmService.ReconcileNormal(ctx, supervisorMachineContext)
verifyOutput(supervisorMachineContext)
})

@sbueringer
Copy link
Member Author

@chrischdi @fabriziopandini if I see correctly we don't have the cluster autoscaler test on the list, let's add it as P2?

@zhanggbj
Copy link
Contributor

zhanggbj commented Jul 3, 2024

@sbueringer I could take this one once #3024 is merged. Thanks!

- [ ] Add clusterctl upgrade tests n-3 (supervisor)

@sbueringer
Copy link
Member Author

sbueringer commented Jul 3, 2024

  • Add clusterctl upgrade tests n-3

@zhanggbj was just chatting with Christian and Fabrizio about it, we just realized that we didn't have supervisor e2e test in CAPV v1.8. So it would be a lot of effort to add v1.8 => main tests.

We would propose instead to wait until after the v1.11 release in ~ 6 weeks, and then we can just have v1.9 => main, v1.10 => main and v1.11 => main (so we get n-3 coverage just a bit later but with a lot less effort).

Probably we can just include it in the "Prepare main branch for development of the new release" task which is part of #3084. Usually we drop the oldest test there, but in that case we will simply keep it around.

@zhanggbj
Copy link
Contributor

zhanggbj commented Jul 5, 2024

@sbueringer Makes sense to me. This will provide n-3 tests with much less effort. Thanks!

@sbueringer
Copy link
Member Author

@zhanggbj Sorry I was just working on prepare main branch and realized that "Add clusterctl upgrade tests n-3" will be covered by that, so I took over that task from you.

@zhanggbj
Copy link
Contributor

@sbueringer Just go ahead, no worries :)

@sbueringer
Copy link
Member Author

@chrischdi Assigned Add test "When using the autoscaler with Cluster API using ClusterClass" to you + updated the PR description to add "part of ..."

@sbueringer
Copy link
Member Author

Moved remaining sub-tasks to separate issues:

/close

Great work everyone!

@k8s-ci-robot
Copy link
Contributor

@sbueringer: Closing this issue.

In response to this:

Moved remaining sub-tasks to separate issues:

/close

Great work everyone!

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/supervisor Issues or PRs related to the supervisor mode area/testing
Projects
None yet
Development

No branches or pull requests

5 participants