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

Refactor Beat packaging and cross-building #7388

Merged
merged 6 commits into from
Jun 29, 2018

Conversation

andrewkroh
Copy link
Member

@andrewkroh andrewkroh commented Jun 21, 2018

This refactors Beat packaging to use a declarative YAML based packaging
specification. This makes it easier to customize the contents of packages (e.g.
adding additional X-Pack content or simply tailoring what's included for single
Beat).

The specification itself is pretty simple. It consists of package metadata and a
list of files to include. The values can be templated which allows for a single
package spec to be reused across Beats. Here's an example spec for an OSS
Windows zip package.

spec:
  name:         '{{.BeatName}}-oss'
  service_name: '{{.BeatServiceName}}'
  os:           '{{.GOOS}}'
  arch:         '{{.PackageArch}}'
  vendor:       '{{.BeatVendor}}'
  version:      '{{ beat_version }}'
  license:      ASL 2.0
  url:          '{{.BeatURL}}'
  description: '{{.BeatDescription}}'
  files:
    '{{.BeatName}}{{.BinaryExt}}':
      source: build/golang-crossbuild/{{.BeatName}}-{{.GOOS}}-{{.Platform.Arch}}{{.BinaryExt}}
      mode:   0755
    fields.yml:
      source: fields.yml
      mode:   0644
    LICENSE.txt:
      source: '{{ repo.RootDir }}/licenses/APACHE-LICENSE-2.0.txt'
      mode: 0644
    NOTICE.txt:
      source: '{{ repo.RootDir }}/NOTICE.txt'
      mode:   0644
    README.md:
      template: '{{ elastic_beats_dir }}/dev-tools/mage/templates/common/README.md.tmpl'
      mode:   0644
    .build_hash.txt:
      content: >
        {{ commit }}
      mode:   0644
    '{{.BeatName}}.reference.yml':
      source: '{{.BeatName}}.reference.yml'
      mode:   0644
    '{{.BeatName}}.yml':
      source: '{{.BeatName}}.yml'
      mode: 0600
      config: true
    kibana:
      source: _meta/kibana.generated
      mode:   0644
    install-service-{{.BeatName}}.ps1:
      template: '{{ elastic_beats_dir }}/dev-tools/mage/templates/windows/install-service.ps1.tmpl'
      mode: 0755
    uninstall-service-{{.BeatName}}.ps1:
      template: '{{ elastic_beats_dir }}/dev-tools/mage/templates/windows/uninstall-service.ps1.tmpl'
      mode: 0755

Each Beat has two build targets for packaging.

  • make snapshot
  • make release

The set of target platforms can be influenced by the PLATFORMS environment
variable which accepts an platform selection expression. For example to add
ARMv7 (for your Raspberry Pi) to the default set of platforms you would use
PLATFORMS='+linux/armv7' make snapshot. Or to only build for Windows set
PLATFORMS='windows'. Full details can be found in the godocs for
NewPlatformList.

For the release manager there are two new top-level targets that take care of
ensuring that the proper Go version is used. The naming here aligns with what
several of the other projects are using for their release manager targets.

  • make release-manager-snapshot
  • make release-manager-release

Build Process Details

Below is the command line output of building and packaging Packetbeat for
linux/armv7 only. I'll describe each step in the build process.

$ PLATFORMS='+all linux/armv7' make snapshot
Installing mage from vendor
>> golangCrossBuild: Building for linux/armv7
>> buildGoDaemon: Building for linux/armv7
>> Building using: cmd='build/mage-linux-amd64 golangCrossBuild', env=[CC=arm-linux-gnueabihf-gcc, CXX=arm-linux-gnueabihf-g++, GOARCH=arm, GOARM=7, GOOS=linux, PLATFORM_ID=linux-armv7]
>> Building using: cmd='build/mage-linux-amd64 buildGoDaemon', env=[CC=arm-linux-gnueabihf-gcc, CXX=arm-linux-gnueabihf-g++, GOARCH=arm, GOARM=7, GOOS=linux, PLATFORM_ID=linux-armv7]
grammar.y: warning: 38 shift/reduce conflicts [-Wconflicts-sr]
>> package: Building packetbeat type=tar.gz for platform=linux/armv7
>> package: Building packetbeat-oss type=deb for platform=linux/armv7
>> package: Building packetbeat type=rpm for platform=linux/armv7
>> package: Building packetbeat type=deb for platform=linux/armv7
>> package: Building packetbeat-oss type=tar.gz for platform=linux/armv7
>> package: Building packetbeat-oss type=rpm for platform=linux/armv7
>> Testing package contents
package ran for 1m6.597416206s

