-
Notifications
You must be signed in to change notification settings - Fork 4
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
213 fix gopath #289
213 fix gopath #289
Conversation
@@ -61,6 +65,7 @@ pipeline { | |||
expression { return params.RELEASE_TEST_IMAGES } | |||
} | |||
steps { | |||
sh(label: 'Install virtualenv', script: 'pip install --user virtualenv') |
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.
@jsoriano I think this should be provided by Beats' build system 🤔
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.
In the build system we are assuming that virtualenv
is installed. In different scripts for testing and development (.travis.yml
, Vagrantfile
, Dockerfile
...) we install it in different ways depending on the platform. If we continue with this schema virtualenv
should be already installed in the machine, or it should be installed here.
Also, depending on the environment, the user running the tests may not have permissions to install applications using pip or any other package manager.
.ci/buildBeatsDockerImages.groovy
Outdated
GOPATH = "${env.WORKSPACE}" | ||
GOROOT = "${env.HOME}/.gimme/versions/go${env.GO_VERSION}.linux.amd64" |
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.
This variable is needed as we are installing Go using Travis CI's gimme.
I'd see very convenient installing Go from a step in the library, as we use it a lot across repos. Wdyt?
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.
Go version to use with beats is available in the .go-version
file, could we use this version instead of setting it through a param so we only need to keep the version updated in one place?
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.
This is not required, gimme should be executed within the same context. For instance:
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.
What do I mean?
- Create a similar script like https://github.com/elastic/apm-pipeline-library/blob/master/.ci/scripts/install-mage.sh for the other go commands
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.
🤔 So it would mean creating a script for running MODULE=mysql mage compose:buildSupportedVersions
after installing GIMME, right?
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.
So it would mean creating a script for running MODULE=mysql mage compose:buildSupportedVersions after installing GIMME, right?
maybe something like:
.ci/scripts/build-module.sh <MODULE>
cat .ci/scripts/build-module.sh
MODULE=$1
<content for .ci/scripts/install-mage.sh >
MODULE=${MODULE} mage compose:buildSupportedVersions
and even if .ci/scripts/build-module.sh
gets called several times, the GIMME would be installed the very first time in that particular CI worker, and the next calls it will be cached and won't do the installation but it will run the eval
command with all the required env variables in place within the shell step context.
Does it make sense?
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.
Ok, that will solve my concerns about future parallel executions.
Like the proposal!
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 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.
@jsoriano we are defining the Go version in just one place
Oh, if this place is .go-version
then this sounds great to me, thanks!
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.
No, it's the Jenkinsfile, as an input parameter
Co-Authored-By: Victor Martinez <victormartinezrubio@gmail.com>
Co-Authored-By: Victor Martinez <victormartinezrubio@gmail.com>
What does this PR do?
It refactors how the release process of the test Docker image for metricbeat modules is performed. First, we install virtualenv and GIMME in the build agent, but instead of adding GIMME paths to the PATH, we execute it in-context: we install and use it in the same shell script.
This refactor will allow us to implement parallel execution in the near future, as the script is accepting the module to be built, which could be defined as the input parameter to a parallel branch in Jenkins
It also adds an input parameter for the Go version, which will allow the build to configure the specific version to be used.
Why is it important?
It fixes the build process for the test Docker images of beats.
Related issues
Completes #280