chore: standardise how unit/integration tests is run on CI, adding a build command to compress the system-tests files #26209

mdelapenya · 2021-06-08T13:59:25Z

What does this PR do?

This PR adds two things:

a mage packageSystemTests command, plugged into filebeat and metricbeat modules (both OSS and xpack flavours).
a Tar method under the commons.go mage build file, so that it's possible to compress a source directory using TAR+GZIP algorithms, and writing it to the build dir in the root directory of the Beats repository.

Besides that, we are removing a python script that detected the path to the Python system-tests (used in the pipeline to store the folder name in a variable), and we are updating the Jenkins pipeline to call the new build system command instead of the previous script. To keep existing behaviour, that we could consider it wrong and would possible discusse about it in a follow-up PR, we are passing a new argument to the groovy method in the pipeline, representing the directory in which the Beat live (i.e. filebeat, x-pack/filebeat), so that it's possible to run the mage command in the right context.

Finally, we are keeping Pipeline2.0-consistency across Beats calling the Python system-tests, using OSS metricbeat's as the canonical reference: run unit tests, then Go integration tests, and finally Python integration tests when needed. This change applies to filebeat and metricbeat in both OSS and xpack flavours.

UPDATE: I finally revisited each Beat and made sure all of them run the unit, goInteg and pythonInteg tests in a consistent manner: instead of calling the obscure mage test command, we separate it in three different stages when needed: mage unitTest, mage goIntegTest and mage pythonIntegTest. I've verified one by one that each of those three commands is present in the build for the related Beat, adding or removing them from the CI descriptor (Jenkinsfile.yml) when/if needed, creating the consistent experience of running atomic commands for each test type.

Why is it important?

First one is simplicity: instead of adding code into the Jenkins pipeline that makes it almost impossible to reproduce it locally, we are moving to the build system, which will allow developers to run this target locally and verify what the CI will do.

Second is consistency: metricbeat and filebeat are running system-tests, and we want to operate them on CI in the same manner. That's why we are making the descriptors very similar.

And third, separation of concers: we want to run unit > integration > e2e tests in different stages, so that we acknowledge the power of not building/running code that could have been broken after a possible failure in a previous stage.

Checklist

My code follows the style guidelines of this project
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
I have made corresponding change to the default configuration files
I have added tests that prove my fix is effective or that my feature works
I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

Author's Checklist

[ ]

How to test this PR locally

$ cd filebeat
$ mage pythonIntegTest
$ mage packageSystemTests
>> skipping symlink: build/system-tests/run/test_harvester.Test.test_ignore_symlink/log/symlink.log
>> skipping symlink: build/system-tests/run/test_harvester.Test.test_symlink_and_file/log/symlink.log
>> skipping symlink: build/system-tests/run/test_harvester.Test.test_symlink_rotated/log/symlink.log
>> skipping symlink: build/system-tests/run/test_harvester.Test.test_symlinks_enabled/log/symlink.log
>> skipping symlink: build/system-tests/run/test_harvester.Test.test_truncate/log/symlink.log
>> skipping symlink: build/system-tests/run/test_input.Test.test_skip_symlinks/log/test.log

# checking file system contents
$ find build/system-tests/run -name "*" | wc -l
    3940
    
# checking TAR contents
$ tar -tf ../build/system-tests-filebeat.tar.gz | wc -l
    3934

Expected behaviour: a TAR file in the root's build directory named as the Beat will exist (i.e. ./build/system-tests-filebeat.tar.gz)

Related issues

Relates [Build System] Add a command to package system tests files into a TAR file #26208

Follow-ups

We should consider following same separation of concerns in all Beats running mage build test, so that they run unitTest instead, and then having separate stages for integrations tests.

This command will replace the search_system_tests.py script

Filebeat and Metricbeat, including x-pack

botelastic · 2021-06-08T13:59:32Z

This pull request doesn't have a Team:<team> label.

