docs(e2e): trim the comments added in this series

Review feedback on kube-logging#2301. The comments this series added explained more than the
code needed. Cut to one line each where they earn it and dropped where they did
not: 27 lines of prose down to 8, plus assertion messages that only restated the
expected value.

Signed-off-by: Vyncint Ng <115854244+vyncint@users.noreply.github.com>

Author: vyncint

Makefile

@@ -54,10 +54,7 @@ CRD_OPTIONS ?= crd:maxDescLen=0
 
 E2E_TEST_TIMEOUT ?= 20m
 
-# go test runs this many suite binaries at a time, which bounds peak kind
-# clusters by the parallel tests inside them instead of by the core count. With
-# 4 the heaviest combination is fluentd-aggregator's 4 tests, volumedrain's 2 and
-# one each from two more, so at most 8 against the 17 an unbounded run creates.
+# Concurrent suite binaries, so peak kind clusters stops following the core count.
 E2E_CLUSTERS ?= 4
 
 TEST_COV_DIR := $(shell mkdir -p build/_test_coverage && realpath build/_test_coverage)

e2e/common/cluster.go

@@ -35,9 +35,7 @@ import (
 	"github.com/kube-logging/logging-operator/e2e/internal/kind"
 )
 
-// clusterStopTimeout bounds the wait for cluster.Start to return once the
-// context is canceled, so a runnable that will not stop is named here instead
-// of running the whole binary out of its -timeout.
+// clusterStopTimeout bounds the wait for cluster.Start to return after cancel.
 const clusterStopTimeout = time.Minute
 
 type Cluster interface {
@@ -78,18 +76,14 @@ func WithCluster(name string, t *testing.T, fn func(*testing.T, Cluster), before
 		assert.NoError(t, cluster.Cleanup())
 		cancel()
 
-		// Start's error is checked here rather than in the goroutine: FailNow is
-		// undefined off the test goroutine, so a cluster that never started used
-		// to be dropped and resurface as an unrelated Eventually timeout.
+		// Checked here, not in the goroutine: FailNow is undefined off the test one.
 		select {
 		case err := <-startErr:
 			assert.NoError(t, err, "starting the cluster")
 		case <-time.After(clusterStopTimeout):
 			assert.Fail(t, "cluster.Start did not return after cancellation")
 		}
 
-		// A cluster that came down cleanly must not fail the test on kind's own
-		// kubeconfig bookkeeping, so this matches the assertions above.
 		assert.NoError(t, DeleteTestCluster(name))
 	}()
 

e2e/common/kind.go

@@ -32,9 +32,7 @@ const KindClusterCreationTimeout = "3m"
 
 var kindCLI = kind.New()
 
-// kubeconfigDir is one directory per run, created 0700 by MkdirTemp. The file
-// names inside it are predictable, the directory is not, so nothing planted at a
-// guessable path in the shared temp directory can be written through.
+// kubeconfigDir is one 0700 directory per run, so the paths in it are unguessable.
 var kubeconfigDir = sync.OnceValues(func() (string, error) {
 	return os.MkdirTemp("", "e2e-kubeconfig-*")
 })
@@ -50,11 +48,8 @@ func ClusterKubeconfigPath(name string) (string, error) {
 	return filepath.Join(dir, "kind-"+name+".kubeconfig"), nil
 }
 
-// RemoveClusterKubeconfig drops the file and the lock kind leaves beside it.
-// kind rewrites the kubeconfig rather than unlinking it, so without this a run
-// that was killed hands its leftovers to the next one. The directory goes with
-// the last cluster of the binary; until then it is not empty and Remove is a
-// no-op on it.
+// RemoveClusterKubeconfig drops the file and the lock kind leaves beside it,
+// which kind itself does not. The directory goes with the binary's last cluster.
 func RemoveClusterKubeconfig(name string) error {
 	path, err := ClusterKubeconfigPath(name)
 	if err != nil {

e2e/common/kind_test.go

@@ -28,15 +28,15 @@ func TestClusterKubeconfigPath(t *testing.T) {
 	beta, err := ClusterKubeconfigPath("beta")
 	require.NoError(t, err)
 
-	require.NotEqual(t, alpha, beta, "two clusters sharing one file is the lock contention")
+	require.NotEqual(t, alpha, beta)
 	require.Equal(t, filepath.Dir(alpha), filepath.Dir(beta))
 
 	again, err := ClusterKubeconfigPath("alpha")
 	require.NoError(t, err)
 	require.Equal(t, alpha, again, "create and delete have to name the same file")
 
 	dir := filepath.Dir(alpha)
-	require.NotEqual(t, os.TempDir(), dir, "a path directly in the shared temp directory is guessable")
+	require.NotEqual(t, os.TempDir(), dir, "a path in the shared temp directory is guessable")
 
 	info, err := os.Stat(dir)
 	require.NoError(t, err)
@@ -51,15 +51,14 @@ func TestRemoveClusterKubeconfig(t *testing.T) {
 	require.NoError(t, RemoveClusterKubeconfig("gamma"))
 
 	require.NoFileExists(t, path)
-	require.NoFileExists(t, lock, "kind leaves the lock behind when it is killed mid-write")
+	require.NoFileExists(t, lock)
 }
 
 func TestRemoveClusterKubeconfigToleratesMissingFiles(t *testing.T) {
 	require.NoError(t, RemoveClusterKubeconfig("never-created"))
 }
 
-// stubKubeconfig recreates the per-run directory when an earlier case has
-// already taken it away, so the cases here do not depend on their order.
+// stubKubeconfig recreates the directory a previous case may have removed.
 func stubKubeconfig(t *testing.T, name string) string {
 	t.Helper()
 

e2e/internal/kind/commands.go

@@ -110,9 +110,7 @@ func (k *Kind) CreateCluster(options CreateClusterOptions) error {
 
 	case errors.Is(err, ErrTimeout):
 		// kind removes a half-built cluster when one of its own actions fails,
-		// but not when we kill it, so the leftovers have to go explicitly. The
-		// kubeconfig has to come along: this delete runs when creates are timing
-		// out together, which is when a shared one loses the lock.
+		// but not when we kill it, so the leftovers have to go explicitly.
 		cleanupErr := k.deleteCluster(k.CleanupTimeout, DeleteClusterOptions{
 			Name:       options.Name,
 			Kubeconfig: options.Kubeconfig,

e2e/internal/kind/commands_test.go

@@ -266,9 +266,7 @@ func TestInvocations(t *testing.T) {
 			want:    []string{"create cluster --name fine"},
 		},
 		// kind tears down a half-built cluster when one of its own actions
-		// fails, but not when we kill it, so the delete has to be ours. It has
-		// to name the create's kubeconfig too: this is the delete that runs while
-		// several creates are timing out together.
+		// fails, but not when we kill it, so the delete has to be ours.
 		"a stalled create deletes the partial cluster": {
 			sleepEnv: "FAKE_KIND_CREATE_SLEEP",
 			timeout:  shortTimeout,