-
Notifications
You must be signed in to change notification settings - Fork 32
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
Add dependencies for x-pack/auditbeat and Journalbeat #58
Add dependencies for x-pack/auditbeat and Journalbeat #58
Conversation
libicu57:arm64 \ | ||
icu-devtools:arm64 \ | ||
libsystemd-dev:arm64 \ | ||
# librpm-dev:armhf \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we cannot install the three ARM variants, the packages conflict with each other. I have to take a look at https://wiki.debian.org/CrossCompiling to see if there is a way to make it. An alternative is to have an image for each ARM variant
@@ -17,6 +17,7 @@ RUN \ | |||
mingw-w64 \ | |||
mingw-w64-tools \ | |||
patch \ | |||
librpm-dev:i386 \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kvch I think we are missing journalbeat dependencies here. What is required for amd64 and i386 builds (I think i386 requires us to jump through some extra hoops).
Is the main
image used for building i386 and amd64 builds? Maybe we should provide separate images. Especially the systemd dependency can be a pain.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess so, but I could find a place where are defined the packages needed for any beat or the place where they are installed when you run the mage package
In any case, this would be done in a follow-up PR this one is to fix missing dependencies.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there is a PR that create a new amd64 Docker image #53, I will make a review of it with the changes in here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure I follow why this PR is needed. Journalbeat installs its dependencies before running GolangCrossbuild. Once it is merged, we need to follow-up in the magefile of the Beat? Is this template is used for all Beats? If yes, why would we install Journalbeat deps for the image when it builds e.g. Packetbeat?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had a chat with @urso offline about the goal of this PR. You can find the deps of Journalbeat by platform here: https://github.com/elastic/beats/blob/master/journalbeat/magefile.go#L121-#L133
BTW this generates images for the Go version 1.13.12 (among others), we use 1.13.10 on Beats, so or we bump the version in Beats or we generate a version 1.13.10 here |
@urso I have changed the Go version to 1.13.10 the same Beats use, to use the generated images in the package job |
+1 for providing 1.13.10. We still try to update to go1.14.4: elastic/beats#18829 |
ab24022
to
3a5bc90
Compare
it installs cross-compile dependencies into the cross-build Docker images, finally, the trick is to add all the cross-packages manually, it seems it does not follow the dependencies.
This PR superseded #39