dev-tools/mage/pkg.go

dev-tools/mage/common.go

x-pack/filebeat/Jenkinsfile.yml

elasticmachine · 2021-06-08T14:51:00Z

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Start Time: 2021-07-27T07:13:01.130+0000
Duration: 130 min 10 sec
Commit: a230581

Test stats 🧪

Test	Results
Failed	0
Passed	49331
Skipped	5320
Total	54651

Trends 🧪

💚 Flaky test report

Tests succeeded.

Expand to view the summary

Test stats 🧪

Test	Results
Failed	0
Passed	49331
Skipped	5320
Total	54651

v1v

💯

jsoriano

👍 added some suggestions.

dev-tools/mage/common.go

dev-tools/mage/pkg.go

Co-authored-by: Jaime Soriano Pastor <[email protected]>

v1v · 2021-06-30T12:14:26Z

Jenkinsfile

-          def os_suffix = isArm() ? 'linux' : nodeOS()
-          def name = folder.replaceAll('/', '-').replaceAll('\\\\', '-').replaceAll('build', '').replaceAll('^-', '') + '-' + os_suffix
-          tarAndUploadArtifacts(file: "${name}.tgz", location: folder)
+        withMageEnv(version: "${env.GO_VERSION}"){


Can it be done this change in a different PR? Then, we can remove the scripts that configure the tools too?

dev-tools/mage/target/integtest/notests/integtest.go

cachedout · 2021-06-30T15:17:15Z

Jenkinsfile

-          def name = folder.replaceAll('/', '-').replaceAll('\\\\', '-').replaceAll('build', '').replaceAll('^-', '') + '-' + os_suffix
-          tarAndUploadArtifacts(file: "${name}.tgz", location: folder)
+        withMageEnv(version: "${env.GO_VERSION}"){
+          dir(directory){


What happens if this is '' as it would be if the .get() returned its default value?

We are setting the variable with setEnvVar('GO_VERSION', readFile(".go-version").trim()). Do you mean that the readFile can return empty value? That would be the case if the .go-version file disappears, which will represent a breaking change in how Go is installed.

dev-tools/mage/target/integtest/integtest.go

We want to avoid collitions in beats overriding some commands

* master: [MetricBeat] [AWS] Fix aws metric tags with resourcegroupstaggingapi paginator (elastic#26385) (elastic#26443) Move openmetrics module to oss (elastic#26561) Skip flaky test TestFilestreamMetadataUpdatedOnRename (elastic#26609) [filebeat][fortinet] Use default add_locale for fortinet.firewall (elastic#26524) Enroll proxy settings (elastic#26514)

mergify · 2021-07-12T15:02:35Z

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b archive-system-tests upstream/archive-system-tests
git merge upstream/master
git push upstream archive-system-tests

* master: (61 commits) Add disk queue unit tests based on the queuetest package [Heartbeat] redact authorization headers from logger (elastic#26892) Expose custom process metrics (elastic#26912) [gcp/billing] always quote table name identifier (elastic#26870) Add Beats central management removal to BCs (elastic#26400) Add custom suffix to identifiers in filestream input when needed (elastic#26669) Update asa-ftd-pipeline.yml (elastic#26265) Use common host parser in vsphere module (elastic#26904) [automation] Update go release version 1.16.6 (elastic#26860) Skip flaky test: filestream and harvester group (elastic#26728) [Filebeat] Remove alias fields from Suricata and Traefik module mappings (elastic#26627) docs: apm-server.auth (elastic#26831) [Automation] Update elastic stack version to 8.0.0-2f008f4a for testing (elastic#26881) Clarify the scope of start/end multiline example (elastic#26786) [Heartbeat]: update Node.js version for synthetics (elastic#26867) [fix][httpjson] Fix incorrect key for template data (elastic#26848) [httpjson] Add value_type parameter to httpjson transforms (elastic#26847) [Heartbeat]: capture error from journey/end events (elastic#26781) [Winlogbeat] Fixes for wineventlog experimental api (elastic#26826) Set agent.id to Fleet Agent ID for each metric/log monitoring input (elastic#26776) ...

This reverts commit 7aa8e53.

* master: Forward port 7.13.4 to master (elastic#26971) Use MustAddMetricSet in all metricsets (elastic#26907) add_process_metadata: enrich process info with process owner (elastic#21068) (elastic#21111) Use aws sdk paginator for FilterLogEvents and GetMetricData (elastic#26852) [Filebeat] Allow - for source IP for AWS S3 Access pipeline (elastic#26940) Increase timeout to 30secs (elastic#26841) Add Cluster filter on Kubernetes Overview ECS dashboard (elastic#26919)

mdelapenya · 2021-07-20T15:40:10Z

jenkins run the tests please

This reverts commit 6c39510.

mdelapenya · 2021-07-22T08:34:46Z

jenkins run the tests please

mdelapenya · 2021-07-26T10:20:15Z

@jsoriano I think this is ready to merge. Waiting for final 👍 to do so

jsoriano

Requesting changes mainly for the use of withModule, I am not sure if this can be generaly used in other Beats beyond Metricbeat.

I am also a bit worried about the refactor of mage targets, we may be missing the execution of some tests by mistake. I think that the use of a central test target was introduced for this. I wouldn't be in any case opposed to the use more specific targets, specially now that we have separated Jenkinsfiles, but I would like to have a second opinion by @andrewkroh about this.

jsoriano · 2021-07-26T17:53:41Z

dev-tools/mage/target/common/package.go

+// PackageSystemTests packages the python system tests results
+func PackageSystemTests() error {


Nit, if this target packages test results, could it be called PackageSystemTestsResults()? :)

Suggested change

// PackageSystemTests packages the python system tests results

func PackageSystemTests() error {

// PackageSystemTestsResults packages the python system tests results

func PackageSystemTestsResults() error {

filebeat/Jenkinsfile.yml

x-pack/filebeat/Jenkinsfile.yml

jsoriano · 2021-07-26T18:02:16Z

heartbeat/Jenkinsfile.yml

+        withModule: true
+        stage: mandatory
+    pythonIntegTest:
+        mage: "mage pythonIntegTest"
+        withModule: true


Same here, not sure if we can use withModule. Heartbeat doesn't have modules, it has monitors, but they have a different directory layout.

libbeat/Jenkinsfile.yml

x-pack/heartbeat/Jenkinsfile.yml

jsoriano · 2021-07-26T18:13:32Z

x-pack/heartbeat/Jenkinsfile.yml

+    unitTest:
+        mage: "mage build unitTest"
+        stage: mandatory
+    goIntegTest:


I think heartbeat doesn't have integration tests.

x-pack/libbeat/Jenkinsfile.yml

x-pack/osquerybeat/Jenkinsfile.yml

jsoriano · 2021-07-26T18:16:30Z

Jenkinsfile

-          tarAndUploadArtifacts(file: "${name}.tgz", location: folder)
+        withMageEnv(version: "${env.GO_VERSION}"){
+          dir(directory){
+            cmd(label: "Archive system tests files", script: 'mage packageSystemTests')


If packageSystemTests is only called here, are all the other changes required? I am a bit concerned that we may be missing the execution of some tests in some cases, it may be good to separate these changes to a different PR if possible, so we can merge this feature with less risks.

Ok, let me split the PR in two:

changing test execution layout on CI

adding the new mage command

Co-authored-by: Jaime Soriano Pastor <[email protected]>

mergify · 2021-08-23T12:38:36Z

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b archive-system-tests upstream/archive-system-tests
git merge upstream/master
git push upstream archive-system-tests

mdelapenya · 2021-08-23T12:46:11Z

Ok, this PR is not needed anymore, as it has been superseded by #27069 and #27295

mdelapenya added 10 commits June 7, 2021 12:05

feat: add a mage command to list all system tests

b8d8f85

This command will replace the search_system_tests.py script

feat: add packageSystemTests commands for beat running system tests

d8fe22c

Filebeat and Metricbeat, including x-pack

feat: support compressing integration test files

ccead60

chore: evaluate if it's a symlink the soonest

dd212e2

chore(ci): run integration tests for metricbeat in a consistent manner

81d44cd

chore(ci): reorder xpack/metricbeat's pipeline to match oss'

dddf17c

chore(ci): run filebeat's integration tests in separate stages

5fe3f32

chore(ci): upload to GCP bucket system-tests tar files

5494848

chore(ci): invoke new mage command before archiving files

aa87f5f

chore(ci): remove unused script

d1dd718

mdelapenya self-assigned this Jun 8, 2021

mdelapenya added the Team:Automation Label for the Observability productivity team label Jun 8, 2021

mdelapenya requested a review from a team June 8, 2021 13:59

botelastic bot added needs_team Indicates that the issue/PR needs a Team:* label and removed needs_team Indicates that the issue/PR needs a Team:* label labels Jun 8, 2021

mdelapenya requested a review from jsoriano June 8, 2021 14:02

mdelapenya commented Jun 8, 2021

View reviewed changes

dev-tools/mage/pkg.go Outdated Show resolved Hide resolved

kuisathaverat reviewed Jun 8, 2021

View reviewed changes

dev-tools/mage/common.go Show resolved Hide resolved

v1v reviewed Jun 8, 2021

View reviewed changes

x-pack/filebeat/Jenkinsfile.yml Outdated Show resolved Hide resolved

x-pack/filebeat/Jenkinsfile.yml Outdated Show resolved Hide resolved

v1v added backport-v7.13.0 Automated backport with mergify backport-v7.14.0 Automated backport with mergify labels Jun 8, 2021

mdelapenya added 2 commits June 8, 2021 16:43

fix(ci): remove non-needed field in CI descriptor

57b6efb

chore: fix typo

d5d9f9d

mdelapenya requested a review from v1v June 8, 2021 15:52

v1v approved these changes Jun 8, 2021

View reviewed changes

mdelapenya marked this pull request as ready for review June 8, 2021 16:32

jsoriano reviewed Jun 8, 2021

View reviewed changes

dev-tools/mage/common.go Outdated Show resolved Hide resolved

dev-tools/mage/common.go Show resolved Hide resolved

dev-tools/mage/pkg.go Outdated Show resolved Hide resolved

chore: use ioutil for writing the tar file

c67f530

Co-authored-by: Jaime Soriano Pastor <[email protected]>

mdelapenya requested review from v1v and cachedout June 30, 2021 11:06

mdelapenya changed the title ~~feat: add a build command to compress the system-tests files~~ chore: standardise how unit/integration tests is run on CI, adding a build command to compress the system-tests files Jun 30, 2021

v1v reviewed Jun 30, 2021

View reviewed changes

chore: update comment

368417a

jsoriano reviewed Jun 30, 2021

View reviewed changes

dev-tools/mage/target/integtest/notests/integtest.go Outdated Show resolved Hide resolved

cachedout reviewed Jun 30, 2021

View reviewed changes

jsoriano reviewed Jun 30, 2021

View reviewed changes

dev-tools/mage/target/integtest/integtest.go Outdated Show resolved Hide resolved

mdelapenya added 6 commits June 30, 2021 17:42

chore: move new command to common package

0a17e48

We want to avoid collitions in beats overriding some commands

fix: create a plain directory layout for tar files

d99d0b0

chore: simplify creating path

f7b150a

chore: remove missing import comment

2ed629b

chore: add debug trace for files to upload

7aa8e53

mdelapenya added 4 commits July 19, 2021 08:44

Revert "chore: add debug trace for files to upload"

6c39510

This reverts commit 7aa8e53.

fix: bring parameter back after resolving conflicts

cf33568

mdelapenya added 2 commits July 21, 2021 09:12

Revert "Revert "chore: add debug trace for files to upload""

7bf7938

This reverts commit 6c39510.

chore: use absolute paths

83996c1

jsoriano requested changes Jul 26, 2021

View reviewed changes

chore: remove withModule from beats without modules

a230581

Co-authored-by: Jaime Soriano Pastor <[email protected]>

mdelapenya mentioned this pull request Jul 27, 2021

chore: decouple unit and integration tests on CI #27069

Merged

6 tasks

mdelapenya closed this Aug 23, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: standardise how unit/integration tests is run on CI, adding a build command to compress the system-tests files #26209

chore: standardise how unit/integration tests is run on CI, adding a build command to compress the system-tests files #26209

mdelapenya commented Jun 8, 2021 •

edited

Loading

botelastic bot commented Jun 8, 2021

elasticmachine commented Jun 8, 2021 •

edited by jenkins-beats-ci bot

Loading

Build stats

Test stats 🧪

Trends 🧪

Test stats 🧪

v1v left a comment

jsoriano left a comment

v1v Jun 30, 2021

cachedout Jun 30, 2021

mdelapenya Jul 1, 2021

mergify bot commented Jul 12, 2021

mdelapenya commented Jul 20, 2021

mdelapenya commented Jul 22, 2021

mdelapenya commented Jul 26, 2021

jsoriano left a comment

jsoriano Jul 26, 2021 •

edited

Loading

jsoriano Jul 26, 2021

jsoriano Jul 26, 2021

jsoriano Jul 26, 2021

mdelapenya Jul 27, 2021

mergify bot commented Aug 23, 2021

mdelapenya commented Aug 23, 2021

		// PackageSystemTests packages the python system tests results
		func PackageSystemTests() error {

chore: standardise how unit/integration tests is run on CI, adding a build command to compress the system-tests files #26209

chore: standardise how unit/integration tests is run on CI, adding a build command to compress the system-tests files #26209

Conversation

mdelapenya commented Jun 8, 2021 • edited Loading

What does this PR do?

Why is it important?

Checklist

Author's Checklist

How to test this PR locally

Related issues

Follow-ups

botelastic bot commented Jun 8, 2021

elasticmachine commented Jun 8, 2021 • edited by jenkins-beats-ci bot Loading

💚 Build Succeeded

Build stats

Test stats 🧪

Trends 🧪

💚 Flaky test report

Test stats 🧪

v1v left a comment

Choose a reason for hiding this comment

jsoriano left a comment

Choose a reason for hiding this comment

v1v Jun 30, 2021

Choose a reason for hiding this comment

cachedout Jun 30, 2021

Choose a reason for hiding this comment

mdelapenya Jul 1, 2021

Choose a reason for hiding this comment

mergify bot commented Jul 12, 2021

mdelapenya commented Jul 20, 2021

mdelapenya commented Jul 22, 2021

mdelapenya commented Jul 26, 2021

jsoriano left a comment

Choose a reason for hiding this comment

jsoriano Jul 26, 2021 • edited Loading

Choose a reason for hiding this comment

jsoriano Jul 26, 2021

Choose a reason for hiding this comment

jsoriano Jul 26, 2021

Choose a reason for hiding this comment

jsoriano Jul 26, 2021

Choose a reason for hiding this comment

mdelapenya Jul 27, 2021

Choose a reason for hiding this comment

mergify bot commented Aug 23, 2021

mdelapenya commented Aug 23, 2021

mdelapenya commented Jun 8, 2021 •

edited

Loading

elasticmachine commented Jun 8, 2021 •

edited by jenkins-beats-ci bot

Loading

jsoriano Jul 26, 2021 •

edited

Loading