Skip to content

Mage targets for faster development workflow#4731

Merged
jalvz merged 4 commits intoelastic:masterfrom
jalvz:quick-agent-dev
Feb 19, 2021
Merged

Mage targets for faster development workflow#4731
jalvz merged 4 commits intoelastic:masterfrom
jalvz:quick-agent-dev

Conversation

@jalvz
Copy link
Copy Markdown
Contributor

@jalvz jalvz commented Feb 15, 2021

Motivation/summary

Adds script to speed up fleet-related development:

run_agent.sh builds and packages an apm-server binary for Linux, creates a new Elastic Agent container based off the most recent image, mount-binds the apm-server package at distributions/apm-server-8.0.0-linux-x86_64 in the install directory, connects to the apm-integration-testing network, and runs it.

This is useful when a developing a feature that needs to be tested while running under Fleet.

@ghost
Copy link
Copy Markdown

ghost commented Feb 15, 2021

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: Pull request #4731 updated

  • Start Time: 2021-02-19T07:52:54.949+0000

  • Duration: 57 min 59 sec

  • Commit: 79db0ba

Test stats 🧪

Test Results
Failed 0
Passed 4751
Skipped 124
Total 4875

Trends 🧪

Image of Build Times

Image of Tests

Copy link
Copy Markdown
Member

@axw axw left a comment

Choose a reason for hiding this comment

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

Looks useful!

The main thing I'm not too happy about is that this adds dependencies to the main go.mod and consequently modifies NOTICE.txt. End-users shouldn't really be affected by this.

Could we do this as a couple of bash scripts instead? The QuickPackage target should be as simple as this:

#!/bin/bash
set -xe

version=$(mage QualifiedVersion)
builddir=build/distributions/apm-server-$version-linux-x86_64
mkdir -p $builddir

cp -f LICENSE.txt NOTICE.txt README.md apm-server.yml $builddir
GOOS=linux go build -o $builddir/apm-server ./x-pack/apm-server
tar cvzf $builddir.tar.gz $builddir

(Requires the addition of a mage target called "QualifiedVersion" to magefile.go for printing out mage.BeatQualifiedVersion))

The RunAgent target looks like it should be doable by invoking the docker CLI, but I haven't pored over it so maybe I missed something.

magefile.go Outdated
Comment on lines +213 to +221
err = sh.Run("tar", "cvf", buildDir+".tar", buildDir)
if err != nil {
return err
}

err = sh.Run("gzip", buildDir+".tar")
if err != nil {
return err
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
err = sh.Run("tar", "cvf", buildDir+".tar", buildDir)
if err != nil {
return err
}
err = sh.Run("gzip", buildDir+".tar")
if err != nil {
return err
}
err = sh.Run("tar", "cvzf", buildDir+".tar.gz", buildDir)
if err != nil {
return err
}

@jalvz
Copy link
Copy Markdown
Contributor Author

jalvz commented Feb 16, 2021

Could we do this as a couple of bash scripts instead?

That seems so much easier... 😅 🤦

Copy link
Copy Markdown
Member

@axw axw left a comment

Choose a reason for hiding this comment

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

Much simpler :)
Looks good, please revert NOTICE.txt and go.sum changes.

@jalvz
Copy link
Copy Markdown
Contributor Author

jalvz commented Feb 18, 2021

doh, I thought go mod tidy would fix that. thanks

Copy link
Copy Markdown
Member

@axw axw left a comment

Choose a reason for hiding this comment

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

Not sure why there are whitespace changes to NOTICE.txt -- probably should be reverted? Otherwise LGTM

@jalvz
Copy link
Copy Markdown
Contributor Author

jalvz commented Feb 18, 2021

Not sure why there are whitespace changes to NOTICE.txt

I don't know either, but since they are removed it seems more correct like this?

@axw
Copy link
Copy Markdown
Member

axw commented Feb 19, 2021

I don't know either, but since they are removed it seems more correct like this?

Only if it doesn't break automation (doesn't seem to have? I'm surprised). I'm not bothered otherwise.

@jalvz jalvz merged commit ce812f6 into elastic:master Feb 19, 2021
v1v added a commit to v1v/apm-server that referenced this pull request Feb 22, 2021
…chemas-to-agents

* upstream/master: (111 commits)
  Introduce metricset.name (elastic#4857)
  processor/otel: test service.version handling (elastic#4853)
  docs: Add PHP agent information to shared docs (elastic#4740)
  Script for faster development workflow (elastic#4731)
  Update to elastic/beats@1b31c26 (elastic#4763)
  backport: add 7.12 to .backportrc.json (elastic#4807)
  backport: enable auto-merge on backport PRs (elastic#4777)
  Support for Node.js profiles (elastic#4728)
  docs: readds .NET link (elastic#4764)
  [DOCS] Fixes URLs on Secure communication with APM Agents page (elastic#4746)
  ci: set proper parameters for the tar step (elastic#4696)
  docs: add 7.11.1 release notes (elastic#4727)
  Disable sourcemap upload endpoint when data streams enabled (elastic#4735)
  Add service name to dataset field (elastic#4674)
  Update to elastic/beats@ba423212a660 (elastic#4733)
  sampling: require a default policy (elastic#4729)
  processor/otel: add unit test for span status (elastic#4734)
  Add support for consuming OTLP/gRPC metrics (elastic#4722)
  [apmpackage] Add config options supported in ESS (elastic#4690)
  Use the apm-server version everywhere* (elastic#4725)
  ...
v1v added a commit to v1v/apm-server that referenced this pull request Feb 22, 2021
…te-schema-json-1

* upstream/master: (111 commits)
  Introduce metricset.name (elastic#4857)
  processor/otel: test service.version handling (elastic#4853)
  docs: Add PHP agent information to shared docs (elastic#4740)
  Script for faster development workflow (elastic#4731)
  Update to elastic/beats@1b31c26 (elastic#4763)
  backport: add 7.12 to .backportrc.json (elastic#4807)
  backport: enable auto-merge on backport PRs (elastic#4777)
  Support for Node.js profiles (elastic#4728)
  docs: readds .NET link (elastic#4764)
  [DOCS] Fixes URLs on Secure communication with APM Agents page (elastic#4746)
  ci: set proper parameters for the tar step (elastic#4696)
  docs: add 7.11.1 release notes (elastic#4727)
  Disable sourcemap upload endpoint when data streams enabled (elastic#4735)
  Add service name to dataset field (elastic#4674)
  Update to elastic/beats@ba423212a660 (elastic#4733)
  sampling: require a default policy (elastic#4729)
  processor/otel: add unit test for span status (elastic#4734)
  Add support for consuming OTLP/gRPC metrics (elastic#4722)
  [apmpackage] Add config options supported in ESS (elastic#4690)
  Use the apm-server version everywhere* (elastic#4725)
  ...
jalvz added a commit to jalvz/apm-server that referenced this pull request Mar 3, 2021
jalvz added a commit to jalvz/apm-server that referenced this pull request Mar 3, 2021
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.

2 participants