Skip to content

Commit

Permalink
Fix version comparison for final version after RC
Browse files Browse the repository at this point in the history
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]>
  • Loading branch information
saschagrunert committed May 29, 2024
1 parent 38d66ed commit c0d739f
Show file tree
Hide file tree
Showing 2 changed files with 80 additions and 1 deletion.
29 changes: 28 additions & 1 deletion pkg/release/publish.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"path/filepath"
"strings"

"github.com/blang/semver/v4"
"github.com/sirupsen/logrus"

"sigs.k8s.io/release-sdk/gcli"
Expand Down Expand Up @@ -283,7 +284,7 @@ func (p *Publisher) VerifyLatestUpdate(
return false, fmt.Errorf("invalid GCS version format %s", gcsVersion)
}

if sv.LTE(gcsSemverVersion) {
if IsUpToDate(gcsSemverVersion, sv) {
logrus.Infof(
"Not updating version, because %s <= %s", version, gcsVersion,
)
Expand All @@ -294,6 +295,32 @@ func (p *Publisher) VerifyLatestUpdate(
return true, nil
}

func IsUpToDate(oldVersion, newVersion semver.Version) bool {
oldPre := oldVersion.Pre
newPre := newVersion.Pre

oldStrippedPre := semver.Version{Major: oldVersion.Major, Minor: oldVersion.Minor, Patch: oldVersion.Patch}
newStrippedPre := semver.Version{Major: newVersion.Major, Minor: newVersion.Minor, Patch: newVersion.Patch}

// Verfy specific use case in our tagging logic:
// 1.30.0-rc.2.10+00000000000000
// needs to be considered lower than
// 1.30.0-11+00000000000000
// which is not given by newVersion.LTE(oldVersion) below.
if len(oldPre) == 3 && // [rc 2 10]
oldPre[0].String() == "rc" &&
len(newPre) == 1 && // [11]
newPre[0].IsNum &&
// For example when:
// oldVersion: 1.29.0-rc.0.20+00000000000000
// newVersion: 1.28.1-2+00000000000000
newStrippedPre.GE(oldStrippedPre) {
return false
}

return newVersion.LTE(oldVersion)
}

// PublishToGcs publishes a release to GCS
// publishFile - the GCS location to look in
// buildDir - build output directory
Expand Down
52 changes: 52 additions & 0 deletions pkg/release/publish_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"os"
"testing"

"github.com/blang/semver/v4"
"github.com/stretchr/testify/require"
"k8s.io/release/pkg/release"
"k8s.io/release/pkg/release/releasefakes"
Expand Down Expand Up @@ -325,3 +326,54 @@ func TestPublishReleaseNotesIndex(t *testing.T) {
}
}
}

func TestIsUpToDate(t *testing.T) {
t.Parallel()

for _, tc := range []struct {
name, oldVersion, newVersion string
expected bool
}{
{
name: "Final version after RC",
oldVersion: "1.30.0-rc.2.10+00000000000000",
newVersion: "1.30.0-11+00000000000000",
expected: false,
},
{
name: "More commits",
oldVersion: "1.30.0-10+00000000000000",
newVersion: "1.30.0-11+00000000000000",
expected: false,
},
{
name: "Newer release",
oldVersion: "1.29.0-0+00000000000000",
newVersion: "1.29.1-0+00000000000000",
expected: false,
},
{
name: "Counter reset after RC",
oldVersion: "1.30.0-rc.2.10+00000000000000",
newVersion: "1.30.0-1+00000000000000",
expected: false,
},
{
name: "Patch after newer RC (artificial corner case)",
oldVersion: "1.29.0-rc.0.20+00000000000000",
newVersion: "1.28.1-2+00000000000000",
expected: true,
},
} {
oldVersion := semver.MustParse(tc.oldVersion)
newVersion := semver.MustParse(tc.newVersion)
expected := tc.expected

t.Run(tc.name, func(t *testing.T) {
t.Parallel()

res := release.IsUpToDate(oldVersion, newVersion)
require.Equal(t, expected, res)
})
}
}

0 comments on commit c0d739f

Please sign in to comment.