Use the correct kubeconfig when running in-cluster (#437)

* Improve a docstring.
* When returning non-nil error, return nil config.

This does not change kuttl behaviour, since all callers of Harness.Config() do
not look at the config if a non-nil error is returned.

The benefit is less cognitive effort when reading this function.

* Extract a waitForFunctionalCluster method.

Just to keep Config() more focused and not worry about details such as trying
another namespace.

Co-authored-by: Ken Sipe <kensipe@gmail.com>
Signed-off-by: Marcin Owsiany <porridge@redhat.com>

pkg/test/harness.go

@@ -227,6 +227,8 @@ func (h *Harness) RunTestEnv() (*rest.Config, error) {
 
 // Config returns the current Kubernetes configuration - either from the environment
 // or from the created temporary control plane.
+// As a side effect, on first successful call this method also writes a kubernetes client config file in YAML format
+// to a file called "kubeconfig" in the current directory.
 func (h *Harness) Config() (*rest.Config, error) {
 	h.configLock.Lock()
 	defer h.configLock.Unlock()
@@ -253,51 +255,54 @@ func (h *Harness) Config() (*rest.Config, error) {
 			return nil, err
 		}
 		h.config.WarningHandler = rest.NewWarningWriter(os.Stderr, rest.WarningWriterOptions{Deduplicate: true})
-		inCluster, err := testutils.InClusterConfig()
-		if err != nil {
-			return nil, err
-		}
-		if inCluster {
-			return h.config, nil
-		}
 	}
 	if err != nil {
-		return h.config, err
+		return nil, err
 	}
 
-	// if not the mocked control plane
+	// Newly started clusters aren't ready until default service account is ready.
+	// We need to wait until one is present. Otherwise, we sometimes hit an error such as:
+	//   error looking up service account <namespace>/default: serviceaccount "default" not found
+	//
+	// We avoid doing this for the mocked control plane case (because in that case the default service
+	// account is not provided anyway?)
+	// We still do this when running inside a cluster, because the cluster kuttl is pointed *at* might
+	// be different from the cluster it is running *in*, and it does not hurt when it is the same cluster.
 	if !h.TestSuite.StartControlPlane {
-		// newly started clusters aren't ready until default service account is ready
-		// fixes: error looking up service account <namespace>/default: serviceaccount "default" not found
-		// we avoid this with "inCluster" as the cluster must be already be up since we're running on it
-		err = testutils.WaitForSA(h.config, "default", "default")
-		if err != nil {
-			// if there is a namespace provided but no "default"/"default" SA found, also check a SA in the provided NS
-			if h.TestSuite.Namespace != "" {
-				tempErr := testutils.WaitForSA(h.config, "default", h.TestSuite.Namespace)
-
-				// if it still does not have a SA then return the first "default"/"default" error
-				if tempErr != nil {
-					return h.config, err
-				}
-			} else {
-				return h.config, err
-			}
+		if err := h.waitForFunctionalCluster(); err != nil {
+			return nil, err
 		}
 		h.T.Logf("Successful connection to cluster at: %s", h.config.Host)
 	}
 
-	// The creation of the "kubeconfig" is necessary for out of cluster execution of kubectl
+	// The creation of the "kubeconfig" is necessary for out of cluster execution of kubectl,
+	// as well as in-cluster when the supplied KUBECONFIG is some *other* cluster.
 	f, err := os.Create("kubeconfig")
 	if err != nil {
-		return h.config, err
+		return nil, err
 	}
 
 	defer f.Close()
 
 	return h.config, testutils.Kubeconfig(h.config, f)
 }
 
+func (h *Harness) waitForFunctionalCluster() error {
+	err := testutils.WaitForSA(h.config, "default", "default")
+	if err == nil {
+		return nil
+	}
+	// if there is a namespace provided but no "default"/"default" SA found, also check a SA in the provided NS
+	if h.TestSuite.Namespace != "" {
+		tempErr := testutils.WaitForSA(h.config, "default", h.TestSuite.Namespace)
+		if tempErr == nil {
+			return nil
+		}
+	}
+	// either way, return the first "default"/"default" error
+	return err
+}
+
 // Client returns the current Kubernetes client for the test harness.
 func (h *Harness) Client(forceNew bool) (client.Client, error) {
 	h.clientLock.Lock()

pkg/test/utils/kubernetes.go

@@ -1232,15 +1232,3 @@ func Kubeconfig(cfg *rest.Config, w io.Writer) error {
 		},
 	}, w)
 }
-
-// InClusterConfig returns true if in cluster, false if not
-func InClusterConfig() (bool, error) {
-	_, err := rest.InClusterConfig()
-	if err == nil {
-		return true, nil
-	}
-	if errors.Is(err, rest.ErrNotInCluster) {
-		return false, nil
-	}
-	return false, err
-}