e2e: revise complete report creation

The previous approach was based on the observation that some Prow jobs use the
--report-dir parameter instead of the E2E_REPORT_DIR env variable. Parsing the
command line was necessary to use the --json-report and --junit-report
parameters.

But that is complex and can be avoided by triggering the creation of complete
reports in the E2E test suite. The paths are hard-coded and relative to the
report directory to keep the code simple.

There was a report that k8s-triage started processing more data after
6db4b741dd was merged. It's unclear whether
that was because of the new <report-dir>/ginkgo_report.xml file. To avoid
this potential problem, the reports are now in a "ginkgo" sub-directory.

While at it, error checking gets enhanced:
- Create directories at the start of
  the suite and bail out early if that fails.
- *All* e2e suites using the framework do this, not just test/e2e.
- Added missing error checking of truncated JUnit report writing.

hack/ginkgo-e2e.sh

@@ -162,18 +162,6 @@ if [[ "${GINKGO_NO_COLOR}" == "y" ]]; then
   ginkgo_args+=("--no-color")
 fi
 
-if [[ -n "${E2E_REPORT_DIR:-}" ]]; then
-    report_dir="${E2E_REPORT_DIR}"
-else
-    # Some jobs don't use E2E_REPORT_DIR and instead pass --report-dir=<dir>
-    # as parameter.
-    for arg in "${@}"; do
-        # shellcheck disable=SC2001
-        # (style): See if you can use ${variable//search/replace} instead.
-        case "$arg" in -report-dir=*|--report-dir=*) report_dir="$(echo "$arg" | sed -e 's/^[^=]*=//')";; esac
-    done
-fi
-
 # The --host setting is used only when providing --auth_config
 # If --kubeconfig is used, the host to use is retrieved from the .kubeconfig
 # file and the one provided with --host is ignored.
@@ -194,19 +182,6 @@ case "${E2E_TEST_DEBUG_TOOL:-ginkgo}" in
       program+=("--nodes=25")
     fi
     program+=("${ginkgo_args[@]:+${ginkgo_args[@]}}")
-
-    if [[ -n "${report_dir:-}" ]]; then
-        # The JUnit report written by the E2E suite gets truncated to avoid
-        # overwhelming the tools that need to process it. For manual analysis
-        # it is useful to have the full reports in both formats that Ginkgo
-        # supports:
-        # - JUnit for comparison with the truncated report.
-        # - JSON because it is a faithful representation of
-        #   all available information.
-        #
-        # This has to be passed to the CLI, the suite doesn't support --output-dir.
-        program+=("--output-dir=${report_dir}" "--junit-report=ginkgo_report.xml" "--json-report=ginkgo_report.json")
-    fi
     ;;
   delve) program=("dlv" "exec") ;;
   gdb) program=("gdb") ;;
@@ -222,6 +197,11 @@ if [ "${E2E_TEST_DEBUG_TOOL:-ginkgo}" != "ginkgo" ]; then
   done
 fi
 
+# Generate full dumps of the test result and progress in <report-dir>/ginkgo/,
+# using the Ginkgo-specific JSON format and JUnit XML. Ignored if --report-dir
+# is not used.
+suite_args+=(--report-complete-ginkgo --report-complete-junit)
+
 # The following invocation is fairly complex. Let's dump it to simplify
 # determining what the final options are. Enabled by default in CI
 # environments like Prow.

test/e2e/e2e.go

