-
Notifications
You must be signed in to change notification settings - Fork 4.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[CI] Set MODULE per relevant stage #18777
Conversation
💚 Build SucceededExpand to view the summary
Build stats
Test stats 🧪
Steps errorsExpand to view the steps failures
|
a02ff7b
to
d8663f9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this! I have added a couple of suggestions.
Mage will be the way to go in the future, for such let's search for x-pack in the directory parameter or target (target is a bit more complex as it support more values than the folder only)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 if it looks good to Jenkins, it looks good to me too. Thanks!
…-stage-level * upstream/master: (30 commits) Add a GRPC listener service for Agent (elastic#18827) Disable host.* fields by default for iptables module (elastic#18756) [WIP] Clarify capabilities of the Filebeat auditd module (elastic#17068) fix: rename file and remove extra separator (elastic#18881) ci: enable JJBB (elastic#18812) Disable host.* fields by default for Checkpoint module (elastic#18754) Disable host.* fields by default for Cisco module (elastic#18753) Update latest.yml testing env to 7.7.0 (elastic#18535) Upgrade k8s.io/client-go and k8s keystore tests (elastic#18817) Add missing Jenkins stages for Auditbeat (elastic#18835) [Elastic Log Driver] Create a config shim between libbeat and the user (elastic#18605) Use indexers and matchers in config when defaults are enabled (elastic#18818) Fix panic on `metricbeat test modules` (elastic#18797) [CI] Fix permissions in MacOSX agents (elastic#18847) [Ingest Manager] When not port are specified and the https is used fallback to 443 (elastic#18844) [Ingest Manager] Fix install service script for windows (elastic#18814) [Metricbeat] Fix getting compute instance metadata with partial zone/region config (elastic#18757) Improve error messages in s3 input (elastic#18824) Add memory metrics into compute googlecloud (elastic#18802) include bucket name when logging error (elastic#18679) ...
❕ Build Aborted
Expand to view the summary
Build stats
Log outputExpand to view the last 100 lines of log output
|
…-stage-level * upstream/master: [CI] Fix permissions should not fail (elastic#18899) Revert "Allow the Docker image to be run with a random user id (elastic#12905)" (elastic#18872) Add new fields to HAProxy module of Metricbeat (elastic#18523) Avoid duplicate names in dynamic_templates (elastic#18849)
❕ Build Aborted
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
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM as well, as long as CI is happy.
jenkins run the tests please |
@@ -848,10 +862,27 @@ def mageTargetWin(String context, String directory, String target) { | |||
} | |||
} | |||
|
|||
def withBeatsEnv(boolean archive, Closure body) { | |||
def getModulePattern(String toCompare) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because this method seems processing the directory internally, I'd not delegate its call to the client code (the caller), instead calling it within the withBeatsEnv
method.
As an example:
//...
withBeatsEnv(archive: true, withModule: withModule, directory: directory) {
//...
withBeatsEnvWin(withModule: withModule, directory: directory) {
//...
if (withModule) {
modulePattern = args.containsKey('directory') ? getModulePattern(args.directory) : error('withBeatsEnv: directory parameter is required.')
}
//...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Uhm, I'm not sure if I understood correctly, there is only one minor discrepancy with the directory
. withBeatsEnv
doesn't need the directory argument, and makeTarget
does not have any directory at all but uses withBeatsEnv
. I think we cannot move in that direction
I've found what's going on
That's exactly the same error that happened in the master branch see build here
|
@ycombinator @jsoriano , I've just raised #18977 , as far as I see the issue seems to be with something in the master branch rather than in this particular PR. What do you want me to do? |
Yeah, I don't think this issue is related to this PR, I guess that this can be merged. |
💔 Build FailedExpand to view the summary
Build stats
Test stats 🧪
Steps errorsExpand to view the steps failures
Log outputExpand to view the last 100 lines of log output
|
I'll merge it now as the latest build didn't fail for the above-mentioned error but it's not related with the changes in this PR. |
@v1v could this be backported to the 7.8 branch? it is also affected by this issue. Thanks! |
Let me do it, for some reason I missed, although there is a dependency with #18835, so I'll backport it too otherwise the merge conflict might be a bit tedious |
What does this PR do?
Honor the env variable for those modules that are implementing the MODULE feature. For such, I did change the method signatures to use a map and use the withEnv, since env.MODULE does apply globally.
Filter specifically per top level folder. To avoid the issues spotted in [ci] speed builds for module specific changes #18592 (comment)
Why is it important?
Otherwise it causes errors if the MODULE is not valid.
How to test this PR locally
In the CI
Related issues
Closes #18741
Tasks