Merge pull request #1417 from Nordix/lentzi90/e2e-log-fix

🌱 e2e: Change logging to avoid use of By

Author: k8s-ci-robot

test/e2e/shared/cluster.go

@@ -59,7 +59,7 @@ func createClusterctlLocalRepository(config *clusterctl.E2EConfig, repositoryFol
 
 // setupBootstrapCluster installs Cluster API components via clusterctl.
 func setupBootstrapCluster(config *clusterctl.E2EConfig, scheme *runtime.Scheme, useExistingCluster bool) (bootstrap.ClusterProvider, framework.ClusterProxy) {
-	Byf("Running setupBootstrapCluster (useExistingCluster: %t)", useExistingCluster)
+	Logf("Running setupBootstrapCluster (useExistingCluster: %t)", useExistingCluster)
 
 	// We only want to set clusterProvider if we create a new bootstrap cluster in this test.
 	// If we re-use an existing one, we don't want to delete it afterwards, so we don't set it.
@@ -78,7 +78,7 @@ func setupBootstrapCluster(config *clusterctl.E2EConfig, scheme *runtime.Scheme,
 			// Only use the kubeconfigPath if the current context is the configured kubeContext
 			// Otherwise we might deploy to the wrong cluster.
 			// TODO(sbuerin): this logic could be a lot nicer if we could hand over a kubeContext to NewClusterProxy
-			Byf("Found currentContext %q in %q (configured kubeContext is %q)", kubecfg.CurrentContext, testKubeconfigPath, kubeContext)
+			Logf("Found currentContext %q in %q (configured kubeContext is %q)", kubecfg.CurrentContext, testKubeconfigPath, kubeContext)
 			if kubecfg.CurrentContext == kubeContext {
 				kubeconfigPath = testKubeconfigPath
 			}

test/e2e/shared/common.go

@@ -40,7 +40,7 @@ import (
 )
 
 func SetupSpecNamespace(ctx context.Context, specName string, e2eCtx *E2EContext) *corev1.Namespace {
-	Byf("Creating a namespace for hosting the %q test spec", specName)
+	Logf("Creating a namespace for hosting the %q test spec", specName)
 	namespace, cancelWatches := framework.CreateNamespaceAndWatchEvents(ctx, framework.CreateNamespaceAndWatchEventsInput{
 		Creator:   e2eCtx.Environment.BootstrapClusterProxy.GetClient(),
 		ClientSet: e2eCtx.Environment.BootstrapClusterProxy.GetClientSet(),
@@ -54,14 +54,14 @@ func SetupSpecNamespace(ctx context.Context, specName string, e2eCtx *E2EContext
 }
 
 func DumpSpecResourcesAndCleanup(ctx context.Context, specName string, namespace *corev1.Namespace, e2eCtx *E2EContext) {
-	Byf("Running DumpSpecResourcesAndCleanup for namespace %q", namespace.Name)
+	Logf("Running DumpSpecResourcesAndCleanup for namespace %q", namespace.Name)
 	// Dump all Cluster API related resources to artifacts before deleting them.
 	cancelWatches := e2eCtx.Environment.Namespaces[namespace]
 	dumpSpecResources(ctx, e2eCtx, namespace)
 
 	dumpOpenStack(ctx, e2eCtx, e2eCtx.Environment.BootstrapClusterProxy.GetName())
 
-	Byf("Dumping all OpenStack server instances in the %q namespace", namespace.Name)
+	Logf("Dumping all OpenStack server instances in the %q namespace", namespace.Name)
 	dumpMachines(ctx, e2eCtx, namespace)
 
 	if !e2eCtx.Settings.SkipCleanup {
@@ -70,7 +70,7 @@ func DumpSpecResourcesAndCleanup(ctx context.Context, specName string, namespace
 			Namespace: namespace.Name,
 		}, e2eCtx.E2EConfig.GetIntervals(specName, "wait-delete-cluster")...)
 
-		Byf("Deleting namespace used for hosting the %q test spec", specName)
+		Logf("Deleting namespace used for hosting the %q test spec", specName)
 		framework.DeleteNamespace(ctx, framework.DeleteNamespaceInput{
 			Deleter: e2eCtx.Environment.BootstrapClusterProxy.GetClient(),
 			Name:    namespace.Name,
@@ -207,13 +207,13 @@ func dumpSpecResources(ctx context.Context, e2eCtx *E2EContext, namespace *corev
 	})
 }
 
-func Byf(format string, a ...interface{}) {
-	By("[" + time.Now().Format(time.RFC3339) + "] " + fmt.Sprintf(format, a...))
+func Logf(format string, a ...interface{}) {
+	fmt.Fprintf(GinkgoWriter, "["+time.Now().Format(time.RFC3339)+"] "+format+"\n", a...)
 }
 
 func Debugf(debug bool, format string, a ...interface{}) {
 	if debug {
-		By("[DEBUG] [" + time.Now().Format(time.RFC3339) + "] " + fmt.Sprintf(format, a...))
+		fmt.Fprintf(GinkgoWriter, "[DEBUG] ["+time.Now().Format(time.RFC3339)+"] "+format+"\n", a...)
 	}
 }
 
@@ -232,6 +232,6 @@ func SetEnvVar(key, value string, private bool) {
 		printableValue = value
 	}
 
-	Byf("Setting environment variable: key=%s, value=%s", key, printableValue)
+	Logf("Setting environment variable: key=%s, value=%s", key, printableValue)
 	_ = os.Setenv(key, value)
 }

test/e2e/shared/openstack.go

@@ -63,7 +63,7 @@ type ServerExtWithIP struct {
 
 // ensureSSHKeyPair ensures A SSH key is present under the name.
 func ensureSSHKeyPair(e2eCtx *E2EContext) {
-	Byf("Ensuring presence of SSH key %q in OpenStack", DefaultSSHKeyPairName)
+	Logf("Ensuring presence of SSH key %q in OpenStack", DefaultSSHKeyPairName)
 
 	providerClient, clientOpts, _, err := GetTenantProviderClient(e2eCtx)
 	Expect(err).NotTo(HaveOccurred())
@@ -83,7 +83,7 @@ func ensureSSHKeyPair(e2eCtx *E2EContext) {
 	}
 
 	sshDir := filepath.Join(e2eCtx.Settings.ArtifactFolder, "ssh")
-	Byf("Storing keypair in %q", sshDir)
+	Logf("Storing keypair in %q", sshDir)
 
 	err = os.MkdirAll(sshDir, 0o750)
 	Expect(err).NotTo(HaveOccurred())
@@ -96,7 +96,7 @@ func ensureSSHKeyPair(e2eCtx *E2EContext) {
 }
 
 func dumpOpenStack(_ context.Context, e2eCtx *E2EContext, bootstrapClusterProxyName string) {
-	Byf("Running dumpOpenStack")
+	Logf("Running dumpOpenStack")
 	logPath := filepath.Join(e2eCtx.Settings.ArtifactFolder, "clusters", bootstrapClusterProxyName, "openstack-resources")
 	if err := os.MkdirAll(logPath, os.ModePerm); err != nil {
 		_, _ = fmt.Fprintf(GinkgoWriter, "error creating directory %s: %s\n", logPath, err)

test/e2e/shared/suite.go

@@ -52,17 +52,17 @@ type synchronizedBeforeTestSuiteConfig struct {
 
 // Node1BeforeSuite is the common setup down on the first ginkgo node before the test suite runs.
 func Node1BeforeSuite(e2eCtx *E2EContext) []byte {
-	Byf("Running Node1BeforeSuite")
-	defer Byf("Finished Node1BeforeSuite")
+	Logf("Running Node1BeforeSuite")
+	defer Logf("Finished Node1BeforeSuite")
 
 	flag.Parse()
 	Expect(e2eCtx.Settings.ConfigPath).To(BeAnExistingFile(), "Invalid test suite argument. configPath should be an existing file.")
 	Expect(os.MkdirAll(e2eCtx.Settings.ArtifactFolder, 0o750)).To(Succeed(), "Invalid test suite argument. Can't create artifacts-folder %q", e2eCtx.Settings.ArtifactFolder)
-	Byf("Loading the e2e test configuration from %q", e2eCtx.Settings.ConfigPath)
+	Logf("Loading the e2e test configuration from %q", e2eCtx.Settings.ConfigPath)
 	e2eCtx.E2EConfig = LoadE2EConfig(e2eCtx.Settings.ConfigPath)
 
 	Expect(e2eCtx.E2EConfig.GetVariable(OpenStackCloudYAMLFile)).To(BeAnExistingFile(), "Invalid test suite argument. Value of environment variable OPENSTACK_CLOUD_YAML_FILE should be an existing file: %s", e2eCtx.E2EConfig.GetVariable(OpenStackCloudYAMLFile))
-	Byf("Loading the clouds.yaml from %q", e2eCtx.E2EConfig.GetVariable(OpenStackCloudYAMLFile))
+	Logf("Loading the clouds.yaml from %q", e2eCtx.E2EConfig.GetVariable(OpenStackCloudYAMLFile))
 
 	// TODO(sbuerin): we always need ci artifacts, because we don't have images for every Kubernetes version
 	err := filepath.WalkDir(path.Join(e2eCtx.Settings.DataFolder, "infrastructure-openstack"), func(f string, d fs.DirEntry, _ error) error {
@@ -114,13 +114,13 @@ func Node1BeforeSuite(e2eCtx *E2EContext) []byte {
 
 	ensureSSHKeyPair(e2eCtx)
 
-	Byf("Creating a clusterctl local repository into %q", e2eCtx.Settings.ArtifactFolder)
+	Logf("Creating a clusterctl local repository into %q", e2eCtx.Settings.ArtifactFolder)
 	e2eCtx.Environment.ClusterctlConfigPath = createClusterctlLocalRepository(e2eCtx.E2EConfig, filepath.Join(e2eCtx.Settings.ArtifactFolder, "repository"))
 
-	Byf("Setting up the bootstrap cluster")
+	Logf("Setting up the bootstrap cluster")
 	e2eCtx.Environment.BootstrapClusterProvider, e2eCtx.Environment.BootstrapClusterProxy = setupBootstrapCluster(e2eCtx.E2EConfig, e2eCtx.Environment.Scheme, e2eCtx.Settings.UseExistingCluster)
 
-	Byf("Initializing the bootstrap cluster")
+	Logf("Initializing the bootstrap cluster")
 	initBootstrapCluster(e2eCtx)
 
 	conf := synchronizedBeforeTestSuiteConfig{
@@ -142,8 +142,8 @@ func Node1BeforeSuite(e2eCtx *E2EContext) []byte {
 
 // AllNodesBeforeSuite is the common setup down on each ginkgo parallel node before the test suite runs.
 func AllNodesBeforeSuite(e2eCtx *E2EContext, data []byte) {
-	Byf("Running AllNodesBeforeSuite")
-	defer Byf("Finished AllNodesBeforeSuite")
+	Logf("Running AllNodesBeforeSuite")
+	defer Logf("Finished AllNodesBeforeSuite")
 
 	conf := &synchronizedBeforeTestSuiteConfig{}
 	err := yaml.UnmarshalStrict(data, conf)
@@ -222,20 +222,20 @@ func AllNodesBeforeSuite(e2eCtx *E2EContext, data []byte) {
 
 // AllNodesAfterSuite is cleanup that runs on all ginkgo parallel nodes after the test suite finishes.
 func AllNodesAfterSuite(e2eCtx *E2EContext) {
-	Byf("Running AllNodesAfterSuite")
-	defer Byf("Finished AllNodesAfterSuite")
+	Logf("Running AllNodesAfterSuite")
+	defer Logf("Finished AllNodesAfterSuite")
 
-	Byf("Stopping ResourceTicker")
+	Logf("Stopping ResourceTicker")
 	if e2eCtx.Environment.ResourceTickerDone != nil {
 		e2eCtx.Environment.ResourceTickerDone <- true
 	}
-	Byf("Stopped ResourceTicker")
+	Logf("Stopped ResourceTicker")
 
-	Byf("Stopping MachineTicker")
+	Logf("Stopping MachineTicker")
 	if e2eCtx.Environment.MachineTickerDone != nil {
 		e2eCtx.Environment.MachineTickerDone <- true
 	}
-	Byf("Stopped MachineTicker")
+	Logf("Stopped MachineTicker")
 
 	ctx, cancel := context.WithTimeout(context.TODO(), 5*time.Minute)
 	defer cancel()
@@ -252,10 +252,10 @@ func AllNodesAfterSuite(e2eCtx *E2EContext) {
 
 // Node1AfterSuite is cleanup that runs on the first ginkgo node after the test suite finishes.
 func Node1AfterSuite(e2eCtx *E2EContext) {
-	Byf("Running Node1AfterSuite")
-	defer Byf("Finished Node1AfterSuite")
+	Logf("Running Node1AfterSuite")
+	defer Logf("Finished Node1AfterSuite")
 
-	Byf("Tearing down the management cluster")
+	Logf("Tearing down the management cluster")
 	if !e2eCtx.Settings.SkipCleanup {
 		tearDown(e2eCtx.Environment.BootstrapClusterProvider, e2eCtx.Environment.BootstrapClusterProxy)
 	}

test/e2e/suites/e2e/clusterctl_upgrade_test.go

@@ -21,7 +21,6 @@ package e2e
 
 import (
 	"context"
-	"os"
 
 	. "github.com/onsi/ginkgo/v2"
 	capi_e2e "sigs.k8s.io/cluster-api/test/e2e"
@@ -36,13 +35,8 @@ import (
 // if we want to continue testing v0.6.
 var _ = Describe("When testing clusterctl upgrades (v0.6=>current) [clusterctl-upgrade]", func() {
 	ctx := context.TODO()
-	// Set environment variables for rendering the cluster templates.
-	// We cannot use shared.SetEnvVar here because it tries to log things
-	// using `By`, which cannot be done outside of `It` blocks.
-	// We cannot put this in an `It` block either since `ClusterctlUpgradeSpec`
-	// contains `BeforeEach` which cannot be inside `It`.
-	_ = os.Setenv("USE_CI_ARTIFACTS", "true")
-	_ = os.Setenv("DOWNLOAD_E2E_IMAGE", "true")
+	shared.SetEnvVar("USE_CI_ARTIFACTS", "true", false)
+	shared.SetEnvVar("DOWNLOAD_E2E_IMAGE", "true", false)
 
 	capi_e2e.ClusterctlUpgradeSpec(ctx, func() capi_e2e.ClusterctlUpgradeSpecInput {
 		return capi_e2e.ClusterctlUpgradeSpecInput{