@@ -113,15 +113,6 @@ func RunE2ETests(t *testing.T) {
 	gomega.RegisterFailHandler(framework.Fail)
 
 	// Run tests through the Ginkgo runner with output to console + JUnit for Jenkins
-	if framework.TestContext.ReportDir != "" {
-		// TODO: we should probably only be trying to create this directory once
-		// rather than once-per-Ginkgo-node.
-		// NOTE: junit report can be simply created by executing your tests with the new --junit-report flags instead.
-		if err := os.MkdirAll(framework.TestContext.ReportDir, 0755); err != nil {
-			klog.Errorf("Failed creating report directory: %v", err)
-		}
-	}
-
 	suiteConfig, reporterConfig := framework.CreateGinkgoConfig()
 	klog.Infof("Starting e2e run %q on Ginkgo node %d", framework.RunID, suiteConfig.ParallelProcess)
 	ginkgo.RunSpecs(t, "Kubernetes e2e suite", suiteConfig, reporterConfig)

test/e2e/framework/internal/junit/junit.go

@@ -26,7 +26,7 @@ import (
 // normally written by `ginkgo --junit-report`. This is needed because the full
 // report can become too large for tools like Spyglass
 // (https://github.com/kubernetes/kubernetes/issues/111510).
-func WriteJUnitReport(report ginkgo.Report, filename string) {
+func WriteJUnitReport(report ginkgo.Report, filename string) error {
 	config := reporters.JunitReportConfig{
 		// Remove details for specs where we don't care.
 		OmitTimelinesForSpecState: types.SpecStatePassed | types.SpecStateSkipped,
@@ -38,5 +38,5 @@ func WriteJUnitReport(report ginkgo.Report, filename string) {
 		OmitFailureMessageAttr: true,
 	}
 
-	reporters.GenerateJUnitReportWithConfig(report, filename, config)
+	return reporters.GenerateJUnitReportWithConfig(report, filename, config)
 }

test/e2e/framework/test_context.go

@@ -31,6 +31,7 @@ import (
 	"time"
 
 	"github.com/onsi/ginkgo/v2"
+	"github.com/onsi/ginkgo/v2/reporters"
 	"github.com/onsi/ginkgo/v2/types"
 	"github.com/onsi/gomega"
 	gomegaformat "github.com/onsi/gomega/format"
@@ -115,6 +116,8 @@ type TestContextType struct {
 	OutputDir                   string
 	ReportDir                   string
 	ReportPrefix                string
+	ReportCompleteGinkgo        bool
+	ReportCompleteJUnit         bool
 	Prefix                      string
 	MinStartupPods              int
 	EtcdUpgradeStorage          string
@@ -335,7 +338,9 @@ func RegisterCommonFlags(flags *flag.FlagSet) {
 
 	flags.StringVar(&TestContext.Host, "host", "", fmt.Sprintf("The host, or apiserver, to connect to. Will default to %s if this argument and --kubeconfig are not set.", defaultHost))
 	flags.StringVar(&TestContext.ReportPrefix, "report-prefix", "", "Optional prefix for JUnit XML reports. Default is empty, which doesn't prepend anything to the default name.")
-	flags.StringVar(&TestContext.ReportDir, "report-dir", "", "Path to the directory where the JUnit XML reports and other tests results should be saved. Default is empty, which doesn't generate these reports. If ginkgo's -junit-report parameter is used, that parameter instead of -report-dir determines the location of a single JUnit report.")
+	flags.StringVar(&TestContext.ReportDir, "report-dir", "", "Path to the directory where the simplified JUnit XML reports and other tests results should be saved. Default is empty, which doesn't generate these reports.  If ginkgo's -junit-report parameter is used, that parameter instead of -report-dir determines the location of a single JUnit report.")
+	flags.BoolVar(&TestContext.ReportCompleteGinkgo, "report-complete-ginkgo", false, "Enables writing a complete test report as Ginkgo JSON to <report dir>/ginkgo/report.json. Ignored if --report-dir is not set.")
+	flags.BoolVar(&TestContext.ReportCompleteJUnit, "report-complete-junit", false, "Enables writing a complete test report as JUnit XML to <report dir>/ginkgo/report.json. Ignored if --report-dir is not set.")
 	flags.StringVar(&TestContext.ContainerRuntimeEndpoint, "container-runtime-endpoint", "unix:///var/run/containerd/containerd.sock", "The container runtime endpoint of cluster VM instances.")
 	flags.StringVar(&TestContext.ContainerRuntimeProcessName, "container-runtime-process-name", "dockerd", "The name of the container runtime process.")
 	flags.StringVar(&TestContext.ContainerRuntimePidFile, "container-runtime-pid-file", "/var/run/docker.pid", "The pid file of the container runtime.")
@@ -545,6 +550,31 @@ func AfterReadingAllFlags(t *TestContextType) {
 	}
 
 	if TestContext.ReportDir != "" {
+		// Create the directory before running the suite. If
+		// --report-dir is not unusable, we should report
+		// that as soon as possible. This will be done by each worker
+		// in parallel, so we will get "exists" error in most of them.
+		if err := os.MkdirAll(TestContext.ReportDir, 0777); err != nil && !os.IsExist(err) {
+			klog.Errorf("Create report dir: %v", err)
+			os.Exit(1)
+		}
+		ginkgoDir := path.Join(TestContext.ReportDir, "ginkgo")
+		if TestContext.ReportCompleteGinkgo || TestContext.ReportCompleteJUnit {
+			if err := os.MkdirAll(ginkgoDir, 0777); err != nil && !os.IsExist(err) {
+				klog.Errorf("Create <report-dir>/ginkgo: %v", err)
+				os.Exit(1)
+			}
+		}
+
+		if TestContext.ReportCompleteGinkgo {
+			ginkgo.ReportAfterSuite("Ginkgo JSON report", func(report ginkgo.Report) {
+				ExpectNoError(reporters.GenerateJSONReport(report, path.Join(ginkgoDir, "report.json")))
+			})
+			ginkgo.ReportAfterSuite("JUnit XML report", func(report ginkgo.Report) {
+				ExpectNoError(reporters.GenerateJUnitReport(report, path.Join(ginkgoDir, "report.xml")))
+			})
+		}
+
 		ginkgo.ReportAfterSuite("Kubernetes e2e JUnit report", func(report ginkgo.Report) {
 			// With Ginkgo v1, we used to write one file per
 			// parallel node. Now Ginkgo v2 automatically merges
@@ -559,9 +589,7 @@ func AfterReadingAllFlags(t *TestContextType) {
 			// needed because the full report can become too large
 			// for tools like Spyglass
 			// (https://github.com/kubernetes/kubernetes/issues/111510).
-			//
+			ExpectNoError(junit.WriteJUnitReport(report, junitReport))
-			// Users who want the full report can use `--junit-report`.
-			junit.WriteJUnitReport(report, junitReport)
 		})
 	}
 }