[ci] speed builds for module specific changes #18592
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
botelastic
bot
added
the
needs_team
4 tasks
v1v
force-pushed
the
feature/ci-for-single-modules
branch
from
1cb0924 to
f22c835
Compare
💔 Tests Failed
Expand to view the summary
Build stats
Test stats 🧪
Test errorsExpand to view the tests failures
Steps errorsExpand to view the steps failures
Log outputExpand to view the last 100 lines of log output
|
…ngle-modules * upstream/master: [Auditbeat] Update auditbeat ECS mappings (elastic#18596)
This reverts commit f22c835.
v1v
commented
May 16, 2020
Jenkinsfile
Outdated
| @@ -1,6 +1,6 @@ | |||
| #!/usr/bin/env groovy | |||
|
|
|||
| @Library('apm@current') _ | |||
| @Library('apm@test/single-module') _ | |||
botelastic
bot
removed
the
needs_team
|
jenkins run the tests please |
v1v
requested review from
kuisathaverat,
ycombinator,
cachedout,
mikemadden42 and
andrewkroh
kuisathaverat
approved these changes
May 18, 2020
|
Hi @v1v, question about the commit you made to test this PR: 6b65f05. The changeset for that commit was: Given that, I'd expect only the Metricbeat stages to have run, but looking at https://beats-ci.elastic.co/blue/organizations/jenkins/Beats%2Fbeats-beats-mbp/detail/PR-18592/6/pipeline it looks like all stages (Auditbeat, Agent, etc.) were run. |
ycombinator
reviewed
May 19, 2020
| // Run the ITs by running only if the changeset affects a specific module. | ||
| // For such, it's required to look for changes under the module folder and exclude anything else | ||
| // such as ascidoc and png files. | ||
| env.MODULE = getGitMatchingGroup(pattern: '[a-z0-9]+beat\\/module\\/([^\\/]+)\\/.*', exclude: '^(((?!\\/module\\/).)*$|.*\\.asciidoc|.*\\.png)') |
There was a problem hiding this comment.
Given that we're now excluding anything that's not under /module/, do we also need the .*\.asciidoc and .*\.png patterns in the exclude list?
There was a problem hiding this comment.
Yes, we do, otherwise all those PRs that change the top-level CHANGELOG.next.asciidoc will not match the pattern, besides, the exclude does include the png files similar to the pattern that it is used for whether the PR is only related to docs changes. We could potentially remove the png if required, although I don't know all the cases, but the asciidoc is needed.
For instance:
There was a problem hiding this comment.
Thanks, lets keep this for now and optimize later.
Expected behaviour, in this PR there are changes in the
|
ycombinator
approved these changes
May 20, 2020
v1v
changed the title
v1v
added a commit
to v1v/beats
that referenced
this pull request
May 20, 2020
This was referenced May 20, 2020
v1v
added a commit
to v1v/beats
that referenced
this pull request
May 20, 2020
v1v
added a commit
that referenced
this pull request
May 20, 2020
v1v
added a commit
that referenced
this pull request
May 20, 2020
6 tasks
|
There is one issue with this change in some builds. Some of the current mage targets fail if the module specified by This makes me wonder what happens if multiple modules are modified, what is the result of the call to |
|
Yes, I think we need to do two things now:
1. As you say, look into supporting multiple modules’ changes.
2. Move the setting of the MODULE env var from the global scope into each
relevant build stage (Filebeat OSS, Metricbeat OSS, Filebeat X-Pack, etc.)
and adjust the patterns to fit each stage.
Shaunak
…On Mon, May 25, 2020 at 11:18 AM Jaime Soriano Pastor < ***@***.***> wrote:
There is one issue with this change in some builds. Some of the current
mage targets fail if the module specified by MODULE doesn't exist. This
happens at least with mage goIntegTest in Metricbeat. This can be an
issue if a change triggers builds for multiple beats, because not all of
them are going to have all the modules.
For example in this build
<https://beats-ci.elastic.co/blue/organizations/jenkins/Beats%2Fbeats-beats-mbp/detail/PR-18711/4/pipeline>
MODULE seems to be set to kafka, what is correct because a test is
modified in the kafka module in OSS metricbeat, but x-pack metricbeat build
fails because it doesn't have a kafka module.
This can be locally reproduced with MODULE=kafka mage goIntegTest from
the x-pack/metricbeat directory.
I guess that one option to solve this is to use MODULE only as a filter,
so it only executes tests for the module in MODULE.
This makes me wonder what happens if multiple modules are modified, what
is the result of the call to getGitMatchingGroup?
In principle current mage targets that make use of MODULE expect a single
module, but it could be interesting to support multiple modules.
—
You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub
<#18592 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAMO5K65EWS4EBUB5UVYOTRTKD33ANCNFSM4NBYCQPQ>
.
|
An empty value, so far it does only look for all the changes in PRs with only one module. In other words, if there are PRs with multiple changes in multiple modules then the MODULE env variable will be set to empty.
Would you mind raising an issue with those requirements? |
Done: #18741 |
Good. Then I guess that the only urgent thing to do would be #18741. Thanks @v1v and @ycombinator for the explanations and creating the issues! |
This was referenced May 28, 2020
6 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Use the step to gather what's the module that has been changed when the changes are related to a specific module only. This will help to skip certain tests that are not related to the module that has been changes.
Why is it important?
Fast builds with the tests that required to be tested rather than everything else.
How to test this PR locally
In the CI, touch certain files for the same module and see whether it does what's expected or the other way around, touch a few files and see if it works.
6b65f05 caused
See the very last line in the log