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

ci(jenkins): release process #66

Merged
merged 4 commits into from
Dec 6, 2019
Merged

Conversation

v1v
Copy link
Member

@v1v v1v commented Nov 28, 2019

Highlights

  • enable ITs: async when running as a PR or sync when running in a branch/tag
  • Continuous delivery when running the pipeline either in a branch or tag. master branch refers to latest while others will name the image with their names.
  • Script to bump versions for the Node.js agent.

Issues

Tasks

  • Versioning
  • Release automation
  • ITs validation should be synchronous when running a release.

@v1v v1v self-assigned this Nov 28, 2019
@v1v v1v added the automation label Nov 28, 2019
@v1v v1v added the ci label Nov 28, 2019
@v1v v1v marked this pull request as ready for review November 28, 2019 13:59
@v1v v1v requested review from watson and axw November 28, 2019 13:59
Copy link
Contributor

@watson watson left a comment

Choose a reason for hiding this comment

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

I don't think you need to run npm install again as long as you just ensure to build a new Docker image as the version dependency is already specified as a range. If we do that you also don't risk upgrading to a version with a higher major component as that will be a breaking change.

.ci/bump-version.sh Outdated Show resolved Hide resolved
Co-Authored-By: Thomas Watson <w@tson.dk>
@v1v
Copy link
Member Author

v1v commented Nov 29, 2019

I don't think you need to run npm install again as long as you just ensure to build a new Docker image as the version dependency is already specified as a range. If we do that you also don't risk upgrading to a version with a higher major component as that will be a breaking change.

The idea of this particular approach is to ensure the version for the apm-agent-nodejs which has been shipped got an opbeans docker image with it. Therefore we got reproducibility, using ranges might be good for certain cases, but for this particular one, there is a requirement to ensure all the releases for all the apm-agents got a specific opbeans docker image matching with the same version.

The next follow-ups will consist of running some functional tests to validate whether those dependencies are not breaking anything. The one which will do that is the:

It does run something but requires to add more test coverage for those use cases.

At the moment, the automation is in place, we need to add more test coverage to be in a more healthy place.

Not sure if I answered your concerns, please let me know If I need to clarify it.

@watson
Copy link
Contributor

watson commented Nov 29, 2019

What I mean is that I think it's redundant. If I understand the process of how a new Docker image is being build, the 3 steps (npm install ..., git add ..., and git commit ...) doesn't do any difference in how the Docker image turns out as long as the Docker image is rebuilt, it will be built with a fresh npm install and hence get the latest version allowed in the range specified in package.json.

I would always recommend doing a manual upgrade between each major version of the agent, as it most likely involves manual checking and upgrading of code due to the breaking changes.

But maybe I misunderstood how the Docker images are built?

@kuisathaverat
Copy link
Contributor

On the Docker, file installs the version that it is configured in the package.json, this means you have to manually edit this file before making an Opbeans release to set a fixed version for the Node.jas agent, the automated process that @v1v is putting in place make this change and trigger a push to build a new Docker image.

@watson
Copy link
Contributor

watson commented Dec 6, 2019

In this repository, we don't have a checked-in package-lock.json file, so as long as you're running npm install from a freshly cloned version of the repo, you will get the latest version of the agent within the range specified in package.json. So if we specify ^3.0.0 in package.json, we'll get the latest 3.x version.

You can verify this by running npm ls elastic-apm-node after finishing the npm install. That will show the actual version that has been installed.

Copy link
Contributor

@watson watson left a comment

Choose a reason for hiding this comment

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

Just had a chat with @v1v. While the bump-version.sh script would technically work without manually running npm install elastic-apm-node@${AGENT_VERSION} it would only work for installing whatever is the latest version at the time the script runs.

There can be race conditions where two versions of the Node agent has been published and then later this script is being run twice, both times installing the latest of those two versions. That's obviously not a good thing, so being very specific about the version like the script is now is a good idea.

However, for this to be 100% dependent, you also need to change the version defined in package.json to no longer be a range:

-    "elastic-apm-node": "^3.0.0",
+    "elastic-apm-node": "3.0.0",

By removing the ^ sign, you ensure that only the version specified will be installed. If you don't do this, running npm install elastic-apm-node@3.1.0 would just change ^3.0.0 to ^3.1.0 which isn't that dependable and might create weird issues down the line.

@watson
Copy link
Contributor

watson commented Dec 6, 2019

For details of what these prefixes to the version (like ^) means in package.json, see: https://docs.npmjs.com/files/package.json#dependencies

@v1v v1v changed the title ci(jenkins): script to bump the given version ci(jenkins): release process Dec 6, 2019
@v1v v1v merged commit 7fa9c78 into elastic:master Dec 6, 2019
@v1v v1v deleted the feature/release-opbeans-step branch December 6, 2019 14:40
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.

4 participants