Skip to content
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 #18741

Closed
ycombinator opened this issue May 26, 2020 · 3 comments · Fixed by #18777
Closed

[CI] Set MODULE per relevant stage #18741

ycombinator opened this issue May 26, 2020 · 3 comments · Fixed by #18777
Assignees
Labels
automation ci Team:Automation Label for the Observability productivity team

Comments

@ycombinator
Copy link
Contributor

ycombinator commented May 26, 2020

Follow up to #18592. In #18592, we set the MODULE environment variable at a global scope. As such, once the MODULE is determined and set, it gets used by all stages of the Jenkins CI pipeline. This can cause errors when the value of MODULE isn't valid for a a stage (e.g. X-Pack Filebeat doesn't implement the module specified in MODULE).

One fix would be to move the setting of MODULE from the global scope into each relevant stage of the Jenkins CI pipeline. We would only set it in those stages that implement Beats modules, viz. OSS Filebeat, OSS Metricbeat, OSS Auditbeat, X-Pack Filebeat, X-Pack Metricbeat, X-Pack Auditbeat, and X-Pack Winlogbeat. In each stage, the pattern used to determine whether module files have changed would need to be scoped to files relevant to that stage, e.g. x-pack/filebeat/module/* for the X-Pack Filebeat stage.

@ycombinator ycombinator added the ci label May 26, 2020
@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label May 26, 2020
@ycombinator ycombinator added the Team:Automation Label for the Observability productivity team label May 26, 2020
@botelastic botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label May 26, 2020
@v1v v1v self-assigned this May 27, 2020
@v1v v1v added the automation label May 27, 2020
@v1v
Copy link
Member

v1v commented May 27, 2020

@ycombinator, got a question regarding the description:

(e.g. X-Pack Filebeat doesn't implement the module specified in MODULE).

..., OSS Auditbeat, X-Pack Filebeat, X-Pack Metricbeat,

There is a contradiction with X-Pack Filebeat as far as I understood.

BTW, I just coded some changes in the CI Pipeline, to explicitly enable the MODULE env variable in the above relevant stages, the stages I did enable are the ones that are running the tests targets, please let me know if I missed any other stage:

#18777

See the changes with , withModule: true)

@ycombinator
Copy link
Contributor Author

There is a contradiction with X-Pack Filebeat as far as I understood.

Ah, sorry for the confusion. X-Pack FIlebeat does have modules so the MODULE environment variable should be set for it's build stage.

I was trying to give an example of the current state of affairs where the MODULE is being set at the global level. Today, MODULE could get set to a value that is invalid for certain stages. Using the same example that @jsoriano gave in #18652

For example in this build 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.

I was just trying to illustrate the same situation, but with X-Pack Filebeat and OSS Filebeat instead of X-Pack Metricbeat and OSS Metricbeat in @jsoriano's example. Sorry for causing confusion!

@jsoriano
Copy link
Member

Another example of this happened in #18802, a change for googlecloud module triggered all Metricbeat builds, but only x-pack Metricbeat has this module, then in OSS Metricbeat:

Error: no module googlecloud

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automation ci Team:Automation Label for the Observability productivity team
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants