Improve test logging to make errors actionable (#172)

* Better log message for app status
* Improve error message when `E2E_KUBECONFIG` points to a non-existing file
* Show finalizers if object still exists
* Do not unnecessarily allocate TTY for running commands, return stderr content even if command fails with error exit code

CHANGELOG.md

@@ -7,6 +7,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Changed
+
+- Better log message for app status
+- Improve error message when `E2E_KUBECONFIG` points to a non-existing file
+- Show finalizers if object still exists
+- Do not unnecessarily allocate TTY for running commands, return stderr content even if command fails with error exit code
+
 ## [0.18.0] - 2024-04-18
 
 ### Added

pkg/client/client.go

@@ -95,7 +95,10 @@ func NewWithContext(kubeconfigPath string, contextName string) (*Client, error)
 		return nil, fmt.Errorf("a kubeconfig file must be provided")
 	}
 
-	data, _ := os.ReadFile(kubeconfigPath)
+	data, err := os.ReadFile(kubeconfigPath)
+	if err != nil {
+		return nil, fmt.Errorf("failed to create context from kubeconfig file %q - %v", kubeconfigPath, err)
+	}
 	clusterName, err := getClusterNameFromKubeConfig(data, contextName)
 	if err != nil {
 		return nil, fmt.Errorf("failed to get cluster name - %v", err)

pkg/client/exec.go

@@ -16,7 +16,7 @@ import (
 func (c *Client) ExecInPod(ctx context.Context, podName, namespace, containerName string, command []string) (string, string, error) {
 	logger.Log("Running %v in container %q in pod %q", command, containerName, podName)
 
-	tty := true
+	tty := false
 
 	coreClient, err := kubernetes.NewForConfig(c.config)
 	if err != nil {
@@ -51,7 +51,7 @@ func (c *Client) ExecInPod(ctx context.Context, podName, namespace, containerNam
 		Tty:    tty,
 	})
 	if err != nil {
-		return "", "", fmt.Errorf("failed to exec command in pod - %v", err)
+		return stdout.String(), stderr.String(), fmt.Errorf("failed to exec command in pod - %v", err)
 	}
 
 	return stdout.String(), stderr.String(), err

pkg/wait/conditions.go

@@ -2,6 +2,7 @@ package wait
 
 import (
 	"context"
+	"fmt"
 	"reflect"
 	"strings"
 	"time"
@@ -94,11 +95,12 @@ func IsResourceDeleted(ctx context.Context, kubeClient *client.Client, resource
 		logger.Log("Checking if %s '%s' still exists", getResourceKind(kubeClient, resource), resource.GetName())
 		err := kubeClient.Client.Get(ctx, cr.ObjectKeyFromObject(resource), resource, &cr.GetOptions{})
 		if cr.IgnoreNotFound(err) != nil {
-			return false, err
+			return false, fmt.Errorf("failed to check if %s '%s' still exists: %v", getResourceKind(kubeClient, resource), resource.GetName(), err)
 		} else if apierrors.IsNotFound(err) {
 			return true, nil
 		}
 
+		logger.Log("Still exists: %s '%s' (finalizers: %s)", getResourceKind(kubeClient, resource), resource.GetName(), strings.Join(resource.GetFinalizers(), ", "))
 		return false, nil
 	}
 }
@@ -153,8 +155,13 @@ func IsAppStatus(ctx context.Context, kubeClient *client.Client, appName string,
 		}
 
 		actualStatus := app.Status.Release.Status
-		logger.Log("Checking if App status for %s is equal to '%s': %s", appName, expectedStatus, actualStatus)
-		return expectedStatus == actualStatus, nil
+		if expectedStatus == actualStatus {
+			logger.Log("App status for %s is as expected: %s", appName, actualStatus)
+			return true, nil
+		} else {
+			logger.Log("App status for %s is not yet as expected: expectedStatus=%q actualStatus=%q (reason: %q)", appName, expectedStatus, actualStatus, app.Status.Release.Reason)
+			return false, nil
+		}
 	}
 }
 
@@ -178,8 +185,10 @@ func IsAllAppStatus(ctx context.Context, kubeClient *client.Client, appNamespace
 			}
 
 			actualStatus := app.Status.Release.Status
-			logger.Log("Checking if App status for %s is equal to '%s': %s", namespacedName.Name, expectedStatus, actualStatus)
-			if expectedStatus != actualStatus {
+			if expectedStatus == actualStatus {
+				logger.Log("App status for %s is as expected: %s", namespacedName.Name, actualStatus)
+			} else {
+				logger.Log("App status for %s is not yet as expected: expectedStatus=%q actualStatus=%q (reason: %q)", namespacedName.Name, expectedStatus, actualStatus, app.Status.Release.Reason)
 				isSuccess = false
 			}
 		}