From ecfe60c875cbccc74de2cca3d69698cae0f83e8b Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Wed, 12 Jun 2024 14:28:17 +0200 Subject: [PATCH 01/44] Prepare RunSuite to trigger tests in parallel --- internal/testrunner/globaltestconfig.go | 47 +++++++ internal/testrunner/runners/asset/tester.go | 5 + .../testrunner/runners/pipeline/tester.go | 5 + internal/testrunner/runners/policy/tester.go | 5 + internal/testrunner/runners/static/tester.go | 5 + internal/testrunner/runners/system/tester.go | 23 +++- internal/testrunner/testrunner.go | 120 +++++++++++++++++- 7 files changed, 203 insertions(+), 7 deletions(-) create mode 100644 internal/testrunner/globaltestconfig.go diff --git a/internal/testrunner/globaltestconfig.go b/internal/testrunner/globaltestconfig.go new file mode 100644 index 000000000..75c5fc7eb --- /dev/null +++ b/internal/testrunner/globaltestconfig.go @@ -0,0 +1,47 @@ +package testrunner + +import ( + "errors" + "fmt" + "os" + "path/filepath" + + "github.com/elastic/go-ucfg" + "github.com/elastic/go-ucfg/yaml" +) + +type GlobalTestConfig struct { + Asset GlobalRunnerTestConfig `config:"asset"` + Pipeline GlobalRunnerTestConfig `config:"pipeline"` + Policy GlobalRunnerTestConfig `config:"policy"` + Static GlobalRunnerTestConfig `config:"static"` + System GlobalRunnerTestConfig `config:"system"` +} + +type GlobalRunnerTestConfig struct { + Sequential bool `config:"sequential"` + SkippableConfig `config:",inline"` +} + +func AGlobalTestConfig(packageRootPath string) (*GlobalTestConfig, error) { + configFilePath := filepath.Join(packageRootPath, "_dev", "test", "config.yml") + + data, err := os.ReadFile(configFilePath) + if errors.Is(err, os.ErrNotExist) { + return &GlobalTestConfig{}, nil + } + if err != nil { + return nil, fmt.Errorf("failed to read %s: %w", configFilePath, err) + } + + var c GlobalTestConfig + cfg, err := yaml.NewConfig(data, ucfg.PathSep(".")) + if err != nil { + return nil, fmt.Errorf("unable to load global test configuration file: %s: %w", configFilePath, err) + } + if err := cfg.Unpack(&c); err != nil { + return nil, fmt.Errorf("unable to unpack global test configuration file: %s: %w", configFilePath, err) + } + + return &c, nil +} diff --git a/internal/testrunner/runners/asset/tester.go b/internal/testrunner/runners/asset/tester.go index e253ab565..7093d17f6 100644 --- a/internal/testrunner/runners/asset/tester.go +++ b/internal/testrunner/runners/asset/tester.go @@ -57,6 +57,11 @@ func (r tester) String() string { return "asset loading" } +// Parallel indicates if this tester can run in parallel or not. +func (r tester) Parallel() bool { + return false +} + // Run runs the asset loading tests func (r *tester) Run(ctx context.Context) ([]testrunner.TestResult, error) { return r.run(ctx) diff --git a/internal/testrunner/runners/pipeline/tester.go b/internal/testrunner/runners/pipeline/tester.go index caefcb4a8..a6b1237d6 100644 --- a/internal/testrunner/runners/pipeline/tester.go +++ b/internal/testrunner/runners/pipeline/tester.go @@ -127,6 +127,11 @@ func (r *tester) String() string { return "pipeline" } +// Parallel indicates if this tester can run in parallel or not. +func (r tester) Parallel() bool { + return false +} + // Run runs the pipeline tests defined under the given folder func (r *tester) Run(ctx context.Context) ([]testrunner.TestResult, error) { return r.run(ctx) diff --git a/internal/testrunner/runners/policy/tester.go b/internal/testrunner/runners/policy/tester.go index 670e994fa..2a6b8afb1 100644 --- a/internal/testrunner/runners/policy/tester.go +++ b/internal/testrunner/runners/policy/tester.go @@ -59,6 +59,11 @@ func (r *tester) String() string { return string(TestType) } +// Parallel indicates if this tester can run in parallel or not. +func (r tester) Parallel() bool { + return false +} + func (r *tester) Run(ctx context.Context) ([]testrunner.TestResult, error) { var results []testrunner.TestResult diff --git a/internal/testrunner/runners/static/tester.go b/internal/testrunner/runners/static/tester.go index 3b1222e38..ce422ec03 100644 --- a/internal/testrunner/runners/static/tester.go +++ b/internal/testrunner/runners/static/tester.go @@ -47,6 +47,11 @@ func (r tester) String() string { return "static files" } +// Parallel indicates if this tester can run in parallel or not. +func (r tester) Parallel() bool { + return false +} + func (r tester) Run(ctx context.Context) ([]testrunner.TestResult, error) { return r.run(ctx) } diff --git a/internal/testrunner/runners/system/tester.go b/internal/testrunner/runners/system/tester.go index 4024d406b..caac5bd95 100644 --- a/internal/testrunner/runners/system/tester.go +++ b/internal/testrunner/runners/system/tester.go @@ -152,6 +152,8 @@ type tester struct { serviceStateFilePath string + globalConfig testrunner.GlobalRunnerTestConfig + // Execution order of following handlers is defined in runner.TearDown() method. removeAgentHandler func(context.Context) error deleteTestPolicyHandler func(context.Context) error @@ -231,6 +233,13 @@ func NewSystemTester(options SystemTesterOptions) (*tester, error) { if err != nil { return nil, fmt.Errorf("cannot request Kibana version: %w", err) } + + globalTestConfig, err := testrunner.AGlobalTestConfig(r.packageRootPath) + if err != nil { + return nil, fmt.Errorf("failed to read global config: %w", err) + } + r.globalConfig = globalTestConfig.System + logger.Debugf("Read config file for system tests:\n%v+", r.globalConfig) return &r, nil } @@ -247,6 +256,11 @@ func (r *tester) String() string { return "system" } +// Parallel indicates if this tester can run in parallel or not. +func (r tester) Parallel() bool { + return !r.globalConfig.Sequential +} + // Run runs the system tests defined under the given folder func (r *tester) Run(ctx context.Context) ([]testrunner.TestResult, error) { if !r.runSetup && !r.runTearDown && !r.runTestsOnly { @@ -1367,6 +1381,13 @@ func (r *tester) runTest(ctx context.Context, config *testConfig, svcInfo servic return result.WithSkip(config.Skip) } + if r.globalConfig.Skip != nil { + logger.Warnf("skipping %s test for %s/%s: %s (details: %s)", + TestType, r.testFolder.Package, r.testFolder.DataStream, + r.globalConfig.Skip.Reason, r.globalConfig.Skip.Link.String()) + return result.WithSkip(config.Skip) + } + logger.Debugf("running test with configuration '%s'", config.Name()) scenario, err := r.prepareScenario(ctx, config, svcInfo) @@ -1391,7 +1412,7 @@ func checkEnrolledAgents(ctx context.Context, client *kibana.Client, agentInfo a } else { agents = filterAgents(allAgents, svcInfo) } - logger.Debugf("found %d enrolled agent(s)", len(agents)) + logger.Debugf("found %d enrolled agent(s) - policyID %s (%s) - expected agent %s", len(agents), agentInfo.Policy.ID, agentInfo.Policy.Name, agentInfo.Hostname) if len(agents) == 0 { return false, nil // selected agents are unavailable yet } diff --git a/internal/testrunner/testrunner.go b/internal/testrunner/testrunner.go index 3e56e840c..6d998152d 100644 --- a/internal/testrunner/testrunner.go +++ b/internal/testrunner/testrunner.go @@ -11,16 +11,23 @@ import ( "os" "path/filepath" "sort" + "strconv" "strings" + "sync" "time" "github.com/elastic/elastic-package/internal/elasticsearch" + "github.com/elastic/elastic-package/internal/environment" "github.com/elastic/elastic-package/internal/kibana" "github.com/elastic/elastic-package/internal/logger" "github.com/elastic/elastic-package/internal/packages" "github.com/elastic/elastic-package/internal/profile" ) +const DEFAULT_MAXIMUM_ROUTINES = 1 + +var maximumNumberParallelTest = environment.WithElasticPackagePrefix("MAXIMUM_NUMBER_PARALLEL_TESTS") + // TestType represents the various supported test types type TestType string @@ -60,6 +67,9 @@ type Tester interface { // TearDown cleans up any test runner resources. It must be called // after the test runner has finished executing. TearDown(context.Context) error + + // Parallel indicates if this test can be run in parallel or not + Parallel() bool } type TesterFactory func(TestFolder) (Tester, error) @@ -307,28 +317,126 @@ func RunSuite(ctx context.Context, runner TestRunner) ([]TestResult, error) { } return nil, fmt.Errorf("failed to setup %s runner: %w", runner.Type(), err) } - results, err := runWithFactory(ctx, testers) + + var parallelTesters, sequentialTesters []Tester + for _, tester := range testers { + if tester.Parallel() { + logger.Debugf("Found tester parallel") + parallelTesters = append(parallelTesters, tester) + } else { + logger.Debugf("Found tester sequential") + sequentialTesters = append(sequentialTesters, tester) + } + } + + var allResults, results []TestResult + var parallelErr, sequentialErr error + + results, parallelErr = runSuiteParallel(ctx, parallelTesters) + allResults = append(allResults, results...) + + results, sequentialErr = runSuite(ctx, sequentialTesters) + allResults = append(allResults, results...) // Avoid cancellations during cleanup. cleanupCtx := context.WithoutCancel(ctx) tdErr := runner.TearDownRunner(cleanupCtx) if tdErr != nil { - return results, fmt.Errorf("failed to tear down %s runner: %w", runner.Type(), err) + return allResults, fmt.Errorf("failed to tear down %s runner: %w", runner.Type(), tdErr) } - return results, err + if parallelErr != nil { + return allResults, parallelErr + } + if sequentialErr != nil { + return allResults, sequentialErr + } + + return allResults, nil +} + +func maxNumberRoutines() (int, error) { + var err error + maxRoutines := DEFAULT_MAXIMUM_ROUTINES + v, ok := os.LookupEnv(maximumNumberParallelTest) + if ok { + maxRoutines, err = strconv.Atoi(v) + if err != nil { + return 0, fmt.Errorf("failed to read number of maximum routines from environment variable: %w", err) + } + } + return maxRoutines, nil } -// runWithFactory method delegates execution of tests to the runners generated through the factory function. -func runWithFactory(ctx context.Context, testers []Tester) ([]TestResult, error) { +func runSuite(ctx context.Context, testers []Tester) ([]TestResult, error) { + if len(testers) == 0 { + return nil, nil + } var results []TestResult for _, tester := range testers { r, err := run(ctx, tester) if err != nil { - return nil, fmt.Errorf("error running package %s tests: %w", tester.Type(), err) + return results, fmt.Errorf("error running package %s tests: %w", tester.Type(), err) } results = append(results, r...) } + + return results, nil +} + +// runSuiteParallel method delegates execution of tests to the runners generated through the factory function. +func runSuiteParallel(ctx context.Context, testers []Tester) ([]TestResult, error) { + if len(testers) == 0 { + return nil, nil + } + maxRoutines, err := maxNumberRoutines() + if err != nil { + return nil, err + } + + var wg sync.WaitGroup + type routineResult struct { + results []TestResult + err error + } + chResults := make(chan routineResult, len(testers)) + + logger.Debugf(">>> Maximum routines: %d", maxRoutines) + // Use channel as a semaphore to limit the number of test executions in parallel + sem := make(chan int, maxRoutines) + + for _, tester := range testers { + wg.Add(1) + aTester := tester + sem <- 1 + go func() { + defer wg.Done() + r, err := run(ctx, aTester) + chResults <- routineResult{r, err} + <-sem + }() + } + + wg.Wait() + var results []TestResult + var multiErr error + testType := testers[0].Type() + for range testers { + testResults := <-chResults + + if testResults.err != nil { + multiErr = errors.Join(multiErr, testResults.err) + } + + results = append(results, testResults.results...) + } + close(chResults) + close(sem) + + if multiErr != nil { + return results, fmt.Errorf("error running package %s tests: %w", testType, multiErr) + } + return results, nil } From bc822c7e8146a39c57df3ca361cc669a338e5948 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Wed, 12 Jun 2024 15:09:06 +0200 Subject: [PATCH 02/44] Reduce log from container status - temporal --- internal/compose/compose.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/internal/compose/compose.go b/internal/compose/compose.go index 0f9b0c21e..cf8f8a24a 100644 --- a/internal/compose/compose.go +++ b/internal/compose/compose.go @@ -370,29 +370,33 @@ func (p *Project) WaitForHealthy(ctx context.Context, opts CommandOptions) error } for _, containerDescription := range descriptions { - logger.Debugf("Container status: %s", containerDescription.String()) // No healthcheck defined for service if containerDescription.State.Status == "running" && containerDescription.State.Health == nil { + logger.Debugf("Container %s status: %s (no health status)", containerDescription.ID, containerDescription.State.Status) continue } // Service is up and running and it's healthy if containerDescription.State.Status == "running" && containerDescription.State.Health.Status == "healthy" { + logger.Debugf("Container %s status: %s (health: %s)", containerDescription.ID, containerDescription.State.Status, containerDescription.State.Health.Status) continue } // Container started and finished with exit code 0 if containerDescription.State.Status == "exited" && containerDescription.State.ExitCode == 0 { + logger.Debugf("Container %s status: %s (exit code: %d)", containerDescription.ID, containerDescription.State.Status, containerDescription.State.ExitCode) continue } // Container exited with code > 0 if containerDescription.State.Status == "exited" && containerDescription.State.ExitCode > 0 { + logger.Debugf("Container %s status: %s (exit code: %d)", containerDescription.ID, containerDescription.State.Status, containerDescription.State.ExitCode) return fmt.Errorf("container (ID: %s) exited with code %d", containerDescription.ID, containerDescription.State.ExitCode) } // Any different status is considered unhealthy + logger.Debugf("Container %s status: unhealthy", containerDescription.ID) healthy = false } From ef1383c8aaa4ec37e27d245136f281eb93ecea1c Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Thu, 13 Jun 2024 16:56:21 +0200 Subject: [PATCH 03/44] Add debug message --- internal/testrunner/testrunner.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/testrunner/testrunner.go b/internal/testrunner/testrunner.go index 6d998152d..e86c0b2ba 100644 --- a/internal/testrunner/testrunner.go +++ b/internal/testrunner/testrunner.go @@ -424,6 +424,7 @@ func runSuiteParallel(ctx context.Context, testers []Tester) ([]TestResult, erro for range testers { testResults := <-chResults + logger.Debugf("Processing test result %s", testResults.results[0].Name) if testResults.err != nil { multiErr = errors.Join(multiErr, testResults.err) } From 85eb196b99e390cbcc74943781efe44e51982bfc Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Thu, 13 Jun 2024 17:16:17 +0200 Subject: [PATCH 04/44] Rename field - default parallel false --- internal/testrunner/globaltestconfig.go | 2 +- internal/testrunner/runners/system/tester.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/testrunner/globaltestconfig.go b/internal/testrunner/globaltestconfig.go index 75c5fc7eb..d8064e6cf 100644 --- a/internal/testrunner/globaltestconfig.go +++ b/internal/testrunner/globaltestconfig.go @@ -19,7 +19,7 @@ type GlobalTestConfig struct { } type GlobalRunnerTestConfig struct { - Sequential bool `config:"sequential"` + Parallel bool `config:"parallel"` SkippableConfig `config:",inline"` } diff --git a/internal/testrunner/runners/system/tester.go b/internal/testrunner/runners/system/tester.go index caac5bd95..6d08fb820 100644 --- a/internal/testrunner/runners/system/tester.go +++ b/internal/testrunner/runners/system/tester.go @@ -258,7 +258,7 @@ func (r *tester) String() string { // Parallel indicates if this tester can run in parallel or not. func (r tester) Parallel() bool { - return !r.globalConfig.Sequential + return r.globalConfig.Parallel } // Run runs the system tests defined under the given folder From 4e928a0b18418d5cd75f71f995a86c8647793593 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Thu, 13 Jun 2024 17:40:19 +0200 Subject: [PATCH 05/44] Avod checking agent logs if test is skipped --- internal/testrunner/runners/system/tester.go | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/internal/testrunner/runners/system/tester.go b/internal/testrunner/runners/system/tester.go index 6d08fb820..7f82c54b6 100644 --- a/internal/testrunner/runners/system/tester.go +++ b/internal/testrunner/runners/system/tester.go @@ -239,7 +239,6 @@ func NewSystemTester(options SystemTesterOptions) (*tester, error) { return nil, fmt.Errorf("failed to read global config: %w", err) } r.globalConfig = globalTestConfig.System - logger.Debugf("Read config file for system tests:\n%v+", r.globalConfig) return &r, nil } @@ -519,12 +518,17 @@ func (r *tester) run(ctx context.Context) (results []testrunner.TestResult, err startTesting := time.Now() - partial, err := r.runTestPerVariant(ctx, result, r.configFileName, r.serviceVariant) - results = append(results, partial...) + results, err = r.runTestPerVariant(ctx, result, r.configFileName, r.serviceVariant) if err != nil { return results, err } + // Now there is just one test executed + if len(results) > 0 && results[0].Skipped != nil { + logger.Debugf("Test skipped, avoid checking agent logs") + return results, nil + } + tempDir, err := os.MkdirTemp("", "test-system-") if err != nil { return nil, fmt.Errorf("can't create temporal directory: %w", err) @@ -570,7 +574,6 @@ func (r *tester) runTestPerVariant(ctx context.Context, result *testrunner.Resul if err != nil { return nil, fmt.Errorf("unable to load system test case file '%s': %w", configFile, err) } - logger.Debugf("Using config: %q", testConfig.Name()) partial, err := r.runTest(ctx, testConfig, svcInfo) @@ -1385,7 +1388,7 @@ func (r *tester) runTest(ctx context.Context, config *testConfig, svcInfo servic logger.Warnf("skipping %s test for %s/%s: %s (details: %s)", TestType, r.testFolder.Package, r.testFolder.DataStream, r.globalConfig.Skip.Reason, r.globalConfig.Skip.Link.String()) - return result.WithSkip(config.Skip) + return result.WithSkip(r.globalConfig.Skip) } logger.Debugf("running test with configuration '%s'", config.Name()) From 7719ea84033a4511feba1b81c0dc52a0145ef3cf Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Thu, 13 Jun 2024 18:27:33 +0200 Subject: [PATCH 06/44] Show agent data instead of JSON --- internal/kibana/agents.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/kibana/agents.go b/internal/kibana/agents.go index 5014a852f..0b0fb6eee 100644 --- a/internal/kibana/agents.go +++ b/internal/kibana/agents.go @@ -118,7 +118,8 @@ func (c *Client) waitUntilPolicyAssigned(ctx context.Context, a Agent, p Policy) if err != nil { return fmt.Errorf("can't get the agent: %w", err) } - logger.Debugf("Agent data: %s", agent.String()) + logger.Debugf("Agent %s (Host: %s): Policy ID %s LogLevel: %s Status: %s", + agent.ID, agent.LocalMetadata.Host.Name, agent.PolicyID, agent.LocalMetadata.Host.Name, agent.LocalMetadata.Elastic.Agent.LogLevel, agent.Status) if agent.PolicyID == p.ID && agent.PolicyRevision >= p.Revision { logger.Debugf("Policy revision assigned to the agent (ID: %s)...", a.ID) From cfb1905e8864e459c23191b87d2256b5796d4ad7 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Thu, 13 Jun 2024 18:28:35 +0200 Subject: [PATCH 07/44] Move pkgManifest and dataStreamManifest to constructor tester --- internal/testrunner/runners/system/tester.go | 85 ++++++++++---------- 1 file changed, 44 insertions(+), 41 deletions(-) diff --git a/internal/testrunner/runners/system/tester.go b/internal/testrunner/runners/system/tester.go index 7f82c54b6..9a26f07f5 100644 --- a/internal/testrunner/runners/system/tester.go +++ b/internal/testrunner/runners/system/tester.go @@ -145,10 +145,12 @@ type tester struct { pipelines []ingest.Pipeline - dataStreamPath string - stackVersion kibana.VersionInfo - locationManager *locations.LocationManager - resourcesManager *resources.Manager + dataStreamPath string + stackVersion kibana.VersionInfo + locationManager *locations.LocationManager + resourcesManager *resources.Manager + pkgManifest *packages.PackageManifest + dataStreamManifest *packages.DataStreamManifest serviceStateFilePath string @@ -239,6 +241,30 @@ func NewSystemTester(options SystemTesterOptions) (*tester, error) { return nil, fmt.Errorf("failed to read global config: %w", err) } r.globalConfig = globalTestConfig.System + + r.pkgManifest, err = packages.ReadPackageManifestFromPackageRoot(r.packageRootPath) + if err != nil { + return nil, fmt.Errorf("reading package manifest failed: %w", err) + } + + r.dataStreamManifest, err = packages.ReadDataStreamManifest(filepath.Join(r.dataStreamPath, packages.DataStreamManifestFile)) + if err != nil { + return nil, fmt.Errorf("reading data stream manifest failed: %w", err) + } + + // Temporarily until independent Elastic Agents are enabled by default, + // enable independent Elastic Agents if package defines that requires root privileges + if pkg, ds := r.pkgManifest, r.dataStreamManifest; pkg.Agent.Privileges.Root || (ds != nil && ds.Agent.Privileges.Root) { + r.runIndependentElasticAgent = true + } + + // If the environment variable is present, it always has preference over the root + // privileges value (if any) defined in the manifest file + v, ok := os.LookupEnv(enableIndependentAgents) + if ok { + r.runIndependentElasticAgent = strings.ToLower(v) == "true" + } + return &r, nil } @@ -257,7 +283,8 @@ func (r *tester) String() string { // Parallel indicates if this tester can run in parallel or not. func (r tester) Parallel() bool { - return r.globalConfig.Parallel + // it is required independent Elastic Agents to run in parallel system tests + return r.runIndependentElasticAgent && r.globalConfig.Parallel } // Run runs the system tests defined under the given folder @@ -745,8 +772,6 @@ func (r *tester) getDocs(ctx context.Context, dataStream string) (*hits, error) type scenarioTest struct { dataStream string policyTemplateName string - pkgManifest *packages.PackageManifest - dataStreamManifest *packages.DataStreamManifest kibanaDataStream kibana.PackageDataStream syntheticEnabled bool docs []common.MapStr @@ -790,40 +815,18 @@ func (r *tester) prepareScenario(ctx context.Context, config *testConfig, svcInf } } - scenario.pkgManifest, err = packages.ReadPackageManifestFromPackageRoot(r.packageRootPath) - if err != nil { - return nil, fmt.Errorf("reading package manifest failed: %w", err) - } - - scenario.dataStreamManifest, err = packages.ReadDataStreamManifest(filepath.Join(r.dataStreamPath, packages.DataStreamManifestFile)) - if err != nil { - return nil, fmt.Errorf("reading data stream manifest failed: %w", err) - } - - // Temporarily until independent Elastic Agents are enabled by default, - // enable independent Elastic Agents if package defines that requires root privileges - if pkg, ds := scenario.pkgManifest, scenario.dataStreamManifest; pkg.Agent.Privileges.Root || (ds != nil && ds.Agent.Privileges.Root) { - r.runIndependentElasticAgent = true - } - - // If the environment variable is present, it always has preference over the root - // privileges value (if any) defined in the manifest file - v, ok := os.LookupEnv(enableIndependentAgents) - if ok { - r.runIndependentElasticAgent = strings.ToLower(v) == "true" - } serviceOptions.DeployIndependentAgent = r.runIndependentElasticAgent policyTemplateName := config.PolicyTemplate if policyTemplateName == "" { - policyTemplateName, err = findPolicyTemplateForInput(*scenario.pkgManifest, *scenario.dataStreamManifest, config.Input) + policyTemplateName, err = findPolicyTemplateForInput(*r.pkgManifest, *r.dataStreamManifest, config.Input) if err != nil { return nil, fmt.Errorf("failed to determine the associated policy_template: %w", err) } } scenario.policyTemplateName = policyTemplateName - policyTemplate, err := selectPolicyTemplateByName(scenario.pkgManifest.PolicyTemplates, scenario.policyTemplateName) + policyTemplate, err := selectPolicyTemplateByName(r.pkgManifest.PolicyTemplates, scenario.policyTemplateName) if err != nil { return nil, fmt.Errorf("failed to find the selected policy_template: %w", err) } @@ -914,7 +917,7 @@ func (r *tester) prepareScenario(ctx context.Context, config *testConfig, svcInf policy = policyCurrent } - agentDeployed, agentInfo, err := r.setupAgent(ctx, config, serviceStateData, policy, scenario.pkgManifest.Agent) + agentDeployed, agentInfo, err := r.setupAgent(ctx, config, serviceStateData, policy, r.pkgManifest.Agent) if err != nil { return nil, err } @@ -939,7 +942,7 @@ func (r *tester) prepareScenario(ctx context.Context, config *testConfig, svcInf scenario.startTestTime = time.Now() logger.Debug("adding package data stream to test policy...") - ds := createPackageDatastream(*policyToTest, *scenario.pkgManifest, policyTemplate, *scenario.dataStreamManifest, *config, policyToTest.Namespace) + ds := createPackageDatastream(*policyToTest, *r.pkgManifest, policyTemplate, *r.dataStreamManifest, *config, policyToTest.Namespace) if r.runTearDown { logger.Debug("Skip adding data stream config to policy") } else { @@ -954,7 +957,7 @@ func (r *tester) prepareScenario(ctx context.Context, config *testConfig, svcInf // Input packages can set `data_stream.dataset` by convention to customize the dataset. dataStreamDataset := ds.Inputs[0].Streams[0].DataStream.Dataset - if scenario.pkgManifest.Type == "input" { + if r.pkgManifest.Type == "input" { v, _ := config.Vars.GetValue("data_stream.dataset") if dataset, ok := v.(string); ok && dataset != "" { dataStreamDataset = dataset @@ -1301,13 +1304,13 @@ func (r *tester) validateTestScenario(ctx context.Context, result *testrunner.Re if expectedDatasets == nil { var expectedDataset string if ds := r.testFolder.DataStream; ds != "" { - expectedDataset = getDataStreamDataset(*scenario.pkgManifest, *scenario.dataStreamManifest) + expectedDataset = getDataStreamDataset(*r.pkgManifest, *r.dataStreamManifest) } else { - expectedDataset = scenario.pkgManifest.Name + "." + scenario.policyTemplateName + expectedDataset = r.pkgManifest.Name + "." + scenario.policyTemplateName } expectedDatasets = []string{expectedDataset} } - if scenario.pkgManifest.Type == "input" { + if r.pkgManifest.Type == "input" { v, _ := config.Vars.GetValue("data_stream.dataset") if dataset, ok := v.(string); ok && dataset != "" { expectedDatasets = append(expectedDatasets, dataset) @@ -1315,7 +1318,7 @@ func (r *tester) validateTestScenario(ctx context.Context, result *testrunner.Re } fieldsValidator, err := fields.CreateValidatorForDirectory(r.dataStreamPath, - fields.WithSpecVersion(scenario.pkgManifest.SpecVersion), + fields.WithSpecVersion(r.pkgManifest.SpecVersion), fields.WithNumericKeywordFields(config.NumericKeywordFields), fields.WithExpectedDatasets(expectedDatasets), fields.WithEnabledImportAllECSSChema(true), @@ -1341,9 +1344,9 @@ func (r *tester) validateTestScenario(ctx context.Context, result *testrunner.Re } } - specVersion, err := semver.NewVersion(scenario.pkgManifest.SpecVersion) + specVersion, err := semver.NewVersion(r.pkgManifest.SpecVersion) if err != nil { - return result.WithError(fmt.Errorf("failed to parse format version %q: %w", scenario.pkgManifest.SpecVersion, err)) + return result.WithError(fmt.Errorf("failed to parse format version %q: %w", r.pkgManifest.SpecVersion, err)) } // Write sample events file from first doc, if requested @@ -1357,7 +1360,7 @@ func (r *tester) validateTestScenario(ctx context.Context, result *testrunner.Re } // Check transforms if present - if err := r.checkTransforms(ctx, config, scenario.pkgManifest, scenario.kibanaDataStream, scenario.dataStream); err != nil { + if err := r.checkTransforms(ctx, config, r.pkgManifest, scenario.kibanaDataStream, scenario.dataStream); err != nil { return result.WithError(err) } From a68f6fd1ca3f6e7109021f01cf326e1b3042b047 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Thu, 13 Jun 2024 18:30:52 +0200 Subject: [PATCH 08/44] Remove extra parameter in Debugf --- internal/kibana/agents.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/kibana/agents.go b/internal/kibana/agents.go index 0b0fb6eee..041d27e16 100644 --- a/internal/kibana/agents.go +++ b/internal/kibana/agents.go @@ -119,7 +119,7 @@ func (c *Client) waitUntilPolicyAssigned(ctx context.Context, a Agent, p Policy) return fmt.Errorf("can't get the agent: %w", err) } logger.Debugf("Agent %s (Host: %s): Policy ID %s LogLevel: %s Status: %s", - agent.ID, agent.LocalMetadata.Host.Name, agent.PolicyID, agent.LocalMetadata.Host.Name, agent.LocalMetadata.Elastic.Agent.LogLevel, agent.Status) + agent.ID, agent.LocalMetadata.Host.Name, agent.PolicyID, agent.LocalMetadata.Elastic.Agent.LogLevel, agent.Status) if agent.PolicyID == p.ID && agent.PolicyRevision >= p.Revision { logger.Debugf("Policy revision assigned to the agent (ID: %s)...", a.ID) From ac95e8b8ebaa3be0cc63f9ae53fd29fd77c29495 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Thu, 13 Jun 2024 19:00:45 +0200 Subject: [PATCH 09/44] Update package-spec dep --- go.mod | 2 ++ go.sum | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/go.mod b/go.mod index fc7c80322..6e910ce07 100644 --- a/go.mod +++ b/go.mod @@ -184,3 +184,5 @@ require ( sigs.k8s.io/structured-merge-diff/v4 v4.4.1 // indirect sigs.k8s.io/yaml v1.4.0 // indirect ) + +replace github.com/elastic/package-spec/v3 => github.com/mrodm/package-spec/v3 v3.0.0-20240613145150-8d065e83d217 diff --git a/go.sum b/go.sum index ad3f83f25..99a8fd5e7 100644 --- a/go.sum +++ b/go.sum @@ -102,8 +102,6 @@ github.com/elastic/gojsonschema v1.2.1 h1:cUMbgsz0wyEB4x7xf3zUEvUVDl6WCz2RKcQPul github.com/elastic/gojsonschema v1.2.1/go.mod h1:biw5eBS2Z4T02wjATMRSfecfjCmwaDPvuaqf844gLrg= github.com/elastic/kbncontent v0.1.4 h1:GoUkJkqkn2H6iJTnOHcxEqYVVYyjvcebLQVaSR1aSvU= github.com/elastic/kbncontent v0.1.4/go.mod h1:kOPREITK9gSJsiw/WKe7QWSO+PRiZMyEFQCw+CMLAHI= -github.com/elastic/package-spec/v3 v3.1.5 h1:BoUvKZI3WhbxsWAvsOqIasgpCo8fmjmj7lZgO1JyB2s= -github.com/elastic/package-spec/v3 v3.1.5/go.mod h1:kHltfTMhDn9Whp3LZluVwI0WFW5uHwqtlByx6+c8ZHE= github.com/elazarl/goproxy v0.0.0-20230808193330-2592e75ae04a h1:mATvB/9r/3gvcejNsXKSkQ6lcIaNec2nyfOdlTBR2lU= github.com/elazarl/goproxy v0.0.0-20230808193330-2592e75ae04a/go.mod h1:Ro8st/ElPeALwNFlcTpWmkr6IoMFfkjXAvTHpevnDsM= github.com/emicklei/go-restful/v3 v3.11.0 h1:rAQeMHw1c7zTmncogyy8VvRZwtkmkZ4FxERmMY4rD+g= @@ -316,6 +314,8 @@ github.com/modern-go/reflect2 v1.0.2/go.mod h1:yWuevngMOJpCy52FWWMvUC8ws7m/LJsjY github.com/monochromegane/go-gitignore v0.0.0-20200626010858-205db1a8cc00 h1:n6/2gBQ3RWajuToeY6ZtZTIKv2v7ThUy5KKusIT0yc0= github.com/monochromegane/go-gitignore v0.0.0-20200626010858-205db1a8cc00/go.mod h1:Pm3mSP3c5uWn86xMLZ5Sa7JB9GsEZySvHYXCTK4E9q4= github.com/montanaflynn/stats v0.0.0-20171201202039-1bf9dbcd8cbe/go.mod h1:wL8QJuTMNUDYhXwkmfOly8iTdp5TEcJFWZD2D7SIkUc= +github.com/mrodm/package-spec/v3 v3.0.0-20240613145150-8d065e83d217 h1:Ywx/eMe8dX0yx3xf7nWwlHMp6Bp+NyBqXtl95KvNpXU= +github.com/mrodm/package-spec/v3 v3.0.0-20240613145150-8d065e83d217/go.mod h1:Qdq/tqETSbbTpCfUAlVTtIvO2RiAdXp6aXTbqeGdHfE= github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 h1:C3w9PqII01/Oq1c1nUAm88MOHcQC9l5mIlSMApZMrHA= github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822/go.mod h1:+n7T8mK8HuQTcFwEeznm/DIxMOiR9yIdICNftLE1DvQ= github.com/mxk/go-flowrate v0.0.0-20140419014527-cca7078d478f h1:y5//uYreIhSUg3J1GEMiLbxo1LJaP8RfCpH6pymGZus= From fa31ae53fb3128cb12b5b3a9e48facf7c6fbb0ae Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Thu, 13 Jun 2024 19:02:48 +0200 Subject: [PATCH 10/44] Ensure routines finished if context cancelled or signal received --- internal/testrunner/testrunner.go | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/internal/testrunner/testrunner.go b/internal/testrunner/testrunner.go index e86c0b2ba..3c183ace0 100644 --- a/internal/testrunner/testrunner.go +++ b/internal/testrunner/testrunner.go @@ -411,9 +411,17 @@ func runSuiteParallel(ctx context.Context, testers []Tester) ([]TestResult, erro sem <- 1 go func() { defer wg.Done() + defer func() { + <-sem + }() + if err := ctx.Err(); err != nil { + logger.Errorf("context error: %s", context.Cause(ctx)) + chResults <- routineResult{nil, err} + return + } + // TODO: How to check if ctx is Done or Cancelled, to not call to "run" method r, err := run(ctx, aTester) chResults <- routineResult{r, err} - <-sem }() } @@ -424,7 +432,9 @@ func runSuiteParallel(ctx context.Context, testers []Tester) ([]TestResult, erro for range testers { testResults := <-chResults - logger.Debugf("Processing test result %s", testResults.results[0].Name) + if len(testResults.results) > 0 { + logger.Debugf("Processing test result %s", testResults.results[0].Name) + } if testResults.err != nil { multiErr = errors.Join(multiErr, testResults.err) } From 4c926bf38607d209171c387bdc42d04c98a8018c Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Thu, 13 Jun 2024 19:36:42 +0200 Subject: [PATCH 11/44] Reorder where handlers are defined --- internal/testrunner/runners/system/tester.go | 135 ++++++++++--------- 1 file changed, 71 insertions(+), 64 deletions(-) diff --git a/internal/testrunner/runners/system/tester.go b/internal/testrunner/runners/system/tester.go index 9a26f07f5..06f6e444f 100644 --- a/internal/testrunner/runners/system/tester.go +++ b/internal/testrunner/runners/system/tester.go @@ -516,6 +516,7 @@ func (r *tester) tearDownTest(ctx context.Context) error { if r.deleteTestPolicyHandler != nil { if err := r.deleteTestPolicyHandler(cleanupCtx); err != nil { + logger.Debugf("Failed deleting test policy handler...: %v", err) return err } r.deleteTestPolicyHandler = nil @@ -606,7 +607,7 @@ func (r *tester) runTestPerVariant(ctx context.Context, result *testrunner.Resul tdErr := r.tearDownTest(ctx) if err != nil { - return partial, err + return partial, errors.Join(err, fmt.Errorf("failed to tear down runner: %w", tdErr)) } if tdErr != nil { return partial, fmt.Errorf("failed to tear down runner: %w", tdErr) @@ -924,66 +925,6 @@ func (r *tester) prepareScenario(ctx context.Context, config *testConfig, svcInf scenario.agent = agentDeployed - service, svcInfo, err := r.setupService(ctx, config, serviceOptions, svcInfo, agentInfo, agentDeployed, policy, serviceStateData) - if errors.Is(err, os.ErrNotExist) { - logger.Debugf("No service deployer defined for this test") - } else if err != nil { - return nil, err - } - - // Reload test config with ctx variable substitution. - config, err = newConfig(config.Path, svcInfo, serviceOptions.Variant) - if err != nil { - return nil, fmt.Errorf("unable to reload system test case configuration: %w", err) - } - - // store the time just before adding the Test Policy, this time will be used to check - // the agent logs from that time onwards to avoid possible previous errors present in logs - scenario.startTestTime = time.Now() - - logger.Debug("adding package data stream to test policy...") - ds := createPackageDatastream(*policyToTest, *r.pkgManifest, policyTemplate, *r.dataStreamManifest, *config, policyToTest.Namespace) - if r.runTearDown { - logger.Debug("Skip adding data stream config to policy") - } else { - if err := r.kibanaClient.AddPackageDataStreamToPolicy(ctx, ds); err != nil { - return nil, fmt.Errorf("could not add data stream config to policy: %w", err) - } - } - scenario.kibanaDataStream = ds - - // Delete old data - logger.Debug("deleting old data in data stream...") - - // Input packages can set `data_stream.dataset` by convention to customize the dataset. - dataStreamDataset := ds.Inputs[0].Streams[0].DataStream.Dataset - if r.pkgManifest.Type == "input" { - v, _ := config.Vars.GetValue("data_stream.dataset") - if dataset, ok := v.(string); ok && dataset != "" { - dataStreamDataset = dataset - } - } - scenario.dataStream = fmt.Sprintf( - "%s-%s-%s", - ds.Inputs[0].Streams[0].DataStream.Type, - dataStreamDataset, - ds.Namespace, - ) - componentTemplatePackage := fmt.Sprintf( - "%s-%s@package", - ds.Inputs[0].Streams[0].DataStream.Type, - dataStreamDataset, - ) - - r.cleanTestScenarioHandler = func(ctx context.Context) error { - logger.Debugf("Deleting data stream for testing %s", scenario.dataStream) - r.deleteDataStream(ctx, scenario.dataStream) - if err != nil { - return fmt.Errorf("failed to delete data stream %s: %w", scenario.dataStream, err) - } - return nil - } - // FIXME: running per stages does not work when multiple agents are created var origPolicy kibana.Policy agents, err := checkEnrolledAgents(ctx, r.kibanaClient, agentInfo, svcInfo, r.runIndependentElasticAgent) @@ -1058,6 +999,66 @@ func (r *tester) prepareScenario(ctx context.Context, config *testConfig, svcInf return nil } + service, svcInfo, err := r.setupService(ctx, config, serviceOptions, svcInfo, agentInfo, agentDeployed, policy, serviceStateData) + if errors.Is(err, os.ErrNotExist) { + logger.Debugf("No service deployer defined for this test") + } else if err != nil { + return nil, err + } + + // Reload test config with ctx variable substitution. + config, err = newConfig(config.Path, svcInfo, serviceOptions.Variant) + if err != nil { + return nil, fmt.Errorf("unable to reload system test case configuration: %w", err) + } + + // store the time just before adding the Test Policy, this time will be used to check + // the agent logs from that time onwards to avoid possible previous errors present in logs + scenario.startTestTime = time.Now() + + logger.Debug("adding package data stream to test policy...") + ds := createPackageDatastream(*policyToTest, *r.pkgManifest, policyTemplate, *r.dataStreamManifest, *config, policyToTest.Namespace) + if r.runTearDown { + logger.Debug("Skip adding data stream config to policy") + } else { + if err := r.kibanaClient.AddPackageDataStreamToPolicy(ctx, ds); err != nil { + return nil, fmt.Errorf("could not add data stream config to policy: %w", err) + } + } + scenario.kibanaDataStream = ds + + // Delete old data + logger.Debug("deleting old data in data stream...") + + // Input packages can set `data_stream.dataset` by convention to customize the dataset. + dataStreamDataset := ds.Inputs[0].Streams[0].DataStream.Dataset + if r.pkgManifest.Type == "input" { + v, _ := config.Vars.GetValue("data_stream.dataset") + if dataset, ok := v.(string); ok && dataset != "" { + dataStreamDataset = dataset + } + } + scenario.dataStream = fmt.Sprintf( + "%s-%s-%s", + ds.Inputs[0].Streams[0].DataStream.Type, + dataStreamDataset, + ds.Namespace, + ) + componentTemplatePackage := fmt.Sprintf( + "%s-%s@package", + ds.Inputs[0].Streams[0].DataStream.Type, + dataStreamDataset, + ) + + r.cleanTestScenarioHandler = func(ctx context.Context) error { + logger.Debugf("Deleting data stream for testing %s", scenario.dataStream) + r.deleteDataStream(ctx, scenario.dataStream) + if err != nil { + return fmt.Errorf("failed to delete data stream %s: %w", scenario.dataStream, err) + } + return nil + } + if r.runTearDown { logger.Debug("Skip assigning package data stream to agent") } else { @@ -1197,14 +1198,16 @@ func (r *tester) setupService(ctx context.Context, config *testConfig, serviceOp } service, err := serviceDeployer.SetUp(ctx, svcInfo) - if err != nil { - return nil, svcInfo, fmt.Errorf("could not setup service: %w", err) - } + // Set shutdownServiceHandler even if there is any error, to remove service + // this is important mainly for terraform r.shutdownServiceHandler = func(ctx context.Context) error { if r.runTestsOnly { return nil } + if service == nil { + return nil + } logger.Debug("tearing down service...") if err := service.TearDown(ctx); err != nil { return fmt.Errorf("error tearing down service: %w", err) @@ -1212,6 +1215,10 @@ func (r *tester) setupService(ctx context.Context, config *testConfig, serviceOp return nil } + + if err != nil { + return nil, svcInfo, fmt.Errorf("could not setup service: %w", err) + } return service, service.Info(), nil } From ae89c2ea69a4e702bfc1b5360c5da02db4e77152 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Thu, 13 Jun 2024 19:37:04 +0200 Subject: [PATCH 12/44] Return service to be able to set the shutdown service handler --- internal/servicedeployer/compose.go | 4 ++-- internal/servicedeployer/custom_agent.go | 2 +- internal/servicedeployer/terraform.go | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/internal/servicedeployer/compose.go b/internal/servicedeployer/compose.go index 989e15d81..c7eca0106 100644 --- a/internal/servicedeployer/compose.go +++ b/internal/servicedeployer/compose.go @@ -124,7 +124,7 @@ func (d *DockerComposeServiceDeployer) SetUp(ctx context.Context, svcInfo Servic } else { err = p.Up(ctx, opts) if err != nil { - return nil, fmt.Errorf("could not boot up service using Docker Compose: %w", err) + return &service, fmt.Errorf("could not boot up service using Docker Compose: %w", err) } } @@ -133,7 +133,7 @@ func (d *DockerComposeServiceDeployer) SetUp(ctx context.Context, svcInfo Servic processServiceContainerLogs(context.WithoutCancel(ctx), p, compose.CommandOptions{ Env: opts.Env, }, svcInfo.Name) - return nil, fmt.Errorf("service is unhealthy: %w", err) + return &service, fmt.Errorf("service is unhealthy: %w", err) } // Added a specific alias when connecting the service to the network. diff --git a/internal/servicedeployer/custom_agent.go b/internal/servicedeployer/custom_agent.go index 2c8e01ab8..a178def24 100644 --- a/internal/servicedeployer/custom_agent.go +++ b/internal/servicedeployer/custom_agent.go @@ -169,7 +169,7 @@ func (d *CustomAgentDeployer) SetUp(ctx context.Context, svcInfo ServiceInfo) (D processServiceContainerLogs(ctx, p, compose.CommandOptions{ Env: opts.Env, }, svcInfo.Name) - return nil, fmt.Errorf("service is unhealthy: %w", err) + return &service, fmt.Errorf("service is unhealthy: %w", err) } logger.Debugf("adding service container %s internal ports to context", p.ContainerName(serviceName)) diff --git a/internal/servicedeployer/terraform.go b/internal/servicedeployer/terraform.go index 72bd58e2d..8235bf32a 100644 --- a/internal/servicedeployer/terraform.go +++ b/internal/servicedeployer/terraform.go @@ -149,7 +149,7 @@ func (tsd TerraformServiceDeployer) SetUp(ctx context.Context, svcInfo ServiceIn } err = p.Up(ctx, opts) if err != nil { - return nil, fmt.Errorf("could not boot up service using Docker Compose: %w", err) + return &service, fmt.Errorf("could not boot up service using Docker Compose: %w", err) } err = p.WaitForHealthy(ctx, opts) @@ -158,7 +158,7 @@ func (tsd TerraformServiceDeployer) SetUp(ctx context.Context, svcInfo ServiceIn Env: opts.Env, }, svcInfo.Name) //lint:ignore ST1005 error starting with product name can be capitalized - return nil, fmt.Errorf("Terraform deployer is unhealthy: %w", err) + return &service, fmt.Errorf("Terraform deployer is unhealthy: %w", err) } svcInfo.Agent.Host.NamePrefix = "docker-fleet-agent" From c36f4bd9ae6a8563037bafc95d9ba03f339f3033 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Thu, 13 Jun 2024 19:59:57 +0200 Subject: [PATCH 13/44] Add license header --- internal/testrunner/globaltestconfig.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/internal/testrunner/globaltestconfig.go b/internal/testrunner/globaltestconfig.go index d8064e6cf..594a0a8ce 100644 --- a/internal/testrunner/globaltestconfig.go +++ b/internal/testrunner/globaltestconfig.go @@ -1,3 +1,7 @@ +// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one +// or more contributor license agreements. Licensed under the Elastic License; +// you may not use this file except in compliance with the Elastic License. + package testrunner import ( From 32137514b1a7f3c86a2b94a31628c38d1e4edf20 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Thu, 13 Jun 2024 21:52:28 +0200 Subject: [PATCH 14/44] Revert reorder of checkEnrolledAgents and handlers --- internal/testrunner/runners/system/tester.go | 122 ++++++++++--------- 1 file changed, 62 insertions(+), 60 deletions(-) diff --git a/internal/testrunner/runners/system/tester.go b/internal/testrunner/runners/system/tester.go index 06f6e444f..759b58756 100644 --- a/internal/testrunner/runners/system/tester.go +++ b/internal/testrunner/runners/system/tester.go @@ -925,8 +925,70 @@ func (r *tester) prepareScenario(ctx context.Context, config *testConfig, svcInf scenario.agent = agentDeployed + service, svcInfo, err := r.setupService(ctx, config, serviceOptions, svcInfo, agentInfo, agentDeployed, policy, serviceStateData) + if errors.Is(err, os.ErrNotExist) { + logger.Debugf("No service deployer defined for this test") + } else if err != nil { + return nil, err + } + + // Reload test config with ctx variable substitution. + config, err = newConfig(config.Path, svcInfo, serviceOptions.Variant) + if err != nil { + return nil, fmt.Errorf("unable to reload system test case configuration: %w", err) + } + + // store the time just before adding the Test Policy, this time will be used to check + // the agent logs from that time onwards to avoid possible previous errors present in logs + scenario.startTestTime = time.Now() + + logger.Debug("adding package data stream to test policy...") + ds := createPackageDatastream(*policyToTest, *r.pkgManifest, policyTemplate, *r.dataStreamManifest, *config, policyToTest.Namespace) + if r.runTearDown { + logger.Debug("Skip adding data stream config to policy") + } else { + if err := r.kibanaClient.AddPackageDataStreamToPolicy(ctx, ds); err != nil { + return nil, fmt.Errorf("could not add data stream config to policy: %w", err) + } + } + scenario.kibanaDataStream = ds + + // Delete old data + logger.Debug("deleting old data in data stream...") + + // Input packages can set `data_stream.dataset` by convention to customize the dataset. + dataStreamDataset := ds.Inputs[0].Streams[0].DataStream.Dataset + if r.pkgManifest.Type == "input" { + v, _ := config.Vars.GetValue("data_stream.dataset") + if dataset, ok := v.(string); ok && dataset != "" { + dataStreamDataset = dataset + } + } + scenario.dataStream = fmt.Sprintf( + "%s-%s-%s", + ds.Inputs[0].Streams[0].DataStream.Type, + dataStreamDataset, + ds.Namespace, + ) + componentTemplatePackage := fmt.Sprintf( + "%s-%s@package", + ds.Inputs[0].Streams[0].DataStream.Type, + dataStreamDataset, + ) + + r.cleanTestScenarioHandler = func(ctx context.Context) error { + logger.Debugf("Deleting data stream for testing %s", scenario.dataStream) + r.deleteDataStream(ctx, scenario.dataStream) + if err != nil { + return fmt.Errorf("failed to delete data stream %s: %w", scenario.dataStream, err) + } + return nil + } + // FIXME: running per stages does not work when multiple agents are created var origPolicy kibana.Policy + // While there could be created Elastic Agents within `setupService()` (custom agents and k8s agents), + // this "checkEnrolledAgents" call to must be located after creating the service. agents, err := checkEnrolledAgents(ctx, r.kibanaClient, agentInfo, svcInfo, r.runIndependentElasticAgent) if err != nil { return nil, fmt.Errorf("can't check enrolled agents: %w", err) @@ -999,66 +1061,6 @@ func (r *tester) prepareScenario(ctx context.Context, config *testConfig, svcInf return nil } - service, svcInfo, err := r.setupService(ctx, config, serviceOptions, svcInfo, agentInfo, agentDeployed, policy, serviceStateData) - if errors.Is(err, os.ErrNotExist) { - logger.Debugf("No service deployer defined for this test") - } else if err != nil { - return nil, err - } - - // Reload test config with ctx variable substitution. - config, err = newConfig(config.Path, svcInfo, serviceOptions.Variant) - if err != nil { - return nil, fmt.Errorf("unable to reload system test case configuration: %w", err) - } - - // store the time just before adding the Test Policy, this time will be used to check - // the agent logs from that time onwards to avoid possible previous errors present in logs - scenario.startTestTime = time.Now() - - logger.Debug("adding package data stream to test policy...") - ds := createPackageDatastream(*policyToTest, *r.pkgManifest, policyTemplate, *r.dataStreamManifest, *config, policyToTest.Namespace) - if r.runTearDown { - logger.Debug("Skip adding data stream config to policy") - } else { - if err := r.kibanaClient.AddPackageDataStreamToPolicy(ctx, ds); err != nil { - return nil, fmt.Errorf("could not add data stream config to policy: %w", err) - } - } - scenario.kibanaDataStream = ds - - // Delete old data - logger.Debug("deleting old data in data stream...") - - // Input packages can set `data_stream.dataset` by convention to customize the dataset. - dataStreamDataset := ds.Inputs[0].Streams[0].DataStream.Dataset - if r.pkgManifest.Type == "input" { - v, _ := config.Vars.GetValue("data_stream.dataset") - if dataset, ok := v.(string); ok && dataset != "" { - dataStreamDataset = dataset - } - } - scenario.dataStream = fmt.Sprintf( - "%s-%s-%s", - ds.Inputs[0].Streams[0].DataStream.Type, - dataStreamDataset, - ds.Namespace, - ) - componentTemplatePackage := fmt.Sprintf( - "%s-%s@package", - ds.Inputs[0].Streams[0].DataStream.Type, - dataStreamDataset, - ) - - r.cleanTestScenarioHandler = func(ctx context.Context) error { - logger.Debugf("Deleting data stream for testing %s", scenario.dataStream) - r.deleteDataStream(ctx, scenario.dataStream) - if err != nil { - return fmt.Errorf("failed to delete data stream %s: %w", scenario.dataStream, err) - } - return nil - } - if r.runTearDown { logger.Debug("Skip assigning package data stream to agent") } else { From 06d5b77329569edf6ab61da5a3594559ace6fc68 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Thu, 13 Jun 2024 21:54:02 +0200 Subject: [PATCH 15/44] Restore debug message --- internal/testrunner/runners/system/tester.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/testrunner/runners/system/tester.go b/internal/testrunner/runners/system/tester.go index 759b58756..f203c70ab 100644 --- a/internal/testrunner/runners/system/tester.go +++ b/internal/testrunner/runners/system/tester.go @@ -1427,7 +1427,7 @@ func checkEnrolledAgents(ctx context.Context, client *kibana.Client, agentInfo a } else { agents = filterAgents(allAgents, svcInfo) } - logger.Debugf("found %d enrolled agent(s) - policyID %s (%s) - expected agent %s", len(agents), agentInfo.Policy.ID, agentInfo.Policy.Name, agentInfo.Hostname) + logger.Debugf("found %d enrolled agent(s)", len(agents)) if len(agents) == 0 { return false, nil // selected agents are unavailable yet } From 74bdd7284874dc4b8956924c83b26443af4f40a1 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Mon, 17 Jun 2024 10:59:12 +0200 Subject: [PATCH 16/44] Update comments --- internal/testrunner/runners/system/tester.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/internal/testrunner/runners/system/tester.go b/internal/testrunner/runners/system/tester.go index f203c70ab..40f189099 100644 --- a/internal/testrunner/runners/system/tester.go +++ b/internal/testrunner/runners/system/tester.go @@ -516,6 +516,7 @@ func (r *tester) tearDownTest(ctx context.Context) error { if r.deleteTestPolicyHandler != nil { if err := r.deleteTestPolicyHandler(cleanupCtx); err != nil { + // TODO: to be removed logger.Debugf("Failed deleting test policy handler...: %v", err) return err } @@ -551,7 +552,9 @@ func (r *tester) run(ctx context.Context) (results []testrunner.TestResult, err return results, err } - // Now there is just one test executed + // Every tester is in charge of just one test, so if there is no error, + // then there should be just one result for tests. As an exception, there could + // be two results if there is any issue checking Elastic Agent logs. if len(results) > 0 && results[0].Skipped != nil { logger.Debugf("Test skipped, avoid checking agent logs") return results, nil @@ -602,6 +605,7 @@ func (r *tester) runTestPerVariant(ctx context.Context, result *testrunner.Resul if err != nil { return nil, fmt.Errorf("unable to load system test case file '%s': %w", configFile, err) } + logger.Debugf("Using config: %q", testConfig.Name()) partial, err := r.runTest(ctx, testConfig, svcInfo) From b50f5cd7b96bc89ac29f658c2832ec2cd0b53ccd Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Mon, 17 Jun 2024 11:01:08 +0200 Subject: [PATCH 17/44] Rename function and set private struct --- internal/testrunner/globaltestconfig.go | 8 ++++---- internal/testrunner/runners/system/tester.go | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/internal/testrunner/globaltestconfig.go b/internal/testrunner/globaltestconfig.go index 594a0a8ce..d5539e287 100644 --- a/internal/testrunner/globaltestconfig.go +++ b/internal/testrunner/globaltestconfig.go @@ -14,7 +14,7 @@ import ( "github.com/elastic/go-ucfg/yaml" ) -type GlobalTestConfig struct { +type globalTestConfig struct { Asset GlobalRunnerTestConfig `config:"asset"` Pipeline GlobalRunnerTestConfig `config:"pipeline"` Policy GlobalRunnerTestConfig `config:"policy"` @@ -27,18 +27,18 @@ type GlobalRunnerTestConfig struct { SkippableConfig `config:",inline"` } -func AGlobalTestConfig(packageRootPath string) (*GlobalTestConfig, error) { +func ReadGlobalTestConfig(packageRootPath string) (*globalTestConfig, error) { configFilePath := filepath.Join(packageRootPath, "_dev", "test", "config.yml") data, err := os.ReadFile(configFilePath) if errors.Is(err, os.ErrNotExist) { - return &GlobalTestConfig{}, nil + return &globalTestConfig{}, nil } if err != nil { return nil, fmt.Errorf("failed to read %s: %w", configFilePath, err) } - var c GlobalTestConfig + var c globalTestConfig cfg, err := yaml.NewConfig(data, ucfg.PathSep(".")) if err != nil { return nil, fmt.Errorf("unable to load global test configuration file: %s: %w", configFilePath, err) diff --git a/internal/testrunner/runners/system/tester.go b/internal/testrunner/runners/system/tester.go index 40f189099..b6f8ffee8 100644 --- a/internal/testrunner/runners/system/tester.go +++ b/internal/testrunner/runners/system/tester.go @@ -236,7 +236,7 @@ func NewSystemTester(options SystemTesterOptions) (*tester, error) { return nil, fmt.Errorf("cannot request Kibana version: %w", err) } - globalTestConfig, err := testrunner.AGlobalTestConfig(r.packageRootPath) + globalTestConfig, err := testrunner.ReadGlobalTestConfig(r.packageRootPath) if err != nil { return nil, fmt.Errorf("failed to read global config: %w", err) } From 37f8d44642c623f5f683e910e30314cf39c2647d Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Mon, 17 Jun 2024 11:36:48 +0200 Subject: [PATCH 18/44] Enable parallel tests in apache and nginx --- .buildkite/pipeline.yml | 1 + test/packages/parallel/apache/_dev/test/config.yml | 2 ++ test/packages/parallel/nginx/_dev/test/config.yml | 2 ++ 3 files changed, 5 insertions(+) create mode 100644 test/packages/parallel/apache/_dev/test/config.yml create mode 100644 test/packages/parallel/nginx/_dev/test/config.yml diff --git a/.buildkite/pipeline.yml b/.buildkite/pipeline.yml index 72264b8eb..ab09c381f 100644 --- a/.buildkite/pipeline.yml +++ b/.buildkite/pipeline.yml @@ -1,6 +1,7 @@ env: SETUP_GVM_VERSION: 'v0.5.2' # https://github.com/andrewkroh/gvm/issues/44#issuecomment-1013231151 ELASTIC_PACKAGE_COMPOSE_DISABLE_VERBOSE_OUTPUT: "true" + ELASTIC_PACKAGE_MAXIMUM_NUMBER_PARALLEL_TESTS: 3 DOCKER_COMPOSE_VERSION: "v2.24.1" DOCKER_VERSION: "26.1.2" KIND_VERSION: 'v0.20.0' diff --git a/test/packages/parallel/apache/_dev/test/config.yml b/test/packages/parallel/apache/_dev/test/config.yml new file mode 100644 index 000000000..6c4442219 --- /dev/null +++ b/test/packages/parallel/apache/_dev/test/config.yml @@ -0,0 +1,2 @@ +system: + parallel: true diff --git a/test/packages/parallel/nginx/_dev/test/config.yml b/test/packages/parallel/nginx/_dev/test/config.yml new file mode 100644 index 000000000..6c4442219 --- /dev/null +++ b/test/packages/parallel/nginx/_dev/test/config.yml @@ -0,0 +1,2 @@ +system: + parallel: true From f608d06aa37bb3da51d03b3eebc19a975060ea37 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Mon, 17 Jun 2024 11:37:22 +0200 Subject: [PATCH 19/44] Remove duplicated debug log (shown in system runner) --- internal/testrunner/runners/system/tester.go | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/internal/testrunner/runners/system/tester.go b/internal/testrunner/runners/system/tester.go index b6f8ffee8..83ee6c5b4 100644 --- a/internal/testrunner/runners/system/tester.go +++ b/internal/testrunner/runners/system/tester.go @@ -207,22 +207,16 @@ func NewSystemTester(options SystemTesterOptions) (*tester, error) { r.serviceStateFilePath = filepath.Join(stateFolderPath(r.profile.ProfilePath), serviceStateFileName) var err error - var found bool r.locationManager, err = locations.NewLocationManager() if err != nil { return nil, fmt.Errorf("reading service logs directory failed: %w", err) } - r.dataStreamPath, found, err = packages.FindDataStreamRootForPath(r.testFolder.Path) + r.dataStreamPath, _, err = packages.FindDataStreamRootForPath(r.testFolder.Path) if err != nil { return nil, fmt.Errorf("locating data stream root failed: %w", err) } - if found { - logger.Debugf("Running system tests for data stream %q", r.testFolder.DataStream) - } else { - logger.Debug("Running system tests for package") - } if r.esAPI == nil { return nil, errors.New("missing Elasticsearch client") From e6424606147037ec841a408ea98fcdb3e928c393 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Mon, 17 Jun 2024 11:56:46 +0200 Subject: [PATCH 20/44] Fix format global config --- test/packages/parallel/apache/_dev/test/config.yml | 2 +- test/packages/parallel/nginx/_dev/test/config.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/packages/parallel/apache/_dev/test/config.yml b/test/packages/parallel/apache/_dev/test/config.yml index 6c4442219..a57750f85 100644 --- a/test/packages/parallel/apache/_dev/test/config.yml +++ b/test/packages/parallel/apache/_dev/test/config.yml @@ -1,2 +1,2 @@ system: - parallel: true + parallel: true diff --git a/test/packages/parallel/nginx/_dev/test/config.yml b/test/packages/parallel/nginx/_dev/test/config.yml index 6c4442219..a57750f85 100644 --- a/test/packages/parallel/nginx/_dev/test/config.yml +++ b/test/packages/parallel/nginx/_dev/test/config.yml @@ -1,2 +1,2 @@ system: - parallel: true + parallel: true From 6107bcaeea1957303f81feab3471e553359945ff Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Mon, 17 Jun 2024 12:46:09 +0200 Subject: [PATCH 21/44] Test duplicating configuration files in nginx - to be removed --- .../access/_dev/test/system/test-other-config.yml | 5 +++++ .../error/_dev/test/system/test-other-config.yml | 5 +++++ .../stubstatus/_dev/test/system/test-other-config.yml | 6 ++++++ 3 files changed, 16 insertions(+) create mode 100644 test/packages/parallel/nginx/data_stream/access/_dev/test/system/test-other-config.yml create mode 100644 test/packages/parallel/nginx/data_stream/error/_dev/test/system/test-other-config.yml create mode 100644 test/packages/parallel/nginx/data_stream/stubstatus/_dev/test/system/test-other-config.yml diff --git a/test/packages/parallel/nginx/data_stream/access/_dev/test/system/test-other-config.yml b/test/packages/parallel/nginx/data_stream/access/_dev/test/system/test-other-config.yml new file mode 100644 index 000000000..0f73c5dd9 --- /dev/null +++ b/test/packages/parallel/nginx/data_stream/access/_dev/test/system/test-other-config.yml @@ -0,0 +1,5 @@ +vars: ~ +data_stream: + vars: + paths: + - "{{SERVICE_LOGS_DIR}}/access.log*" diff --git a/test/packages/parallel/nginx/data_stream/error/_dev/test/system/test-other-config.yml b/test/packages/parallel/nginx/data_stream/error/_dev/test/system/test-other-config.yml new file mode 100644 index 000000000..ec7356ee9 --- /dev/null +++ b/test/packages/parallel/nginx/data_stream/error/_dev/test/system/test-other-config.yml @@ -0,0 +1,5 @@ +vars: ~ +data_stream: + vars: + paths: + - "{{SERVICE_LOGS_DIR}}/error.log*" diff --git a/test/packages/parallel/nginx/data_stream/stubstatus/_dev/test/system/test-other-config.yml b/test/packages/parallel/nginx/data_stream/stubstatus/_dev/test/system/test-other-config.yml new file mode 100644 index 000000000..0e5231dc8 --- /dev/null +++ b/test/packages/parallel/nginx/data_stream/stubstatus/_dev/test/system/test-other-config.yml @@ -0,0 +1,6 @@ +vars: + hosts: + - http://{{Hostname}}:{{Port}} +data_stream: + vars: + server_status_path: /server-status From d6113ba4f6a907ebe32e522397c0d7456eb49c52 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Mon, 17 Jun 2024 13:56:58 +0200 Subject: [PATCH 22/44] Use channel to loop for results --- internal/testrunner/testrunner.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/testrunner/testrunner.go b/internal/testrunner/testrunner.go index 3c183ace0..28a97d164 100644 --- a/internal/testrunner/testrunner.go +++ b/internal/testrunner/testrunner.go @@ -426,11 +426,13 @@ func runSuiteParallel(ctx context.Context, testers []Tester) ([]TestResult, erro } wg.Wait() + close(chResults) + close(sem) + var results []TestResult var multiErr error testType := testers[0].Type() - for range testers { - testResults := <-chResults + for testResults := range chResults { if len(testResults.results) > 0 { logger.Debugf("Processing test result %s", testResults.results[0].Name) @@ -441,8 +443,6 @@ func runSuiteParallel(ctx context.Context, testers []Tester) ([]TestResult, erro results = append(results, testResults.results...) } - close(chResults) - close(sem) if multiErr != nil { return results, fmt.Errorf("error running package %s tests: %w", testType, multiErr) From 2cdeacf4883c30552cff5291318c7587b080c6a8 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Mon, 17 Jun 2024 14:02:52 +0200 Subject: [PATCH 23/44] Reverted changes in service deployer --- internal/servicedeployer/compose.go | 4 ++-- internal/servicedeployer/custom_agent.go | 2 +- internal/servicedeployer/terraform.go | 4 ++-- internal/testrunner/runners/system/tester.go | 8 +++----- 4 files changed, 8 insertions(+), 10 deletions(-) diff --git a/internal/servicedeployer/compose.go b/internal/servicedeployer/compose.go index c7eca0106..989e15d81 100644 --- a/internal/servicedeployer/compose.go +++ b/internal/servicedeployer/compose.go @@ -124,7 +124,7 @@ func (d *DockerComposeServiceDeployer) SetUp(ctx context.Context, svcInfo Servic } else { err = p.Up(ctx, opts) if err != nil { - return &service, fmt.Errorf("could not boot up service using Docker Compose: %w", err) + return nil, fmt.Errorf("could not boot up service using Docker Compose: %w", err) } } @@ -133,7 +133,7 @@ func (d *DockerComposeServiceDeployer) SetUp(ctx context.Context, svcInfo Servic processServiceContainerLogs(context.WithoutCancel(ctx), p, compose.CommandOptions{ Env: opts.Env, }, svcInfo.Name) - return &service, fmt.Errorf("service is unhealthy: %w", err) + return nil, fmt.Errorf("service is unhealthy: %w", err) } // Added a specific alias when connecting the service to the network. diff --git a/internal/servicedeployer/custom_agent.go b/internal/servicedeployer/custom_agent.go index a178def24..2c8e01ab8 100644 --- a/internal/servicedeployer/custom_agent.go +++ b/internal/servicedeployer/custom_agent.go @@ -169,7 +169,7 @@ func (d *CustomAgentDeployer) SetUp(ctx context.Context, svcInfo ServiceInfo) (D processServiceContainerLogs(ctx, p, compose.CommandOptions{ Env: opts.Env, }, svcInfo.Name) - return &service, fmt.Errorf("service is unhealthy: %w", err) + return nil, fmt.Errorf("service is unhealthy: %w", err) } logger.Debugf("adding service container %s internal ports to context", p.ContainerName(serviceName)) diff --git a/internal/servicedeployer/terraform.go b/internal/servicedeployer/terraform.go index 8235bf32a..72bd58e2d 100644 --- a/internal/servicedeployer/terraform.go +++ b/internal/servicedeployer/terraform.go @@ -149,7 +149,7 @@ func (tsd TerraformServiceDeployer) SetUp(ctx context.Context, svcInfo ServiceIn } err = p.Up(ctx, opts) if err != nil { - return &service, fmt.Errorf("could not boot up service using Docker Compose: %w", err) + return nil, fmt.Errorf("could not boot up service using Docker Compose: %w", err) } err = p.WaitForHealthy(ctx, opts) @@ -158,7 +158,7 @@ func (tsd TerraformServiceDeployer) SetUp(ctx context.Context, svcInfo ServiceIn Env: opts.Env, }, svcInfo.Name) //lint:ignore ST1005 error starting with product name can be capitalized - return &service, fmt.Errorf("Terraform deployer is unhealthy: %w", err) + return nil, fmt.Errorf("Terraform deployer is unhealthy: %w", err) } svcInfo.Agent.Host.NamePrefix = "docker-fleet-agent" diff --git a/internal/testrunner/runners/system/tester.go b/internal/testrunner/runners/system/tester.go index 83ee6c5b4..579eb7073 100644 --- a/internal/testrunner/runners/system/tester.go +++ b/internal/testrunner/runners/system/tester.go @@ -1198,9 +1198,10 @@ func (r *tester) setupService(ctx context.Context, config *testConfig, serviceOp } service, err := serviceDeployer.SetUp(ctx, svcInfo) + if err != nil { + return nil, svcInfo, fmt.Errorf("could not setup service: %w", err) + } - // Set shutdownServiceHandler even if there is any error, to remove service - // this is important mainly for terraform r.shutdownServiceHandler = func(ctx context.Context) error { if r.runTestsOnly { return nil @@ -1216,9 +1217,6 @@ func (r *tester) setupService(ctx context.Context, config *testConfig, serviceOp return nil } - if err != nil { - return nil, svcInfo, fmt.Errorf("could not setup service: %w", err) - } return service, service.Info(), nil } From 5c8f500639f6cbe80eddcaded320b24edb81eba1 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Mon, 17 Jun 2024 16:51:10 +0200 Subject: [PATCH 24/44] Move reading global config to cmd --- cmd/testrunner.go | 6 ++++++ internal/testrunner/runners/system/runner.go | 5 +++++ internal/testrunner/runners/system/tester.go | 14 +++++--------- 3 files changed, 16 insertions(+), 9 deletions(-) diff --git a/cmd/testrunner.go b/cmd/testrunner.go index b9b0ff2ec..1c077fb04 100644 --- a/cmd/testrunner.go +++ b/cmd/testrunner.go @@ -532,6 +532,11 @@ func testRunnerSystemCommandAction(cmd *cobra.Command, args []string) error { return fmt.Errorf("reading package manifest failed (path: %s): %w", packageRootPath, err) } + globalTestConfig, err := testrunner.ReadGlobalTestConfig(packageRootPath) + if err != nil { + return fmt.Errorf("failed to read global config: %w", err) + } + runner := system.NewSystemTestRunner(system.SystemTestRunnerOptions{ Profile: profile, PackageRootPath: packageRootPath, @@ -547,6 +552,7 @@ func testRunnerSystemCommandAction(cmd *cobra.Command, args []string) error { GenerateTestResult: generateTestResult, DeferCleanup: deferCleanup, RunIndependentElasticAgent: false, + GlobalTestConfig: globalTestConfig.System, }) logger.Debugf("Running suite...") diff --git a/internal/testrunner/runners/system/runner.go b/internal/testrunner/runners/system/runner.go index f04ade1d2..553b95379 100644 --- a/internal/testrunner/runners/system/runner.go +++ b/internal/testrunner/runners/system/runner.go @@ -32,6 +32,7 @@ type runner struct { dataStreams []string serviceVariant string + globalTestConfig testrunner.GlobalRunnerTestConfig failOnMissingTests bool generateTestResult bool runIndependentElasticAgent bool @@ -63,6 +64,8 @@ type SystemTestRunnerOptions struct { RunTestsOnly bool ConfigFilePath string + GlobalTestConfig testrunner.GlobalRunnerTestConfig + FailOnMissingTests bool GenerateTestResult bool RunIndependentElasticAgent bool @@ -85,6 +88,7 @@ func NewSystemTestRunner(options SystemTestRunnerOptions) *runner { generateTestResult: options.GenerateTestResult, runIndependentElasticAgent: options.RunIndependentElasticAgent, deferCleanup: options.DeferCleanup, + globalTestConfig: options.GlobalTestConfig, } r.resourcesManager = resources.NewManager() @@ -247,6 +251,7 @@ func (r *runner) GetTests(ctx context.Context) ([]testrunner.Tester, error) { RunTearDown: r.runTearDown, ConfigFileName: config, RunIndependentElasticAgent: r.runIndependentElasticAgent, + GlobalTestConfig: r.globalTestConfig, }) if err != nil { return nil, fmt.Errorf( diff --git a/internal/testrunner/runners/system/tester.go b/internal/testrunner/runners/system/tester.go index 579eb7073..4e6e23002 100644 --- a/internal/testrunner/runners/system/tester.go +++ b/internal/testrunner/runners/system/tester.go @@ -176,9 +176,10 @@ type SystemTesterOptions struct { RunIndependentElasticAgent bool - DeferCleanup time.Duration - ServiceVariant string - ConfigFileName string + DeferCleanup time.Duration + ServiceVariant string + ConfigFileName string + GlobalTestConfig testrunner.GlobalRunnerTestConfig RunSetup bool RunTearDown bool @@ -200,6 +201,7 @@ func NewSystemTester(options SystemTesterOptions) (*tester, error) { runSetup: options.RunSetup, runTestsOnly: options.RunTestsOnly, runTearDown: options.RunTearDown, + globalConfig: options.GlobalTestConfig, } r.resourcesManager = resources.NewManager() r.resourcesManager.RegisterProvider(resources.DefaultKibanaProviderName, &resources.KibanaProvider{Client: r.kibanaClient}) @@ -230,12 +232,6 @@ func NewSystemTester(options SystemTesterOptions) (*tester, error) { return nil, fmt.Errorf("cannot request Kibana version: %w", err) } - globalTestConfig, err := testrunner.ReadGlobalTestConfig(r.packageRootPath) - if err != nil { - return nil, fmt.Errorf("failed to read global config: %w", err) - } - r.globalConfig = globalTestConfig.System - r.pkgManifest, err = packages.ReadPackageManifestFromPackageRoot(r.packageRootPath) if err != nil { return nil, fmt.Errorf("reading package manifest failed: %w", err) From b84c8dc17e7ac98536e0deae3a00b66c44769df9 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Mon, 17 Jun 2024 17:00:42 +0200 Subject: [PATCH 25/44] Read global config in commands for all runners --- cmd/testrunner.go | 28 +++++++++++++++++-- internal/testrunner/runners/asset/runner.go | 22 +++++++++------ internal/testrunner/runners/asset/tester.go | 15 ++++++---- .../testrunner/runners/pipeline/runner.go | 10 +++++-- .../testrunner/runners/pipeline/tester.go | 3 ++ internal/testrunner/runners/policy/runner.go | 4 +++ internal/testrunner/runners/policy/tester.go | 12 +++++--- internal/testrunner/runners/static/runner.go | 8 ++++-- internal/testrunner/runners/static/tester.go | 15 ++++++---- 9 files changed, 85 insertions(+), 32 deletions(-) diff --git a/cmd/testrunner.go b/cmd/testrunner.go index 1c077fb04..fba82fdbd 100644 --- a/cmd/testrunner.go +++ b/cmd/testrunner.go @@ -165,9 +165,15 @@ func testRunnerAssetCommandAction(cmd *cobra.Command, args []string) error { return fmt.Errorf("can't create Kibana client: %w", err) } + globalTestConfig, err := testrunner.ReadGlobalTestConfig(packageRootPath) + if err != nil { + return fmt.Errorf("failed to read global config: %w", err) + } + runner := asset.NewAssetTestRunner(asset.AssetTestRunnerOptions{ - PackageRootPath: packageRootPath, - KibanaClient: kibanaClient, + PackageRootPath: packageRootPath, + KibanaClient: kibanaClient, + GlobalTestConfig: globalTestConfig.Asset, }) results, err := testrunner.RunSuite(ctx, runner) @@ -247,10 +253,16 @@ func testRunnerStaticCommandAction(cmd *cobra.Command, args []string) error { ctx, stop := signal.Enable(cmd.Context(), logger.Info) defer stop() + globalTestConfig, err := testrunner.ReadGlobalTestConfig(packageRootPath) + if err != nil { + return fmt.Errorf("failed to read global config: %w", err) + } + runner := static.NewStaticTestRunner(static.StaticTestRunnerOptions{ PackageRootPath: packageRootPath, DataStreams: dataStreams, FailOnMissingTests: failOnMissing, + GlobalTestConfig: globalTestConfig.Static, }) results, err := testrunner.RunSuite(ctx, runner) @@ -355,6 +367,11 @@ func testRunnerPipelineCommandAction(cmd *cobra.Command, args []string) error { return fmt.Errorf("reading package manifest failed (path: %s): %w", packageRootPath, err) } + globalTestConfig, err := testrunner.ReadGlobalTestConfig(packageRootPath) + if err != nil { + return fmt.Errorf("failed to read global config: %w", err) + } + runner := pipeline.NewPipelineTestRunner(pipeline.PipelineTestRunnerOptions{ Profile: profile, PackageRootPath: packageRootPath, @@ -365,6 +382,7 @@ func testRunnerPipelineCommandAction(cmd *cobra.Command, args []string) error { WithCoverage: testCoverage, CoverageType: testCoverageFormat, DeferCleanup: deferCleanup, + GlobalTestConfig: globalTestConfig.Pipeline, }) results, err := testrunner.RunSuite(ctx, runner) @@ -652,12 +670,18 @@ func testRunnerPolicyCommandAction(cmd *cobra.Command, args []string) error { return fmt.Errorf("reading package manifest failed (path: %s): %w", packageRootPath, err) } + globalTestConfig, err := testrunner.ReadGlobalTestConfig(packageRootPath) + if err != nil { + return fmt.Errorf("failed to read global config: %w", err) + } + runner := policy.NewPolicyTestRunner(policy.PolicyTestRunnerOptions{ PackageRootPath: packageRootPath, KibanaClient: kibanaClient, DataStreams: dataStreams, FailOnMissingTests: failOnMissing, GenerateTestResult: generateTestResult, + GlobalTestConfig: globalTestConfig.Policy, }) results, err := testrunner.RunSuite(ctx, runner) diff --git a/internal/testrunner/runners/asset/runner.go b/internal/testrunner/runners/asset/runner.go index 7112b58c9..6047d9686 100644 --- a/internal/testrunner/runners/asset/runner.go +++ b/internal/testrunner/runners/asset/runner.go @@ -17,19 +17,22 @@ const ( ) type runner struct { - packageRootPath string - kibanaClient *kibana.Client + packageRootPath string + kibanaClient *kibana.Client + globalTestConfig testrunner.GlobalRunnerTestConfig } type AssetTestRunnerOptions struct { - PackageRootPath string - KibanaClient *kibana.Client + PackageRootPath string + KibanaClient *kibana.Client + GlobalTestConfig testrunner.GlobalRunnerTestConfig } func NewAssetTestRunner(options AssetTestRunnerOptions) *runner { runner := runner{ - packageRootPath: options.PackageRootPath, - kibanaClient: options.KibanaClient, + packageRootPath: options.PackageRootPath, + kibanaClient: options.KibanaClient, + globalTestConfig: options.GlobalTestConfig, } return &runner } @@ -48,9 +51,10 @@ func (r *runner) TearDownRunner(ctx context.Context) error { func (r *runner) GetTests(ctx context.Context) ([]testrunner.Tester, error) { testers := []testrunner.Tester{ NewAssetTester(AssetTesterOptions{ - PackageRootPath: r.packageRootPath, - KibanaClient: r.kibanaClient, - TestFolder: testrunner.TestFolder{Package: r.packageRootPath}, + PackageRootPath: r.packageRootPath, + KibanaClient: r.kibanaClient, + TestFolder: testrunner.TestFolder{Package: r.packageRootPath}, + GlobalTestConfig: r.globalTestConfig, }), } return testers, nil diff --git a/internal/testrunner/runners/asset/tester.go b/internal/testrunner/runners/asset/tester.go index 7093d17f6..6231c375a 100644 --- a/internal/testrunner/runners/asset/tester.go +++ b/internal/testrunner/runners/asset/tester.go @@ -22,19 +22,22 @@ type tester struct { packageRootPath string kibanaClient *kibana.Client resourcesManager *resources.Manager + globalTestConfig testrunner.GlobalRunnerTestConfig } type AssetTesterOptions struct { - TestFolder testrunner.TestFolder - PackageRootPath string - KibanaClient *kibana.Client + TestFolder testrunner.TestFolder + PackageRootPath string + KibanaClient *kibana.Client + GlobalTestConfig testrunner.GlobalRunnerTestConfig } func NewAssetTester(options AssetTesterOptions) *tester { tester := tester{ - testFolder: options.TestFolder, - packageRootPath: options.PackageRootPath, - kibanaClient: options.KibanaClient, + testFolder: options.TestFolder, + packageRootPath: options.PackageRootPath, + kibanaClient: options.KibanaClient, + globalTestConfig: options.GlobalTestConfig, } manager := resources.NewManager() diff --git a/internal/testrunner/runners/pipeline/runner.go b/internal/testrunner/runners/pipeline/runner.go index e6eeb3ae1..01de91c64 100644 --- a/internal/testrunner/runners/pipeline/runner.go +++ b/internal/testrunner/runners/pipeline/runner.go @@ -31,9 +31,10 @@ type runner struct { failOnMissingTests bool generateTestResult bool - withCoverage bool - coverageType string - deferCleanup time.Duration + withCoverage bool + coverageType string + deferCleanup time.Duration + globalTestConfig testrunner.GlobalRunnerTestConfig } type PipelineTestRunnerOptions struct { @@ -46,6 +47,7 @@ type PipelineTestRunnerOptions struct { WithCoverage bool CoverageType string DeferCleanup time.Duration + GlobalTestConfig testrunner.GlobalRunnerTestConfig } func NewPipelineTestRunner(options PipelineTestRunnerOptions) *runner { @@ -59,6 +61,7 @@ func NewPipelineTestRunner(options PipelineTestRunnerOptions) *runner { withCoverage: options.WithCoverage, coverageType: options.CoverageType, deferCleanup: options.DeferCleanup, + globalTestConfig: options.GlobalTestConfig, } return &runner } @@ -134,6 +137,7 @@ func (r *runner) GetTests(ctx context.Context) ([]testrunner.Tester, error) { Profile: r.profile, API: r.esAPI, TestCaseFile: caseFile, + GlobalTestConfig: r.globalTestConfig, }) if err != nil { return nil, fmt.Errorf("failed to create pipeline tester: %w", err) diff --git a/internal/testrunner/runners/pipeline/tester.go b/internal/testrunner/runners/pipeline/tester.go index a6b1237d6..b09ea79dc 100644 --- a/internal/testrunner/runners/pipeline/tester.go +++ b/internal/testrunner/runners/pipeline/tester.go @@ -45,6 +45,7 @@ type tester struct { generateTestResult bool withCoverage bool coverageType string + globalTestConfig testrunner.GlobalRunnerTestConfig testCaseFile string @@ -65,6 +66,7 @@ type PipelineTesterOptions struct { WithCoverage bool CoverageType string TestCaseFile string + GlobalTestConfig testrunner.GlobalRunnerTestConfig } func NewPipelineTester(options PipelineTesterOptions) (*tester, error) { @@ -78,6 +80,7 @@ func NewPipelineTester(options PipelineTesterOptions) (*tester, error) { generateTestResult: options.GenerateTestResult, withCoverage: options.WithCoverage, coverageType: options.CoverageType, + globalTestConfig: options.GlobalTestConfig, } stackConfig, err := stack.LoadConfig(r.profile) diff --git a/internal/testrunner/runners/policy/runner.go b/internal/testrunner/runners/policy/runner.go index 3d0cb9e8a..a563cca17 100644 --- a/internal/testrunner/runners/policy/runner.go +++ b/internal/testrunner/runners/policy/runner.go @@ -28,6 +28,7 @@ type runner struct { dataStreams []string failOnMissingTests bool generateTestResult bool + globalTestConfig testrunner.GlobalRunnerTestConfig resourcesManager *resources.Manager cleanup func(context.Context) error @@ -42,6 +43,7 @@ type PolicyTestRunnerOptions struct { DataStreams []string FailOnMissingTests bool GenerateTestResult bool + GlobalTestConfig testrunner.GlobalRunnerTestConfig } func NewPolicyTestRunner(options PolicyTestRunnerOptions) *runner { @@ -51,6 +53,7 @@ func NewPolicyTestRunner(options PolicyTestRunnerOptions) *runner { dataStreams: options.DataStreams, failOnMissingTests: options.FailOnMissingTests, generateTestResult: options.GenerateTestResult, + globalTestConfig: options.GlobalTestConfig, } runner.resourcesManager = resources.NewManager() runner.resourcesManager.RegisterProvider(resources.DefaultKibanaProviderName, &resources.KibanaProvider{Client: runner.kibanaClient}) @@ -131,6 +134,7 @@ func (r *runner) GetTests(ctx context.Context) ([]testrunner.Tester, error) { KibanaClient: r.kibanaClient, GenerateTestResult: r.generateTestResult, TestPath: test, + GlobalTestConfig: r.globalTestConfig, })) } diff --git a/internal/testrunner/runners/policy/tester.go b/internal/testrunner/runners/policy/tester.go index 2a6b8afb1..8836904f4 100644 --- a/internal/testrunner/runners/policy/tester.go +++ b/internal/testrunner/runners/policy/tester.go @@ -18,11 +18,13 @@ import ( ) type tester struct { - testFolder testrunner.TestFolder - packageRootPath string + testFolder testrunner.TestFolder + packageRootPath string + kibanaClient *kibana.Client + testPath string + generateTestResult bool - kibanaClient *kibana.Client - testPath string + globalTestConfig testrunner.GlobalRunnerTestConfig resourcesManager *resources.Manager } @@ -36,6 +38,7 @@ type PolicyTesterOptions struct { KibanaClient *kibana.Client PackageRootPath string GenerateTestResult bool + GlobalTestConfig testrunner.GlobalRunnerTestConfig } func NewPolicyTester(options PolicyTesterOptions) *tester { @@ -45,6 +48,7 @@ func NewPolicyTester(options PolicyTesterOptions) *tester { packageRootPath: options.PackageRootPath, generateTestResult: options.GenerateTestResult, testPath: options.TestPath, + globalTestConfig: options.GlobalTestConfig, } tester.resourcesManager = resources.NewManager() tester.resourcesManager.RegisterProvider(resources.DefaultKibanaProviderName, &resources.KibanaProvider{Client: tester.kibanaClient}) diff --git a/internal/testrunner/runners/static/runner.go b/internal/testrunner/runners/static/runner.go index a4ae6a0b8..54b228c01 100644 --- a/internal/testrunner/runners/static/runner.go +++ b/internal/testrunner/runners/static/runner.go @@ -25,12 +25,14 @@ type runner struct { packageRootPath string failOnMissingTests bool dataStreams []string + globalTestConfig testrunner.GlobalRunnerTestConfig } type StaticTestRunnerOptions struct { PackageRootPath string FailOnMissingTests bool DataStreams []string + GlobalTestConfig testrunner.GlobalRunnerTestConfig } func NewStaticTestRunner(options StaticTestRunnerOptions) *runner { @@ -38,6 +40,7 @@ func NewStaticTestRunner(options StaticTestRunnerOptions) *runner { packageRootPath: options.PackageRootPath, failOnMissingTests: options.FailOnMissingTests, dataStreams: options.DataStreams, + globalTestConfig: options.GlobalTestConfig, } return &runner } @@ -89,8 +92,9 @@ func (r *runner) GetTests(ctx context.Context) ([]testrunner.Tester, error) { var testers []testrunner.Tester for _, t := range tests { testers = append(testers, NewStaticTester(StaticTesterOptions{ - PackageRootPath: r.packageRootPath, - TestFolder: t, + PackageRootPath: r.packageRootPath, + TestFolder: t, + GlobalTestConfig: r.globalTestConfig, })) } return testers, nil diff --git a/internal/testrunner/runners/static/tester.go b/internal/testrunner/runners/static/tester.go index ce422ec03..7c28eb348 100644 --- a/internal/testrunner/runners/static/tester.go +++ b/internal/testrunner/runners/static/tester.go @@ -20,18 +20,21 @@ import ( ) type tester struct { - testFolder testrunner.TestFolder - packageRootPath string + testFolder testrunner.TestFolder + packageRootPath string + globalTestConfig testrunner.GlobalRunnerTestConfig } type StaticTesterOptions struct { - TestFolder testrunner.TestFolder - PackageRootPath string + TestFolder testrunner.TestFolder + PackageRootPath string + GlobalTestConfig testrunner.GlobalRunnerTestConfig } func NewStaticTester(options StaticTesterOptions) *tester { runner := tester{ - testFolder: options.TestFolder, - packageRootPath: options.PackageRootPath, + testFolder: options.TestFolder, + packageRootPath: options.PackageRootPath, + globalTestConfig: options.GlobalTestConfig, } return &runner } From a8e1ee1a55c47ba6723a2be3675303e262aa23d3 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Mon, 17 Jun 2024 17:36:56 +0200 Subject: [PATCH 26/44] Add support to skip tests with global config in all runners --- internal/testrunner/runners/asset/tester.go | 7 +++++++ internal/testrunner/runners/pipeline/tester.go | 9 +++++++++ internal/testrunner/runners/policy/testconfig.go | 4 ++++ internal/testrunner/runners/policy/tester.go | 15 +++++++++++++++ internal/testrunner/runners/static/tester.go | 7 +++++++ 5 files changed, 42 insertions(+) diff --git a/internal/testrunner/runners/asset/tester.go b/internal/testrunner/runners/asset/tester.go index 6231c375a..229544525 100644 --- a/internal/testrunner/runners/asset/tester.go +++ b/internal/testrunner/runners/asset/tester.go @@ -102,6 +102,13 @@ func (r *tester) run(ctx context.Context) ([]testrunner.TestResult, error) { return result.WithSkip(testConfig.Skip) } + if r.globalTestConfig.Skip != nil { + logger.Warnf("skipping %s test for %s/%s: %s (details: %s)", + TestType, r.testFolder.Package, r.testFolder.DataStream, + r.globalTestConfig.Skip.Reason, r.globalTestConfig.Skip.Link.String()) + return result.WithSkip(r.globalTestConfig.Skip) + } + logger.Debug("installing package...") _, err = r.resourcesManager.ApplyCtx(ctx, r.resources(true)) if err != nil { diff --git a/internal/testrunner/runners/pipeline/tester.go b/internal/testrunner/runners/pipeline/tester.go index b09ea79dc..5e98d07eb 100644 --- a/internal/testrunner/runners/pipeline/tester.go +++ b/internal/testrunner/runners/pipeline/tester.go @@ -324,6 +324,15 @@ func (r *tester) runTestCase(ctx context.Context, testCaseFile string, dsPath st return tr, nil } + if r.globalTestConfig.Skip != nil { + logger.Warnf("skipping %s test for %s/%s: %s (details: %s)", + TestType, r.testFolder.Package, r.testFolder.DataStream, + r.globalTestConfig.Skip.Reason, r.globalTestConfig.Skip.Link.String()) + + tr.Skipped = r.globalTestConfig.Skip + return tr, nil + } + simulateDataStream := dsType + "-" + r.testFolder.Package + "." + r.testFolder.DataStream + "-default" processedEvents, err := ingest.SimulatePipeline(ctx, r.esAPI, pipeline, tc.events, simulateDataStream) if err != nil { diff --git a/internal/testrunner/runners/policy/testconfig.go b/internal/testrunner/runners/policy/testconfig.go index 437d1559b..acae9a1a7 100644 --- a/internal/testrunner/runners/policy/testconfig.go +++ b/internal/testrunner/runners/policy/testconfig.go @@ -9,9 +9,13 @@ import ( "os" "gopkg.in/yaml.v3" + + "github.com/elastic/elastic-package/internal/testrunner" ) type testConfig struct { + testrunner.SkippableConfig `config:",inline"` + Input string `yaml:"input,omitempty"` Vars map[string]any `yaml:"vars,omitempty"` DataStream struct { diff --git a/internal/testrunner/runners/policy/tester.go b/internal/testrunner/runners/policy/tester.go index 8836904f4..a3d58c46a 100644 --- a/internal/testrunner/runners/policy/tester.go +++ b/internal/testrunner/runners/policy/tester.go @@ -94,6 +94,21 @@ func (r *tester) runTest(ctx context.Context, manager *resources.Manager, testPa } testName := testNameFromPath(testPath) + + if testConfig.Skip != nil { + logger.Warnf("skipping %s test for %s/%s: %s (details: %s)", + TestType, r.testFolder.Package, r.testFolder.DataStream, + testConfig.Skip.Reason, testConfig.Skip.Link.String()) + return result.WithSkip(testConfig.Skip) + } + + if r.globalTestConfig.Skip != nil { + logger.Warnf("skipping %s test for %s/%s: %s (details: %s)", + TestType, r.testFolder.Package, r.testFolder.DataStream, + r.globalTestConfig.Skip.Reason, r.globalTestConfig.Skip.Link.String()) + return result.WithSkip(r.globalTestConfig.Skip) + } + policy := resources.FleetAgentPolicy{ Name: testName, Namespace: "ep", diff --git a/internal/testrunner/runners/static/tester.go b/internal/testrunner/runners/static/tester.go index 7c28eb348..4db1fa034 100644 --- a/internal/testrunner/runners/static/tester.go +++ b/internal/testrunner/runners/static/tester.go @@ -78,6 +78,13 @@ func (r tester) run(ctx context.Context) ([]testrunner.TestResult, error) { return result.WithSkip(testConfig.Skip) } + if r.globalTestConfig.Skip != nil { + logger.Warnf("skipping %s test for %s/%s: %s (details: %s)", + TestType, r.testFolder.Package, r.testFolder.DataStream, + r.globalTestConfig.Skip.Reason, r.globalTestConfig.Skip.Link.String()) + return result.WithSkip(r.globalTestConfig.Skip) + } + pkgManifest, err := packages.ReadPackageManifestFromPackageRoot(r.packageRootPath) if err != nil { return result.WithError(fmt.Errorf("failed to read manifest: %w", err)) From 80c231c8d807898c49a3aa361d7f708273676b20 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Mon, 17 Jun 2024 17:39:06 +0200 Subject: [PATCH 27/44] Add comment about runners wihtout parallel system test support --- internal/testrunner/runners/asset/tester.go | 1 + internal/testrunner/runners/pipeline/tester.go | 1 + internal/testrunner/runners/policy/tester.go | 1 + internal/testrunner/runners/static/tester.go | 1 + 4 files changed, 4 insertions(+) diff --git a/internal/testrunner/runners/asset/tester.go b/internal/testrunner/runners/asset/tester.go index 229544525..26f8887e8 100644 --- a/internal/testrunner/runners/asset/tester.go +++ b/internal/testrunner/runners/asset/tester.go @@ -62,6 +62,7 @@ func (r tester) String() string { // Parallel indicates if this tester can run in parallel or not. func (r tester) Parallel() bool { + // Not supported yet parallel tests even if it is indicated in the global config r.globalTestConfig return false } diff --git a/internal/testrunner/runners/pipeline/tester.go b/internal/testrunner/runners/pipeline/tester.go index 5e98d07eb..fd7f2ee2e 100644 --- a/internal/testrunner/runners/pipeline/tester.go +++ b/internal/testrunner/runners/pipeline/tester.go @@ -132,6 +132,7 @@ func (r *tester) String() string { // Parallel indicates if this tester can run in parallel or not. func (r tester) Parallel() bool { + // Not supported yet parallel tests even if it is indicated in the global config r.globalTestConfig return false } diff --git a/internal/testrunner/runners/policy/tester.go b/internal/testrunner/runners/policy/tester.go index a3d58c46a..5a1a037e5 100644 --- a/internal/testrunner/runners/policy/tester.go +++ b/internal/testrunner/runners/policy/tester.go @@ -65,6 +65,7 @@ func (r *tester) String() string { // Parallel indicates if this tester can run in parallel or not. func (r tester) Parallel() bool { + // Not supported yet parallel tests even if it is indicated in the global config r.globalTestConfig return false } diff --git a/internal/testrunner/runners/static/tester.go b/internal/testrunner/runners/static/tester.go index 4db1fa034..882d68618 100644 --- a/internal/testrunner/runners/static/tester.go +++ b/internal/testrunner/runners/static/tester.go @@ -52,6 +52,7 @@ func (r tester) String() string { // Parallel indicates if this tester can run in parallel or not. func (r tester) Parallel() bool { + // Not supported yet parallel tests even if it is indicated in the global config r.globalTestConfig return false } From c933c0d33dc9228dc6243caf9cb279e3691ce797 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Mon, 17 Jun 2024 17:46:20 +0200 Subject: [PATCH 28/44] Update debug messages --- internal/testrunner/testrunner.go | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/internal/testrunner/testrunner.go b/internal/testrunner/testrunner.go index 28a97d164..cccc8bbd5 100644 --- a/internal/testrunner/testrunner.go +++ b/internal/testrunner/testrunner.go @@ -321,10 +321,8 @@ func RunSuite(ctx context.Context, runner TestRunner) ([]TestResult, error) { var parallelTesters, sequentialTesters []Tester for _, tester := range testers { if tester.Parallel() { - logger.Debugf("Found tester parallel") parallelTesters = append(parallelTesters, tester) } else { - logger.Debugf("Found tester sequential") sequentialTesters = append(sequentialTesters, tester) } } @@ -372,6 +370,7 @@ func runSuite(ctx context.Context, testers []Tester) ([]TestResult, error) { if len(testers) == 0 { return nil, nil } + logger.Debugf("Running tests sequentially") var results []TestResult for _, tester := range testers { r, err := run(ctx, tester) @@ -401,7 +400,7 @@ func runSuiteParallel(ctx context.Context, testers []Tester) ([]TestResult, erro } chResults := make(chan routineResult, len(testers)) - logger.Debugf(">>> Maximum routines: %d", maxRoutines) + logger.Debugf("Running tests in parallel. Maximum routines to run in parallel: %d", maxRoutines) // Use channel as a semaphore to limit the number of test executions in parallel sem := make(chan int, maxRoutines) @@ -433,10 +432,6 @@ func runSuiteParallel(ctx context.Context, testers []Tester) ([]TestResult, erro var multiErr error testType := testers[0].Type() for testResults := range chResults { - - if len(testResults.results) > 0 { - logger.Debugf("Processing test result %s", testResults.results[0].Name) - } if testResults.err != nil { multiErr = errors.Join(multiErr, testResults.err) } From 9758666f38ec28ddca04d6ca7c506a0d16aca4c1 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Mon, 17 Jun 2024 17:52:31 +0200 Subject: [PATCH 29/44] Update package-spec to elastic/main 572e54732 --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index c9a246871..5f069db76 100644 --- a/go.mod +++ b/go.mod @@ -185,4 +185,4 @@ require ( sigs.k8s.io/yaml v1.4.0 // indirect ) -replace github.com/elastic/package-spec/v3 => github.com/mrodm/package-spec/v3 v3.0.0-20240613145150-8d065e83d217 +replace github.com/elastic/package-spec/v3 => github.com/elastic/package-spec/v3 v3.1.6-0.20240617154842-572e54732385 diff --git a/go.sum b/go.sum index 0c1513683..1eebe77ad 100644 --- a/go.sum +++ b/go.sum @@ -102,6 +102,8 @@ github.com/elastic/gojsonschema v1.2.1 h1:cUMbgsz0wyEB4x7xf3zUEvUVDl6WCz2RKcQPul github.com/elastic/gojsonschema v1.2.1/go.mod h1:biw5eBS2Z4T02wjATMRSfecfjCmwaDPvuaqf844gLrg= github.com/elastic/kbncontent v0.1.4 h1:GoUkJkqkn2H6iJTnOHcxEqYVVYyjvcebLQVaSR1aSvU= github.com/elastic/kbncontent v0.1.4/go.mod h1:kOPREITK9gSJsiw/WKe7QWSO+PRiZMyEFQCw+CMLAHI= +github.com/elastic/package-spec/v3 v3.1.6-0.20240617154842-572e54732385 h1:nZcfkILwSR+OkQujlke73NjlFw2KSFcKV5LyvE7hI8s= +github.com/elastic/package-spec/v3 v3.1.6-0.20240617154842-572e54732385/go.mod h1:dH3ca6UEfn4c4LX3SwznyOEV8ZJN/YJ2PE+x0WeNHvM= github.com/elazarl/goproxy v0.0.0-20230808193330-2592e75ae04a h1:mATvB/9r/3gvcejNsXKSkQ6lcIaNec2nyfOdlTBR2lU= github.com/elazarl/goproxy v0.0.0-20230808193330-2592e75ae04a/go.mod h1:Ro8st/ElPeALwNFlcTpWmkr6IoMFfkjXAvTHpevnDsM= github.com/emicklei/go-restful/v3 v3.11.0 h1:rAQeMHw1c7zTmncogyy8VvRZwtkmkZ4FxERmMY4rD+g= @@ -314,8 +316,6 @@ github.com/modern-go/reflect2 v1.0.2/go.mod h1:yWuevngMOJpCy52FWWMvUC8ws7m/LJsjY github.com/monochromegane/go-gitignore v0.0.0-20200626010858-205db1a8cc00 h1:n6/2gBQ3RWajuToeY6ZtZTIKv2v7ThUy5KKusIT0yc0= github.com/monochromegane/go-gitignore v0.0.0-20200626010858-205db1a8cc00/go.mod h1:Pm3mSP3c5uWn86xMLZ5Sa7JB9GsEZySvHYXCTK4E9q4= github.com/montanaflynn/stats v0.0.0-20171201202039-1bf9dbcd8cbe/go.mod h1:wL8QJuTMNUDYhXwkmfOly8iTdp5TEcJFWZD2D7SIkUc= -github.com/mrodm/package-spec/v3 v3.0.0-20240613145150-8d065e83d217 h1:Ywx/eMe8dX0yx3xf7nWwlHMp6Bp+NyBqXtl95KvNpXU= -github.com/mrodm/package-spec/v3 v3.0.0-20240613145150-8d065e83d217/go.mod h1:Qdq/tqETSbbTpCfUAlVTtIvO2RiAdXp6aXTbqeGdHfE= github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 h1:C3w9PqII01/Oq1c1nUAm88MOHcQC9l5mIlSMApZMrHA= github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822/go.mod h1:+n7T8mK8HuQTcFwEeznm/DIxMOiR9yIdICNftLE1DvQ= github.com/mxk/go-flowrate v0.0.0-20140419014527-cca7078d478f h1:y5//uYreIhSUg3J1GEMiLbxo1LJaP8RfCpH6pymGZus= From 1c4b9cf564177f7e8357788809954b9dc5043a90 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Mon, 17 Jun 2024 18:01:24 +0200 Subject: [PATCH 30/44] Update log message --- internal/testrunner/testrunner.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/testrunner/testrunner.go b/internal/testrunner/testrunner.go index cccc8bbd5..40a3d1d08 100644 --- a/internal/testrunner/testrunner.go +++ b/internal/testrunner/testrunner.go @@ -313,7 +313,7 @@ func RunSuite(ctx context.Context, runner TestRunner) ([]TestResult, error) { cleanupCtx := context.WithoutCancel(ctx) tdErr := runner.TearDownRunner(cleanupCtx) if tdErr != nil { - logger.Debugf("failed to tear down %s runner: %w", runner.Type(), tdErr) + logger.Debugf("failed to tear down %s runner: %s", runner.Type(), tdErr) } return nil, fmt.Errorf("failed to setup %s runner: %w", runner.Type(), err) } From 419f8ea3999f1fb1ef40960d8d024c85b580dea3 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Mon, 17 Jun 2024 19:56:38 +0200 Subject: [PATCH 31/44] Update docs --- docs/howto/asset_testing.md | 11 +++++++++++ docs/howto/pipeline_testing.md | 11 +++++++++++ docs/howto/policy_testing.md | 11 +++++++++++ docs/howto/static_testing.md | 11 +++++++++++ docs/howto/system_testing.md | 35 ++++++++++++++++++++++++++++++++++ 5 files changed, 79 insertions(+) diff --git a/docs/howto/asset_testing.md b/docs/howto/asset_testing.md index 45dc373ba..f979c45d6 100644 --- a/docs/howto/asset_testing.md +++ b/docs/howto/asset_testing.md @@ -49,3 +49,14 @@ Finally, when you are done running all asset loading tests, bring down the Elast ``` elastic-package stack down ``` + +## Global test configuration + +Each package could define a configuration file in `_dev/test/config.yml` to skip all the asset tests. + +```yaml +asset: + skip: + reason: + link: +``` \ No newline at end of file diff --git a/docs/howto/pipeline_testing.md b/docs/howto/pipeline_testing.md index 7a5517ef0..a59b21ce0 100644 --- a/docs/howto/pipeline_testing.md +++ b/docs/howto/pipeline_testing.md @@ -175,3 +175,14 @@ Finally, when you are done running all pipeline tests, bring down the Elastic St ``` elastic-package stack down ``` + +## Global test configuration + +Each package could define a configuration file in `_dev/test/config.yml` to skip all the pipeline tests. + +```yaml +pipeline: + skip: + reason: + link: +``` \ No newline at end of file diff --git a/docs/howto/policy_testing.md b/docs/howto/policy_testing.md index de16dd8c6..a0d7bcab1 100644 --- a/docs/howto/policy_testing.md +++ b/docs/howto/policy_testing.md @@ -41,6 +41,17 @@ It is possible, and encouraged, to define multiple policy tests for each package or data stream. +## Global test configuration + +Each package could define a configuration file in `_dev/test/config.yml` to skip all the policy tests. + +```yaml +policy: + skip: + reason: + link: +``` + ### Defining the configuration of the policy Test configuration for the policy is defined in a YAML file prefixed with diff --git a/docs/howto/static_testing.md b/docs/howto/static_testing.md index 185974c90..f479deb6c 100644 --- a/docs/howto/static_testing.md +++ b/docs/howto/static_testing.md @@ -26,3 +26,14 @@ If you want to run pipeline tests for **specific data streams** in a package, na ``` elastic-package test static --data-streams [,,...] ``` + +## Global test configuration + +Each package could define a configuration file in `_dev/test/config.yml` to skip all the static tests. + +```yaml +static: + skip: + reason: + link: +``` \ No newline at end of file diff --git a/docs/howto/system_testing.md b/docs/howto/system_testing.md index 07ec66d5a..47a1320d4 100644 --- a/docs/howto/system_testing.md +++ b/docs/howto/system_testing.md @@ -553,6 +553,20 @@ Placeholders used in the `test--config.yml` must be enclosed in `{{{` **NOTE**: Terraform variables in the form of environment variables (prefixed with `TF_VAR_`) are not injected and cannot be used as placeholder (their value will always be empty). +## Global test configuration + +Each package could define a configuration file in `_dev/test/config.yml` that allows to: +- skip all the system tests defined. +- set if these system tests should be running in parallel or not. + +```yaml +system: + parallel: true + skip: + reason: + link: +``` + ## Running a system test Once the two levels of configurations are defined as described in the previous section, you are ready to run system tests for a package's data streams. @@ -761,6 +775,27 @@ Considerations for this mode of running Elastic Agents: - Create a new `_dev/deploy/docker` adding the service container if needed. - Define the settings required for your Elastic Agents in all the test configuration files. +#### Running system tests in parallel (technical preview) + +By default, `elatic-package` runs every system test sequentially. This could be changed to allow running in parallel `N` tests. + +In order to do so, it is required that the tests are run using independent Elasic Agents. + +To enable this feature, package must define the global test configuration file with these contents: +```yaml +system: + parallel: true +``` + +And run `elastic-package test` with the following environment variables: +```shell +ELASTIC_PACKAGE_MAXIMUM_NUMBER_PARALLEL_TESTS=5 \ + ELASTIC_PACKAGE_TEST_ENABLE_INDEPENDENT_AGENT=true \ + elastic-package test system -v +``` + +Currently, just system tests support to run the tests in parallel. + ### Detecting ignored fields As part of the system test, `elastic-package` checks whether any documents couldn't successfully map any fields. Common issues are the configured field limit being exceeded or keyword fields receiving values longer than `ignore_above`. You can learn more in the [Elasticsearch documentation](https://www.elastic.co/guide/en/elasticsearch/reference/current/mapping-ignored-field.html). From d579fb7285e74f50fe7d5e44e545998fdb77ff4b Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Tue, 18 Jun 2024 10:17:22 +0200 Subject: [PATCH 32/44] Update docs --- docs/howto/system_testing.md | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/docs/howto/system_testing.md b/docs/howto/system_testing.md index 47a1320d4..6152f6be0 100644 --- a/docs/howto/system_testing.md +++ b/docs/howto/system_testing.md @@ -777,30 +777,32 @@ Considerations for this mode of running Elastic Agents: #### Running system tests in parallel (technical preview) -By default, `elatic-package` runs every system test sequentially. This could be changed to allow running in parallel `N` tests. - -In order to do so, it is required that the tests are run using independent Elasic Agents. +By default, `elatic-package` runs every system test defined in the package sequentially. +This could be changed to allow running in parallel tests. For that it is needed: +- running tests using independent Elastic Agents (see [section](#running-system-tests-with-independent-elastic-agents-in-each-test-technical-preview)). +- package must define the global test configuration file with these contents to enable system test parallelization: + ```yaml + system: + parallel: true + ``` +- define how many tests in parallel should be running + - This is done defining the environment variable `ELASTIC_PACKAGE_MAXIMUM_NUMBER_PARALLEL_TESTS` -To enable this feature, package must define the global test configuration file with these contents: -```yaml -system: - parallel: true -``` -And run `elastic-package test` with the following environment variables: +Given those requirements, this is an example to run system tests in parallel: ```shell ELASTIC_PACKAGE_MAXIMUM_NUMBER_PARALLEL_TESTS=5 \ ELASTIC_PACKAGE_TEST_ENABLE_INDEPENDENT_AGENT=true \ elastic-package test system -v ``` -Currently, just system tests support to run the tests in parallel. +**NOTE**: Currently, just system tests support to run the tests in parallel. ### Detecting ignored fields As part of the system test, `elastic-package` checks whether any documents couldn't successfully map any fields. Common issues are the configured field limit being exceeded or keyword fields receiving values longer than `ignore_above`. You can learn more in the [Elasticsearch documentation](https://www.elastic.co/guide/en/elasticsearch/reference/current/mapping-ignored-field.html). -In this case, `elastic-package test system` will fail with an error and print a sample of affected documents. To fix the issue, check which fields got ignored and the `ignored_field_values` and either adapt the mapping or the ingest pipeline to accomodate for the problematic values. In case an ignored field can't be meaningfully mitigated, it's possible to skip the check by listing the field under the `skip_ignored_fields` property in the system test config of the data stream: +In this case, `elastic-package test system` will fail with an error and print a sample of affected documents. To fix the issue, check which fields got ignored and the `ignored_field_values` and either adapt the mapping or the ingest pipeline to accommodate for the problematic values. In case an ignored field can't be meaningfully mitigated, it's possible to skip the check by listing the field under the `skip_ignored_fields` property in the system test config of the data stream: ``` # data_stream//_dev/test/system/test-default-config.yml skip_ignored_fields: From d8a16e964d3989baff82afc711bdf1a8480bb97d Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Tue, 18 Jun 2024 13:30:20 +0200 Subject: [PATCH 33/44] Return just the error from tests --- internal/testrunner/runners/system/tester.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/testrunner/runners/system/tester.go b/internal/testrunner/runners/system/tester.go index c78f47ffe..f7a393933 100644 --- a/internal/testrunner/runners/system/tester.go +++ b/internal/testrunner/runners/system/tester.go @@ -601,7 +601,7 @@ func (r *tester) runTestPerVariant(ctx context.Context, result *testrunner.Resul tdErr := r.tearDownTest(ctx) if err != nil { - return partial, errors.Join(err, fmt.Errorf("failed to tear down runner: %w", tdErr)) + return partial, err } if tdErr != nil { return partial, fmt.Errorf("failed to tear down runner: %w", tdErr) From 94b533f85c9804849630642ba9e4d4572322502d Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Tue, 18 Jun 2024 13:35:03 +0200 Subject: [PATCH 34/44] Run system tests in parallel for sql_input --- test/packages/parallel/sql_input/_dev/test/config.yml | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 test/packages/parallel/sql_input/_dev/test/config.yml diff --git a/test/packages/parallel/sql_input/_dev/test/config.yml b/test/packages/parallel/sql_input/_dev/test/config.yml new file mode 100644 index 000000000..a57750f85 --- /dev/null +++ b/test/packages/parallel/sql_input/_dev/test/config.yml @@ -0,0 +1,2 @@ +system: + parallel: true From 7b520326f6bb7235ac9bf3d3a42ea17ae3e0a668 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Tue, 18 Jun 2024 13:37:30 +0200 Subject: [PATCH 35/44] Update variable name --- internal/testrunner/testrunner.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/testrunner/testrunner.go b/internal/testrunner/testrunner.go index 40a3d1d08..81291628a 100644 --- a/internal/testrunner/testrunner.go +++ b/internal/testrunner/testrunner.go @@ -406,7 +406,7 @@ func runSuiteParallel(ctx context.Context, testers []Tester) ([]TestResult, erro for _, tester := range testers { wg.Add(1) - aTester := tester + tester := tester sem <- 1 go func() { defer wg.Done() @@ -419,7 +419,7 @@ func runSuiteParallel(ctx context.Context, testers []Tester) ([]TestResult, erro return } // TODO: How to check if ctx is Done or Cancelled, to not call to "run" method - r, err := run(ctx, aTester) + r, err := run(ctx, tester) chResults <- routineResult{r, err} }() } From a3c7288749aa78eadc2bc9f4e020fcc9266ccf73 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Tue, 18 Jun 2024 13:49:08 +0200 Subject: [PATCH 36/44] Set maximum routines dynamically --- internal/testrunner/testrunner.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/internal/testrunner/testrunner.go b/internal/testrunner/testrunner.go index 81291628a..123eb83ae 100644 --- a/internal/testrunner/testrunner.go +++ b/internal/testrunner/testrunner.go @@ -10,6 +10,7 @@ import ( "fmt" "os" "path/filepath" + "runtime" "sort" "strconv" "strings" @@ -24,9 +25,10 @@ import ( "github.com/elastic/elastic-package/internal/profile" ) -const DEFAULT_MAXIMUM_ROUTINES = 1 - -var maximumNumberParallelTest = environment.WithElasticPackagePrefix("MAXIMUM_NUMBER_PARALLEL_TESTS") +var ( + DEFAULT_MAXIMUM_ROUTINES = runtime.GOMAXPROCS(0) / 2 + maximumNumberParallelTest = environment.WithElasticPackagePrefix("MAXIMUM_NUMBER_PARALLEL_TESTS") +) // TestType represents the various supported test types type TestType string From 3cc537d547b6f6d6375577e87b4dd16c66044d77 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Tue, 18 Jun 2024 15:51:04 +0200 Subject: [PATCH 37/44] Sort test results --- cmd/testrunner.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/cmd/testrunner.go b/cmd/testrunner.go index fba82fdbd..1eb1b3aaf 100644 --- a/cmd/testrunner.go +++ b/cmd/testrunner.go @@ -10,6 +10,7 @@ import ( "os" "path/filepath" "slices" + "sort" "strings" "github.com/spf13/cobra" @@ -693,6 +694,18 @@ func testRunnerPolicyCommandAction(cmd *cobra.Command, args []string) error { } func processResults(results []testrunner.TestResult, testType testrunner.TestType, reportFormat, reportOutput, packageRootPath, packageName, packageType, testCoverageFormat string, testCoverage bool) error { + sort.Slice(results, func(i, j int) bool { + if results[i].Package != results[j].Package { + return results[i].Package < results[j].Package + } + if results[i].TestType != results[j].TestType { + return results[i].TestType < results[j].TestType + } + if results[i].DataStream != results[j].DataStream { + return results[i].DataStream < results[j].DataStream + } + return results[i].Name < results[j].Name + }) format := testrunner.TestReportFormat(reportFormat) report, err := testrunner.FormatReport(format, results) if err != nil { From e89184295c62bee5288293b98d3ce1ed7af627c7 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Tue, 18 Jun 2024 15:51:49 +0200 Subject: [PATCH 38/44] Remove duplicated test config files --- .../access/_dev/test/system/test-other-config.yml | 5 ----- .../error/_dev/test/system/test-other-config.yml | 5 ----- .../stubstatus/_dev/test/system/test-other-config.yml | 6 ------ 3 files changed, 16 deletions(-) delete mode 100644 test/packages/parallel/nginx/data_stream/access/_dev/test/system/test-other-config.yml delete mode 100644 test/packages/parallel/nginx/data_stream/error/_dev/test/system/test-other-config.yml delete mode 100644 test/packages/parallel/nginx/data_stream/stubstatus/_dev/test/system/test-other-config.yml diff --git a/test/packages/parallel/nginx/data_stream/access/_dev/test/system/test-other-config.yml b/test/packages/parallel/nginx/data_stream/access/_dev/test/system/test-other-config.yml deleted file mode 100644 index 0f73c5dd9..000000000 --- a/test/packages/parallel/nginx/data_stream/access/_dev/test/system/test-other-config.yml +++ /dev/null @@ -1,5 +0,0 @@ -vars: ~ -data_stream: - vars: - paths: - - "{{SERVICE_LOGS_DIR}}/access.log*" diff --git a/test/packages/parallel/nginx/data_stream/error/_dev/test/system/test-other-config.yml b/test/packages/parallel/nginx/data_stream/error/_dev/test/system/test-other-config.yml deleted file mode 100644 index ec7356ee9..000000000 --- a/test/packages/parallel/nginx/data_stream/error/_dev/test/system/test-other-config.yml +++ /dev/null @@ -1,5 +0,0 @@ -vars: ~ -data_stream: - vars: - paths: - - "{{SERVICE_LOGS_DIR}}/error.log*" diff --git a/test/packages/parallel/nginx/data_stream/stubstatus/_dev/test/system/test-other-config.yml b/test/packages/parallel/nginx/data_stream/stubstatus/_dev/test/system/test-other-config.yml deleted file mode 100644 index 0e5231dc8..000000000 --- a/test/packages/parallel/nginx/data_stream/stubstatus/_dev/test/system/test-other-config.yml +++ /dev/null @@ -1,6 +0,0 @@ -vars: - hosts: - - http://{{Hostname}}:{{Port}} -data_stream: - vars: - server_status_path: /server-status From 5f04b26146b44fad9b6f2b29e9dcda083bee8994 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Tue, 18 Jun 2024 16:05:24 +0200 Subject: [PATCH 39/44] Add function to check if there is any skip configuration --- internal/testrunner/runners/asset/tester.go | 13 +++---------- .../testrunner/runners/pipeline/tester.go | 16 +++------------- internal/testrunner/runners/policy/tester.go | 13 +++---------- internal/testrunner/runners/static/tester.go | 13 +++---------- internal/testrunner/runners/system/tester.go | 19 ++++++------------- internal/testrunner/testrunner.go | 9 +++++++++ 6 files changed, 27 insertions(+), 56 deletions(-) diff --git a/internal/testrunner/runners/asset/tester.go b/internal/testrunner/runners/asset/tester.go index f241b1e42..20891f10d 100644 --- a/internal/testrunner/runners/asset/tester.go +++ b/internal/testrunner/runners/asset/tester.go @@ -96,18 +96,11 @@ func (r *tester) run(ctx context.Context) ([]testrunner.TestResult, error) { return result.WithError(fmt.Errorf("unable to load asset loading test config file: %w", err)) } - if testConfig != nil && testConfig.Skip != nil { - logger.Warnf("skipping %s test for %s: %s (details: %s)", - TestType, r.testFolder.Package, - testConfig.Skip.Reason, testConfig.Skip.Link) - return result.WithSkip(testConfig.Skip) - } - - if r.globalTestConfig.Skip != nil { + if skip := testrunner.AnySkipConfig(testConfig.Skip, r.globalTestConfig.Skip); skip != nil { logger.Warnf("skipping %s test for %s/%s: %s (details: %s)", TestType, r.testFolder.Package, r.testFolder.DataStream, - r.globalTestConfig.Skip.Reason, r.globalTestConfig.Skip.Link.String()) - return result.WithSkip(r.globalTestConfig.Skip) + skip.Reason, skip.Link) + return result.WithSkip(skip) } logger.Debug("installing package...") diff --git a/internal/testrunner/runners/pipeline/tester.go b/internal/testrunner/runners/pipeline/tester.go index 1137cb388..84e838f7a 100644 --- a/internal/testrunner/runners/pipeline/tester.go +++ b/internal/testrunner/runners/pipeline/tester.go @@ -316,21 +316,11 @@ func (r *tester) runTestCase(ctx context.Context, testCaseFile string, dsPath st } tr.Name = tc.name - if tc.config.Skip != nil { + if skip := testrunner.AnySkipConfig(tc.config.Skip, r.globalTestConfig.Skip); skip != nil { logger.Warnf("skipping %s test for %s/%s: %s (details: %s)", TestType, r.testFolder.Package, r.testFolder.DataStream, - tc.config.Skip.Reason, tc.config.Skip.Link) - - tr.Skipped = tc.config.Skip - return tr, nil - } - - if r.globalTestConfig.Skip != nil { - logger.Warnf("skipping %s test for %s/%s: %s (details: %s)", - TestType, r.testFolder.Package, r.testFolder.DataStream, - r.globalTestConfig.Skip.Reason, r.globalTestConfig.Skip.Link.String()) - - tr.Skipped = r.globalTestConfig.Skip + skip.Reason, skip.Link) + tr.Skipped = skip return tr, nil } diff --git a/internal/testrunner/runners/policy/tester.go b/internal/testrunner/runners/policy/tester.go index 5a1a037e5..e0c277443 100644 --- a/internal/testrunner/runners/policy/tester.go +++ b/internal/testrunner/runners/policy/tester.go @@ -96,18 +96,11 @@ func (r *tester) runTest(ctx context.Context, manager *resources.Manager, testPa testName := testNameFromPath(testPath) - if testConfig.Skip != nil { + if skip := testrunner.AnySkipConfig(testConfig.Skip, r.globalTestConfig.Skip); skip != nil { logger.Warnf("skipping %s test for %s/%s: %s (details: %s)", TestType, r.testFolder.Package, r.testFolder.DataStream, - testConfig.Skip.Reason, testConfig.Skip.Link.String()) - return result.WithSkip(testConfig.Skip) - } - - if r.globalTestConfig.Skip != nil { - logger.Warnf("skipping %s test for %s/%s: %s (details: %s)", - TestType, r.testFolder.Package, r.testFolder.DataStream, - r.globalTestConfig.Skip.Reason, r.globalTestConfig.Skip.Link.String()) - return result.WithSkip(r.globalTestConfig.Skip) + skip.Reason, skip.Link) + return result.WithSkip(skip) } policy := resources.FleetAgentPolicy{ diff --git a/internal/testrunner/runners/static/tester.go b/internal/testrunner/runners/static/tester.go index b9f8e9095..724ae6dd1 100644 --- a/internal/testrunner/runners/static/tester.go +++ b/internal/testrunner/runners/static/tester.go @@ -72,18 +72,11 @@ func (r tester) run(ctx context.Context) ([]testrunner.TestResult, error) { return result.WithError(fmt.Errorf("unable to load asset loading test config file: %w", err)) } - if testConfig != nil && testConfig.Skip != nil { - logger.Warnf("skipping %s test for %s: %s (details: %s)", - TestType, r.testFolder.Package, - testConfig.Skip.Reason, testConfig.Skip.Link) - return result.WithSkip(testConfig.Skip) - } - - if r.globalTestConfig.Skip != nil { + if skip := testrunner.AnySkipConfig(testConfig.Skip, r.globalTestConfig.Skip); skip != nil { logger.Warnf("skipping %s test for %s/%s: %s (details: %s)", TestType, r.testFolder.Package, r.testFolder.DataStream, - r.globalTestConfig.Skip.Reason, r.globalTestConfig.Skip.Link.String()) - return result.WithSkip(r.globalTestConfig.Skip) + skip.Reason, skip.Link) + return result.WithSkip(skip) } pkgManifest, err := packages.ReadPackageManifestFromPackageRoot(r.packageRootPath) diff --git a/internal/testrunner/runners/system/tester.go b/internal/testrunner/runners/system/tester.go index f7a393933..71ee7585a 100644 --- a/internal/testrunner/runners/system/tester.go +++ b/internal/testrunner/runners/system/tester.go @@ -154,7 +154,7 @@ type tester struct { serviceStateFilePath string - globalConfig testrunner.GlobalRunnerTestConfig + globalTestConfig testrunner.GlobalRunnerTestConfig // Execution order of following handlers is defined in runner.TearDown() method. removeAgentHandler func(context.Context) error @@ -201,7 +201,7 @@ func NewSystemTester(options SystemTesterOptions) (*tester, error) { runSetup: options.RunSetup, runTestsOnly: options.RunTestsOnly, runTearDown: options.RunTearDown, - globalConfig: options.GlobalTestConfig, + globalTestConfig: options.GlobalTestConfig, } r.resourcesManager = resources.NewManager() r.resourcesManager.RegisterProvider(resources.DefaultKibanaProviderName, &resources.KibanaProvider{Client: r.kibanaClient}) @@ -274,7 +274,7 @@ func (r *tester) String() string { // Parallel indicates if this tester can run in parallel or not. func (r tester) Parallel() bool { // it is required independent Elastic Agents to run in parallel system tests - return r.runIndependentElasticAgent && r.globalConfig.Parallel + return r.runIndependentElasticAgent && r.globalTestConfig.Parallel } // Run runs the system tests defined under the given folder @@ -1381,18 +1381,11 @@ func (r *tester) validateTestScenario(ctx context.Context, result *testrunner.Re func (r *tester) runTest(ctx context.Context, config *testConfig, svcInfo servicedeployer.ServiceInfo) ([]testrunner.TestResult, error) { result := r.newResult(config.Name()) - if config.Skip != nil { + if skip := testrunner.AnySkipConfig(config.Skip, r.globalTestConfig.Skip); skip != nil { logger.Warnf("skipping %s test for %s/%s: %s (details: %s)", TestType, r.testFolder.Package, r.testFolder.DataStream, - config.Skip.Reason, config.Skip.Link) - return result.WithSkip(config.Skip) - } - - if r.globalConfig.Skip != nil { - logger.Warnf("skipping %s test for %s/%s: %s (details: %s)", - TestType, r.testFolder.Package, r.testFolder.DataStream, - r.globalConfig.Skip.Reason, r.globalConfig.Skip.Link.String()) - return result.WithSkip(r.globalConfig.Skip) + skip.Reason, skip.Link) + return result.WithSkip(skip) } logger.Debugf("running test with configuration '%s'", config.Name()) diff --git a/internal/testrunner/testrunner.go b/internal/testrunner/testrunner.go index 123eb83ae..156e53fe2 100644 --- a/internal/testrunner/testrunner.go +++ b/internal/testrunner/testrunner.go @@ -492,3 +492,12 @@ func PackageHasDataStreams(manifest *packages.PackageManifest) (bool, error) { return false, fmt.Errorf("unexpected package type %q", manifest.Type) } } + +func AnySkipConfig(configs ...*SkipConfig) *SkipConfig { + for _, config := range configs { + if config != nil { + return config + } + } + return nil +} From 59f7ef980732783f30575c5e6dd40453fd0050b0 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Tue, 18 Jun 2024 16:55:59 +0200 Subject: [PATCH 40/44] Check if test config is nil --- internal/testrunner/runners/asset/tester.go | 7 ++++++- internal/testrunner/runners/static/tester.go | 7 ++++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/internal/testrunner/runners/asset/tester.go b/internal/testrunner/runners/asset/tester.go index 20891f10d..b1aa14e71 100644 --- a/internal/testrunner/runners/asset/tester.go +++ b/internal/testrunner/runners/asset/tester.go @@ -96,7 +96,12 @@ func (r *tester) run(ctx context.Context) ([]testrunner.TestResult, error) { return result.WithError(fmt.Errorf("unable to load asset loading test config file: %w", err)) } - if skip := testrunner.AnySkipConfig(testConfig.Skip, r.globalTestConfig.Skip); skip != nil { + skipConfigs := []*testrunner.SkipConfig{r.globalTestConfig.Skip} + if testConfig != nil { + skipConfigs = append(skipConfigs, testConfig.Skip) + } + + if skip := testrunner.AnySkipConfig(skipConfigs...); skip != nil { logger.Warnf("skipping %s test for %s/%s: %s (details: %s)", TestType, r.testFolder.Package, r.testFolder.DataStream, skip.Reason, skip.Link) diff --git a/internal/testrunner/runners/static/tester.go b/internal/testrunner/runners/static/tester.go index 724ae6dd1..2c912e2ed 100644 --- a/internal/testrunner/runners/static/tester.go +++ b/internal/testrunner/runners/static/tester.go @@ -72,7 +72,12 @@ func (r tester) run(ctx context.Context) ([]testrunner.TestResult, error) { return result.WithError(fmt.Errorf("unable to load asset loading test config file: %w", err)) } - if skip := testrunner.AnySkipConfig(testConfig.Skip, r.globalTestConfig.Skip); skip != nil { + skipConfigs := []*testrunner.SkipConfig{r.globalTestConfig.Skip} + if testConfig != nil { + skipConfigs = append(skipConfigs, testConfig.Skip) + } + + if skip := testrunner.AnySkipConfig(skipConfigs...); skip != nil { logger.Warnf("skipping %s test for %s/%s: %s (details: %s)", TestType, r.testFolder.Package, r.testFolder.DataStream, skip.Reason, skip.Link) From ec7cb7862206685385fa2fbf11e59ca0daed45fa Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Wed, 19 Jun 2024 12:16:52 +0200 Subject: [PATCH 41/44] Remove TODO comments --- internal/testrunner/runners/system/tester.go | 2 -- internal/testrunner/testrunner.go | 1 - 2 files changed, 3 deletions(-) diff --git a/internal/testrunner/runners/system/tester.go b/internal/testrunner/runners/system/tester.go index 71ee7585a..c7f98999d 100644 --- a/internal/testrunner/runners/system/tester.go +++ b/internal/testrunner/runners/system/tester.go @@ -506,8 +506,6 @@ func (r *tester) tearDownTest(ctx context.Context) error { if r.deleteTestPolicyHandler != nil { if err := r.deleteTestPolicyHandler(cleanupCtx); err != nil { - // TODO: to be removed - logger.Debugf("Failed deleting test policy handler...: %v", err) return err } r.deleteTestPolicyHandler = nil diff --git a/internal/testrunner/testrunner.go b/internal/testrunner/testrunner.go index 156e53fe2..393ec314c 100644 --- a/internal/testrunner/testrunner.go +++ b/internal/testrunner/testrunner.go @@ -420,7 +420,6 @@ func runSuiteParallel(ctx context.Context, testers []Tester) ([]TestResult, erro chResults <- routineResult{nil, err} return } - // TODO: How to check if ctx is Done or Cancelled, to not call to "run" method r, err := run(ctx, tester) chResults <- routineResult{r, err} }() From 8d8fb5d75394647a432766687899b75174c10e54 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Wed, 19 Jun 2024 12:17:07 +0200 Subject: [PATCH 42/44] Rename variable --- internal/testrunner/testrunner.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/testrunner/testrunner.go b/internal/testrunner/testrunner.go index 393ec314c..13acf971e 100644 --- a/internal/testrunner/testrunner.go +++ b/internal/testrunner/testrunner.go @@ -26,7 +26,7 @@ import ( ) var ( - DEFAULT_MAXIMUM_ROUTINES = runtime.GOMAXPROCS(0) / 2 + defaultMaximumRoutines = runtime.GOMAXPROCS(0) / 2 maximumNumberParallelTest = environment.WithElasticPackagePrefix("MAXIMUM_NUMBER_PARALLEL_TESTS") ) @@ -357,7 +357,7 @@ func RunSuite(ctx context.Context, runner TestRunner) ([]TestResult, error) { func maxNumberRoutines() (int, error) { var err error - maxRoutines := DEFAULT_MAXIMUM_ROUTINES + maxRoutines := defaultMaximumRoutines v, ok := os.LookupEnv(maximumNumberParallelTest) if ok { maxRoutines, err = strconv.Atoi(v) From 9a228b9896f4cbc69ad2656203bccd8e8a4f21a5 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Wed, 19 Jun 2024 17:07:46 +0200 Subject: [PATCH 43/44] Add notes about running system tests in parallel --- docs/howto/system_testing.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/howto/system_testing.md b/docs/howto/system_testing.md index 6152f6be0..b804f4e76 100644 --- a/docs/howto/system_testing.md +++ b/docs/howto/system_testing.md @@ -796,7 +796,9 @@ ELASTIC_PACKAGE_MAXIMUM_NUMBER_PARALLEL_TESTS=5 \ elastic-package test system -v ``` -**NOTE**: Currently, just system tests support to run the tests in parallel. +**NOTE**: +- Currently, just system tests support to run tests in parallel. +- **Not recommended** to enable system tests in parallel for packages that make use of the Terraform or Kubernetes service deployers. ### Detecting ignored fields From a47379ed64c2e82fe2e8c2f529b04b8c522b644b Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Wed, 19 Jun 2024 17:35:08 +0200 Subject: [PATCH 44/44] Remove package-spec replace in go.mod --- go.mod | 2 -- go.sum | 4 ++-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/go.mod b/go.mod index 5f069db76..183ddc861 100644 --- a/go.mod +++ b/go.mod @@ -184,5 +184,3 @@ require ( sigs.k8s.io/structured-merge-diff/v4 v4.4.1 // indirect sigs.k8s.io/yaml v1.4.0 // indirect ) - -replace github.com/elastic/package-spec/v3 => github.com/elastic/package-spec/v3 v3.1.6-0.20240617154842-572e54732385 diff --git a/go.sum b/go.sum index 1eebe77ad..ab75469d2 100644 --- a/go.sum +++ b/go.sum @@ -102,8 +102,8 @@ github.com/elastic/gojsonschema v1.2.1 h1:cUMbgsz0wyEB4x7xf3zUEvUVDl6WCz2RKcQPul github.com/elastic/gojsonschema v1.2.1/go.mod h1:biw5eBS2Z4T02wjATMRSfecfjCmwaDPvuaqf844gLrg= github.com/elastic/kbncontent v0.1.4 h1:GoUkJkqkn2H6iJTnOHcxEqYVVYyjvcebLQVaSR1aSvU= github.com/elastic/kbncontent v0.1.4/go.mod h1:kOPREITK9gSJsiw/WKe7QWSO+PRiZMyEFQCw+CMLAHI= -github.com/elastic/package-spec/v3 v3.1.6-0.20240617154842-572e54732385 h1:nZcfkILwSR+OkQujlke73NjlFw2KSFcKV5LyvE7hI8s= -github.com/elastic/package-spec/v3 v3.1.6-0.20240617154842-572e54732385/go.mod h1:dH3ca6UEfn4c4LX3SwznyOEV8ZJN/YJ2PE+x0WeNHvM= +github.com/elastic/package-spec/v3 v3.1.5 h1:BoUvKZI3WhbxsWAvsOqIasgpCo8fmjmj7lZgO1JyB2s= +github.com/elastic/package-spec/v3 v3.1.5/go.mod h1:kHltfTMhDn9Whp3LZluVwI0WFW5uHwqtlByx6+c8ZHE= github.com/elazarl/goproxy v0.0.0-20230808193330-2592e75ae04a h1:mATvB/9r/3gvcejNsXKSkQ6lcIaNec2nyfOdlTBR2lU= github.com/elazarl/goproxy v0.0.0-20230808193330-2592e75ae04a/go.mod h1:Ro8st/ElPeALwNFlcTpWmkr6IoMFfkjXAvTHpevnDsM= github.com/emicklei/go-restful/v3 v3.11.0 h1:rAQeMHw1c7zTmncogyy8VvRZwtkmkZ4FxERmMY4rD+g=