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

CONTRIBUTING: how to build a package #330

Merged
merged 20 commits into from
Apr 9, 2020

Conversation

mtojek
Copy link
Contributor

@mtojek mtojek commented Apr 7, 2020

This PR updates CONTRIBUTING docs with description on how to build/migrate an integration.

Preview: https://github.com/mtojek/package-registry/blob/how-to-build-package/CONTRIBUTING.md

I will mention few reviewers to verify if all steps are correct (agent, Kibana, etc.)

Issue: #220

@mtojek mtojek self-assigned this Apr 7, 2020
@mtojek mtojek changed the title CONTRIBUTINGHow to build package CONTRIBUTING: how to build a package Apr 7, 2020
@mtojek mtojek mentioned this pull request Apr 7, 2020
5 tasks
Copy link

@exekias exekias left a comment

Choose a reason for hiding this comment

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

This is great, thank you!

Would it make sense to add a note to create an issue per integration explaining every manual change that was needed? That should help us improve the script later

CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated
Make sure you don't have any manual changes applied as they will reflect on the integration.
2. Clone/refresh the Elastic Package Registry (EPR) to always use the latest version of the script:
* https://github.com/elastic/package-registry
3. Make sure you've the `mage` tool installed.
Copy link

Choose a reason for hiding this comment

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

nit: how about adding the command line for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed. Added a one-liner for mage.

CONTRIBUTING.md Outdated Show resolved Hide resolved
Comment on lines +276 to +286
```bash
$ cd $GOPATH/src/github.com/elastic/beats/x-pack/elastic-agent
$ PLATFORMS=darwin mage package
```

Unpack the distribution you'd like to use (e.g. tar.gz):
```bash
$ cd build/distributions/
$ tar xzf elastic-agent-8.0.0-darwin-x86_64.tar.gz
$ cd elastic-agent-8.0.0-darwin-x86_64/
```
Copy link

Choose a reason for hiding this comment

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

I think this is fine, wondering if we have a snapshot image being produced already? cc @ph @ruflin

Copy link

Choose a reason for hiding this comment

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

I will take a look and get back to you @exekias

mtojek and others added 3 commits April 7, 2020 13:08
Co-Authored-By: Carlos Pérez-Aradros Herce <[email protected]>
Co-Authored-By: Carlos Pérez-Aradros Herce <[email protected]>
@mtojek
Copy link
Contributor Author

mtojek commented Apr 7, 2020

This is great, thank you!

Would it make sense to add a note to create an issue per integration explaining every manual change that was needed? That should help us improve the script later

Good idea, done.

@mtojek mtojek requested a review from exekias April 7, 2020 11:27
@mtojek
Copy link
Contributor Author

mtojek commented Apr 7, 2020

Thank you @exekias for the approval. I'm looking forward to see comments from other reviewers too.

Copy link
Contributor

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

This is great! Lets get it in and let some other contributors use it and provide feedback if some parts are missing.

@alakahakai you will be interested in this one.


### Elements

Link: https://github.com/elastic/package-registry/blob/master/ASSETS.md
Copy link
Contributor

Choose a reason for hiding this comment

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

I will have to update that with the most recent details one day ;-)

The directory contains mandatory manifest files defining the integration and its datasets. All manifests have fields
annotated with comments to better understand their goals.

_Keep in mind that this package doesn't contain all file resources (images, screenshots, icons) referenced in manifests.
Copy link
Contributor

Choose a reason for hiding this comment

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

Ups, we should add those. I plan to have validation for these in the future.


5. Review fields file and exported fields in docs.

The fields files (`package-fields.yml`, `fields.yml` and `ecs.yml`) in the package were created from original
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add an additional note here that the sum of all the fields.yml for a dataset should only contain the fields that are really used by the dataset? For example not just have all the ecs fields that exist?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, done.

5. Start Kibana with enabled Ingest Manager:

```bash
$ yarn start --xpack.ingestManager.enabled=true --xpack.ingestManager.epm.enabled=true --xpack.ingestManager.fleet.enabled=true --xpack.ingestManager.epm.registryUrl=http://localhost:8080/
Copy link
Contributor

Choose a reason for hiding this comment

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

Most users new to KB will forget to run yarn kbn bootstrap ;-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, added.

@mtojek mtojek merged commit 9499d10 into elastic:master Apr 9, 2020
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