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

Update beats #1053

Merged
merged 4 commits into from
Jul 4, 2018
Merged

Update beats #1053

merged 4 commits into from
Jul 4, 2018

Conversation

jalvz
Copy link
Contributor

@jalvz jalvz commented Jun 29, 2018

This update includes packaging refactor (elastic/beats#7388)

required before #1010

@jalvz
Copy link
Contributor Author

jalvz commented Jun 29, 2018

@andrewkroh thanks for explicitly testing the new packaging with APM, seems to work smoothly 👏

how do we use mage for the snapshot / release targets?

@andrewkroh
Copy link
Member

andrewkroh commented Jun 29, 2018

I'm happy to help with any changes. Here's a summary of what I think needs to be done.

  1. Add magefile.go from this gist.
  2. Add package/README.md.tmpl from the gist. Where to put it is up to you. If you change the location then update the path in magefile.go.
  3. Apply the patch to Makefile from the gist.
  4. Update vendor/ to include:
    • github.com/elastic/beats/dev-tools/mage
    • github.com/magefile/mage/^
    • golang.org/x/tools/go/vcs

This enables make mage to work and satisfies the dependencies of magefile.go.

Testing

  • Run make mage and test that mage is installed to $GOPATH/bin/mage.

  • Test that mage -l works and shows a list of build targets.

  • Test make snapshot and verify snapshots are in build/distributions.

  • Test make release and verify non-snapshots are in build/distributions.

External Changes

I would hold off on merging this in until after we have tested and verified everything with Beats.

release-manager

apm-server-docker

jenkins

@jalvz
Copy link
Contributor Author

jalvz commented Jul 3, 2018

Many thanks @andrewkroh, done after some minor tweakings, tested the release targets as well and tried the darwin build.

elastic/apm-server-docker#20
https://github.com/elastic/infra/pull/5798
https://github.com/elastic/release-manager/pull/365

Anything else you found out lately?

@jalvz
Copy link
Contributor Author

jalvz commented Jul 3, 2018

@simitt @graphaelli would be great if any of you has a look as well when you have some time

Copy link
Member

@andrewkroh andrewkroh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. But I would rely more on CI's opinion 😄

We merged one more change to add DMG packaging, but I would add that in a separate update. It will require a small add to your customizePackaging() method to overwrite the appropriate files inside the macos installer.

Makefile Outdated
@@ -11,12 +10,10 @@ TEST_ENVIRONMENT=true
ES_BEATS?=./_beats
PREFIX?=.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think PREFIX is no longer used anywhere.

Copy link
Contributor

@simitt simitt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apart from one minor comment LGTM afaict.

@@ -1,5 +1,4 @@
BEAT_NAME=apm-server
BEAT_DESCRIPTION=Elastic Application Performance Monitoring Server
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From what I saw this would be actually used in mage. Where does the description now come from?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants