Update metricbeat to use mage for all scenarios along with Makefile shim #17799

blakerouse · 2020-04-17T16:23:26Z

What does this PR do?

Update's metricbeat to use mage for all scenarios and replace Makefile with the mage Makefile shim.

Why is it important?

While working on #17656 I ran into issues with running the integration tests because the current Makefile would override the usage of docker-compose instead of using the logic that was already in the mage work.

This ensures that the same logic is used for x-pack/metricbeat and metricbeat.

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.~~

How to test this PR locally

cd metricbeat && mage update build test

elasticmachine · 2020-04-17T16:23:29Z

Pinging @elastic/integrations-platforms (Team:Platforms)

elasticmachine · 2020-04-17T16:23:30Z

Pinging @elastic/integrations-services (Team:Services)

blakerouse · 2020-04-17T16:24:15Z

@andrewkroh This is probably of interest to you.

jsoriano

I love this change, I have added some comments by now, I will try it also locally before approving.

Let's wait to have some greener builds too. Some failures in CI are fixed in master.

dev-tools/make/mage.mk

metricbeat/tests/system/test_base.py

metricbeat/Makefile

metricbeat/magefile.go

blakerouse · 2020-04-20T14:19:34Z

@jsoriano I added the create-metricset back into the Makefile that calls mage createMetricset so the python environment can be setup correctly without have to use the old python-env.

I rebased with master, so hopefully it will go all green.

jsoriano · 2020-04-20T14:31:08Z

run beats-ci/package

x-pack/metricbeat/tests/system/test_xpack_base.py

blakerouse · 2020-04-20T17:34:45Z

run beats-ci/package

blakerouse · 2020-04-21T11:40:18Z

@jsoriano Been trying since yesterday to get all green, no matter what there always seems to be something random. At least this time Travis passed completely.

The filebeat tests are un-related to any changes here.

Build and Test / Filebeat Mac OS X / test_crawler.Test.test_file_disappear
Build and Test / Filebeat Mac OS X / test_registrar.Test.test_clean_removed_with_clean_inactive
Build and Test / Filebeat Mac OS X / test_registrar.Test.test_registrar_files
Build and Test / Filebeat Mac OS X / test_registrar.Test.test_rotating_file_with_restart
Build and Test / Filebeat Mac OS X / test_registrar_upgrade.Test.test_upgrade_from_6_3_1
Build and Test / Filebeat Mac OS X / test_registrar_upgrade.Test.test_upgrade_from_faulty_6_3_1
Build and Test / Filebeat Mac OS X / test_registrar_upgrade.Test.test_upgrade_from_latest

blakerouse · 2020-04-21T11:40:41Z

run beats-ci/package

jsoriano · 2020-04-21T13:31:46Z

CI looks good, yes, I don't think the failures in OSX are related to this change.

jsoriano

It LGTM, but I would like to see a second opinion. @andrewkroh could you take a quick look in case we are missing something? 😇

andrewkroh

Nice improvements. Let some comments and questions. Nothing major.

Jenkinsfile

andrewkroh · 2020-04-22T02:10:13Z

dev-tools/make/mage.mk

+
+.PHONY: unit-tests
+unit-tests: mage
+	mage update unitTest


Since this is a new target, I don't think update should be required. The unitTest targets should have the appropriate dependencies wired up already. In most cases I think they do now, like requiring the Fields before running the python tests.

The diff is wierd, because this is actually just xpack.mk that was renamed to mage.mk because now its not only x-pack projects that use the file.

I will remove it but it does affect, all x-pack projects. Hopefully those do the correct thing.

andrewkroh · 2020-04-22T02:12:34Z

dev-tools/make/mage.mk

-	@-mage -clean
-endif
-	@true
+# This is a minimal Makefile for Beats that are built with Mage. Its only


Why are you adding new targets to the file as opposed to directly invoking mage from Travis without any shim? Is it because the Travis file isn't setup to do that?

The goes with my comment above. This is really just xpack.mk renamed to mage.mk. These targets are the same targets shared with the other x-pack beats. The travis file does use these a lot.

If we want to remove the use of a Makefile shim completely, then I think we should move that to a different PR. This is currently trying to make metricbeat just like x-pack/metricbeat.

