Fix version comparison in VerifyLatestUpdate #3223
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Marko Mudrinić <[email protected]>
k8s-ci-robot
added
release-note
k8s-ci-robot
added
cncf-cla: yes
k8s-ci-robot
added
sig/release
|
/test pull-release-integration-test |
saschagrunert
approved these changes
Aug 25, 2023
k8s-ci-robot
added
the
lgtm
|
Unfortunately the linter does not seem to be happy with this change. /lgtm cancel |
k8s-ci-robot
removed
the
lgtm
Signed-off-by: Marko Mudrinić <[email protected]>
xmudrii
force-pushed
the
fix-release-comparison
branch
from
78c8377 to
23c723f
Compare
|
/retest |
|
@saschagrunert The linter issue should be fixed now. |
k8s-ci-robot
removed
the
do-not-merge/hold
|
/retest |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cpanato, saschagrunert, xmudrii The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added a commit
that referenced
this pull request
Sep 7, 2023
Revert "Merge pull request #3223 from xmudrii/fix-release-comparison"
saschagrunert
added a commit
to saschagrunert/release
that referenced
this pull request
May 14, 2024
Fixing the very specific use case of the failing semver comparison mentioned in kubernetes/kubernetes#117115 Follow-up on kubernetes#3223 and kubernetes#3254 Signed-off-by: Sascha Grunert <[email protected]>
saschagrunert
added a commit
to saschagrunert/release
that referenced
this pull request
May 28, 2024
Fixing the very specific use case of the failing semver comparison mentioned in kubernetes/kubernetes#117115 Follow-up on kubernetes#3223 and kubernetes#3254 Signed-off-by: Sascha Grunert <[email protected]>
saschagrunert
added a commit
to saschagrunert/release
that referenced
this pull request
May 29, 2024
Fixing the very specific use case of the failing semver comparison mentioned in kubernetes/kubernetes#117115 Follow-up on kubernetes#3223 and kubernetes#3254 Signed-off-by: Sascha Grunert <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What type of PR is this?
/kind bug
What this PR does / why we need it:
We observed that version markers are not getting updated properly on many occasions which is documented in kubernetes/kubernetes#117115
This issue appears when the version marker is supposed to switch from prerelease (rc) to stable. For example:
v1.28.0-rc.1.9+3fb5377b25ec51->v1.28.0-7+c4e17abb04728eis not going to happen even though left version is lower than right version.The reason for that is that both
v1.28.0-rc.1.9+3fb5377b25ec51andv1.28.0-7+c4e17abb04728eare considered as "prereleases" byblang/semverlibrary. Version is modeled as:v1.28.0-rc.1.9+3fb5377b25ec51v1.28.0-7+c4e17abb04728eComparing versions includes comparing elements in both prerelease slices and that doesn't behave as we would expect in this case.
Given this logic, we always know that:
prereleasesliceprereleasesliceThis can be used when comparing the version to properly switch from prerelease (rc) to stable.
There might be a better approach, but I didn't find such an approach from the
blang/semverdocumentation. Unit tests are added to cover various cases and ensure that this still works as expected.Which issue(s) this PR fixes:
Fixes kubernetes/kubernetes#117115
Does this PR introduce a user-facing change?
/assign @saschagrunert @puerco @jeremyrickard
cc @kubernetes/release-engineering
/hold for discussion