Improve execution speed of unit tests

[sbueringer]

User Story

As a developer I would like to have very fast local feedback loop with unit tests.

Detailed Description

In most of our test suites (probably all?) we start an entire tes tenv before starting the unit tests. This is of course necessary for some of our unit tests in the respective packages. But it’s also really painful when developing new unit tests which don’t need test env, because it still takes >10s to run those tests. Imho if somehow possible the feedback loop for something like that (i.e. running a subset of tests which don’t require test env) should be a lot faster. It would be really nice if we can find a way to improve this for local test development / execution.

In addition, it would be great if we can also speed up the unit tests which require test env.

Anything else you would like to add:

This topic was briefly discussed in Slack (https://kubernetes.slack.com/archives/C8TSNPY4T/p1627483162464600) and we came up with the following solutions:

1. Make it possible to skip the test env creation entirely (This is useful when only a subset of unit tests which doesn't need test env is executed, e.g. via IDE)

   • This could be achieved by adjusting our TestMain funcs to skip the test env creation if e.g. the CAPI_DISABLE_TEST_ENV environment variable is set. The env var can then be configured either on the command line or in the DIE.

2. Make it possible to spin up test env once and then reuse it for multiple test runs (This is useful when unit tests which require a test env are executed, which is not adressed by 1.)

I think we should implement both as:

• When we only implement 1., we still have slow unit tests with test env.
• When we only implement 2., we still have to spin up a test env for unit tests which don't need it.

[Miscellaneous information that will assist in solving the issue.]

/kind feature

[sbueringer]

/cc @randomvariable @vincepri @fabriziopandini @CecileRobertMichon @enxebre @srm09

[sbueringer]

I think solution 2 is already implemented by controller-runtime: https://github.com/kubernetes-sigs/controller-runtime/blob/master/pkg/envtest/server.go#L40

[vincepri]

The only issue I see with using an existing cluster is how we go about registering webhooks and whatnot. We can try to work around a quick way to spin up a local control plane with envtest by making sure we register all our webhooks and CRDs automatically when we start it up.

[vincepri]

/area testing
/priority important-longterm
/milestone v0.4

[sbueringer]

Tested ./controllers/... test with testenv with an existing kind cluster (on Mac). Results:

Setup

kind create cluster
# Append to /etc/hosts
# 127.0.0.1 docker.for.mac.localhost

Patch envtest/webhooks.go

diff --git a/internal/envtest/webhooks.go b/internal/envtest/webhooks.go
--- a/internal/envtest/webhooks.go	(revision Staged)
+++ b/internal/envtest/webhooks.go	(date 1627590181567)
@@ -75,10 +75,11 @@
 		klog.Fatalf("Failed to append controlplane controller webhook config: %v", err)
 	}
 	return envtest.WebhookInstallOptions{
-		MaxTime:            20 * time.Second,
-		PollInterval:       time.Second,
-		ValidatingWebhooks: validatingWebhooks,
-		MutatingWebhooks:   mutatingWebhooks,
+		MaxTime:                      20 * time.Second,
+		PollInterval:                 time.Second,
+		ValidatingWebhooks:           validatingWebhooks,
+		MutatingWebhooks:             mutatingWebhooks,
+		LocalServingHostExternalName: "docker.for.mac.localhost",
 	}
 }

Note: /etc/hosts and LocalServingHostExternalName was necessary, because the kube-apiserver running in kind on Docker for Mac could not reach the locally running webhooks otherwise.

Tests (via Intellij with env var USE_EXISTING_CLUSTER=true)

First run all unit tests in ./controllers/...

• all green

Second run all unit tests in ./controllers/... (on same kind cluster)

• failed tests:
  • TestNodeToMachine: test-node-to-machine-1
  • TestClusterReconciler/Should_successfully_set_ControlPlaneInitialized_on_the_cluster_object_if_controlplane_is_ready nodes "id-node-1" already exists
  • TestWatches: would have to spend some more time analyzing the logs

Thx @GrigoriyMikhalkin, you're recently merged PRs for the randomly generated namespaces made that possible!! 🎉
(Before I used the latest merged commits I had a lot of failed test because of already existing namespaces)

[sbueringer]

1 is done 2 is still open (will implement "kind support" the next few days)
/reopen

[k8s-ci-robot]

@sbueringer: Reopened this issue.

In response to this:

1 is done 2 is still open (will implement "kind support" the next few days)
/reopen

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/test-infra repository.