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

feat: new release process using PRs #3422

Closed
wants to merge 1 commit into from
Closed

Conversation

amannocci
Copy link
Contributor

What is the change being made?

  • Use PRs to release a new version of the apm-agent-java.
  • The release process has been split into two workflows.
  • The first one will create a PR with pre/post-release changes labeled with release.
  • It will push artifacts in staging/snapshot maven repositories.
  • It will also block any further change happening in the main branch.
  • The new PR should be reviewed by a code owner.
  • If everything looks good then the release process will continue after the PR has been approved.
  • The second workflow will perform the remaining tasks.
  • It will also create a new PR to update the 1.x major branch.
  • This PR could be merged after review.

Why is the change being made?

  • The new branch protection doesn't allow direct push in the main branch even for the automation.

@amannocci amannocci self-assigned this Nov 9, 2023
@amannocci
Copy link
Contributor Author

Given the significant alterations to the release process introduced by this PR, I'm seeking feedback on its implementation.
What do you though @v1v @reakaleek @SylvainJuge?

@SylvainJuge
Copy link
Member

Is there any particular reason to use a separate PR for the version bump ? This is part of the usual release process, and merging non-snapshot version directly into main will make the CI attempt to build non-snapshot artifacts when merged which is not necessary.

Ideally if everything is in the same PR, it means we only have to review the changes once, however in this case the commit that will be used to generate the release would be the HEAD~1 from the release branch (and not the state that we see in the PR review).

Alternatively, we could have one PR with two reviews needed: one for the commit that generates the release binaries, then another one for the next snapshot version (which should only contain version changes in the pom.xml files, thus tedious but not complicated to review).

@amannocci
Copy link
Contributor Author

Is there any particular reason to use a separate PR for the version bump ? This is part of the usual release process, and merging non-snapshot version directly into main will make the CI attempt to build non-snapshot artifacts when merged which is not necessary.

Are talking about the PR to push changes in the major 1.x branch?

@SylvainJuge
Copy link
Member

Are talking about the PR to push changes in the major 1.x branch?

My bad, I misunderstood which branch we were talking about here. The 1.x branch update will of course require a separate PR and that makes sense, I thought that it was bumping the agent version to the next snapshot, which is one of the commits that the release process does.

Copy link
Member

@v1v v1v left a comment

Choose a reason for hiding this comment

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

I cannot find the changes related to using the nexus staging rather than the production one. IIRC, that's configured in the settings.xml or pom.xml

url: ${{ secrets.VAULT_ADDR }}
roleId: ${{ secrets.VAULT_ROLE_ID }}
secretId: ${{ secrets.VAULT_SECRET_ID }}
- name: "Build docker image"
Copy link
Member

Choose a reason for hiding this comment

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

I'd envision the release-from-pr.yml to consume the artifacts generated in the release.yml. This can help with ensuring that no artifacts need to be generated in two different places the ones in release.yml and the ones in release-from-pr.yml. But making the release-from-pr.yml to be the orchestrator.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

AFAIU, the docker build script will consume artifacts from the Sonatype repository since they aren't present locally.

Copy link
Member

Choose a reason for hiding this comment

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

I'm not talking about the docker build image but a more generic aspect, I'd say the release.yml workflow shall be the one creating all the required artifacts, and the release-from-pr.yml workflow being the one promoting them - while promotion means interacting with Nexus, Docker, AWS or similar artifact managers.

Copy link
Member

Choose a reason for hiding this comment

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

On side note regarding the docker build script, will it work when using the staging nexus repository? As I mentioned #3422 (review), this proposal will need to use the staging nexus repository, so releases won't be public available, only after the PR with the bumps is merged (that's the task in release-from-pr.yml).

The promotion in Nexus from staging to production could be done in a follow up. The process for the Release Manager, aka RM, could be something like:

  1. RM runs the release.yml workflow with the relevant input parameters
  2. RM waits for some notification regarding the PR with the bumps to be approved.
    a) Once approved the release-from-pr.yml workflow will do the remaining steps.
  3. RM promotes from stating to production in Nexus (manually if no automated).

@@ -12,28 +12,6 @@ on:
version:
description: 'The version to release (e.g. 1.2.3). This workflow will automatically perform the required version bumps'
required: true
update_changelog:
Copy link
Member

Choose a reason for hiding this comment

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

What's the proposal 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.

That's a good point. Let me try to reintroduce some of them where possible or relevant.

@v1v
Copy link
Member

v1v commented Feb 9, 2024

run docs-build

@amannocci
Copy link
Contributor Author

Superseded by #3567

@amannocci amannocci closed this Mar 20, 2024
@amannocci amannocci deleted the feat/new-release-process branch March 20, 2024 18:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants