Skip to content

Commit

Permalink
Merge pull request kubernetes#3223 from xmudrii/fix-release-comparison
Browse files Browse the repository at this point in the history 
Fix version comparison in VerifyLatestUpdate
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot authored Aug 25, 2023
2 parents 0e27c38 + 23c723f commit 7288ca7
Show file tree
Hide file tree
Showing 2 changed files with 188 additions and 1 deletion.
12 changes: 11 additions & 1 deletion pkg/release/publish.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,17 @@ func (p *Publisher) VerifyLatestUpdate(
return false, fmt.Errorf("invalid GCS version format %s", gcsVersion)
}

if sv.LTE(gcsSemverVersion) {
// The version format that we use is:
// - Stable: "v<major>.<minor>.<patch>-<number-of-commits>+<build-number>"
// - Prerelease: "v<major>.<minor>.<patch>-<prerelease>.<number-of-commits>+<build-number>"
// blang/semver library considers "-<number-of-commits>" part as the prerelease part even though
// the version is stable. This is causing issues when we're supposed to bump the version marker
// from rc to stable.
// blang/semver models prerelease as a slice of prereleases, so stable releases
// are going to have one element (the number of commits), and prereleases are going to have three elements
// (alpha/beta/rc, number of prerelease, number of commits).
// We can use this to determine when we're switching from rc to stable and act accordingly.
if sv.LTE(gcsSemverVersion) && len(sv.Pre) >= len(gcsSemverVersion.Pre) {
logrus.Infof(
"Not updating version, because %s <= %s", version, gcsVersion,
)
Expand Down
177 changes: 177 additions & 0 deletions pkg/release/publish_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -325,3 +325,180 @@ func TestPublishReleaseNotesIndex(t *testing.T) {
}
}
}

func TestVerifyLatestUpdate(t *testing.T) {
for _, tc := range []struct {
prepare func(*releasefakes.FakePublisherClient, string)
version string
gcsVersion string
needsUpdate bool
shouldError bool
}{
{ // success same version
prepare: func(mock *releasefakes.FakePublisherClient, gcsVersion string) {
mock.NormalizePathReturns("", nil)
mock.GSUtilOutputReturns(gcsVersion, nil)
},
version: "v1.24.0",
gcsVersion: "v1.24.0",
needsUpdate: false,
shouldError: false,
},

{ // success version > gcsVersion (patch)
prepare: func(mock *releasefakes.FakePublisherClient, gcsVersion string) {
mock.NormalizePathReturns("", nil)
mock.GSUtilOutputReturns(gcsVersion, nil)
},
version: "v1.24.1",
gcsVersion: "v1.24.0",
needsUpdate: true,
shouldError: false,
},
{ // success version < gcsVersion (patch)
prepare: func(mock *releasefakes.FakePublisherClient, gcsVersion string) {
mock.NormalizePathReturns("", nil)
mock.GSUtilOutputReturns(gcsVersion, nil)
},
version: "v1.24.0",
gcsVersion: "v1.24.1",
needsUpdate: false,
shouldError: false,
},

{ // success version > gcsVersion (minor)
prepare: func(mock *releasefakes.FakePublisherClient, gcsVersion string) {
mock.NormalizePathReturns("", nil)
mock.GSUtilOutputReturns(gcsVersion, nil)
},
version: "v1.25.0",
gcsVersion: "v1.24.0",
needsUpdate: true,
shouldError: false,
},
{ // success version < gcsVersion (minor)
prepare: func(mock *releasefakes.FakePublisherClient, gcsVersion string) {
mock.NormalizePathReturns("", nil)
mock.GSUtilOutputReturns(gcsVersion, nil)
},
version: "v1.23.0",
gcsVersion: "v1.24.0",
needsUpdate: false,
shouldError: false,
},

{ // success version = gcsVersion (with build version)
prepare: func(mock *releasefakes.FakePublisherClient, gcsVersion string) {
mock.NormalizePathReturns("", nil)
mock.GSUtilOutputReturns(gcsVersion, nil)
},
version: "v1.28.0-7+c4e17abb04728e",
gcsVersion: "v1.28.0-7+c4e17abb04728e",
needsUpdate: false,
shouldError: false,
},
{ // success version > gcsVersion (with build version)
prepare: func(mock *releasefakes.FakePublisherClient, gcsVersion string) {
mock.NormalizePathReturns("", nil)
mock.GSUtilOutputReturns(gcsVersion, nil)
},
version: "v1.28.0-9+aaaaaabb04728e",
gcsVersion: "v1.28.0-7+c4e17abb04728e",
needsUpdate: true,
shouldError: false,
},
{ // success version < gcsVersion (with build version)
prepare: func(mock *releasefakes.FakePublisherClient, gcsVersion string) {
mock.NormalizePathReturns("", nil)
mock.GSUtilOutputReturns(gcsVersion, nil)
},
version: "v1.28.0-7+c4e17abb04728e",
gcsVersion: "v1.28.0-9+aaaaaabb04728e",
needsUpdate: false,
shouldError: false,
},
{ // success version > gcsVersion (with build version)
prepare: func(mock *releasefakes.FakePublisherClient, gcsVersion string) {
mock.NormalizePathReturns("", nil)
mock.GSUtilOutputReturns(gcsVersion, nil)
},
version: "v1.28.1-1+aaaaaabb04728e",
gcsVersion: "v1.28.0-7+c4e17abb04728e",
needsUpdate: true,
shouldError: false,
},
{ // success version = gcsVersion (with build version, prerelease)
prepare: func(mock *releasefakes.FakePublisherClient, gcsVersion string) {
mock.NormalizePathReturns("", nil)
mock.GSUtilOutputReturns(gcsVersion, nil)
},
version: "v1.28.0-rc.1.9+3fb5377b25ec51",
gcsVersion: "v1.28.0-rc.1.9+3fb5377b25ec51",
needsUpdate: false,
shouldError: false,
},
{ // success version > gcsVersion (with build version, prerelease)
prepare: func(mock *releasefakes.FakePublisherClient, gcsVersion string) {
mock.NormalizePathReturns("", nil)
mock.GSUtilOutputReturns(gcsVersion, nil)
},
version: "v1.28.0-rc.1.10+3fb5377b25ec51",
gcsVersion: "v1.28.0-rc.1.9+3fb5377b25ec51",
needsUpdate: true,
shouldError: false,
},
{ // success version < gcsVersion (with build version, prerelease)
prepare: func(mock *releasefakes.FakePublisherClient, gcsVersion string) {
mock.NormalizePathReturns("", nil)
mock.GSUtilOutputReturns(gcsVersion, nil)
},
version: "v1.28.0-rc.1.9+3fb5377b25ec51",
gcsVersion: "v1.28.0-rc.1.10+3fb5377b25ec51",
needsUpdate: false,
shouldError: false,
},
{ // success version < gcsVersion (with build version, prerelease)
prepare: func(mock *releasefakes.FakePublisherClient, gcsVersion string) {
mock.NormalizePathReturns("", nil)
mock.GSUtilOutputReturns(gcsVersion, nil)
},
version: "v1.28.0-beta.1.9+3fb5377b25ec51",
gcsVersion: "v1.28.0-rc.1.9+3fb5377b25ec51",
needsUpdate: false,
shouldError: false,
},
{ // success version > gcsVersion (with build version, prerelease)
prepare: func(mock *releasefakes.FakePublisherClient, gcsVersion string) {
mock.NormalizePathReturns("", nil)
mock.GSUtilOutputReturns(gcsVersion, nil)
},
version: "v1.28.0-rc.1.9+3fb5377b25ec51",
gcsVersion: "v1.28.0-beta.1.9+3fb5377b25ec51",
needsUpdate: true,
shouldError: false,
},
{ // success version > gcsVersion (with build version, stable and prerelease)
prepare: func(mock *releasefakes.FakePublisherClient, gcsVersion string) {
mock.NormalizePathReturns("", nil)
mock.GSUtilOutputReturns(gcsVersion, nil)
},
version: "v1.28.0-7+c4e17abb04728e",
gcsVersion: "v1.28.0-rc.1.9+3fb5377b25ec51",
needsUpdate: true,
shouldError: false,
},
} {
sut := release.NewPublisher()
clientMock := &releasefakes.FakePublisherClient{}
sut.SetClient(clientMock)
tc.prepare(clientMock, tc.gcsVersion)

needsUpdate, err := sut.VerifyLatestUpdate("", "", tc.version)
if tc.shouldError {
require.NotNil(t, err)
} else {
require.Nil(t, err)
require.Equal(t, tc.needsUpdate, needsUpdate)
}
}
}

0 comments on commit 7288ca7

Please sign in to comment.