The reason I asked was that unit-tests and system-tests were not in the former xpack.mk. But I see that this file is now being used to shim targets used from metricbeat/Makefile. So no problem. We'll be able to remove this shim makefile soon.

dev-tools/make/mage.mk

andrewkroh · 2020-04-22T02:13:36Z

metricbeat/Makefile

-
-# Collects all module configs
-.PHONY: configs
-configs: python-env


metricbeat/magefile.go

blakerouse · 2020-04-22T12:26:01Z

@andrewkroh Ready for another look.

andrewkroh · 2020-04-22T18:03:24Z

This is an issue in the generator tests w.r.t. the dashboards target.

https://beats-ci.elastic.co/blue/organizations/jenkins/Beats%2Fbeats-beats-mbp/detail/PR-17799/12/pipeline#step-1156-log-277

It might be something I broke.

blakerouse · 2020-04-23T16:30:09Z

@andrewkroh I fixed the generator test, the other failures are unrelated to my change.

… less time and not run over the timeline.

andrewkroh

Thanks!

LGTM.

…him (elastic#17799) * Update metricbeat to use mage and the mage make targets. * Run update and fmt. * Remove assets.go as its no longer used. * Split apart the metricbeat tests in travis into multiple jobs to take less time and not run over the timeline. * Add back in create-metricset with mage. * Re-add test_xpack_base.py. * Add kafka to requirements.txt. * Fix kafka to work with python 3.7. * Remove the extra kafka-python from rebase on master. * Fix issues from code review. * Add back in comment from merge. * Fix comment spacing. * Fix update target in generate metricbeat. * Update magefile in metricbeat generator. * Run fmt and update. (cherry picked from commit 8786d05)

…arios along with Makefile shim (#17951) * Update metricbeat to use mage for all scenarios along with Makefile shim (#17799) * Update metricbeat to use mage and the mage make targets. * Run update and fmt. * Remove assets.go as its no longer used. * Split apart the metricbeat tests in travis into multiple jobs to take less time and not run over the timeline. * Add back in create-metricset with mage. * Re-add test_xpack_base.py. * Add kafka to requirements.txt. * Fix kafka to work with python 3.7. * Remove the extra kafka-python from rebase on master. * Fix issues from code review. * Add back in comment from merge. * Fix comment spacing. * Fix update target in generate metricbeat. * Update magefile in metricbeat generator. * Run fmt and update. (cherry picked from commit 8786d05) * Fix merge issue. * Run fmt and update.

blakerouse added enhancement Team:Services (Deprecated) Label for the former Integrations-Services team Team:Platforms Label for the Integrations - Platforms team labels Apr 17, 2020

blakerouse requested a review from a team as a code owner April 17, 2020 16:23

blakerouse mentioned this pull request Apr 17, 2020

Run kubernetes integration tests inside of a pod and use kind to setup a kubernetes cluster #17656

Merged

5 tasks

jsoriano self-requested a review April 17, 2020 17:03

andresrc added [zube]: Inbox [zube]: In Review and removed [zube]: Inbox labels Apr 19, 2020

jsoriano reviewed Apr 20, 2020

View reviewed changes

dev-tools/make/mage.mk Show resolved Hide resolved

metricbeat/tests/system/test_base.py Outdated Show resolved Hide resolved

metricbeat/Makefile Show resolved Hide resolved

metricbeat/Makefile Show resolved Hide resolved

metricbeat/magefile.go Show resolved Hide resolved

blakerouse force-pushed the metricbeat-mage branch from b575258 to f4f04df Compare April 20, 2020 14:17

jsoriano reviewed Apr 20, 2020

View reviewed changes

x-pack/metricbeat/tests/system/test_xpack_base.py Show resolved Hide resolved

blakerouse force-pushed the metricbeat-mage branch from 8c9a979 to 41cc42a Compare April 21, 2020 01:30

jsoriano reviewed Apr 21, 2020

View reviewed changes

blakerouse force-pushed the metricbeat-mage branch from 41cc42a to 8c83203 Compare April 22, 2020 01:04

andrewkroh reviewed Apr 22, 2020

View reviewed changes

blakerouse added 2 commits April 23, 2020 12:38

Update metricbeat to use mage and the mage make targets.

e376bc7

Run update and fmt.

659d091

blakerouse added 12 commits April 23, 2020 12:38

Remove assets.go as its no longer used.

a76ff78

Split apart the metricbeat tests in travis into multiple jobs to take…

09173fe

… less time and not run over the timeline.

Add back in create-metricset with mage.

ee486f4

Re-add test_xpack_base.py.

10ce059

Add kafka to requirements.txt.

312e96a

Fix kafka to work with python 3.7.

a9b911b

Remove the extra kafka-python from rebase on master.

d3ff2b4

Fix issues from code review.

a6977fc

Add back in comment from merge.

c908517

Fix comment spacing.

a818e6a

Fix update target in generate metricbeat.

8c80bef

Update magefile in metricbeat generator.

8ec4512

blakerouse force-pushed the metricbeat-mage branch from 19d7ba8 to 8ec4512 Compare April 23, 2020 16:38

Run fmt and update.

275ecc9

andrewkroh approved these changes Apr 23, 2020

View reviewed changes

blakerouse merged commit 8786d05 into elastic:master Apr 23, 2020

zube bot added [zube]: Done and removed [zube]: In Review labels Apr 23, 2020

blakerouse deleted the metricbeat-mage branch April 23, 2020 18:09

blakerouse mentioned this pull request Apr 23, 2020

Cherry-pick #17799 to 7.x: Update metricbeat to use mage for all scenarios along with Makefile shim #17951

Merged

2 tasks

blakerouse added the v7.8.0 label Apr 23, 2020

andresrc removed the [zube]: Done label Apr 29, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update metricbeat to use mage for all scenarios along with Makefile shim #17799

Update metricbeat to use mage for all scenarios along with Makefile shim #17799

blakerouse commented Apr 17, 2020 •

edited by andresrc

Loading

elasticmachine commented Apr 17, 2020

elasticmachine commented Apr 17, 2020

blakerouse commented Apr 17, 2020

jsoriano left a comment

blakerouse commented Apr 20, 2020

jsoriano commented Apr 20, 2020

blakerouse commented Apr 20, 2020

blakerouse commented Apr 21, 2020

blakerouse commented Apr 21, 2020

jsoriano commented Apr 21, 2020

jsoriano left a comment

andrewkroh left a comment

andrewkroh Apr 22, 2020

blakerouse Apr 22, 2020

andrewkroh Apr 22, 2020

blakerouse Apr 22, 2020

andrewkroh Apr 22, 2020

andrewkroh Apr 22, 2020

blakerouse commented Apr 22, 2020

andrewkroh commented Apr 22, 2020 •

edited

Loading

blakerouse commented Apr 23, 2020

andrewkroh left a comment

Update metricbeat to use mage for all scenarios along with Makefile shim #17799

Update metricbeat to use mage for all scenarios along with Makefile shim #17799

Conversation

blakerouse commented Apr 17, 2020 • edited by andresrc Loading

What does this PR do?

Why is it important?

Checklist

How to test this PR locally

elasticmachine commented Apr 17, 2020

elasticmachine commented Apr 17, 2020

blakerouse commented Apr 17, 2020

jsoriano left a comment

Choose a reason for hiding this comment

blakerouse commented Apr 20, 2020

jsoriano commented Apr 20, 2020

blakerouse commented Apr 20, 2020

blakerouse commented Apr 21, 2020

blakerouse commented Apr 21, 2020

jsoriano commented Apr 21, 2020

jsoriano left a comment

Choose a reason for hiding this comment

andrewkroh left a comment

Choose a reason for hiding this comment

andrewkroh Apr 22, 2020

Choose a reason for hiding this comment

blakerouse Apr 22, 2020

Choose a reason for hiding this comment

andrewkroh Apr 22, 2020

Choose a reason for hiding this comment

blakerouse Apr 22, 2020

Choose a reason for hiding this comment

andrewkroh Apr 22, 2020

Choose a reason for hiding this comment

andrewkroh Apr 22, 2020

Choose a reason for hiding this comment

blakerouse commented Apr 22, 2020

andrewkroh commented Apr 22, 2020 • edited Loading

blakerouse commented Apr 23, 2020

andrewkroh left a comment

Choose a reason for hiding this comment

blakerouse commented Apr 17, 2020 •

edited by andresrc

Loading

andrewkroh commented Apr 22, 2020 •

edited

Loading