[CI][lint] Enable x-pack linting when OSS changes #23655

v1v · 2021-01-25T14:22:17Z

What does this PR do?

Changes in Beats should trigger builds of their x-pack counterparts

It will only run the linting if OSS changes among x-pack.
It will exclude any mandatory stages for x-pack if only OSS docs changes.

What beats have been enabled?

x-pack/auditbeat
x-pack/filebeat
x-pack/metricbeat
x-pack/packetbeat
x-pack/winlogbeat

Why is it important?

#22584 changed the packetbeat but didn't update the x-pack/packbeat. This broke the linting job for x-pack/packetbeat, but was not detected.

Related issues

Similar to #23378 but the other way around.

Further details

Code changeSet discover is delegated to the .ci/scripts/get-vendor-dependencies.sh but for non-code changes this is the proposal.

elasticmachine · 2021-01-25T15:09:09Z

💚 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

Build Cause: Pull request #23655 updated
- Start Time: 2021-01-26T13:12:33.549+0000
Duration: 45 min 53 sec
Commit: 510bb05

Test stats 🧪

Test	Results
Failed	0
Passed	10199
Skipped	1386
Total	11585

💚 Flaky test report

Tests succeeded.

Expand to view the summary

Test stats 🧪

Test	Results
Failed	0
Passed	10199
Skipped	1386
Total	11585

jsoriano · 2021-01-26T10:11:52Z

x-pack/libbeat/Jenkinsfile.yml

@@ -33,3 +34,7 @@ stages:
            tags: true         ## for all the tags
    build:
        mage: "mage build test"
+        when:                  ## Override the top-level when.
+            not_changeset_full_match: "^libbeat/.*" ## Disable the stage if ONLY changes for the OSS beats


If there are changes in the code of libbeat we should run the tests for x-pack/libbeat. I guess this is done now by @ci or GetProjectDependencies, but will this new condition prevent tests to be run if there are changes only in OSS?

Aha! Then this is totally a different behaviour that I though! good catch. The @xpack macro already contain the "^libbeat/.*", therefore we don't need this when

jsoriano · 2021-01-26T10:12:51Z

x-pack/metricbeat/Jenkinsfile.yml

@@ -20,6 +21,10 @@ stages:
          make check-no-changes;
    unitTest:
        mage: "mage build unitTest"
+        when:                  ## Override the top-level when.
+            not_changeset_full_match: "^metricbeat/.*" ## Disable the stage if ONLY changes for the OSS beats


Same here (and with the other beats), will this prevent x-pack metricbeat tests to be run if there were changes in oss metricbeat only?

Exactly, for clarity for the below x-pack/metricbeat CI stages:

lint stage (L18) will always run as long as the first when condition is set (see L1)

unitTest stage (L22) will run if the changeset does not contain ONLY metricbeat changes or it's a build in any branch/tag

not_changeset_full_match is a negative condition, exactly the opposite of what the majority of the other when conditions.

But we want x-pack tests to be run if there are changes in some dependency in OSS (as detected by GetProjectDependencies).

e.g. if we change only something in metricbeat/mb, we want to run x-pack metricbeat tests as it can be affected.

I'm gonna double check the expected behaviour works as expected, since I'm not convinced now if this might cause that particular regression :/

Since there is no an easy way with the proposal, I finally change a bit the implementation to exclude the x-pack/<beat> stages if there are only <beat>/docs changes.

e.g. if we change only something in metricbeat/mb, we want to run x-pack metricbeat tests as it can be affected.

It should we now be covered. I just executed some tests for the not_changeset_full_match when condition locally:

diff --git a/src/test/groovy/BeatsWhenStepTests.groovy b/src/test/groovy/BeatsWhenStepTests.groovy index 9f53c52..792a3a2 100644 --- a/src/test/groovy/BeatsWhenStepTests.groovy +++ b/src/test/groovy/BeatsWhenStepTests.groovy + @Test + void test_xpackmetricbeat_test_when_metricbeat_mb_changes() throws Exception { + def script = loadScript(scriptName) + def changeset = "metricbeat/mb/my-file.go" + helper.registerAllowedMethod('readFile', [String.class], { return changeset }) + def ret = script.whenNotChangesetFullMatch(content: [ not_changeset_full_match: '^metricbeat/docs/.*']) + printCallStack() + assertTrue(ret) + } +

$ ./mvnw test -Dtest=BeatsWhenStepTests#test_xpackmetricbeat_test_when_metricbeat_mb_changes ...[INFO] Running BeatsWhenStepTests [INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.981 s - in BeatsWhenStepTests [INFO] [INFO] Results: [INFO] [INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0 [INFO] [INFO] ------------------------------------------------------------------------ [INFO] BUILD SUCCESS

x-pack/libbeat/Jenkinsfile.yml

This is already supported with the `@xpack` macro and in fact, it should always run if libbeat changes

…pack-when-oss-changes * upstream/master: [DOCS] Add setup content to Kubernetes and Cloud Foundry docs (elastic#23580) [CI] Mandatory windows support for all the versions (elastic#23615) Add check when retrieving the worker process id using performance counters (elastic#23647) Remove 4912 evtx from testing (elastic#23669) Add missing SSL settings (elastic#23632) Update X-Pack Packetbeat config (elastic#23666) Use hostname check from verify.go to handle patterns in TLS certs (elastic#23661) Fix: Dissect Cisco ASA 302013 message usernames (elastic#21196) Add FAQ entry for MADV settings in older versions (elastic#23429) Sync fixes from Integration Package Testing (elastic#23424) [Filebeat] Add Cisco ASA message '302023' parsing (elastic#23092) [Elastic Log Driver] Change hosts config flag (elastic#23628) Audit and Authentication Policy Change Events (elastic#20684)

…1v/beats into feature/linting-xpack-when-oss-changes * 'feature/linting-xpack-when-oss-changes' of github.com:v1v/beats: Apply suggestions from code review

jsoriano

Thanks! It looks ok, but it is starting to get a bit complicated to follow.

jsoriano · 2021-01-26T16:27:04Z

x-pack/auditbeat/Jenkinsfile.yml

+        when:                  ## Override the top-level when.
+            not_changeset_full_match: "^auditbeat/docs/.*" ## Disable the stage if ONLY DOC changes for the OSS beats
+            branches: true                         ## for all the branches
+            tags: true                             ## for all the tags


Is there a reason to have this override in build and windows*, but not in arm and macos?

A brief explanation:

Top-level when condition is the very first evaluation.

Then, any stages with a when condition will be evaluated again. Therefore when for stages and when for top-level don't work together. The initial approach didn't require anything further.

Is there a reason to have this override in build and windows*, but not in arm and macos?

Mandatory stages with this particular cross-project linting require the when: not_changeset_full_match... while arm and macos are optional stages, so they do need to run on a PR basis unless any of the when conditions.

jsoriano · 2021-01-26T16:28:15Z

x-pack/auditbeat/Jenkinsfile.yml

@@ -2,6 +2,7 @@ when:
    branches: true             ## for all the branches
    changeset:                 ## when PR contains any of those entries in the changeset
        - "^x-pack/auditbeat/.*"
+        - "^auditbeat/.*"      ## when changes in the auditbeat


I guess this cannot be added for linting only, so we don't have to add the negation in all the other builds?

I could not find a different approach with the existing declarative syntax :(

Although,I just found something that I wanna give a go in another PR

My new proposal -> #23695

v1v added 2 commits January 25, 2021 14:13

[CI][lint] Enable x-pack linting when OSS changes

b8d5e7b

Change regex

7c66eb5

v1v requested a review from a team as a code owner January 25, 2021 14:22

botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Jan 25, 2021

v1v requested review from jsoriano and a team January 25, 2021 14:22

v1v added automation ci Team:Automation Label for the Observability productivity team labels Jan 25, 2021

botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Jan 25, 2021

mdelapenya approved these changes Jan 25, 2021

View reviewed changes

kuisathaverat approved these changes Jan 25, 2021

View reviewed changes

cachedout approved these changes Jan 25, 2021

View reviewed changes

jsoriano reviewed Jan 26, 2021

View reviewed changes

v1v commented Jan 26, 2021

View reviewed changes

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

v1v commented Jan 26, 2021

View reviewed changes

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

v1v and others added 4 commits January 26, 2021 10:22

Apply suggestions from code review

014a3c8

This is already supported with the `@xpack` macro and in fact, it should always run if libbeat changes

Merge branch 'feature/linting-xpack-when-oss-changes' of github.com:v…

d6df0b1

…1v/beats into feature/linting-xpack-when-oss-changes * 'feature/linting-xpack-when-oss-changes' of github.com:v1v/beats: Apply suggestions from code review

Exclude builds if only OSS docs changes in x-pack

510bb05

v1v requested a review from jsoriano January 26, 2021 13:22

jsoriano approved these changes Jan 26, 2021

View reviewed changes

v1v mentioned this pull request Jan 26, 2021

[CI] Explicit linting stages for OSS and x-pack #23695

Merged

v1v closed this Jan 27, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[CI][lint] Enable x-pack linting when OSS changes #23655

[CI][lint] Enable x-pack linting when OSS changes #23655

v1v commented Jan 25, 2021 •

edited

Loading

elasticmachine commented Jan 25, 2021 •

edited by jenkins-beats-ci bot

Loading

Build stats

Test stats 🧪

Test stats 🧪

jsoriano Jan 26, 2021

v1v Jan 26, 2021

jsoriano Jan 26, 2021

v1v Jan 26, 2021

jsoriano Jan 26, 2021

v1v Jan 26, 2021

v1v Jan 26, 2021

v1v Jan 26, 2021

jsoriano left a comment

jsoriano Jan 26, 2021

v1v Jan 26, 2021

jsoriano Jan 26, 2021

v1v Jan 26, 2021

v1v Jan 26, 2021

[CI][lint] Enable x-pack linting when OSS changes #23655

[CI][lint] Enable x-pack linting when OSS changes #23655

Conversation

v1v commented Jan 25, 2021 • edited Loading

What does this PR do?

Why is it important?

Related issues

Further details

elasticmachine commented Jan 25, 2021 • edited by jenkins-beats-ci bot Loading

💚 Build Succeeded

Build stats

Test stats 🧪

💚 Flaky test report

Test stats 🧪

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jsoriano left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

v1v commented Jan 25, 2021 •

edited

Loading

elasticmachine commented Jan 25, 2021 •

edited by jenkins-beats-ci bot

Loading