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(snapshot): skip deploy if no snapshot version or release version #238

Merged
merged 7 commits into from
Feb 20, 2024

Conversation

v1v
Copy link
Member

@v1v v1v commented Feb 19, 2024

What

  • Don't run a snapshot deployment if the version is not a snapshot.
  • Don't run a release deployment if the version is a snapshot.

Why

The release process has been changed to accommodate the branch protection, and hence, the main will contain a release version until the PR with the snapshot version is merged.

Therefore, the snapshot GitHub action will run when it should not!

In addition, let's avoid the corner case where the pom.xml file contains a SNAPSHOT version and the inputs.ref points to that particular git reference.

Test

image

### Issues

Caused by #226

@v1v v1v requested review from a team February 19, 2024 14:08
@v1v v1v self-assigned this Feb 19, 2024
@v1v v1v changed the title ci(snapshot): skip deploy if no snapshot version ci(snapshot): skip deploy if no snapshot version or release version Feb 19, 2024
@@ -51,6 +51,15 @@ jobs:
echo "Tag should match pom.xml project.version"
exit 1
fi
- uses: ./.github/workflows/maven-goal
Copy link
Contributor

@JonasKunz JonasKunz Feb 19, 2024

Choose a reason for hiding this comment

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

Do we even need this here? Don't we check for exact equality with the provided end.RELEASE_VERSION above?

Someone would need to actually provide 1.6.0-SNAPSHOT here to accidentally release a snapshot?

Disclaimer: I'm a shell scripting noob

Copy link
Member Author

@v1v v1v Feb 19, 2024

Choose a reason for hiding this comment

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

Do we even need this here? Don't we check for exact equality with the provided end.RELEASE_VERSION above?

You are right!

Although I'm a bit confused about the reason for using the inputs.version, IIUC, it should be the pre-release workflow, the one bumping the version and the release workflow, the one using the pom.xml file, to do the release. Hence inputs.version is not something we need at all, the buildkite pipeline is not using it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Someone would need to actually provide 1.6.0-SNAPSHOT here to accidentally release a snapshot?

Or change the pom.xml version in a different and use a different inputs.ref.

I think the validation is still valid and in fact, the checkout for the given ref is needed, I'll do the change now

Copy link
Member Author

Choose a reason for hiding this comment

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

see a696105

.github/workflows/release.yml Outdated Show resolved Hide resolved
.github/workflows/snapshot.yml Outdated Show resolved Hide resolved
@v1v v1v requested review from JonasKunz and SylvainJuge February 20, 2024 08:31
@@ -40,6 +40,7 @@ jobs:
- name: Checkout
uses: actions/checkout@v4
with:
ref: ${{ inputs.ref }}
Copy link
Member Author

Choose a reason for hiding this comment

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

Fail fast if the ref is not correct. Otherwise, it's required to wait for the Buildkite pipeline to run

@@ -51,6 +52,12 @@ jobs:
echo "Tag should match pom.xml project.version"
exit 1
fi
- name: Validate version is a release version
Copy link
Member Author

Choose a reason for hiding this comment

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

Otherwise, the pom.xml version is a snapshot.

@v1v v1v requested a review from amannocci February 20, 2024 11:14
Copy link
Contributor

@amannocci amannocci left a comment

Choose a reason for hiding this comment

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

LGTM

@JonasKunz JonasKunz merged commit ddcf03a into elastic:main Feb 20, 2024
7 checks passed
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.

4 participants