$ tree build/distributions/
build/distributions/
├── packetbeat-oss-7.0.0-alpha1-SNAPSHOT-armhf.deb
├── packetbeat-oss-7.0.0-alpha1-SNAPSHOT-armhf.deb.sha512
├── packetbeat-oss-7.0.0-alpha1-SNAPSHOT-armhfp.rpm
├── packetbeat-oss-7.0.0-alpha1-SNAPSHOT-armhfp.rpm.sha512
├── packetbeat-oss-7.0.0-alpha1-SNAPSHOT-linux-armv7.tar.gz
└── packetbeat-oss-7.0.0-alpha1-SNAPSHOT-linux-armv7.tar.gz.sha512
  1. Install mage from the vendor directory to $GOPATH/bin.

    Mage is used to provide a simple gmake like wrapper around build logic that's
    written in Go. Additionally it makes is possible to build and package without
    gmake by invoking mage directly [think Windows users]. For example:

   $ mage -l
    Targets:
      build                 builds the Beat binary.
      buildGoDaemon         builds the go-daemon binary (use crossBuildGoDaemon).
      clean                 cleans all generated files and build artifacts.
      crossBuild            cross-builds the beat for all target platforms.
      crossBuildGoDaemon    cross-builds the go-daemon binary using Docker.
      golangCrossBuild      build the Beat binary inside of the golang-builder.
      package               packages the Beat for distribution.
      testPackages          tests the generated packages (i.e.
      update                updates the generated files (aka make update).
  1. Cross-build Packetbeat for linux/armv7. Cross-building requires Docker and
    uses images hosted at docker.elastic.co/beats-dev/golang-crossbuild. The
    repo for these images is https://github.com/elastic/golang-crossbuild. These
    images give us wider platform support for cross-building than we had.

    mage golangCrossBuild is invoked inside of the container. This handles
    cross-compiling libpcap and then invoking go build with the proper args.

  2. Cross-build go-daemon for linux/armv7. This is actually done in parallel with
    the other cross-builds. GOMAXPROCS determines the number of concurrent jobs.

    mage buildGoDaemon is invoked inside of the container.

  3. After all cross-builds complete, packaging begins. The package types are
    decided based on the package specs that are registered for each OS.

    Zip and tar.gz files are built natively with Go. RPM and deb packages are
    first generated as tar.gz where we have full control over the target file
    names, ownership, and modes regardless of the underlying filesystem [think
    Windows]. Then FPM is invoked inside of Docker to translate the tar.gz file
    into a proper RPM or deb.

  4. SHA512 side-car files are generated for each package. Go is used for this so
    no special command line tools are needed.

  5. The generated packages are inspected with dev-tools/package_test.go. This
    looks at the contents of the packages to ensure the files have the expected
    ownership and modes (e.g. the config file should have 0600).

CI Testing

Follow-up Tasks

  • Integrate in the macOS packaging and preference pane
  • Sync to 6.4 branch
  • Set PLATFORMS for beats-ci packaging job to enable snapshot packages for additional platforms (like +linux/armv7 +linux/ppc64le +linux/s390x).
  • Update commands used by release manager on 6.4 and master
  • Retire dev-tools/deploy
  • Test on Windows
  • Work with apm-server to update (I have already done some testing) - magefile.go for apm-server

snapshot-urls.txt

magefile.go Outdated
)

