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 28, 2024
1 parent 38d66ed commit 1b4995a
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 1 deletion.
22 changes: 21 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,25 @@ func (p *Publisher) VerifyLatestUpdate(
return true, nil
}

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

// 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 {
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
40 changes: 40 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,42 @@ func TestPublishReleaseNotesIndex(t *testing.T) {
}
}
}

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

for _, tc := range []struct {
name, oldVersion, newVersion string
}{
{
name: "Final version after RC",
oldVersion: "1.30.0-rc.2.10+00000000000000",
newVersion: "1.30.0-11+00000000000000",
},
{
name: "More commits",
oldVersion: "1.30.0-10+00000000000000",
newVersion: "1.30.0-11+00000000000000",
},
{
name: "Newer release",
oldVersion: "1.29.0-0+00000000000000",
newVersion: "1.29.1-0+00000000000000",
},
{
name: "Counter reset",
oldVersion: "1.30.0-rc.2.10+00000000000000",
newVersion: "1.30.0-1+00000000000000",
},
} {
oldVersion := semver.MustParse(tc.oldVersion)
newVersion := semver.MustParse(tc.newVersion)

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

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

0 comments on commit 1b4995a

Please sign in to comment.