From dd53426296f75294221cb5f6eb320024135fc9e2 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Mon, 20 May 2024 13:16:49 +0200 Subject: [PATCH 01/33] Install/Uninstall once package per data stream folder --- internal/testrunner/runners/system/runner.go | 47 ++++++++++++-------- 1 file changed, 28 insertions(+), 19 deletions(-) diff --git a/internal/testrunner/runners/system/runner.go b/internal/testrunner/runners/system/runner.go index 0e47c7c21..317ff69e3 100644 --- a/internal/testrunner/runners/system/runner.go +++ b/internal/testrunner/runners/system/runner.go @@ -194,7 +194,7 @@ func (r *runner) Run(ctx context.Context, options testrunner.TestOptions) ([]tes } result := r.newResult("(init)") - if err := r.initRun(); err != nil { + if err := r.initRun(ctx); err != nil { return result.WithError(err) } @@ -406,6 +406,15 @@ func (r *runner) createServiceInfo() (servicedeployer.ServiceInfo, error) { // TearDown method doesn't perform any global action as the "tear down" is executed per test case. func (r *runner) TearDown(ctx context.Context) error { + logger.Debugf("Uninstalling package...") + resourcesOptions := resourcesOptions{ + // Keep it installed only if we were running setup, or tests only. + installedPackage: r.options.RunSetup || r.options.RunTestsOnly, + } + _, err := r.resourcesManager.ApplyCtx(ctx, r.resources(resourcesOptions)) + if err != nil { + return err + } return nil } @@ -491,7 +500,7 @@ func (r *runner) newResult(name string) *testrunner.ResultComposer { }) } -func (r *runner) initRun() error { +func (r *runner) initRun(ctx context.Context) error { var err error var found bool r.locationManager, err = locations.NewLocationManager() @@ -565,12 +574,28 @@ func (r *runner) initRun() error { } } + if r.options.RunTearDown { + logger.Debug("Skip installing package") + } else { + // Install the package before creating the policy, so we control exactly what is being + // installed. + logger.Debug("Installing package...") + resourcesOptions := resourcesOptions{ + // Install it unless we are running the tear down only. + installedPackage: !r.options.RunTearDown, + } + _, err = r.resourcesManager.ApplyCtx(ctx, r.resources(resourcesOptions)) + if err != nil { + return fmt.Errorf("can't install the package: %w", err) + } + } + return nil } func (r *runner) run(ctx context.Context) (results []testrunner.TestResult, err error) { result := r.newResult("(init)") - if err = r.initRun(); err != nil { + if err = r.initRun(ctx); err != nil { return result.WithError(err) } @@ -955,22 +980,6 @@ func (r *runner) prepareScenario(ctx context.Context, config *testConfig, svcInf return nil, fmt.Errorf("unable to reload system test case configuration: %w", err) } - if r.options.RunTearDown { - logger.Debug("Skip installing package") - } else { - // Install the package before creating the policy, so we control exactly what is being - // installed. - logger.Debug("Installing package...") - resourcesOptions := resourcesOptions{ - // Install it unless we are running the tear down only. - installedPackage: !r.options.RunTearDown, - } - _, err = r.resourcesManager.ApplyCtx(ctx, r.resources(resourcesOptions)) - if err != nil { - return nil, fmt.Errorf("can't install the package: %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() From 6c448c96be1f577835df59a56aa3be0e094231ac Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Mon, 20 May 2024 14:58:30 +0200 Subject: [PATCH 02/33] Add setup and teardown runner methods to Runner interface --- cmd/testrunner.go | 52 +++++++++----- internal/testrunner/runners/asset/runner.go | 11 +++ .../testrunner/runners/pipeline/runner.go | 11 +++ internal/testrunner/runners/static/runner.go | 11 +++ internal/testrunner/runners/system/runner.go | 69 +++++++++++-------- internal/testrunner/testrunner.go | 16 +++++ 6 files changed, 125 insertions(+), 45 deletions(-) diff --git a/cmd/testrunner.go b/cmd/testrunner.go index 246c11d97..59fa3c6b7 100644 --- a/cmd/testrunner.go +++ b/cmd/testrunner.go @@ -337,25 +337,37 @@ func testTypeCommandActionFactory(runner testrunner.TestRunner) cobraext.Command fmt.Printf("Running tests per stages (technical preview)\n") } + runner, err := testrunner.NewRunner(testType) + if err != nil { + return fmt.Errorf("failed to get runner %s: %w", testType, err) + } + + runnerOptions := testrunner.TestOptions{ + Profile: profile, + PackageRootPath: packageRootPath, + GenerateTestResult: generateTestResult, + API: esAPI, + KibanaClient: kibanaClient, + DeferCleanup: deferCleanup, + ServiceVariant: variantFlag, + WithCoverage: testCoverage, + CoverageType: testCoverageFormat, + ConfigFilePath: configFileFlag, + RunSetup: runSetup, + RunTearDown: runTearDown, + RunTestsOnly: runTestsOnly, + RunIndependentElasticAgent: false, + } + + err = runner.SetupRunner(ctx, runnerOptions) + if err != nil { + return fmt.Errorf("failed to run setup runner process: %w", err) + } + var results []testrunner.TestResult for _, folder := range testFolders { - r, err := testrunner.Run(ctx, testType, testrunner.TestOptions{ - Profile: profile, - TestFolder: folder, - PackageRootPath: packageRootPath, - GenerateTestResult: generateTestResult, - API: esAPI, - KibanaClient: kibanaClient, - DeferCleanup: deferCleanup, - ServiceVariant: variantFlag, - WithCoverage: testCoverage, - CoverageType: testCoverageFormat, - ConfigFilePath: configFileFlag, - RunSetup: runSetup, - RunTearDown: runTearDown, - RunTestsOnly: runTestsOnly, - RunIndependentElasticAgent: false, - }) + runnerOptions.TestFolder = folder + r, err := testrunner.Run(ctx, testType, runnerOptions) // Results must be appended even if there is an error, since there could be // tests (e.g. system tests) that return both error and results. @@ -367,6 +379,11 @@ func testTypeCommandActionFactory(runner testrunner.TestRunner) cobraext.Command } + err = runner.TearDownRunner(ctx) + if err != nil { + return fmt.Errorf("failed to run tear down runner process: %w", err) + } + format := testrunner.TestReportFormat(reportFormat) report, err := testrunner.FormatReport(format, results) if err != nil { @@ -390,6 +407,7 @@ func testTypeCommandActionFactory(runner testrunner.TestRunner) cobraext.Command return errors.New("one or more test cases failed") } } + return nil } } diff --git a/internal/testrunner/runners/asset/runner.go b/internal/testrunner/runners/asset/runner.go index 11451dec9..25e7f4482 100644 --- a/internal/testrunner/runners/asset/runner.go +++ b/internal/testrunner/runners/asset/runner.go @@ -46,6 +46,17 @@ func (r runner) String() string { return "asset loading" } +// SetupRunner prepares global resources required by the test runner. +func (r runner) SetupRunner(ctx context.Context, options testrunner.TestOptions) error { + return nil +} + +// TearDownRunner cleans up any global test runner resources. It must be called +// after the test runner has finished executing all its tests. +func (r runner) TearDownRunner(ctx context.Context) error { + return nil +} + // CanRunPerDataStream returns whether this test runner can run on individual // data streams within the package. func (r runner) CanRunPerDataStream() bool { diff --git a/internal/testrunner/runners/pipeline/runner.go b/internal/testrunner/runners/pipeline/runner.go index 14fa843e0..a8e5f5a45 100644 --- a/internal/testrunner/runners/pipeline/runner.go +++ b/internal/testrunner/runners/pipeline/runner.go @@ -71,6 +71,17 @@ func (r *runner) String() string { return "pipeline" } +// SetupRunner prepares global resources required by the test runner. +func (r runner) SetupRunner(ctx context.Context, options testrunner.TestOptions) error { + return nil +} + +// TearDownRunner cleans up any global test runner resources. It must be called +// after the test runner has finished executing all its tests. +func (r runner) TearDownRunner(ctx context.Context) error { + return nil +} + // Run runs the pipeline tests defined under the given folder func (r *runner) Run(ctx context.Context, options testrunner.TestOptions) ([]testrunner.TestResult, error) { r.options = options diff --git a/internal/testrunner/runners/static/runner.go b/internal/testrunner/runners/static/runner.go index b6580d864..53be20fef 100644 --- a/internal/testrunner/runners/static/runner.go +++ b/internal/testrunner/runners/static/runner.go @@ -45,6 +45,17 @@ func (r runner) String() string { return "static files" } +// SetupRunner prepares global resources required by the test runner. +func (r runner) SetupRunner(ctx context.Context, options testrunner.TestOptions) error { + return nil +} + +// TearDownRunner cleans up any global test runner resources. It must be called +// after the test runner has finished executing all its tests. +func (r runner) TearDownRunner(ctx context.Context) error { + return nil +} + func (r runner) Run(ctx context.Context, options testrunner.TestOptions) ([]testrunner.TestResult, error) { r.options = options return r.run(ctx) diff --git a/internal/testrunner/runners/system/runner.go b/internal/testrunner/runners/system/runner.go index 317ff69e3..317f2c230 100644 --- a/internal/testrunner/runners/system/runner.go +++ b/internal/testrunner/runners/system/runner.go @@ -170,6 +170,47 @@ func (r *runner) String() string { return "system" } +// SetupRunner prepares global resources required by the test runner. +func (r *runner) SetupRunner(ctx context.Context, options testrunner.TestOptions) error { + r.options = options + + r.resourcesManager = resources.NewManager() + r.resourcesManager.RegisterProvider(resources.DefaultKibanaProviderName, &resources.KibanaProvider{Client: r.options.KibanaClient}) + + if r.options.RunTearDown { + logger.Debug("Skip installing package") + } else { + // Install the package before creating the policy, so we control exactly what is being + // installed. + logger.Debug("Installing package...") + resourcesOptions := resourcesOptions{ + // Install it unless we are running the tear down only. + installedPackage: !r.options.RunTearDown, + } + _, err := r.resourcesManager.ApplyCtx(ctx, r.resources(resourcesOptions)) + if err != nil { + return fmt.Errorf("can't install the package: %w", err) + } + } + + return nil +} + +// TearDownRunner cleans up any global test runner resources. It must be called +// after the test runner has finished executing all its tests. +func (r *runner) TearDownRunner(ctx context.Context) error { + logger.Debugf("Uninstalling package...") + resourcesOptions := resourcesOptions{ + // Keep it installed only if we were running setup, or tests only. + installedPackage: r.options.RunSetup || r.options.RunTestsOnly, + } + _, err := r.resourcesManager.ApplyCtx(ctx, r.resources(resourcesOptions)) + if err != nil { + return err + } + return nil +} + // CanRunPerDataStream returns whether this test runner can run on individual // data streams within the package. func (r *runner) CanRunPerDataStream() bool { @@ -406,15 +447,6 @@ func (r *runner) createServiceInfo() (servicedeployer.ServiceInfo, error) { // TearDown method doesn't perform any global action as the "tear down" is executed per test case. func (r *runner) TearDown(ctx context.Context) error { - logger.Debugf("Uninstalling package...") - resourcesOptions := resourcesOptions{ - // Keep it installed only if we were running setup, or tests only. - installedPackage: r.options.RunSetup || r.options.RunTestsOnly, - } - _, err := r.resourcesManager.ApplyCtx(ctx, r.resources(resourcesOptions)) - if err != nil { - return err - } return nil } @@ -508,9 +540,6 @@ func (r *runner) initRun(ctx context.Context) error { return fmt.Errorf("reading service logs directory failed: %w", err) } - r.resourcesManager = resources.NewManager() - r.resourcesManager.RegisterProvider(resources.DefaultKibanaProviderName, &resources.KibanaProvider{Client: r.options.KibanaClient}) - r.serviceStateFilePath = filepath.Join(testrunner.StateFolderPath(r.options.Profile.ProfilePath), testrunner.ServiceStateFileName) r.dataStreamPath, found, err = packages.FindDataStreamRootForPath(r.options.TestFolder.Path) @@ -574,22 +603,6 @@ func (r *runner) initRun(ctx context.Context) error { } } - if r.options.RunTearDown { - logger.Debug("Skip installing package") - } else { - // Install the package before creating the policy, so we control exactly what is being - // installed. - logger.Debug("Installing package...") - resourcesOptions := resourcesOptions{ - // Install it unless we are running the tear down only. - installedPackage: !r.options.RunTearDown, - } - _, err = r.resourcesManager.ApplyCtx(ctx, r.resources(resourcesOptions)) - if err != nil { - return fmt.Errorf("can't install the package: %w", err) - } - } - return nil } diff --git a/internal/testrunner/testrunner.go b/internal/testrunner/testrunner.go index 8a87ec231..c32956a61 100644 --- a/internal/testrunner/testrunner.go +++ b/internal/testrunner/testrunner.go @@ -70,6 +70,13 @@ type TestRunner interface { TestFolderRequired() bool CanRunSetupTeardownIndependent() bool + + // SetupRunner prepares global resources required by the test runner. + SetupRunner(context.Context, TestOptions) error + + // TearDownRunner cleans up any global test runner resources. It must be called + // after the test runner has finished executing all its tests. + TearDownRunner(context.Context) error } var runners = map[TestType]TestRunner{} @@ -284,6 +291,15 @@ func RegisterRunner(runner TestRunner) { runners[runner.Type()] = runner } +func NewRunner(testType TestType) (TestRunner, error) { + runner, defined := runners[testType] + if !defined { + return nil, fmt.Errorf("unregistered runner test: %s", testType) + } + + return runner, nil +} + // Run method delegates execution to the registered test runner, based on the test type. func Run(ctx context.Context, testType TestType, options TestOptions) ([]TestResult, error) { runner, defined := runners[testType] From 0c404176d366d5a2e880e491746e5772af05d822 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Mon, 20 May 2024 15:30:25 +0200 Subject: [PATCH 03/33] Avoid re-creating runner --- cmd/testrunner.go | 2 +- internal/testrunner/testrunner.go | 7 +------ 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/cmd/testrunner.go b/cmd/testrunner.go index 59fa3c6b7..2e5d09200 100644 --- a/cmd/testrunner.go +++ b/cmd/testrunner.go @@ -367,7 +367,7 @@ func testTypeCommandActionFactory(runner testrunner.TestRunner) cobraext.Command var results []testrunner.TestResult for _, folder := range testFolders { runnerOptions.TestFolder = folder - r, err := testrunner.Run(ctx, testType, runnerOptions) + r, err := testrunner.Run(ctx, runner, runnerOptions) // Results must be appended even if there is an error, since there could be // tests (e.g. system tests) that return both error and results. diff --git a/internal/testrunner/testrunner.go b/internal/testrunner/testrunner.go index c32956a61..2c37a1a05 100644 --- a/internal/testrunner/testrunner.go +++ b/internal/testrunner/testrunner.go @@ -301,12 +301,7 @@ func NewRunner(testType TestType) (TestRunner, error) { } // Run method delegates execution to the registered test runner, based on the test type. -func Run(ctx context.Context, testType TestType, options TestOptions) ([]TestResult, error) { - runner, defined := runners[testType] - if !defined { - return nil, fmt.Errorf("unregistered runner test: %s", testType) - } - +func Run(ctx context.Context, runner TestRunner, options TestOptions) ([]TestResult, error) { results, err := runner.Run(ctx, options) tdErr := runner.TearDown(ctx) if err != nil { From 6221c2ef9a5323c4c9550132f07d96cbe683042a Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Mon, 20 May 2024 18:01:33 +0200 Subject: [PATCH 04/33] Debug - show timestamp first doc --- internal/testrunner/runners/system/runner.go | 48 ++++++++++++++++++-- 1 file changed, 43 insertions(+), 5 deletions(-) diff --git a/internal/testrunner/runners/system/runner.go b/internal/testrunner/runners/system/runner.go index 317f2c230..e51197b8f 100644 --- a/internal/testrunner/runners/system/runner.go +++ b/internal/testrunner/runners/system/runner.go @@ -1037,12 +1037,12 @@ func (r *runner) prepareScenario(ctx context.Context, config *testConfig, svcInf case r.options.RunTestsOnly: // In this mode, service is still running and the agent is sending documents, so sometimes // cannot be guaranteed to be zero documents - err := r.deleteOldDocumentsDataStreamAndWait(ctx, scenario.dataStream, false) + err := r.deleteOldDocumentsDataStreamAndWait(ctx, scenario.dataStream, false, scenario.syntheticEnabled) if err != nil { return nil, err } default: - err := r.deleteOldDocumentsDataStreamAndWait(ctx, scenario.dataStream, true) + err := r.deleteOldDocumentsDataStreamAndWait(ctx, scenario.dataStream, true, scenario.syntheticEnabled) if err != nil { return nil, err } @@ -1387,24 +1387,61 @@ func (r *runner) writeScenarioState(opts scenarioStateOpts) error { return nil } -func (r *runner) deleteOldDocumentsDataStreamAndWait(ctx context.Context, dataStream string, mustBeZero bool) error { +func timestampFirstDoc(docs *hits, syntheticEnabled bool) (time.Time, error) { + if docs.size() == 0 { + return time.Now().AddDate(0, 0, -1), nil + } + firstDoc := docs.getDocs(syntheticEnabled)[0] + field, err := firstDoc.GetValue("@timestamp") + if err != nil { + return time.Time{}, fmt.Errorf("failed to get @timestamp key: %w", err) + } + val, ok := field.(string) + if !ok { + return time.Time{}, fmt.Errorf("invalid timestamp: %t", ok) + } + timestampBefore, err := time.Parse(time.RFC3339, val) + if err != nil { + return time.Time{}, fmt.Errorf("failed to parse timestamp %q: %w", val, err) + } + return timestampBefore, nil +} + +func (r *runner) deleteOldDocumentsDataStreamAndWait(ctx context.Context, dataStream string, mustBeZero bool, syntheticEnabled bool) error { logger.Debugf("Delete previous documents in data stream %q", dataStream) + logger.Debugf("First check") + startHits, err := r.getDocs(ctx, dataStream) + if err != nil { + return err + } + + timeBefore, _ := timestampFirstDoc(startHits, syntheticEnabled) + logger.Debugf("Timestamp before doc: %s", timeBefore) + if err := deleteDataStreamDocs(ctx, r.options.API, dataStream); err != nil { return fmt.Errorf("error deleting old data in data stream: %s: %w", dataStream, err) } - startHits, err := r.getDocs(ctx, dataStream) + + logger.Debugf("First check") + hits, err := r.getDocs(ctx, dataStream) if err != nil { return err } + timeAfter, _ := timestampFirstDoc(hits, syntheticEnabled) + logger.Debugf("Timestamp after doc: %s", timeAfter) + // First call already reports zero documents - if startHits.size() == 0 { + if hits.size() == 0 || (!mustBeZero && hits.size() < startHits.size()) { return nil } + logger.Debugf("Loop check") cleared, err := wait.UntilTrue(ctx, func(ctx context.Context) (bool, error) { hits, err := r.getDocs(ctx, dataStream) if err != nil { return false, err } + timeLoop, _ := timestampFirstDoc(hits, syntheticEnabled) + logger.Debugf("Timestamp loop doc: %s", timeLoop) if mustBeZero { return hits.size() == 0, nil @@ -1944,6 +1981,7 @@ func deleteDataStreamDocs(ctx context.Context, api *elasticsearch.API, dataStrea if resp.StatusCode == http.StatusNotFound { // Unavailable index is ok, this means that data is already not there. + logger.Debugf("Failed but ignored with status not found %s: %s", dataStream, resp.String()) return nil } if resp.IsError() { From 0e4e8e1cb09377194d6e8415026ec3f02a120627 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Mon, 20 May 2024 18:25:16 +0200 Subject: [PATCH 05/33] Remove unused context --- internal/testrunner/runners/system/runner.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/testrunner/runners/system/runner.go b/internal/testrunner/runners/system/runner.go index e51197b8f..1e7198986 100644 --- a/internal/testrunner/runners/system/runner.go +++ b/internal/testrunner/runners/system/runner.go @@ -235,7 +235,7 @@ func (r *runner) Run(ctx context.Context, options testrunner.TestOptions) ([]tes } result := r.newResult("(init)") - if err := r.initRun(ctx); err != nil { + if err := r.initRun(); err != nil { return result.WithError(err) } @@ -532,7 +532,7 @@ func (r *runner) newResult(name string) *testrunner.ResultComposer { }) } -func (r *runner) initRun(ctx context.Context) error { +func (r *runner) initRun() error { var err error var found bool r.locationManager, err = locations.NewLocationManager() @@ -608,7 +608,7 @@ func (r *runner) initRun(ctx context.Context) error { func (r *runner) run(ctx context.Context) (results []testrunner.TestResult, err error) { result := r.newResult("(init)") - if err = r.initRun(ctx); err != nil { + if err = r.initRun(); err != nil { return result.WithError(err) } From d85ce79408497ed9b0f60a745ffde02a28fcf8cf Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Mon, 20 May 2024 18:48:06 +0200 Subject: [PATCH 06/33] Check timestamps --- internal/testrunner/runners/system/runner.go | 35 +++++++++++++++----- 1 file changed, 26 insertions(+), 9 deletions(-) diff --git a/internal/testrunner/runners/system/runner.go b/internal/testrunner/runners/system/runner.go index 1e7198986..1df09dd08 100644 --- a/internal/testrunner/runners/system/runner.go +++ b/internal/testrunner/runners/system/runner.go @@ -1415,23 +1415,26 @@ func (r *runner) deleteOldDocumentsDataStreamAndWait(ctx context.Context, dataSt return err } - timeBefore, _ := timestampFirstDoc(startHits, syntheticEnabled) - logger.Debugf("Timestamp before doc: %s", timeBefore) + timestampFound := true + + timestampInitial, err := timestampFirstDoc(startHits, syntheticEnabled) + if err != nil { + logger.Warnf("Not found @timestamp in docs from data stream %s", dataStream) + timestampFound = false + } + logger.Debugf("Timestamp before doc: %s", timestampInitial) if err := deleteDataStreamDocs(ctx, r.options.API, dataStream); err != nil { return fmt.Errorf("error deleting old data in data stream: %s: %w", dataStream, err) } - logger.Debugf("First check") hits, err := r.getDocs(ctx, dataStream) if err != nil { return err } - timeAfter, _ := timestampFirstDoc(hits, syntheticEnabled) - logger.Debugf("Timestamp after doc: %s", timeAfter) // First call already reports zero documents - if hits.size() == 0 || (!mustBeZero && hits.size() < startHits.size()) { + if hits.size() == 0 { return nil } logger.Debugf("Loop check") @@ -1440,13 +1443,27 @@ func (r *runner) deleteOldDocumentsDataStreamAndWait(ctx context.Context, dataSt if err != nil { return false, err } - timeLoop, _ := timestampFirstDoc(hits, syntheticEnabled) - logger.Debugf("Timestamp loop doc: %s", timeLoop) + timestampLoop, err := timestampFirstDoc(hits, syntheticEnabled) + if err != nil { + logger.Warnf("Not found @timestamp in docs from data stream %s", dataStream) + } + logger.Debugf("Timestamp loop doc: %s", timestampLoop) if mustBeZero { return hits.size() == 0, nil } - return startHits.size() > hits.size(), nil + if startHits.size() > hits.size() { + return true, nil + } + + if !timestampFound { + return false, nil + } + + if timestampLoop.After(timestampInitial) { + logger.Debug(">>> Found first doc newer than the original") + } + return timestampLoop.After(timestampInitial), nil }, 1*time.Second, 2*time.Minute) if err != nil || !cleared { if err == nil { From dedb5174e01cf92ed38920cd109f0040a2fc58ce Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Tue, 21 May 2024 10:27:20 +0200 Subject: [PATCH 07/33] Add condition to check if hits is zero to finish loop --- internal/testrunner/runners/system/runner.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/testrunner/runners/system/runner.go b/internal/testrunner/runners/system/runner.go index 1df09dd08..56d788134 100644 --- a/internal/testrunner/runners/system/runner.go +++ b/internal/testrunner/runners/system/runner.go @@ -1389,7 +1389,7 @@ func (r *runner) writeScenarioState(opts scenarioStateOpts) error { func timestampFirstDoc(docs *hits, syntheticEnabled bool) (time.Time, error) { if docs.size() == 0 { - return time.Now().AddDate(0, 0, -1), nil + return time.Now().AddDate(0, 0, 1), nil } firstDoc := docs.getDocs(syntheticEnabled)[0] field, err := firstDoc.GetValue("@timestamp") @@ -1452,7 +1452,7 @@ func (r *runner) deleteOldDocumentsDataStreamAndWait(ctx context.Context, dataSt if mustBeZero { return hits.size() == 0, nil } - if startHits.size() > hits.size() { + if hits.size() == 0 || startHits.size() > hits.size() { return true, nil } From 3c18264a6890e4b30ea09d8114552655fdcc65c9 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Tue, 21 May 2024 12:36:09 +0200 Subject: [PATCH 08/33] Move first check inside loop --- internal/testrunner/runners/system/runner.go | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/internal/testrunner/runners/system/runner.go b/internal/testrunner/runners/system/runner.go index 56d788134..157cca6b7 100644 --- a/internal/testrunner/runners/system/runner.go +++ b/internal/testrunner/runners/system/runner.go @@ -1428,15 +1428,6 @@ func (r *runner) deleteOldDocumentsDataStreamAndWait(ctx context.Context, dataSt return fmt.Errorf("error deleting old data in data stream: %s: %w", dataStream, err) } - hits, err := r.getDocs(ctx, dataStream) - if err != nil { - return err - } - - // First call already reports zero documents - if hits.size() == 0 { - return nil - } logger.Debugf("Loop check") cleared, err := wait.UntilTrue(ctx, func(ctx context.Context) (bool, error) { hits, err := r.getDocs(ctx, dataStream) @@ -1463,8 +1454,9 @@ func (r *runner) deleteOldDocumentsDataStreamAndWait(ctx context.Context, dataSt if timestampLoop.After(timestampInitial) { logger.Debug(">>> Found first doc newer than the original") } - return timestampLoop.After(timestampInitial), nil - }, 1*time.Second, 2*time.Minute) + return false, nil + // return timestampLoop.After(timestampInitial), nil + }, 500*time.Millisecond, 2*time.Minute) if err != nil || !cleared { if err == nil { err = errors.New("unable to clear previous data") From 3e80680f715d856ec1000793ac4edd82906d6ba1 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Tue, 21 May 2024 13:42:56 +0200 Subject: [PATCH 09/33] Set different waiting times --- internal/testrunner/runners/system/runner.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/internal/testrunner/runners/system/runner.go b/internal/testrunner/runners/system/runner.go index 157cca6b7..96ac93456 100644 --- a/internal/testrunner/runners/system/runner.go +++ b/internal/testrunner/runners/system/runner.go @@ -1414,6 +1414,11 @@ func (r *runner) deleteOldDocumentsDataStreamAndWait(ctx context.Context, dataSt if err != nil { return err } + waitingTime := 1 * time.Second + if r.options.RunTestsOnly { + // Service is already running and Elastic Agent is injecting the corresponding documents + waitingTime = 500 * time.Millisecond + } timestampFound := true @@ -1456,7 +1461,7 @@ func (r *runner) deleteOldDocumentsDataStreamAndWait(ctx context.Context, dataSt } return false, nil // return timestampLoop.After(timestampInitial), nil - }, 500*time.Millisecond, 2*time.Minute) + }, waitingTime, 2*time.Minute) if err != nil || !cleared { if err == nil { err = errors.New("unable to clear previous data") From da987333bff889d21e4178b639b24d14ec320d5a Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Thu, 23 May 2024 18:35:01 +0200 Subject: [PATCH 10/33] Create policy for no-provision - missing to re-assign policy --- internal/testrunner/runners/system/runner.go | 30 ++++++++++++++++---- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/internal/testrunner/runners/system/runner.go b/internal/testrunner/runners/system/runner.go index acd1236d1..f3ad1eaca 100644 --- a/internal/testrunner/runners/system/runner.go +++ b/internal/testrunner/runners/system/runner.go @@ -971,13 +971,33 @@ func (r *runner) prepareScenario(ctx context.Context, config *testConfig, svcInf } } + testTime := time.Now().Format("20060102T15:04:05Z") + + policyTesting := kibana.Policy{ + Name: fmt.Sprintf("ep-one-test-system-%s-%s-%s", r.options.TestFolder.Package, r.options.TestFolder.DataStream, testTime), + Description: fmt.Sprintf("test policy created by elastic-package test system for data stream %s/%s", r.options.TestFolder.Package, r.options.TestFolder.DataStream), + Namespace: createTestRunID(), + } + var policyForTesting *kibana.Policy + if r.options.RunTestsOnly { + policyForTesting, err = r.options.KibanaClient.CreatePolicy(ctx, policyTesting) + if err != nil { + return nil, fmt.Errorf("could not create test policy: %w", err) + } + policyToTest = policyForTesting + } + // 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, *scenario.pkgManifest, policyTemplate, *scenario.dataStreamManifest, *config, svcInfo.Test.RunID) - if r.options.RunTearDown || r.options.RunTestsOnly { + suffixDatastream := svcInfo.Test.RunID + if r.options.RunTestsOnly { + suffixDatastream = policyTesting.Namespace + } + ds := createPackageDatastream(*policyToTest, *scenario.pkgManifest, policyTemplate, *scenario.dataStreamManifest, *config, suffixDatastream) + if r.options.RunTearDown { logger.Debug("Skip adding data stream config to policy") } else { if err := r.options.KibanaClient.AddPackageDataStreamToPolicy(ctx, ds); err != nil { @@ -1100,7 +1120,7 @@ func (r *runner) prepareScenario(ctx context.Context, config *testConfig, svcInf return nil } - if r.options.RunTearDown || r.options.RunTestsOnly { + if r.options.RunTearDown { logger.Debug("Skip assiging package data stream to agent") } else { policyWithDataStream, err := r.options.KibanaClient.GetPolicy(ctx, policyToTest.ID) @@ -1579,7 +1599,7 @@ func createIntegrationPackageDatastream( ) kibana.PackageDataStream { r := kibana.PackageDataStream{ Name: fmt.Sprintf("%s-%s-%s", pkg.Name, ds.Name, suffix), - Namespace: "ep", + Namespace: kibanaPolicy.Namespace, PolicyID: kibanaPolicy.ID, Enabled: true, Inputs: []kibana.Input{ @@ -1633,7 +1653,7 @@ func createInputPackageDatastream( ) kibana.PackageDataStream { r := kibana.PackageDataStream{ Name: fmt.Sprintf("%s-%s-%s", pkg.Name, policyTemplate.Name, suffix), - Namespace: "ep", + Namespace: kibanaPolicy.Namespace, PolicyID: kibanaPolicy.ID, Enabled: true, } From dd9bf60c0542245400a77fc12370f9b79b8260f6 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Thu, 23 May 2024 18:54:50 +0200 Subject: [PATCH 11/33] Ensure test policy is re-assigned --- internal/testrunner/runners/system/runner.go | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/internal/testrunner/runners/system/runner.go b/internal/testrunner/runners/system/runner.go index f3ad1eaca..dd97d0e9e 100644 --- a/internal/testrunner/runners/system/runner.go +++ b/internal/testrunner/runners/system/runner.go @@ -278,7 +278,14 @@ func (r *runner) Run(ctx context.Context, options testrunner.TestOptions) ([]tes if err != nil { return result.WithError(fmt.Errorf("failed to prepare scenario: %w", err)) } - return r.validateTestScenario(ctx, result, scenario, testConfig) + results, err := r.validateTestScenario(ctx, result, scenario, testConfig) + // run re-assign policy + tdErr := r.resetAgentPolicyHandler(ctx) + if tdErr != nil { + logger.Errorf("failed to reassign policy: %s", tdErr) + } + return results, err + } if r.options.RunTearDown { @@ -1076,7 +1083,7 @@ func (r *runner) prepareScenario(ctx context.Context, config *testConfig, svcInf return nil } - if r.options.RunTearDown || r.options.RunTestsOnly { + if r.options.RunTearDown { origPolicy = serviceStateData.OrigPolicy logger.Debugf("Got orig policy from file: %q - %q", origPolicy.Name, origPolicy.ID) } else { @@ -1088,7 +1095,7 @@ func (r *runner) prepareScenario(ctx context.Context, config *testConfig, svcInf } // Assign policy to agent r.resetAgentPolicyHandler = func(ctx context.Context) error { - if r.options.RunIndependentElasticAgent { + if r.options.RunIndependentElasticAgent && !r.options.RunTestsOnly { return nil } logger.Debug("reassigning original policy back to agent...") From df6268a96c17bfb10c94f3d7cd5dfa16b87e97cf Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Thu, 23 May 2024 18:59:07 +0200 Subject: [PATCH 12/33] Delete agent policies created for each test --- internal/testrunner/runners/system/runner.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/internal/testrunner/runners/system/runner.go b/internal/testrunner/runners/system/runner.go index dd97d0e9e..5bb6177bf 100644 --- a/internal/testrunner/runners/system/runner.go +++ b/internal/testrunner/runners/system/runner.go @@ -1102,6 +1102,10 @@ func (r *runner) prepareScenario(ctx context.Context, config *testConfig, svcInf if err := r.options.KibanaClient.AssignPolicyToAgent(ctx, agent, origPolicy); err != nil { return fmt.Errorf("error reassigning original policy to agent: %w", err) } + if r.options.RunTestsOnly { + // Clean up policies created + return r.options.KibanaClient.DeletePolicy(ctx, policyToTest.ID) + } return nil } From ed1411e8e0bd9097360c40705bb1a3a0e4ba4018 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Thu, 23 May 2024 19:38:14 +0200 Subject: [PATCH 13/33] delete datastream created for testing --- internal/testrunner/runners/system/runner.go | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/internal/testrunner/runners/system/runner.go b/internal/testrunner/runners/system/runner.go index 5bb6177bf..a0dc0db51 100644 --- a/internal/testrunner/runners/system/runner.go +++ b/internal/testrunner/runners/system/runner.go @@ -1102,9 +1102,21 @@ func (r *runner) prepareScenario(ctx context.Context, config *testConfig, svcInf if err := r.options.KibanaClient.AssignPolicyToAgent(ctx, agent, origPolicy); err != nil { return fmt.Errorf("error reassigning original policy to agent: %w", err) } - if r.options.RunTestsOnly { - // Clean up policies created - return r.options.KibanaClient.DeletePolicy(ctx, policyToTest.ID) + if !r.options.RunTestsOnly { + return nil + } + + logger.Debug("Deleting test policy...") + err = r.options.KibanaClient.DeletePolicy(ctx, policyToTest.ID) + if err != nil { + return fmt.Errorf("failed to delete policy %s: %w", policyToTest.Name, err) + } + logger.Debug("Deleting data stream for testing") + _, err := r.options.API.Indices.DeleteDataStream([]string{scenario.dataStream}, + r.options.API.Indices.DeleteDataStream.WithContext(ctx), + ) + if err != nil { + return fmt.Errorf("failed to delete data stream %s: %w", scenario.dataStream, err) } return nil } From 1c55b1cb607759bd6148d45896c1740b5ec2fbc4 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Fri, 24 May 2024 13:37:42 +0200 Subject: [PATCH 14/33] Apply changes to all scenarios --- internal/testrunner/runners/system/runner.go | 82 +++++++++++++------- 1 file changed, 53 insertions(+), 29 deletions(-) diff --git a/internal/testrunner/runners/system/runner.go b/internal/testrunner/runners/system/runner.go index a0dc0db51..d50461224 100644 --- a/internal/testrunner/runners/system/runner.go +++ b/internal/testrunner/runners/system/runner.go @@ -828,6 +828,37 @@ type scenarioTest struct { startTestTime time.Time } +func (r *runner) shouldCreateNewAgentPolicyForTest() bool { + if r.options.RunTestsOnly { + // always that --no-provision is set, it should create new Agent Policies. + return true + } + if !r.options.RunIndependentElasticAgent { + // keep same behaviour as previously when Elastic Agent of the stack is used. + return false + + } + // No need to create new Agent Policies for these stages + if r.options.RunSetup || r.options.RunTearDown { + return false + } + return true +} + +func (r *runner) deleteDataStream(ctx context.Context, dataStream string) error { + resp, err := r.options.API.Indices.DeleteDataStream([]string{dataStream}, + r.options.API.Indices.DeleteDataStream.WithContext(ctx), + ) + if err != nil { + return fmt.Errorf("failed to delete data stream %s: %w", dataStream, err) + } + defer resp.Body.Close() + if resp.IsError() { + return fmt.Errorf("could not get delete data stream %s: %s", dataStream, resp.String()) + } + return nil +} + func (r *runner) prepareScenario(ctx context.Context, config *testConfig, svcInfo servicedeployer.ServiceInfo) (*scenarioTest, error) { serviceOptions := r.createServiceOptions(config.ServiceVariantName) @@ -978,32 +1009,26 @@ func (r *runner) prepareScenario(ctx context.Context, config *testConfig, svcInf } } - testTime := time.Now().Format("20060102T15:04:05Z") + // 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() + suffixDatastream := svcInfo.Test.RunID + logger.Debug("adding package data stream to test policy...") policyTesting := kibana.Policy{ - Name: fmt.Sprintf("ep-one-test-system-%s-%s-%s", r.options.TestFolder.Package, r.options.TestFolder.DataStream, testTime), + Name: fmt.Sprintf("ep-one-test-system-%s-%s-%s", r.options.TestFolder.Package, r.options.TestFolder.DataStream, scenario.startTestTime.Format("20060102T15:04:05Z")), Description: fmt.Sprintf("test policy created by elastic-package test system for data stream %s/%s", r.options.TestFolder.Package, r.options.TestFolder.DataStream), Namespace: createTestRunID(), } - var policyForTesting *kibana.Policy - if r.options.RunTestsOnly { - policyForTesting, err = r.options.KibanaClient.CreatePolicy(ctx, policyTesting) + policyToAssignDatastreamTests := policyToTest + if r.shouldCreateNewAgentPolicyForTest() { + policyToAssignDatastreamTests, err = r.options.KibanaClient.CreatePolicy(ctx, policyTesting) if err != nil { return nil, fmt.Errorf("could not create test policy: %w", err) } - policyToTest = policyForTesting - } - - // 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...") - suffixDatastream := svcInfo.Test.RunID - if r.options.RunTestsOnly { suffixDatastream = policyTesting.Namespace } - ds := createPackageDatastream(*policyToTest, *scenario.pkgManifest, policyTemplate, *scenario.dataStreamManifest, *config, suffixDatastream) + ds := createPackageDatastream(*policyToAssignDatastreamTests, *scenario.pkgManifest, policyTemplate, *scenario.dataStreamManifest, *config, suffixDatastream) if r.options.RunTearDown { logger.Debug("Skip adding data stream config to policy") } else { @@ -1095,26 +1120,25 @@ func (r *runner) prepareScenario(ctx context.Context, config *testConfig, svcInf } // Assign policy to agent r.resetAgentPolicyHandler = func(ctx context.Context) error { - if r.options.RunIndependentElasticAgent && !r.options.RunTestsOnly { - return nil - } - logger.Debug("reassigning original policy back to agent...") - if err := r.options.KibanaClient.AssignPolicyToAgent(ctx, agent, origPolicy); err != nil { - return fmt.Errorf("error reassigning original policy to agent: %w", err) + if !r.options.RunSetup || !r.options.RunTearDown { + // it should be kept the same policy just when system tests are + // triggered with the flags for just setup or just tear-down + logger.Debug("reassigning original policy back to agent...") + if err := r.options.KibanaClient.AssignPolicyToAgent(ctx, agent, origPolicy); err != nil { + return fmt.Errorf("error reassigning original policy to agent: %w", err) + } } - if !r.options.RunTestsOnly { + if !r.shouldCreateNewAgentPolicyForTest() { return nil } logger.Debug("Deleting test policy...") - err = r.options.KibanaClient.DeletePolicy(ctx, policyToTest.ID) + err = r.options.KibanaClient.DeletePolicy(ctx, policyToAssignDatastreamTests.ID) if err != nil { - return fmt.Errorf("failed to delete policy %s: %w", policyToTest.Name, err) + return fmt.Errorf("failed to delete policy %s: %w", policyToAssignDatastreamTests.Name, err) } logger.Debug("Deleting data stream for testing") - _, err := r.options.API.Indices.DeleteDataStream([]string{scenario.dataStream}, - r.options.API.Indices.DeleteDataStream.WithContext(ctx), - ) + r.deleteDataStream(ctx, scenario.dataStream) if err != nil { return fmt.Errorf("failed to delete data stream %s: %w", scenario.dataStream, err) } @@ -1146,7 +1170,7 @@ func (r *runner) prepareScenario(ctx context.Context, config *testConfig, svcInf if r.options.RunTearDown { logger.Debug("Skip assiging package data stream to agent") } else { - policyWithDataStream, err := r.options.KibanaClient.GetPolicy(ctx, policyToTest.ID) + policyWithDataStream, err := r.options.KibanaClient.GetPolicy(ctx, policyToAssignDatastreamTests.ID) if err != nil { return nil, fmt.Errorf("could not read the policy with data stream: %w", err) } From 2bca8d80a46158281e9743b57b2daa28e832d1f3 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Fri, 24 May 2024 13:40:38 +0200 Subject: [PATCH 15/33] Skip tests with terraform --- .buildkite/pipeline.trigger.integration.tests.sh | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.buildkite/pipeline.trigger.integration.tests.sh b/.buildkite/pipeline.trigger.integration.tests.sh index dc1e4d2ad..4214370ea 100755 --- a/.buildkite/pipeline.trigger.integration.tests.sh +++ b/.buildkite/pipeline.trigger.integration.tests.sh @@ -96,6 +96,10 @@ for package in $(find . -maxdepth 1 -mindepth 1 -type d) ; do label_suffix=" (independent agent)" fi package_name=$(basename "${package}") + if [[ "${package_name}" == "gcp" || "${package_name}" == "aws" || "${package_name}" == "aws_logs" ]] ; then + echoerr "Skip package temporarily ${packager_name}" + continue + fi if [[ "$independent_agent" == "false" && "$package_name" == "oracle" ]]; then echoerr "Package \"${package_name}\" skipped: not supported with Elastic Agent running in the stack (missing required software)." From 60c34da89fcf7233271129a0d9d35f5a5f784633 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Fri, 24 May 2024 16:37:27 +0200 Subject: [PATCH 16/33] Fix env. variable name --- .buildkite/pipeline.trigger.integration.tests.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.buildkite/pipeline.trigger.integration.tests.sh b/.buildkite/pipeline.trigger.integration.tests.sh index 4214370ea..09c6dfe32 100755 --- a/.buildkite/pipeline.trigger.integration.tests.sh +++ b/.buildkite/pipeline.trigger.integration.tests.sh @@ -97,7 +97,7 @@ for package in $(find . -maxdepth 1 -mindepth 1 -type d) ; do fi package_name=$(basename "${package}") if [[ "${package_name}" == "gcp" || "${package_name}" == "aws" || "${package_name}" == "aws_logs" ]] ; then - echoerr "Skip package temporarily ${packager_name}" + echoerr "Skip package temporarily ${package_name}" continue fi From 402b631e52f720585091134b99d8fa76262ca597 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Tue, 28 May 2024 09:02:36 +0200 Subject: [PATCH 17/33] Change order tear down handlers --- internal/testrunner/runners/system/runner.go | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/internal/testrunner/runners/system/runner.go b/internal/testrunner/runners/system/runner.go index d50461224..e31225151 100644 --- a/internal/testrunner/runners/system/runner.go +++ b/internal/testrunner/runners/system/runner.go @@ -428,6 +428,15 @@ func (r *runner) tearDownTest(ctx context.Context) error { // Avoid cancellations during cleanup. cleanupCtx := context.WithoutCancel(ctx) + // Run service shutdown first to ensure that resources created + // by terraform are deleted avoiding errors in other handlers + if r.shutdownServiceHandler != nil { + if err := r.shutdownServiceHandler(cleanupCtx); err != nil { + return err + } + r.shutdownServiceHandler = nil + } + if r.resetAgentPolicyHandler != nil { if err := r.resetAgentPolicyHandler(cleanupCtx); err != nil { return err @@ -465,13 +474,6 @@ func (r *runner) tearDownTest(ctx context.Context) error { return err } - if r.shutdownServiceHandler != nil { - if err := r.shutdownServiceHandler(cleanupCtx); err != nil { - return err - } - r.shutdownServiceHandler = nil - } - if r.shutdownAgentHandler != nil { if err := r.shutdownAgentHandler(cleanupCtx); err != nil { return err From e7ea9ff4a9fb591a0cc1a3026ec6871711dcbe48 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Tue, 28 May 2024 09:07:04 +0200 Subject: [PATCH 18/33] Exit loop if getDocs returns zero documents --- internal/testrunner/runners/system/runner.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/internal/testrunner/runners/system/runner.go b/internal/testrunner/runners/system/runner.go index e31225151..8df7c158e 100644 --- a/internal/testrunner/runners/system/runner.go +++ b/internal/testrunner/runners/system/runner.go @@ -1447,6 +1447,8 @@ func (r *runner) deleteOldDocumentsDataStreamAndWait(ctx context.Context, dataSt if err := deleteDataStreamDocs(ctx, r.options.API, dataStream); err != nil { return fmt.Errorf("error deleting old data in data stream: %s: %w", dataStream, err) } + // TODO: Review if this getDocs call can be deleted after creating + // a new Agent Policy in each test startHits, err := r.getDocs(ctx, dataStream) if err != nil { return err @@ -1464,7 +1466,7 @@ func (r *runner) deleteOldDocumentsDataStreamAndWait(ctx context.Context, dataSt if mustBeZero { return hits.size() == 0, nil } - return startHits.size() > hits.size(), nil + return hits.size() == 0 || startHits.size() > hits.size(), nil }, 1*time.Second, 2*time.Minute) if err != nil || !cleared { if err == nil { From 8e95b4db5fbe87e5950a81d086fb63eb1e88d79c Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Tue, 28 May 2024 11:04:21 +0200 Subject: [PATCH 19/33] Reorder handlers and change condition --- internal/testrunner/runners/system/runner.go | 22 ++++++++++++-------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/internal/testrunner/runners/system/runner.go b/internal/testrunner/runners/system/runner.go index 8df7c158e..1ebed5465 100644 --- a/internal/testrunner/runners/system/runner.go +++ b/internal/testrunner/runners/system/runner.go @@ -428,20 +428,24 @@ func (r *runner) tearDownTest(ctx context.Context) error { // Avoid cancellations during cleanup. cleanupCtx := context.WithoutCancel(ctx) - // Run service shutdown first to ensure that resources created - // by terraform are deleted avoiding errors in other handlers - if r.shutdownServiceHandler != nil { - if err := r.shutdownServiceHandler(cleanupCtx); err != nil { + // This handler should be run before shutting down Elastic Agents (agent deployer) + // or services that could run agents like Custom Agents (service deployer) + // or Kind deployer. + if r.resetAgentPolicyHandler != nil { + if err := r.resetAgentPolicyHandler(cleanupCtx); err != nil { return err } - r.shutdownServiceHandler = nil + r.resetAgentPolicyHandler = nil } - if r.resetAgentPolicyHandler != nil { - if err := r.resetAgentPolicyHandler(cleanupCtx); err != nil { + // Shutting down the service should be run one of the first actions + // to ensure that resources created by terraform are deleted even if other + // errors fail. + if r.shutdownServiceHandler != nil { + if err := r.shutdownServiceHandler(cleanupCtx); err != nil { return err } - r.resetAgentPolicyHandler = nil + r.shutdownServiceHandler = nil } if r.resetAgentLogLevelHandler != nil { @@ -1122,7 +1126,7 @@ func (r *runner) prepareScenario(ctx context.Context, config *testConfig, svcInf } // Assign policy to agent r.resetAgentPolicyHandler = func(ctx context.Context) error { - if !r.options.RunSetup || !r.options.RunTearDown { + if !r.options.RunSetup { // it should be kept the same policy just when system tests are // triggered with the flags for just setup or just tear-down logger.Debug("reassigning original policy back to agent...") From a421e83ab248dff36d17f7c48670e35d2523d54e Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Tue, 28 May 2024 11:14:37 +0200 Subject: [PATCH 20/33] Add new tear down handler --- internal/testrunner/runners/system/runner.go | 42 +++++++++++++------- 1 file changed, 27 insertions(+), 15 deletions(-) diff --git a/internal/testrunner/runners/system/runner.go b/internal/testrunner/runners/system/runner.go index 1ebed5465..1b58b513a 100644 --- a/internal/testrunner/runners/system/runner.go +++ b/internal/testrunner/runners/system/runner.go @@ -150,6 +150,7 @@ type runner struct { // Execution order of following handlers is defined in runner.TearDown() method. removeAgentHandler func(context.Context) error deleteTestPolicyHandler func(context.Context) error + cleanTestScenarioHandler func(context.Context) error resetAgentPolicyHandler func(context.Context) error resetAgentLogLevelHandler func(context.Context) error shutdownServiceHandler func(context.Context) error @@ -448,6 +449,13 @@ func (r *runner) tearDownTest(ctx context.Context) error { r.shutdownServiceHandler = nil } + if r.cleanTestScenarioHandler != nil { + if err := r.cleanTestScenarioHandler(cleanupCtx); err != nil { + return err + } + r.cleanTestScenarioHandler = nil + } + if r.resetAgentLogLevelHandler != nil { if err := r.resetAgentLogLevelHandler(cleanupCtx); err != nil { return err @@ -1075,6 +1083,24 @@ func (r *runner) prepareScenario(ctx context.Context, config *testConfig, svcInf return nil } + r.cleanTestScenarioHandler = func(ctx context.Context) error { + if !r.shouldCreateNewAgentPolicyForTest() { + return nil + } + + logger.Debug("Deleting test policy...") + err = r.options.KibanaClient.DeletePolicy(ctx, policyToAssignDatastreamTests.ID) + if err != nil { + return fmt.Errorf("failed to delete policy %s: %w", policyToAssignDatastreamTests.Name, err) + } + logger.Debug("Deleting data stream for testing") + r.deleteDataStream(ctx, scenario.dataStream) + if err != nil { + return fmt.Errorf("failed to delete data stream %s: %w", scenario.dataStream, err) + } + return nil + } + switch { case r.options.RunTearDown: logger.Debugf("Skipped deleting old data in data stream %q", scenario.dataStream) @@ -1124,7 +1150,7 @@ func (r *runner) prepareScenario(ctx context.Context, config *testConfig, svcInf Revision: agent.PolicyRevision, } } - // Assign policy to agent + r.resetAgentPolicyHandler = func(ctx context.Context) error { if !r.options.RunSetup { // it should be kept the same policy just when system tests are @@ -1134,20 +1160,6 @@ func (r *runner) prepareScenario(ctx context.Context, config *testConfig, svcInf return fmt.Errorf("error reassigning original policy to agent: %w", err) } } - if !r.shouldCreateNewAgentPolicyForTest() { - return nil - } - - logger.Debug("Deleting test policy...") - err = r.options.KibanaClient.DeletePolicy(ctx, policyToAssignDatastreamTests.ID) - if err != nil { - return fmt.Errorf("failed to delete policy %s: %w", policyToAssignDatastreamTests.Name, err) - } - logger.Debug("Deleting data stream for testing") - r.deleteDataStream(ctx, scenario.dataStream) - if err != nil { - return fmt.Errorf("failed to delete data stream %s: %w", scenario.dataStream, err) - } return nil } From 8c9c5d8b927aab8bb8d82af6c48329359f3050ef Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Tue, 28 May 2024 11:57:18 +0200 Subject: [PATCH 21/33] Update comment --- internal/testrunner/runners/system/runner.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/testrunner/runners/system/runner.go b/internal/testrunner/runners/system/runner.go index 1b58b513a..ac0f2f6ab 100644 --- a/internal/testrunner/runners/system/runner.go +++ b/internal/testrunner/runners/system/runner.go @@ -1154,7 +1154,7 @@ func (r *runner) prepareScenario(ctx context.Context, config *testConfig, svcInf r.resetAgentPolicyHandler = func(ctx context.Context) error { if !r.options.RunSetup { // it should be kept the same policy just when system tests are - // triggered with the flags for just setup or just tear-down + // triggered with the flags for running setup stage (--setup) logger.Debug("reassigning original policy back to agent...") if err := r.options.KibanaClient.AssignPolicyToAgent(ctx, agent, origPolicy); err != nil { return fmt.Errorf("error reassigning original policy to agent: %w", err) From c11361f7275284a7a96653aef5b3c35697eac80e Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Tue, 28 May 2024 13:53:29 +0200 Subject: [PATCH 22/33] Add one more test package --- .buildkite/pipeline.trigger.integration.tests.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.buildkite/pipeline.trigger.integration.tests.sh b/.buildkite/pipeline.trigger.integration.tests.sh index 09c6dfe32..1eaa2d7d2 100755 --- a/.buildkite/pipeline.trigger.integration.tests.sh +++ b/.buildkite/pipeline.trigger.integration.tests.sh @@ -96,7 +96,7 @@ for package in $(find . -maxdepth 1 -mindepth 1 -type d) ; do label_suffix=" (independent agent)" fi package_name=$(basename "${package}") - if [[ "${package_name}" == "gcp" || "${package_name}" == "aws" || "${package_name}" == "aws_logs" ]] ; then + if [[ "${package_name}" == "aws" || "${package_name}" == "aws_logs" ]] ; then echoerr "Skip package temporarily ${package_name}" continue fi From 7d715621decc06af8585822298bde558d1146e97 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Tue, 28 May 2024 16:24:52 +0200 Subject: [PATCH 23/33] Remove exceptions in wait loop to delete docs --- internal/testrunner/runners/system/runner.go | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/internal/testrunner/runners/system/runner.go b/internal/testrunner/runners/system/runner.go index ac0f2f6ab..5d590deba 100644 --- a/internal/testrunner/runners/system/runner.go +++ b/internal/testrunner/runners/system/runner.go @@ -1463,16 +1463,6 @@ func (r *runner) deleteOldDocumentsDataStreamAndWait(ctx context.Context, dataSt if err := deleteDataStreamDocs(ctx, r.options.API, dataStream); err != nil { return fmt.Errorf("error deleting old data in data stream: %s: %w", dataStream, err) } - // TODO: Review if this getDocs call can be deleted after creating - // a new Agent Policy in each test - startHits, err := r.getDocs(ctx, dataStream) - if err != nil { - return err - } - // First call already reports zero documents - if startHits.size() == 0 { - return nil - } cleared, err := wait.UntilTrue(ctx, func(ctx context.Context) (bool, error) { hits, err := r.getDocs(ctx, dataStream) if err != nil { @@ -1482,7 +1472,7 @@ func (r *runner) deleteOldDocumentsDataStreamAndWait(ctx context.Context, dataSt if mustBeZero { return hits.size() == 0, nil } - return hits.size() == 0 || startHits.size() > hits.size(), nil + return hits.size() == 0, nil }, 1*time.Second, 2*time.Minute) if err != nil || !cleared { if err == nil { From e4e8751b330330232e75bb2d8e54555ce804297b Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Tue, 28 May 2024 16:28:30 +0200 Subject: [PATCH 24/33] Update some logger calls to use the right formats --- internal/agentdeployer/agent.go | 4 ++-- internal/servicedeployer/compose.go | 4 ++-- internal/servicedeployer/custom_agent.go | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/internal/agentdeployer/agent.go b/internal/agentdeployer/agent.go index 246348255..ce1ac8534 100644 --- a/internal/agentdeployer/agent.go +++ b/internal/agentdeployer/agent.go @@ -391,12 +391,12 @@ func (s *dockerComposeDeployedAgent) TearDown(ctx context.Context) error { defer func() { // Remove the service logs dir for this agent if err := os.RemoveAll(s.agentInfo.Logs.Folder.Local); err != nil { - logger.Errorf("could not remove the agent logs (path: %s): %w", s.agentInfo.Logs.Folder.Local, err) + logger.Errorf("could not remove the agent logs (path: %s): %v", s.agentInfo.Logs.Folder.Local, err) } // Remove the configuration dir for this agent (e.g. compose scenario files) if err := os.RemoveAll(s.agentInfo.ConfigDir); err != nil { - logger.Errorf("could not remove the agent configuration directory (path: %s) %w", s.agentInfo.ConfigDir, err) + logger.Errorf("could not remove the agent configuration directory (path: %s) %v", s.agentInfo.ConfigDir, err) } }() diff --git a/internal/servicedeployer/compose.go b/internal/servicedeployer/compose.go index 9dc274b36..989e15d81 100644 --- a/internal/servicedeployer/compose.go +++ b/internal/servicedeployer/compose.go @@ -98,7 +98,7 @@ func (d *DockerComposeServiceDeployer) SetUp(ctx context.Context, svcInfo Servic // service logs folder must no be deleted to avoid breaking log files written // by the service. If this is required, those files should be rotated or truncated // so the service can still write to them. - logger.Debug("Skipping removing service logs folder folder %s", svcInfo.Logs.Folder.Local) + logger.Debugf("Skipping removing service logs folder folder %s", svcInfo.Logs.Folder.Local) } else { err = files.RemoveContent(svcInfo.Logs.Folder.Local) if err != nil { @@ -242,7 +242,7 @@ func (s *dockerComposeDeployedService) TearDown(ctx context.Context) error { } // Remove the outputs generated by the service container if err = os.RemoveAll(s.svcInfo.OutputDir); err != nil { - logger.Errorf("could not remove the temporary output files %w", err) + logger.Errorf("could not remove the temporary output files %s", err) } }() diff --git a/internal/servicedeployer/custom_agent.go b/internal/servicedeployer/custom_agent.go index 36cc111e8..2c8e01ab8 100644 --- a/internal/servicedeployer/custom_agent.go +++ b/internal/servicedeployer/custom_agent.go @@ -129,7 +129,7 @@ func (d *CustomAgentDeployer) SetUp(ctx context.Context, svcInfo ServiceInfo) (D // service logs folder must no be deleted to avoid breaking log files written // by the service. If this is required, those files should be rotated or truncated // so the service can still write to them. - logger.Debug("Skipping removing service logs folder folder %s", svcInfo.Logs.Folder.Local) + logger.Debugf("Skipping removing service logs folder folder %s", svcInfo.Logs.Folder.Local) } else { err = files.RemoveContent(svcInfo.Logs.Folder.Local) if err != nil { From f43dc7778acb73e6c8ef938237af8c825ab73891 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Tue, 28 May 2024 17:56:25 +0200 Subject: [PATCH 25/33] Restore packages skipped --- .buildkite/pipeline.trigger.integration.tests.sh | 5 ----- 1 file changed, 5 deletions(-) diff --git a/.buildkite/pipeline.trigger.integration.tests.sh b/.buildkite/pipeline.trigger.integration.tests.sh index 1eaa2d7d2..3e99ddf42 100755 --- a/.buildkite/pipeline.trigger.integration.tests.sh +++ b/.buildkite/pipeline.trigger.integration.tests.sh @@ -96,11 +96,6 @@ for package in $(find . -maxdepth 1 -mindepth 1 -type d) ; do label_suffix=" (independent agent)" fi package_name=$(basename "${package}") - if [[ "${package_name}" == "aws" || "${package_name}" == "aws_logs" ]] ; then - echoerr "Skip package temporarily ${package_name}" - continue - fi - if [[ "$independent_agent" == "false" && "$package_name" == "oracle" ]]; then echoerr "Package \"${package_name}\" skipped: not supported with Elastic Agent running in the stack (missing required software)." continue From df1835c62ec25711b36387d098cd97885150b7df Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Tue, 28 May 2024 21:11:14 +0200 Subject: [PATCH 26/33] Remove empty line --- internal/testrunner/runners/system/runner.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/testrunner/runners/system/runner.go b/internal/testrunner/runners/system/runner.go index 5d590deba..f337b7a44 100644 --- a/internal/testrunner/runners/system/runner.go +++ b/internal/testrunner/runners/system/runner.go @@ -850,7 +850,6 @@ func (r *runner) shouldCreateNewAgentPolicyForTest() bool { if !r.options.RunIndependentElasticAgent { // keep same behaviour as previously when Elastic Agent of the stack is used. return false - } // No need to create new Agent Policies for these stages if r.options.RunSetup || r.options.RunTearDown { From d9e800089eb641cc12a254d104bc2a3459e399b2 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Tue, 28 May 2024 21:16:47 +0200 Subject: [PATCH 27/33] Remove mustBeZero parameter from delete docs function --- internal/testrunner/runners/system/runner.go | 19 ++++--------------- 1 file changed, 4 insertions(+), 15 deletions(-) diff --git a/internal/testrunner/runners/system/runner.go b/internal/testrunner/runners/system/runner.go index f337b7a44..0034a640b 100644 --- a/internal/testrunner/runners/system/runner.go +++ b/internal/testrunner/runners/system/runner.go @@ -1100,18 +1100,10 @@ func (r *runner) prepareScenario(ctx context.Context, config *testConfig, svcInf return nil } - switch { - case r.options.RunTearDown: + if r.options.RunTearDown { logger.Debugf("Skipped deleting old data in data stream %q", scenario.dataStream) - case r.options.RunTestsOnly: - // In this mode, service is still running and the agent is sending documents, so sometimes - // cannot be guaranteed to be zero documents - err := r.deleteOldDocumentsDataStreamAndWait(ctx, scenario.dataStream, false) - if err != nil { - return nil, err - } - default: - err := r.deleteOldDocumentsDataStreamAndWait(ctx, scenario.dataStream, true) + } else { + err := r.deleteOldDocumentsDataStreamAndWait(ctx, scenario.dataStream) if err != nil { return nil, err } @@ -1457,7 +1449,7 @@ func (r *runner) writeScenarioState(opts scenarioStateOpts) error { return nil } -func (r *runner) deleteOldDocumentsDataStreamAndWait(ctx context.Context, dataStream string, mustBeZero bool) error { +func (r *runner) deleteOldDocumentsDataStreamAndWait(ctx context.Context, dataStream string) error { logger.Debugf("Delete previous documents in data stream %q", dataStream) if err := deleteDataStreamDocs(ctx, r.options.API, dataStream); err != nil { return fmt.Errorf("error deleting old data in data stream: %s: %w", dataStream, err) @@ -1468,9 +1460,6 @@ func (r *runner) deleteOldDocumentsDataStreamAndWait(ctx context.Context, dataSt return false, err } - if mustBeZero { - return hits.size() == 0, nil - } return hits.size() == 0, nil }, 1*time.Second, 2*time.Minute) if err != nil || !cleared { From 7769e3cb119ae2984d5db912da4a8a4074da4254 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Tue, 28 May 2024 21:23:27 +0200 Subject: [PATCH 28/33] Add handler to clean test scenario - test only stage --- internal/testrunner/runners/system/runner.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/internal/testrunner/runners/system/runner.go b/internal/testrunner/runners/system/runner.go index 0034a640b..33832d8c4 100644 --- a/internal/testrunner/runners/system/runner.go +++ b/internal/testrunner/runners/system/runner.go @@ -280,11 +280,14 @@ func (r *runner) Run(ctx context.Context, options testrunner.TestOptions) ([]tes return result.WithError(fmt.Errorf("failed to prepare scenario: %w", err)) } results, err := r.validateTestScenario(ctx, result, scenario, testConfig) - // run re-assign policy tdErr := r.resetAgentPolicyHandler(ctx) if tdErr != nil { logger.Errorf("failed to reassign policy: %s", tdErr) } + tdErr = r.cleanTestScenarioHandler(ctx) + if tdErr != nil { + logger.Errorf("failed to clean test scenario: %s", tdErr) + } return results, err } From 91676d766424c8445694e124c00e1cd4cd8b5f68 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Tue, 28 May 2024 21:31:55 +0200 Subject: [PATCH 29/33] Skip handlers that should not be executed with --no-provision --- internal/testrunner/runners/system/runner.go | 26 +++++++++++++++----- 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/internal/testrunner/runners/system/runner.go b/internal/testrunner/runners/system/runner.go index 33832d8c4..129058cfb 100644 --- a/internal/testrunner/runners/system/runner.go +++ b/internal/testrunner/runners/system/runner.go @@ -280,13 +280,9 @@ func (r *runner) Run(ctx context.Context, options testrunner.TestOptions) ([]tes return result.WithError(fmt.Errorf("failed to prepare scenario: %w", err)) } results, err := r.validateTestScenario(ctx, result, scenario, testConfig) - tdErr := r.resetAgentPolicyHandler(ctx) - if tdErr != nil { - logger.Errorf("failed to reassign policy: %s", tdErr) - } - tdErr = r.cleanTestScenarioHandler(ctx) + tdErr := r.tearDownTest(ctx) if tdErr != nil { - logger.Errorf("failed to clean test scenario: %s", tdErr) + logger.Errorf("failed to tear down runner: %s", tdErr.Error()) } return results, err @@ -971,6 +967,9 @@ func (r *runner) prepareScenario(ctx context.Context, config *testConfig, svcInf } } r.deleteTestPolicyHandler = func(ctx context.Context) error { + if r.options.RunTestsOnly { + return nil + } logger.Debug("deleting test policies...") if err := r.options.KibanaClient.DeletePolicy(ctx, policyToTest.ID); err != nil { return fmt.Errorf("error cleaning up test policy: %w", err) @@ -1078,6 +1077,9 @@ func (r *runner) prepareScenario(ctx context.Context, config *testConfig, svcInf ) r.wipeDataStreamHandler = func(ctx context.Context) error { + if r.options.RunTestsOnly { + return nil + } logger.Debugf("deleting data in data stream...") if err := deleteDataStreamDocs(ctx, r.options.API, scenario.dataStream); err != nil { return fmt.Errorf("error deleting data in data stream: %w", err) @@ -1122,6 +1124,9 @@ func (r *runner) prepareScenario(ctx context.Context, config *testConfig, svcInf logger.Debugf("Selected enrolled agent %q", agent.ID) r.removeAgentHandler = func(ctx context.Context) error { + if r.options.RunTestsOnly { + return nil + } // When not using independent agents, service deployers like kubernetes or custom agents create new Elastic Agent if !r.options.RunIndependentElasticAgent && !svcInfo.Agent.Independent { return nil @@ -1171,6 +1176,9 @@ func (r *runner) prepareScenario(ctx context.Context, config *testConfig, svcInf } } r.resetAgentLogLevelHandler = func(ctx context.Context) error { + if r.options.RunTestsOnly { + return nil + } logger.Debugf("reassigning original log level %q back to agent...", origLogLevel) if err := r.options.KibanaClient.SetAgentLogLevel(ctx, agent.ID, origLogLevel); err != nil { @@ -1323,6 +1331,9 @@ func (r *runner) setupService(ctx context.Context, config *testConfig, serviceOp } r.shutdownServiceHandler = func(ctx context.Context) error { + if r.options.RunTestsOnly { + return nil + } logger.Debug("tearing down service...") if err := service.TearDown(ctx); err != nil { return fmt.Errorf("error tearing down service: %w", err) @@ -1362,6 +1373,9 @@ func (r *runner) setupAgent(ctx context.Context, config *testConfig, state Servi return nil, agentInfo, fmt.Errorf("could not setup agent: %w", err) } r.shutdownAgentHandler = func(ctx context.Context) error { + if r.options.RunTestsOnly { + return nil + } if agentDeployer == nil { return nil } From 8f4a52ac9cdcad9404133c9885d42e454d835ed4 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Fri, 31 May 2024 13:16:29 +0200 Subject: [PATCH 30/33] Reuse same runner instance --- cmd/testrunner.go | 6 ------ internal/testrunner/runners/asset/runner.go | 16 ++++++++-------- internal/testrunner/runners/pipeline/runner.go | 3 ++- internal/testrunner/runners/static/runner.go | 3 ++- internal/testrunner/runners/system/runner.go | 2 +- internal/testrunner/testrunner.go | 9 --------- 6 files changed, 13 insertions(+), 26 deletions(-) diff --git a/cmd/testrunner.go b/cmd/testrunner.go index 2e5d09200..379f39d99 100644 --- a/cmd/testrunner.go +++ b/cmd/testrunner.go @@ -337,11 +337,6 @@ func testTypeCommandActionFactory(runner testrunner.TestRunner) cobraext.Command fmt.Printf("Running tests per stages (technical preview)\n") } - runner, err := testrunner.NewRunner(testType) - if err != nil { - return fmt.Errorf("failed to get runner %s: %w", testType, err) - } - runnerOptions := testrunner.TestOptions{ Profile: profile, PackageRootPath: packageRootPath, @@ -358,7 +353,6 @@ func testTypeCommandActionFactory(runner testrunner.TestRunner) cobraext.Command RunTestsOnly: runTestsOnly, RunIndependentElasticAgent: false, } - err = runner.SetupRunner(ctx, runnerOptions) if err != nil { return fmt.Errorf("failed to run setup runner process: %w", err) diff --git a/internal/testrunner/runners/asset/runner.go b/internal/testrunner/runners/asset/runner.go index 25e7f4482..39f50758e 100644 --- a/internal/testrunner/runners/asset/runner.go +++ b/internal/testrunner/runners/asset/runner.go @@ -47,7 +47,14 @@ func (r runner) String() string { } // SetupRunner prepares global resources required by the test runner. -func (r runner) SetupRunner(ctx context.Context, options testrunner.TestOptions) error { +func (r *runner) SetupRunner(ctx context.Context, options testrunner.TestOptions) error { + r.packageRootPath = options.PackageRootPath + r.kibanaClient = options.KibanaClient + + manager := resources.NewManager() + manager.RegisterProvider(resources.DefaultKibanaProviderName, &resources.KibanaProvider{Client: r.kibanaClient}) + + r.resourcesManager = manager return nil } @@ -72,13 +79,6 @@ func (r *runner) CanRunSetupTeardownIndependent() bool { // Run runs the asset loading tests func (r *runner) Run(ctx context.Context, options testrunner.TestOptions) ([]testrunner.TestResult, error) { r.testFolder = options.TestFolder - r.packageRootPath = options.PackageRootPath - r.kibanaClient = options.KibanaClient - - manager := resources.NewManager() - manager.RegisterProvider(resources.DefaultKibanaProviderName, &resources.KibanaProvider{Client: r.kibanaClient}) - r.resourcesManager = manager - return r.run(ctx) } diff --git a/internal/testrunner/runners/pipeline/runner.go b/internal/testrunner/runners/pipeline/runner.go index a8e5f5a45..dcdfce6b9 100644 --- a/internal/testrunner/runners/pipeline/runner.go +++ b/internal/testrunner/runners/pipeline/runner.go @@ -73,6 +73,7 @@ func (r *runner) String() string { // SetupRunner prepares global resources required by the test runner. func (r runner) SetupRunner(ctx context.Context, options testrunner.TestOptions) error { + r.options = options return nil } @@ -84,7 +85,7 @@ func (r runner) TearDownRunner(ctx context.Context) error { // Run runs the pipeline tests defined under the given folder func (r *runner) Run(ctx context.Context, options testrunner.TestOptions) ([]testrunner.TestResult, error) { - r.options = options + r.options.TestFolder = options.TestFolder stackConfig, err := stack.LoadConfig(r.options.Profile) if err != nil { diff --git a/internal/testrunner/runners/static/runner.go b/internal/testrunner/runners/static/runner.go index 53be20fef..739c7b268 100644 --- a/internal/testrunner/runners/static/runner.go +++ b/internal/testrunner/runners/static/runner.go @@ -47,6 +47,7 @@ func (r runner) String() string { // SetupRunner prepares global resources required by the test runner. func (r runner) SetupRunner(ctx context.Context, options testrunner.TestOptions) error { + r.options = options return nil } @@ -57,7 +58,7 @@ func (r runner) TearDownRunner(ctx context.Context) error { } func (r runner) Run(ctx context.Context, options testrunner.TestOptions) ([]testrunner.TestResult, error) { - r.options = options + r.options.TestFolder = options.TestFolder return r.run(ctx) } diff --git a/internal/testrunner/runners/system/runner.go b/internal/testrunner/runners/system/runner.go index 0adc40228..73dca297e 100644 --- a/internal/testrunner/runners/system/runner.go +++ b/internal/testrunner/runners/system/runner.go @@ -230,7 +230,7 @@ func (r *runner) CanRunSetupTeardownIndependent() bool { // Run runs the system tests defined under the given folder func (r *runner) Run(ctx context.Context, options testrunner.TestOptions) ([]testrunner.TestResult, error) { - r.options = options + r.options.TestFolder = options.TestFolder if !r.options.RunSetup && !r.options.RunTearDown && !r.options.RunTestsOnly { return r.run(ctx) } diff --git a/internal/testrunner/testrunner.go b/internal/testrunner/testrunner.go index 2c37a1a05..4d0261824 100644 --- a/internal/testrunner/testrunner.go +++ b/internal/testrunner/testrunner.go @@ -291,15 +291,6 @@ func RegisterRunner(runner TestRunner) { runners[runner.Type()] = runner } -func NewRunner(testType TestType) (TestRunner, error) { - runner, defined := runners[testType] - if !defined { - return nil, fmt.Errorf("unregistered runner test: %s", testType) - } - - return runner, nil -} - // Run method delegates execution to the registered test runner, based on the test type. func Run(ctx context.Context, runner TestRunner, options TestOptions) ([]TestResult, error) { results, err := runner.Run(ctx, options) From 4bf6049bec49b10a4e65c58613e1d10522cd326b Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Fri, 31 May 2024 13:19:14 +0200 Subject: [PATCH 31/33] Add pointers --- internal/testrunner/runners/pipeline/runner.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/testrunner/runners/pipeline/runner.go b/internal/testrunner/runners/pipeline/runner.go index dcdfce6b9..a63ef8659 100644 --- a/internal/testrunner/runners/pipeline/runner.go +++ b/internal/testrunner/runners/pipeline/runner.go @@ -72,14 +72,14 @@ func (r *runner) String() string { } // SetupRunner prepares global resources required by the test runner. -func (r runner) SetupRunner(ctx context.Context, options testrunner.TestOptions) error { +func (r *runner) SetupRunner(ctx context.Context, options testrunner.TestOptions) error { r.options = options return nil } // TearDownRunner cleans up any global test runner resources. It must be called // after the test runner has finished executing all its tests. -func (r runner) TearDownRunner(ctx context.Context) error { +func (r *runner) TearDownRunner(ctx context.Context) error { return nil } From e59151c685449117831e386af29d502320882a24 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Fri, 31 May 2024 15:19:58 +0200 Subject: [PATCH 32/33] Add pointer to static runner receiver --- internal/testrunner/runners/static/runner.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/testrunner/runners/static/runner.go b/internal/testrunner/runners/static/runner.go index 739c7b268..7ff15ec09 100644 --- a/internal/testrunner/runners/static/runner.go +++ b/internal/testrunner/runners/static/runner.go @@ -46,14 +46,14 @@ func (r runner) String() string { } // SetupRunner prepares global resources required by the test runner. -func (r runner) SetupRunner(ctx context.Context, options testrunner.TestOptions) error { +func (r *runner) SetupRunner(ctx context.Context, options testrunner.TestOptions) error { r.options = options return nil } // TearDownRunner cleans up any global test runner resources. It must be called // after the test runner has finished executing all its tests. -func (r runner) TearDownRunner(ctx context.Context) error { +func (r *runner) TearDownRunner(ctx context.Context) error { return nil } From 31e421b7d0bd25494d33da56e691b65977d1dbd1 Mon Sep 17 00:00:00 2001 From: Mario Rodriguez Molins Date: Fri, 31 May 2024 16:37:05 +0200 Subject: [PATCH 33/33] Remove uninstall step in tearDown test This step/action is moved after all tests have been executed, and it is now performed in TearDownRunner function. --- internal/testrunner/runners/system/runner.go | 9 --------- 1 file changed, 9 deletions(-) diff --git a/internal/testrunner/runners/system/runner.go b/internal/testrunner/runners/system/runner.go index 73dca297e..5c6927a5d 100644 --- a/internal/testrunner/runners/system/runner.go +++ b/internal/testrunner/runners/system/runner.go @@ -517,15 +517,6 @@ func (r *runner) tearDownTest(ctx context.Context) error { r.deleteTestPolicyHandler = nil } - resourcesOptions := resourcesOptions{ - // Keep it installed only if we were running setup, or tests only. - installedPackage: r.options.RunSetup || r.options.RunTestsOnly, - } - _, err := r.resourcesManager.ApplyCtx(cleanupCtx, r.resources(resourcesOptions)) - if err != nil { - return err - } - if r.shutdownAgentHandler != nil { if err := r.shutdownAgentHandler(cleanupCtx); err != nil { return err