var (
Beats = []string{

Choose a reason for hiding this comment

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

exported var Beats should have comment or be unexported

repoInfoOnce sync.Once
)

func GetProjectRepoInfo() (*ProjectRepoInfo, error) {

Choose a reason for hiding this comment

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

exported function GetProjectRepoInfo should have comment or be unexported

SubDir string
}

func (r *ProjectRepoInfo) IsElasticBeats() bool {

Choose a reason for hiding this comment

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

exported method ProjectRepoInfo.IsElasticBeats should have comment or be unexported


// ProjectRepoInfo

type ProjectRepoInfo struct {

Choose a reason for hiding this comment

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

exported type ProjectRepoInfo should have comment or be unexported

@andrewkroh andrewkroh force-pushed the feature/packaging-refactor branch 3 times, most recently from c1d0f20 to 8c73e7b Compare June 21, 2018 05:13
@andrewkroh andrewkroh force-pushed the feature/packaging-refactor branch 4 times, most recently from 1939166 to 485fc97 Compare June 24, 2018 23:12
@ruflin
Copy link
Member

ruflin commented Jun 25, 2018

@elastic/apm-server This PR might have an affect on apm-server packaging.

Copy link
Member

@ruflin ruflin left a comment

Choose a reason for hiding this comment

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

Thanks for all the work on this. I think it heavily reduces the magic part of our packaging which will make it much easier to contribute to it in the future.

I tried make snapshot in packetbeat and got the following error:

>> Building using: cmd='build/mage-linux-amd64 golangCrossBuild', env=[CC=i686-w64-mingw32-gcc, CXX=i686-w64-mingw32-g++, GOARCH=386, GOARM=, GOOS=windows, PLATFORM_ID=windows-386]
../libbeat/cfgfile/reload.go:31:2: readdirent: input/output error
Error: running "go build -o build/golang-crossbuild/packetbeat-windows-386.exe -ldflags -X github.com/elastic/beats/libbeat/version.buildTime=2018-06-25T07:42:13Z -X github.com/elastic/beats/libbeat/version.commit=67395acbd96df3eb478470f6e9bcedce93e2cec5" failed with exit code 1
Error: failed building for windows/386: exit status 1
failed building for windows/386: exit status 1
package ran for 4m45.530903048s
Error: failed cross-building target=golangCrossBuild for platform=windows/386: running "docker run --env EXEC_UID=501 --env EXEC_GID=20 --rm --env MAGEFILE_VERBOSE= --env MAGEFILE_TIMEOUT= --env CGO_ENABLED=1 -v /Users/ruflin/Dev/gopath/src/github.com/elastic/beats:/go/src/github.com/elastic/beats -w /go/src/github.com/elastic/beats/packetbeat docker.elastic.co/beats-dev/golang-crossbuild:1.10.3-main --build-cmd build/mage-linux-amd64 golangCrossBuild -p windows/386" failed with exit code 1
make: *** [snapshot] Error 1

make release does not seem to exist as a Makefile command.

One strange thing I realise after trying to package is that I had a diff in one of the vendored files:

diff --git a/vendor/github.com/tsg/gopacket/pcap/pcap.go b/vendor/github.com/tsg/gopacket/pcap/pcap.go
index c869bcc15..c2b4a54fb 100644
--- a/vendor/github.com/tsg/gopacket/pcap/pcap.go
+++ b/vendor/github.com/tsg/gopacket/pcap/pcap.go
@@ -8,14 +8,14 @@
 package pcap
 
 /*
-#cgo linux LDFLAGS: -lpcap
-#cgo freebsd LDFLAGS: -lpcap
-#cgo openbsd LDFLAGS: -lpcap
-#cgo darwin LDFLAGS: -lpcap
-#cgo solaris LDFLAGS: -lpcap
-#cgo windows CFLAGS: -I C:/WpdPack/Include
-#cgo windows,386 LDFLAGS: -L C:/WpdPack/Lib -lwpcap
-#cgo windows,amd64 LDFLAGS: -L C:/WpdPack/Lib/x64 -lwpcap
+
+
+
+
+
+
+
+

Not sure if that is expected / related? Packaging for other Beats on my end is still running and will report further when done.

As this is a breaking change for community Beats we should mention it in the Developer changelog and perhaps have some notes on how to migrate?

One more general question from my side is on how exactly an update of the Golang version will look like in the future? Assuming we go to 1.11, what are all the things we have to change / do?

@@ -200,6 +200,16 @@ Vagrant.configure(2) do |config|
c.vm.synced_folder ".", "/vagrant", type: "virtualbox"
end

# Windows Server 2016
Copy link
Member

Choose a reason for hiding this comment

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

I assume this is not directly related to this PR?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's only related in that I was testing these build changes on win2016 and we didn't have a machine for it yet.

)

func init() {
mage.BeatDescription = "Audit the activities of users and processes on your system."
Copy link
Member

Choose a reason for hiding this comment

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

Would be nice to have the description in the future as part of the yaml file instead of the Golang file. Would make it easier to reuse if needed (not now).

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. And a few other things that would be useful to externalize in a single yaml file (because it messy to guess and locate the data in the various repos (elastic/beats, apm-server, and community beats):

  • beat version
  • beat doc branch
  • go version

This can be done a future cleanup. I'd like to move more stuff out of the makefile too.


// Update updates the generated files (aka make update).
func Update() error {
return sh.Run("make", "update")
Copy link
Member

Choose a reason for hiding this comment

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

Looking forward to have it all in Go so this would also work on Windows.

if env == nil {
env = map[string]string{}
}
cgoEnabled := "0"
Copy link
Member

Choose a reason for hiding this comment

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

If I remember correctly we use now CGO by default. Should we also set it here to 1 by default? At the same time I like that community beats can easily built without it.

Copy link
Member Author

Choose a reason for hiding this comment

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

The passed in BuildArgs created by DefaultBuildArgs() and DefaultGolangCrossBuildArgs will set the cgo flag appropriately. (it's a bit complex)

  • mage build (equivalent of make or go build) - This uses DefaultBuildArgs() as its starting point (Beats can further override the args). The logic for enabling cgo is exactly the same as it is for Go because it reuses the stdlib logic. Basically the logic is if env[GOOS] == runtime.GOOS then CGO_ENABLED=1. This ensures that cross-builds (GOOS=netbsd mage build) have CGO turned off since cross-compilers are not typically installed.

  • mage crossBuild - The defaults come from DefaultGolangCrossBuildArgs(). This is the target that builds the binaries inside of docker using golang-crossbuild. CGO is enabled here by default (generally). More precisely if the target platform has CGO support then it will be enabled (that list came from go tool dist list -json).

"fields.yml",
"_meta/fields.generated.yml",
"_meta/kibana.generated",
"_meta/kibana/5/index-pattern/{{.BeatName}}.json",
Copy link
Member

Choose a reason for hiding this comment

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

This should not be needed anymore as the index-patterns end up in the kibana.generated directory.

Copy link
Member Author

Choose a reason for hiding this comment

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

I had removed those, but when working across branches it's really nice to be able to fully clean things left behind. I suggest moving all generated files that are not committed to build/ over time.

"--rm",
"--env", "MAGEFILE_VERBOSE="+verbose,
"--env", "MAGEFILE_TIMEOUT="+EnvOr("MAGEFILE_TIMEOUT", ""),
"--env", "CGO_ENABLED=1",
Copy link
Member

Choose a reason for hiding this comment

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

Should this depend on the environment variable?

Copy link
Member Author

Choose a reason for hiding this comment

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

It doesn't matter that it's set (or not set) here because the decision to use cgo for cross-builds is made by the mage golangCrossBuild. I'll remove this to avoid confusion.

version: '{{ beat_version }}'
license: '{{.BeatLicense}}'
url: '{{.BeatURL}}'
description: '{{.BeatDescription}}'
Copy link
Member

Choose a reason for hiding this comment

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

For community Beats they will have to move the description to the Golang file?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. Or export BEAT_DESCRIPTION from their Makefile. But I'd prefer the magefile.go to be the source of truth for this information (or an external YAML file as you suggested) so that make isn't a requirement for building.

b.Spec.Name, b.Type, b.Platform.Name)
}

// TestPackages executes the package tests on the produced binaries. These tests
Copy link
Member

Choose a reason for hiding this comment

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

Really nice. We could also extend these with things like checking if the dashboard files are all in the correct directory etc. ¨

Copy link
Member Author

Choose a reason for hiding this comment

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

Good idea. I suggest enhancing the test outside of this PR. The package_test.go is executed by the release-manager today.

)

const (
fpmVersion = "1.10.0"
Copy link
Member

Choose a reason for hiding this comment

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

Should this be in sync with the Golang version?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, 1.10.0 is the latest release of FPM.

@@ -47,7 +47,7 @@ const (
)

var (
configFilePattern = regexp.MustCompile(`.*beat\.yml`)
configFilePattern = regexp.MustCompile(`.*beat\.yml|apm-server\.yml`)
Copy link
Member

Choose a reason for hiding this comment

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

I see we are already prepared for apm 👍

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I have tested this with APM. I updated the description to include a link to the magefile.go I used which customizes their packaging to not include the reference YAML and use a customized readme.md.

@ruflin
Copy link
Member

ruflin commented Jun 25, 2018

On a second run, packaging for metricbeat and packetbeat was succesful. The diff on the pcap.go file stays.

I did the following:

  • Start default and OSS distribution for metricbeat and packetbeat. Both work.
  • Packetbeat has the adjusted default file to read from en0 as expected.

@andrewkroh
Copy link
Member Author

One more general question from my side is on how exactly an update of the Golang version will look like in the future? Assuming we go to 1.11, what are all the things we have to change / do?

It's generally the same as it is today, except the docker images are stored in a different repo. Instructions for updating can be found at https://github.com/elastic/golang-crossbuild#releasing-images-for-a-new-go-version.

@andrewkroh
Copy link
Member Author

readdirent: input/output error

That must be some kind of an issue with the shared filesystem between macOS and the VM. I haven't seen that error yet on my machine. I have tested this on Docker for Mac, Boot2Docker (on mac), and Docker CE on Linux.

@andrewkroh
Copy link
Member Author

One strange thing I realise after trying to package is that I had a diff in one of the vendored files:

This isn't new. When packaging Packetbeat we patch github.com/tsg/go-packet so that we can adjust the LDFLAGS and CFLAGS that are hard-coded in the source. As a result, when packaging completes you have a modified source file.

I could "fix" this by adding a defer sh.Run(git checkout vendor/github.com/tsg/gopacket/pcap/pcap.go) at the end. I'll try this.

Thanks for testing on your machine!

@andrewkroh
Copy link
Member Author

@ruflin I pushed an update based on your review. af3cb054f8d7c43962f48d1f57f1fb6c2bf46477

  • Fix naming of make package which is now either make release or make snapshot.
  • Remove unneeded CGO_ENABLED env var from the cross build container
  • Undo go-packet patch with 'git checkout' after crossBuild completes

@@ -19,6 +19,7 @@ https://github.com/elastic/beats/compare/v6.2.3...master[Check the HEAD diff]
- Allow override of dynamic template `match_mapping_type` for fields with object_type. {pull}6691[6691]
- Set default kafka version to 1.0.0 in kafka output. Older versions are still supported by configuring the `version` setting. {pull}7025[7025]
- Add `host.name` field to all events, to avoid mapping conflicts. This could be breaking Logstash configs if you rely on the `host` field being a string. {pull}7051[7051]
- Packaging has been refactored and updates are required. See the PR for migration details. {pull}7388[7388]
Copy link
Member

Choose a reason for hiding this comment

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

Should this be in the developer changelog?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I meant to put it there.

@urso urso requested review from ph and kvch June 26, 2018 11:07
}

// Clean clean generated build artifacts.
func Clean() error {
Copy link

Choose a reason for hiding this comment

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

There are a many defaults/magic paths here. How about making it somewhat configurable by passing the paths as arguments?

func Clean(pathsList ...[]string) error {
  if len(pathsList) == 0 {
    pathsList = [][]string{CleanPaths}
  }
  for _, lst := range pathsList {
    for _, f := range lst {
      ..
    }
  }
}

So the per beat magefile can be written like:

func Clean() error {
  return make.Clean(mage.DefaultCleanPaths, []string{
    "...",
    "...",
  })
}

This is still valid.

func Clean() error {
  return mage.Clean()
}

Copy link
Member Author

Choose a reason for hiding this comment

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

Changed. 👍

return errors.Wrap(CreateSHA512File(spec.OutputFile), "failed to create .sha512 file")
}

func addUidGidEnvArgs(args []string) ([]string, error) {

Choose a reason for hiding this comment

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

func addUidGidEnvArgs should be addUIDGidEnvArgs

return errors.Wrap(CreateSHA512File(spec.OutputFile), "failed to create .sha512 file")
}

func addUidGidEnvArgs(args []string) ([]string, error) {

Choose a reason for hiding this comment

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

func addUidGidEnvArgs should be addUIDGidEnvArgs

shortIn = append(shortIn, "_meta/common.p2.yml")
shortIn = append(shortIn, "../libbeat/_meta/config.yml")
if !mage.IsUpToDate(shortConfigTemplate, shortIn...) {
fmt.Println(">> Building", shortConfigTemplate)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why didn't you extracted this 4 lines into a separate function with parameters template, snippets, permission which passes the correct permissions to MustFileConcat? I think this would look cleaner:

if !mage.IsUpToDate(shortConfigTemplate, shortIn...) {
    updateConfigTemplate(shortConfigTemplate, shortIn, 0600)
}

if !mage.IsUpToDate(referenceConfigTemplate, referenceIn...) {
   updateConfigTemplate(referenceConfigTemplate, referenceIn, 0644)
}

// ...

The new extracted function could be:

func updateConfigTemplates(template string, snippets []string, permission int) {
    fmt.Println(">> Building", template)
    mage.MustFileConcat(template, 0644, snippets...)
    mage.MustFindReplace(template, regexp.MustCompile("beatname"), "{{.BeatName}}")
    mage.MustFindReplace(template, regexp.MustCompile("beat-index-prefix"), "{{.BeatIndexPrefix}}")
}

So if the way config files are updated is changed, we only need to modify it in one function.

// - Config files are Go templates.

const (
configTemplateGlob = "module/*/_meta/config*.yml.tmpl"
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it work on Windows? Shouldn't this be

configTemplateGlob = filepath.Join("module", "*", "_meta", "config*.yml.tmpl")

?

Copy link
Contributor

@kvch kvch left a comment

Choose a reason for hiding this comment

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

🎆 🎆 🎆
I like the work you did here. Especially the enhancements around crosscompiling. It's going to make our life much easier. Also, I am very happy to see that magic shell commands are removed from Makefiles. My eyes are bleeding every time I need to look at those.

I have a few minor questions, but nothing which would prevent this PR from being merged.

Copy link
Contributor

@ph ph left a comment

Choose a reason for hiding this comment

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

@andrewkroh Let me start by saying this is by far the best build system I've ever seen, I've never played with mage before. However, for me having everything using the same language is just fantastic, as a bonus, we can test a lot of the logic. I haven't seen a lot of build toolset having some test.

I am excited to get that merged in even my Firefox can't even switch tabs.

For reviewing this huge PR, I took the following notes:

  1. I went through all the code; I haven't seen any apparent errors or problems with it.
  2. I've looked mostly at the structure, Could I see myself debugging this in a few months and I would say Yes.
  3. With the number of external tests that we have, I am confident we should not break something.
  4. The hound comments are still valid and we should do them.

After that I've played with the CLI, I've tried doing make snapshot in the filebeat and packetbeat
directory and I've played with the binaries and they work!

Concerning the merge, I presume the goal is to merge that in master and 6.4 is the first release to be built with the new toolset?

Since this system is impacting apm-server, I would like someone from their team to look at it this will give an eye from a consumer.

cc @elastic/apm-server

return 64
}
}

Copy link
Contributor

Choose a reason for hiding this comment

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

I've seen this method twice, it should be extracted.

This image requires membership in the Elastic organization in order to be able to pull it.
The saves us from having to pull it from Github during builds so it will help with speed and reliability.
This refactors Beat packaging to use a declarative YAML based packaging
specification. This makes it easier to customize the contents of packages (e.g.
adding additional X-Pack content or simply tailoring what's included for single
Beat).

The specification itself is pretty simple. It consists of package metadata and a
list of files to include. The values can be templated which allows for a single
package spec to be reused across Beats. Here's an example spec for an OSS
Windows zip package.

```
spec:
  name:         '{{.BeatName}}-oss'
  service_name: '{{.BeatServiceName}}'
  os:           '{{.GOOS}}'
  arch:         '{{.PackageArch}}'
  vendor:       '{{.BeatVendor}}'
  version:      '{{ beat_version }}'
  license:      ASL 2.0
  url:          '{{.BeatURL}}'
  description: '{{.BeatDescription}}'
  files:
    '{{.BeatName}}{{.BinaryExt}}':
      source: build/golang-crossbuild/{{.BeatName}}-{{.GOOS}}-{{.Platform.Arch}}{{.BinaryExt}}
      mode:   0755
    fields.yml:
      source: fields.yml
      mode:   0644
    LICENSE.txt:
      source: '{{ repo.RootDir }}/licenses/APACHE-LICENSE-2.0.txt'
      mode: 0644
    NOTICE.txt:
      source: '{{ repo.RootDir }}/NOTICE.txt'
      mode:   0644
    README.md:
      template: '{{ elastic_beats_dir }}/dev-tools/mage/templates/common/README.md.tmpl'
      mode:   0644
    .build_hash.txt:
      content: >
        {{ commit }}
      mode:   0644
    '{{.BeatName}}.reference.yml':
      source: '{{.BeatName}}.reference.yml'
      mode:   0644
    '{{.BeatName}}.yml':
      source: '{{.BeatName}}.yml'
      mode: 0600
      config: true
    kibana:
      source: _meta/kibana.generated
      mode:   0644
    install-service-{{.BeatName}}.ps1:
      template: '{{ elastic_beats_dir }}/dev-tools/mage/templates/windows/install-service.ps1.tmpl'
      mode: 0755
    uninstall-service-{{.BeatName}}.ps1:
      template: '{{ elastic_beats_dir }}/dev-tools/mage/templates/windows/uninstall-service.ps1.tmpl'
      mode: 0755
```

Each Beat has two build targets for packaging.

- `make snapshot`
- `make release`

The set of target platforms can be influenced by the `PLATFORMS` environment
variable which accepts an platform selection expression. For example to add
ARMv7 (for your Raspberry Pi) to the default set of platforms you would use
`PLATFORMS='+linux/armv7' make snapshot`. Or to only build for Windows set
`PLATFORMS='windows'`. Full details can be found in the godocs for
`NewPlatformList`.

For the release manager there are two new top-level targets that take care of
ensuring that the proper Go version is used. The naming here aligns with what
several of the other projects are using for their release manager targets.

- `make release-manager-snapshot`
- `make release-manager-release`

Build Process Details
----

Below is the command line output of building and packaging Packetbeat for
linux/armv7 only. I'll describe each step in the build process.

```
$ PLATFORMS='+all linux/armv7' make snapshot
Installing mage from vendor
>> golangCrossBuild: Building for linux/armv7
>> buildGoDaemon: Building for linux/armv7
>> Building using: cmd='build/mage-linux-amd64 golangCrossBuild', env=[CC=arm-linux-gnueabihf-gcc, CXX=arm-linux-gnueabihf-g++, GOARCH=arm, GOARM=7, GOOS=linux, PLATFORM_ID=linux-armv7]
>> Building using: cmd='build/mage-linux-amd64 buildGoDaemon', env=[CC=arm-linux-gnueabihf-gcc, CXX=arm-linux-gnueabihf-g++, GOARCH=arm, GOARM=7, GOOS=linux, PLATFORM_ID=linux-armv7]
grammar.y: warning: 38 shift/reduce conflicts [-Wconflicts-sr]
>> package: Building packetbeat type=tar.gz for platform=linux/armv7
>> package: Building packetbeat-oss type=deb for platform=linux/armv7
>> package: Building packetbeat type=rpm for platform=linux/armv7
>> package: Building packetbeat type=deb for platform=linux/armv7
>> package: Building packetbeat-oss type=tar.gz for platform=linux/armv7
>> package: Building packetbeat-oss type=rpm for platform=linux/armv7
>> Testing package contents
package ran for 1m6.597416206s

$ tree build/distributions/
build/distributions/
├── packetbeat-oss-7.0.0-alpha1-SNAPSHOT-armhf.deb
├── packetbeat-oss-7.0.0-alpha1-SNAPSHOT-armhf.deb.sha512
├── packetbeat-oss-7.0.0-alpha1-SNAPSHOT-armhfp.rpm
├── packetbeat-oss-7.0.0-alpha1-SNAPSHOT-armhfp.rpm.sha512
├── packetbeat-oss-7.0.0-alpha1-SNAPSHOT-linux-armv7.tar.gz
└── packetbeat-oss-7.0.0-alpha1-SNAPSHOT-linux-armv7.tar.gz.sha512
```

1. Install [mage](https://magefile.org/) from the vendor directory to `$GOPATH/bin`.

   Mage is used to provide a simple gmake like wrapper around build logic that's
   written in Go. Additionally it makes is possible to build and package without
   gmake by invoking mage directly [think Windows users]. For example:

   ```
   $ mage -l
    Targets:
      build                 builds the Beat binary.
      buildGoDaemon         builds the go-daemon binary (use crossBuildGoDaemon).
      clean                 cleans all generated files and build artifacts.
      crossBuild            cross-builds the beat for all target platforms.
      crossBuildGoDaemon    cross-builds the go-daemon binary using Docker.
      golangCrossBuild      build the Beat binary inside of the golang-builder.
      package               packages the Beat for distribution.
      testPackages          tests the generated packages (i.e.
      update                updates the generated files (aka make update).
  ```

2. Cross-build Packetbeat for linux/armv7. Cross-building requires Docker and
   uses images hosted at `docker.elastic.co/beats-dev/golang-crossbuild`. The
   repo for these images is https://github.com/elastic/golang-crossbuild. These
   images give us wider platform support for cross-building than we had.

   `mage golangCrossBuild` is invoked inside of the container. This handles
   cross-compiling libpcap and then invoking `go build` with the proper args.

3. Cross-build go-daemon for linux/armv7. This is actually done in parallel with
   the other cross-builds. GOMAXPROCS determines the number of concurrent jobs.

   `mage buildGoDaemon` is invoked inside of the container.

4. After all cross-builds complete, packaging begins. The package types are
   decided based on the package specs that are registered for each OS.

   Zip and tar.gz files are built natively with Go. RPM and deb packages are
   first generated as tar.gz where we have full control over the target file
   names, ownership, and modes regardless of the underlying filesystem [think
   Windows]. Then FPM is invoked inside of Docker to translate the tar.gz file
   into a proper RPM or deb.

5. SHA512 side-car files are generated for each package. Go is used for this so
   no special command line tools are needed.

6. The generated packages are inspected with `dev-tools/package_test.go`. This
   looks at the contents of the packages to ensure the files have the expected
   ownership and modes (e.g. the config file should have 0600).

Changes

- Add Boot2Docker workaround for shared volume permissions

- Mirror the libpcap source to S3

- Use MAX_PARALLEL to control the number of parallel jobs. The default value is
  the lesser of the NumCPU and NCPU from the Docker daemon.

- Add jenkins_package.sh for Jenkins.
@andrewkroh andrewkroh force-pushed the feature/packaging-refactor branch from b9b9e4c to 23c4bc0 Compare June 29, 2018 12:28
@ruflin ruflin merged commit 74f548b into elastic:master Jun 29, 2018
jalvz added a commit to jalvz/apm-server that referenced this pull request Jun 29, 2018
This update includes packaging refactor (elastic/beats#7388)
andrewkroh added a commit to andrewkroh/beats-docker that referenced this pull request Jun 29, 2018
Prior to elastic/beats#7388 artifacts were written to build/upload
by the Beats build. This directory was renamed to better reflect its
purpose and contents.
mgreau pushed a commit to elastic/beats-docker that referenced this pull request Jul 2, 2018
Prior to elastic/beats#7388 artifacts were written to build/upload
by the Beats build. This directory was renamed to better reflect its
purpose and contents.
jalvz added a commit to jalvz/apm-server that referenced this pull request Jul 2, 2018
This update includes packaging refactor (elastic/beats#7388)
jalvz added a commit to jalvz/apm-server that referenced this pull request Jul 3, 2018
This update includes packaging refactor (elastic/beats#7388)
@andrewkroh andrewkroh deleted the feature/packaging-refactor branch July 3, 2018 23:53
jalvz added a commit to jalvz/apm-server that referenced this pull request Jul 4, 2018
This update includes packaging refactor (elastic/beats#7388)
jalvz added a commit to elastic/apm-server that referenced this pull request Jul 4, 2018
This update includes packaging refactor (elastic/beats#7388)
@@ -1 +0,0 @@
version: "7.0.0-alpha1"
Copy link
Contributor

Choose a reason for hiding this comment

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

@andrewkroh with this file removed, now to bump release version in apm-server, we just wait for beats to do it and then update beats, right?

@andrewkroh
Copy link
Member Author

Right now the beat version used inside of binaries is coming from:

https://github.com/elastic/apm-server/blob/968f8621fc11c1cfd5ec8eeef2d3816869585973/magefile.go#L36

But I think there may be more places that have the Beat version (like a version.asciidoc IIRC). It would be nice to get down to a single file in each repo that is the source of truth for things like beat version, go version, doc branch, etc.

andrewkroh added a commit to andrewkroh/beats-docker that referenced this pull request Jul 20, 2018
Prior to elastic/beats#7388 artifacts were written to build/upload
by the Beats build. This directory was renamed to better reflect its
purpose and contents.
mgreau pushed a commit to elastic/beats-docker that referenced this pull request Jul 23, 2018
Prior to elastic/beats#7388 artifacts were written to build/upload
by the Beats build. This directory was renamed to better reflect its
purpose and contents.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants