From 81e640e86594a75cb54709eac484069097191ae5 Mon Sep 17 00:00:00 2001 From: Branden Clark Date: Wed, 9 Oct 2024 09:54:31 -0400 Subject: [PATCH] Move output dir logic to suite (#29580) --- test/new-e2e/pkg/e2e/suite.go | 30 +++++++ test/new-e2e/pkg/e2e/suite_utils.go | 82 +++++++++++++++++- test/new-e2e/pkg/runner/local_profile.go | 25 ------ test/new-e2e/pkg/runner/profile.go | 83 ++++--------------- .../pkg/utils/e2e/client/agent_client.go | 29 +++---- .../tests/installer/windows/base_suite.go | 3 +- .../windows/base_agent_installer_suite.go | 3 +- .../windows/service-test/startstop_test.go | 5 +- 8 files changed, 148 insertions(+), 112 deletions(-) diff --git a/test/new-e2e/pkg/e2e/suite.go b/test/new-e2e/pkg/e2e/suite.go index f5f125343e15b..50e3578e68f8e 100644 --- a/test/new-e2e/pkg/e2e/suite.go +++ b/test/new-e2e/pkg/e2e/suite.go @@ -146,6 +146,7 @@ import ( "errors" "fmt" "reflect" + "sync" "testing" "time" @@ -189,6 +190,9 @@ type BaseSuite[Env any] struct { currentProvisioners ProvisionerMap firstFailTest string + + testSessionOutputDir string + onceTestSessionOutputDir sync.Once } // @@ -529,6 +533,32 @@ func (bs *BaseSuite[Env]) TearDownSuite() { } } +// GetRootOutputDir returns the root output directory for tests to store output files and artifacts. +// The directory is created on the first call to this function and reused in future calls. +// +// See BaseSuite.CreateTestOutputDir() for a function that returns a directory for the current test. +// +// See CreateRootOutputDir() for details on the root directory creation. +func (bs *BaseSuite[Env]) GetRootOutputDir() (string, error) { + var err error + bs.onceTestSessionOutputDir.Do(func() { + // Store the timestamped directory to be used by all tests in the suite + bs.testSessionOutputDir, err = CreateRootOutputDir() + }) + return bs.testSessionOutputDir, err +} + +// CreateTestOutputDir returns an output directory for the current test. +// +// See also CreateTestOutputDir() +func (bs *BaseSuite[Env]) CreateTestOutputDir() (string, error) { + root, err := bs.GetRootOutputDir() + if err != nil { + return "", err + } + return CreateTestOutputDir(root, bs.T()) +} + // Run is a helper function to run a test suite. // Unfortunately, we cannot use `s Suite[Env]` as Go is not able to match it with a struct // However it's able to verify the same constraint on T diff --git a/test/new-e2e/pkg/e2e/suite_utils.go b/test/new-e2e/pkg/e2e/suite_utils.go index 4aa47c6065a95..ad7f1e540845a 100644 --- a/test/new-e2e/pkg/e2e/suite_utils.go +++ b/test/new-e2e/pkg/e2e/suite_utils.go @@ -5,7 +5,17 @@ package e2e -import "testing" +import ( + "fmt" + "os" + "path/filepath" + "strings" + "time" + + "github.com/DataDog/datadog-agent/test/new-e2e/pkg/runner" + + "testing" +) type testLogger struct { t *testing.T @@ -20,3 +30,73 @@ func (tl testLogger) Write(p []byte) (n int, err error) { tl.t.Log(string(p)) return len(p), nil } + +// CreateRootOutputDir creates and returns a directory for tests to store output files and artifacts. +// A timestamp is included in the path to distinguish between multiple runs, and os.MkdirTemp() is +// used to avoid name collisions between parallel runs. +// +// A new directory is created on each call to this function, it is recommended to save this result +// and use it for all tests in a run. For example see BaseSuite.GetRootOutputDir(). +// +// See runner.GetProfile().GetOutputDir() for the root output directory selection logic. +// +// See CreateTestOutputDir and BaseSuite.CreateTestOutputDir for a function that returns a subdirectory for a specific test. +func CreateRootOutputDir() (string, error) { + outputRoot, err := runner.GetProfile().GetOutputDir() + if err != nil { + return "", err + } + // Append timestamp to distinguish between multiple runs + // Format: YYYY-MM-DD_HH-MM-SS + // Use a custom timestamp format because Windows paths can't contain ':' characters + // and we don't need the timezone information. + timePart := time.Now().Format("2006-01-02_15-04-05") + // create root directory + err = os.MkdirAll(outputRoot, 0755) + if err != nil { + return "", err + } + // Create final output directory + // Use MkdirTemp to avoid name collisions between parallel runs + outputRoot, err = os.MkdirTemp(outputRoot, fmt.Sprintf("%s_*", timePart)) + if err != nil { + return "", err + } + if os.Getenv("CI") == "" { + // Create a symlink to the latest run for user convenience + // TODO: Is there a standard "ci" vs "local" check? + // This code used to be in localProfile.GetOutputDir() + latestLink := filepath.Join(filepath.Dir(outputRoot), "latest") + // Remove the symlink if it already exists + if _, err := os.Lstat(latestLink); err == nil { + err = os.Remove(latestLink) + if err != nil { + return "", err + } + } + err = os.Symlink(outputRoot, latestLink) + if err != nil { + return "", err + } + } + return outputRoot, nil +} + +// CreateTestOutputDir creates a directory for a specific test that can be used to store output files and artifacts. +// The test name is used in the directory name, and invalid characters are replaced with underscores. +// +// Example: +// - test name: TestInstallSuite/TestInstall/install_version=7.50.0 +// - output directory: /TestInstallSuite/TestInstall/install_version_7_50_0 +func CreateTestOutputDir(root string, t *testing.T) (string, error) { + // https://en.wikipedia.org/wiki/Filename#Reserved_characters_and_words + invalidPathChars := strings.Join([]string{"?", "%", "*", ":", "|", "\"", "<", ">", ".", ",", ";", "="}, "") + + testPart := strings.ReplaceAll(t.Name(), invalidPathChars, "_") + path := filepath.Join(root, testPart) + err := os.MkdirAll(path, 0755) + if err != nil { + return "", err + } + return path, nil +} diff --git a/test/new-e2e/pkg/runner/local_profile.go b/test/new-e2e/pkg/runner/local_profile.go index 2cba95a568cd2..de08513ae1426 100644 --- a/test/new-e2e/pkg/runner/local_profile.go +++ b/test/new-e2e/pkg/runner/local_profile.go @@ -10,7 +10,6 @@ import ( "os" "os/user" "path" - "path/filepath" "strings" "github.com/DataDog/datadog-agent/test/new-e2e/pkg/runner/parameters" @@ -119,27 +118,3 @@ func (p localProfile) NamePrefix() string { func (p localProfile) AllowDevMode() bool { return true } - -// GetOutputDir extends baseProfile.GetOutputDir to create a symlink to the latest run -func (p localProfile) GetOutputDir() (string, error) { - outDir, err := p.baseProfile.GetOutputDir() - if err != nil { - return "", err - } - - // Create a symlink to the latest run for user convenience - latestLink := filepath.Join(filepath.Dir(outDir), "latest") - // Remove the symlink if it already exists - if _, err := os.Lstat(latestLink); err == nil { - err = os.Remove(latestLink) - if err != nil { - return "", err - } - } - err = os.Symlink(outDir, latestLink) - if err != nil { - return "", err - } - - return outDir, nil -} diff --git a/test/new-e2e/pkg/runner/profile.go b/test/new-e2e/pkg/runner/profile.go index 5f427aa41bd64..a4048093c2d10 100644 --- a/test/new-e2e/pkg/runner/profile.go +++ b/test/new-e2e/pkg/runner/profile.go @@ -16,11 +16,8 @@ import ( "strconv" "strings" "sync" - "time" "github.com/DataDog/datadog-agent/test/new-e2e/pkg/runner/parameters" - - "testing" ) // CloudProvider alias to string @@ -64,9 +61,9 @@ type Profile interface { // AllowDevMode returns if DevMode is allowed AllowDevMode() bool // GetOutputDir returns the root output directory for tests to store output files and artifacts. - // e.g. /tmp/e2e-output/2020-01-01_00-00-00_ + // e.g. /tmp/e2e-output/ or ~/e2e-output/ // - // See GetTestOutputDir for a function that returns a subdirectory for a specific test. + // It is recommended to use GetTestOutputDir to create a subdirectory for a specific test. GetOutputDir() (string, error) } @@ -78,7 +75,6 @@ type baseProfile struct { secretStore parameters.Store workspaceRootFolder string defaultOutputRootFolder string - outputRootFolder string } func newProfile(projectName string, environments []string, store parameters.Store, secretStore *parameters.Store, defaultOutputRoot string) baseProfile { @@ -140,55 +136,30 @@ func (p baseProfile) SecretStore() parameters.Store { return p.secretStore } -// GetOutputDir returns the root output directory for tests to store output files and artifacts. -// The directory is created on the first call to this function, normally this will be when a -// test calls GetTestOutputDir. +// GetOutputDir returns the root output directory to be used to store output files and artifacts. +// A path is returned but the directory is not created. // // The root output directory is chosen in the following order: // - outputDir parameter from the runner configuration, or E2E_OUTPUT_DIR environment variable -// - default provided by a parent profile, /e2e-output, e.g. $CI_PROJECT_DIR/e2e-output +// - default provided by profile, /e2e-output, e.g. $CI_PROJECT_DIR/e2e-output // - os.TempDir()/e2e-output // -// A timestamp is appended to the root output directory to distinguish between multiple runs, -// and os.MkdirTemp() is used to avoid name collisions between parallel runs. -// // See GetTestOutputDir for a function that returns a subdirectory for a specific test. func (p baseProfile) GetOutputDir() (string, error) { - if p.outputRootFolder == "" { - var outputRoot string - configOutputRoot, err := p.store.GetWithDefault(parameters.OutputDir, "") - if err != nil { - return "", err - } - if configOutputRoot != "" { - // If outputRoot is provided in the config file, use it as the root directory - outputRoot = configOutputRoot - } else if p.defaultOutputRootFolder != "" { - // If a default outputRoot was provided, use it as the root directory - outputRoot = filepath.Join(p.defaultOutputRootFolder, "e2e-output") - } else if outputRoot == "" { - // If outputRoot is not provided, use os.TempDir() as the root directory - outputRoot = filepath.Join(os.TempDir(), "e2e-output") - } - // Append timestamp to distinguish between multiple runs - // Format: YYYY-MM-DD_HH-MM-SS - // Use a custom timestamp format because Windows paths can't contain ':' characters - // and we don't need the timezone information. - timePart := time.Now().Format("2006-01-02_15-04-05") - // create root directory - err = os.MkdirAll(outputRoot, 0755) - if err != nil { - return "", err - } - // Create final output directory - // Use MkdirTemp to avoid name collisions between parallel runs - outputRoot, err = os.MkdirTemp(outputRoot, fmt.Sprintf("%s_*", timePart)) - if err != nil { - return "", err - } - p.outputRootFolder = outputRoot + configOutputRoot, err := p.store.GetWithDefault(parameters.OutputDir, "") + if err != nil { + return "", err } - return p.outputRootFolder, nil + if configOutputRoot != "" { + // If outputRoot is provided in the config file, use it as the root directory + return configOutputRoot, nil + } + if p.defaultOutputRootFolder != "" { + // If a default outputRoot was provided, use it as the root directory + return filepath.Join(p.defaultOutputRootFolder, "e2e-output"), nil + } + // as a final fallback, use os.TempDir() as the root directory + return filepath.Join(os.TempDir(), "e2e-output"), nil } // GetWorkspacePath returns the directory for CI Pulumi workspace. @@ -222,21 +193,3 @@ func GetProfile() Profile { return runProfile } - -// GetTestOutputDir returns the output directory for a specific test. -// The test name is sanitized to remove invalid characters, and the output directory is created. -func GetTestOutputDir(p Profile, t *testing.T) (string, error) { - // https://en.wikipedia.org/wiki/Filename#Reserved_characters_and_words - invalidPathChars := strings.Join([]string{"?", "%", "*", ":", "|", "\"", "<", ">", ".", ",", ";", "="}, "") - root, err := p.GetOutputDir() - if err != nil { - return "", err - } - testPart := strings.ReplaceAll(t.Name(), invalidPathChars, "_") - path := filepath.Join(root, testPart) - err = os.MkdirAll(path, 0755) - if err != nil { - return "", err - } - return path, nil -} diff --git a/test/new-e2e/pkg/utils/e2e/client/agent_client.go b/test/new-e2e/pkg/utils/e2e/client/agent_client.go index 03281542a671d..1a362ac6e4f32 100644 --- a/test/new-e2e/pkg/utils/e2e/client/agent_client.go +++ b/test/new-e2e/pkg/utils/e2e/client/agent_client.go @@ -20,7 +20,6 @@ import ( "github.com/stretchr/testify/require" "github.com/DataDog/datadog-agent/test/new-e2e/pkg/e2e" - "github.com/DataDog/datadog-agent/test/new-e2e/pkg/runner" "github.com/DataDog/datadog-agent/test/new-e2e/pkg/utils/e2e/client/agentclient" "github.com/DataDog/datadog-agent/test/new-e2e/pkg/utils/e2e/client/agentclientparams" ) @@ -197,13 +196,15 @@ func waitForReadyTimeout(t *testing.T, host *Host, commandRunner *agentCommandRu } func generateAndDownloadFlare(t *testing.T, commandRunner *agentCommandRunner, host *Host) error { - profile := runner.GetProfile() - outputDir, err := profile.GetOutputDir() - flareFound := false - + root, err := e2e.CreateRootOutputDir() if err != nil { - return fmt.Errorf("could not get output directory: %v", err) + return fmt.Errorf("could not get root output directory: %w", err) } + outputDir, err := e2e.CreateTestOutputDir(root, t) + if err != nil { + return fmt.Errorf("could not get output directory: %w", err) + } + flareFound := false _, err = commandRunner.FlareWithError(agentclient.WithArgs([]string{"--email", "e2e@test.com", "--send", "--local"})) if err != nil { @@ -213,17 +214,17 @@ func generateAndDownloadFlare(t *testing.T, commandRunner *agentCommandRunner, h flareRegex, err := regexp.Compile(`datadog-agent-.*\.zip`) if err != nil { - return fmt.Errorf("could not compile regex: %v", err) + return fmt.Errorf("could not compile regex: %w", err) } tmpFolder, err := host.GetTmpFolder() if err != nil { - return fmt.Errorf("could not get tmp folder: %v", err) + return fmt.Errorf("could not get tmp folder: %w", err) } entries, err := host.ReadDir(tmpFolder) if err != nil { - return fmt.Errorf("could not read directory: %v", err) + return fmt.Errorf("could not read directory: %w", err) } for _, entry := range entries { @@ -233,7 +234,7 @@ func generateAndDownloadFlare(t *testing.T, commandRunner *agentCommandRunner, h if host.osFamily != osComp.WindowsFamily { _, err = host.Execute(fmt.Sprintf("sudo chmod 744 %s/%s", tmpFolder, entry.Name())) if err != nil { - return fmt.Errorf("could not update permission of flare file %s/%s : %v", tmpFolder, entry.Name(), err) + return fmt.Errorf("could not update permission of flare file %s/%s : %w", tmpFolder, entry.Name(), err) } } @@ -241,7 +242,7 @@ func generateAndDownloadFlare(t *testing.T, commandRunner *agentCommandRunner, h err = host.GetFile(fmt.Sprintf("%s/%s", tmpFolder, entry.Name()), fmt.Sprintf("%s/%s", outputDir, entry.Name())) if err != nil { - return fmt.Errorf("could not download flare file from %s/%s : %v", tmpFolder, entry.Name(), err) + return fmt.Errorf("could not download flare file from %s/%s : %w", tmpFolder, entry.Name(), err) } flareFound = true @@ -253,13 +254,13 @@ func generateAndDownloadFlare(t *testing.T, commandRunner *agentCommandRunner, h logsFolder, err := host.GetLogsFolder() if err != nil { - return fmt.Errorf("could not get logs folder: %v", err) + return fmt.Errorf("could not get logs folder: %w", err) } entries, err = host.ReadDir(logsFolder) if err != nil { - return fmt.Errorf("could not read directory: %v", err) + return fmt.Errorf("could not read directory: %w", err) } for _, entry := range entries { @@ -267,7 +268,7 @@ func generateAndDownloadFlare(t *testing.T, commandRunner *agentCommandRunner, h err = host.GetFile(fmt.Sprintf("%s/%s", logsFolder, entry.Name()), fmt.Sprintf("%s/%s", outputDir, entry.Name())) if err != nil { - return fmt.Errorf("could not download log file from %s/%s : %v", logsFolder, entry.Name(), err) + return fmt.Errorf("could not download log file from %s/%s : %w", logsFolder, entry.Name(), err) } } } diff --git a/test/new-e2e/tests/installer/windows/base_suite.go b/test/new-e2e/tests/installer/windows/base_suite.go index e95fbe8d3adc7..42cf1966c813e 100644 --- a/test/new-e2e/tests/installer/windows/base_suite.go +++ b/test/new-e2e/tests/installer/windows/base_suite.go @@ -12,7 +12,6 @@ import ( agentVersion "github.com/DataDog/datadog-agent/pkg/version" "github.com/DataDog/datadog-agent/test/new-e2e/pkg/e2e" "github.com/DataDog/datadog-agent/test/new-e2e/pkg/environments" - "github.com/DataDog/datadog-agent/test/new-e2e/pkg/runner" suiteasserts "github.com/DataDog/datadog-agent/test/new-e2e/tests/installer/windows/suite-assertions" ) @@ -112,7 +111,7 @@ func (s *BaseInstallerSuite) BeforeTest(suiteName, testName string) { s.BaseSuite.BeforeTest(suiteName, testName) var err error - s.outputDir, err = runner.GetTestOutputDir(runner.GetProfile(), s.T()) + s.outputDir, err = s.CreateTestOutputDir() s.Require().NoError(err, "should get output dir") s.T().Logf("Output dir: %s", s.outputDir) s.installer = NewDatadogInstaller(s.Env(), s.outputDir) diff --git a/test/new-e2e/tests/windows/base_agent_installer_suite.go b/test/new-e2e/tests/windows/base_agent_installer_suite.go index 767ed5aef5028..5f2f6809d33b0 100644 --- a/test/new-e2e/tests/windows/base_agent_installer_suite.go +++ b/test/new-e2e/tests/windows/base_agent_installer_suite.go @@ -11,7 +11,6 @@ import ( "github.com/DataDog/datadog-agent/test/new-e2e/pkg/components" "github.com/DataDog/datadog-agent/test/new-e2e/pkg/e2e" - "github.com/DataDog/datadog-agent/test/new-e2e/pkg/runner" platformCommon "github.com/DataDog/datadog-agent/test/new-e2e/tests/agent-platform/common" windowsAgent "github.com/DataDog/datadog-agent/test/new-e2e/tests/windows/common/agent" ) @@ -45,7 +44,7 @@ func (b *BaseAgentInstallerSuite[Env]) BeforeTest(suiteName, testName string) { b.BaseSuite.BeforeTest(suiteName, testName) var err error - b.OutputDir, err = runner.GetTestOutputDir(runner.GetProfile(), b.T()) + b.OutputDir, err = b.CreateTestOutputDir() if err != nil { b.T().Fatalf("should get output dir") } diff --git a/test/new-e2e/tests/windows/service-test/startstop_test.go b/test/new-e2e/tests/windows/service-test/startstop_test.go index 8d7a8f6dacb87..1f951e713d2ba 100644 --- a/test/new-e2e/tests/windows/service-test/startstop_test.go +++ b/test/new-e2e/tests/windows/service-test/startstop_test.go @@ -19,7 +19,6 @@ import ( "github.com/DataDog/datadog-agent/test/new-e2e/pkg/e2e" "github.com/DataDog/datadog-agent/test/new-e2e/pkg/environments" awsHostWindows "github.com/DataDog/datadog-agent/test/new-e2e/pkg/environments/aws/host/windows" - "github.com/DataDog/datadog-agent/test/new-e2e/pkg/runner" "github.com/DataDog/datadog-agent/test/new-e2e/pkg/utils/e2e/client/agentclientparams" windowsCommon "github.com/DataDog/datadog-agent/test/new-e2e/tests/windows/common" windowsAgent "github.com/DataDog/datadog-agent/test/new-e2e/tests/windows/common/agent" @@ -360,7 +359,7 @@ func (s *baseStartStopSuite) BeforeTest(suiteName, testName string) { func (s *baseStartStopSuite) AfterTest(suiteName, testName string) { s.BaseSuite.AfterTest(suiteName, testName) - outputDir, err := runner.GetTestOutputDir(runner.GetProfile(), s.T()) + outputDir, err := s.CreateTestOutputDir() if err != nil { s.T().Fatalf("should get output dir") } @@ -398,7 +397,7 @@ func (s *baseStartStopSuite) AfterTest(suiteName, testName string) { func (s *baseStartStopSuite) collectAgentLogs() { host := s.Env().RemoteHost - outputDir, err := runner.GetTestOutputDir(runner.GetProfile(), s.T()) + outputDir, err := s.CreateTestOutputDir() if err != nil { s.T().Fatalf("should get output dir") }