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

Fix generator jobs for go modules #16288

Merged
merged 6 commits into from
Mar 4, 2020

Conversation

kvch
Copy link
Contributor

@kvch kvch commented Feb 12, 2020

What does this PR do?

  • Set the correct path to mage for generator jobs
  • Generated beats use go modules for dependency management
    • New option beats_revision is added so users can select which beats revision they want to vendor
    • if one does not want to depend on elastic/beats, he/she needs to add the appropriate replace directive to the generated go.mod

Why is it important?

As Beats is moving to go modules for dependency management, generated Beats should use that as well.

Documentation is provided to help users migrate to go modules in their own Beats. Also, about the new option.

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

How to test

Run the test which is executed by the jobs:

make -C generator/_templates/beat test test-package

Then go to the generated beat and play around with the available make commands. For example run make copyVendor or make create-metricseat, etc.

@kvch kvch added in progress Pull request is currently in progress. Team:Services (Deprecated) Label for the former Integrations-Services team labels Feb 12, 2020
@kvch kvch added [zube]: In Review and removed [zube]: In Progress in progress Pull request is currently in progress. labels Feb 19, 2020
generator/common/beatgen/setup/setup.go Outdated Show resolved Hide resolved
generator/common/beatgen/setup/setup.go Outdated Show resolved Hide resolved
generator/common/beatgen/setup/setup.go Outdated Show resolved Hide resolved
generator/common/beatgen/setup/setup.go Outdated Show resolved Hide resolved
@kvch kvch force-pushed the fix-gomodules-generators-once-and-for-all branch from 16ff9cc to efe5ff9 Compare February 20, 2020 12:05
@jsoriano jsoriano self-requested a review February 21, 2020 09:37
@@ -480,3 +481,35 @@ func main() {

When you're done with your new Beat, how about letting everyone know? Open
a pull request to add your link to the {beats-ref}/community-beats.html[Community Beats] list.

[[newbeat-migrate-gomodules]]
=== Migrate existing Beat to go modules
Copy link
Member

Choose a reason for hiding this comment

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

@radoondas you may be interested on that 🙂

Copy link
Member

Choose a reason for hiding this comment

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

Ou yeah! This is something I will have a look once it is available! Thanks

docs/devguide/newbeat.asciidoc Outdated Show resolved Hide resolved
generator/common/Makefile Outdated Show resolved Hide resolved
Key: "beats_revision",
Help: "Enter the github.com/elastic/beats revision",
Default: func(cfg map[string]string) string {
return "master"
Copy link
Member

Choose a reason for hiding this comment

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

Could we somehow reuse the versions in libbeat/docs/version.asciidoc for this default? So if someone creates a new beat from 7.7 branch, 7.7 is used by default and so on.
Maybe we already have some helper in mage to access these versions.

generator/common/beatgen/setup/setup.go Outdated Show resolved Hide resolved
generator/common/beatgen/setup/setup.go Outdated Show resolved Hide resolved
generator/common/beatgen/setup/setup.go Show resolved Hide resolved
@kvch kvch force-pushed the fix-gomodules-generators-once-and-for-all branch 3 times, most recently from 038a10d to 2d7b72f Compare February 25, 2020 13:05
@kvch kvch mentioned this pull request Feb 25, 2020
2 tasks
@kvch
Copy link
Contributor Author

kvch commented Feb 25, 2020

Opened follow-up issue with possible improvements: #16555

Copy link
Member

@jsoriano jsoriano left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for opening the issue for the follow ups!

@kvch
Copy link
Contributor Author

kvch commented Feb 26, 2020

jenkins test this

@mikemadden42
Copy link
Contributor

jenkins test this please

@mikemadden42
Copy link
Contributor

jenkins retest this please

@kvch kvch force-pushed the go-modules branch 4 times, most recently from 3dbefdb to cc275ad Compare February 28, 2020 14:34
@kvch kvch requested review from a team as code owners March 3, 2020 08:35
@kvch kvch changed the base branch from go-modules to master March 3, 2020 08:35
@kvch kvch force-pushed the fix-gomodules-generators-once-and-for-all branch from c6ce496 to b9d1775 Compare March 3, 2020 08:36
@kvch
Copy link
Contributor Author

kvch commented Mar 3, 2020

jenkins test this

@kvch kvch added the needs_backport PR is waiting to be backported to other branches. label Mar 3, 2020
@mikemadden42
Copy link
Contributor

jenkins retest this please

@kvch kvch force-pushed the fix-gomodules-generators-once-and-for-all branch from 4895c92 to 3ec8e03 Compare March 3, 2020 15:32
@kvch kvch force-pushed the fix-gomodules-generators-once-and-for-all branch from 3ec8e03 to 67e8807 Compare March 4, 2020 14:15
@kvch
Copy link
Contributor Author

kvch commented Mar 4, 2020

This comment is written to trigger a CLA check.

@kvch kvch merged commit a846029 into elastic:master Mar 4, 2020
kvch added a commit to kvch/beats that referenced this pull request Mar 5, 2020
* Set the correct path to `mage` for generator jobs
* Generated beats use go modules for dependency management
   * New option `beats_revision` is added so users can select which beats revision they want to vendor
    * if one does not want to depend on `elastic/beats`, he/she needs to add the appropriate `replace` directive to the generated `go.mod`

As Beats is moving to go modules for dependency management, generated Beats should use that as well.

Documentation is provided to help users migrate to go modules in their own Beats. Also, about the new option.
(cherry picked from commit a846029)
@kvch kvch added v7.7.0 and removed needs_backport PR is waiting to be backported to other branches. labels Mar 5, 2020
kvch added a commit that referenced this pull request Mar 5, 2020
* Set the correct path to `mage` for generator jobs
* Generated beats use go modules for dependency management
   * New option `beats_revision` is added so users can select which beats revision they want to vendor
    * if one does not want to depend on `elastic/beats`, he/she needs to add the appropriate `replace` directive to the generated `go.mod`

As Beats is moving to go modules for dependency management, generated Beats should use that as well.

Documentation is provided to help users migrate to go modules in their own Beats. Also, about the new option.
(cherry picked from commit a846029)
@kvch kvch removed their assignment Mar 6, 2020
@kvch kvch added the test-plan Add this PR to be manual test plan label Mar 6, 2020
@andresrc andresrc added the test-plan-added This PR has been added to the test plan label Mar 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team:Services (Deprecated) Label for the former Integrations-Services team test-plan Add this PR to be manual test plan test-plan-added This PR has been added to the test plan v7.7.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants