Skip to content

Commit 7288ca7

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #3223 from xmudrii/fix-release-comparison
Fix version comparison in VerifyLatestUpdate
2 parents 0e27c38 + 23c723f commit 7288ca7

File tree

2 files changed

+188
-1
lines changed

2 files changed

+188
-1
lines changed

‎pkg/release/publish.go

+11-1
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,17 @@ func (p *Publisher) VerifyLatestUpdate(
282282
return false, fmt.Errorf("invalid GCS version format %s", gcsVersion)
283283
}
284284

285-
if sv.LTE(gcsSemverVersion) {
285+
// The version format that we use is:
286+
// - Stable: "v<major>.<minor>.<patch>-<number-of-commits>+<build-number>"
287+
// - Prerelease: "v<major>.<minor>.<patch>-<prerelease>.<number-of-commits>+<build-number>"
288+
// blang/semver library considers "-<number-of-commits>" part as the prerelease part even though
289+
// the version is stable. This is causing issues when we're supposed to bump the version marker
290+
// from rc to stable.
291+
// blang/semver models prerelease as a slice of prereleases, so stable releases
292+
// are going to have one element (the number of commits), and prereleases are going to have three elements
293+
// (alpha/beta/rc, number of prerelease, number of commits).
294+
// We can use this to determine when we're switching from rc to stable and act accordingly.
295+
if sv.LTE(gcsSemverVersion) && len(sv.Pre) >= len(gcsSemverVersion.Pre) {
286296
logrus.Infof(
287297
"Not updating version, because %s <= %s", version, gcsVersion,
288298
)

‎pkg/release/publish_test.go

+177
Original file line numberDiff line numberDiff line change
@@ -325,3 +325,180 @@ func TestPublishReleaseNotesIndex(t *testing.T) {
325325
}
326326
}
327327
}
328+
329+
func TestVerifyLatestUpdate(t *testing.T) {
330+
for _, tc := range []struct {
331+
prepare func(*releasefakes.FakePublisherClient, string)
332+
version string
333+
gcsVersion string
334+
needsUpdate bool
335+
shouldError bool
336+
}{
337+
{ // success same version
338+
prepare: func(mock *releasefakes.FakePublisherClient, gcsVersion string) {
339+
mock.NormalizePathReturns("", nil)
340+
mock.GSUtilOutputReturns(gcsVersion, nil)
341+
},
342+
version: "v1.24.0",
343+
gcsVersion: "v1.24.0",
344+
needsUpdate: false,
345+
shouldError: false,
346+
},
347+
348+
{ // success version > gcsVersion (patch)
349+
prepare: func(mock *releasefakes.FakePublisherClient, gcsVersion string) {
350+
mock.NormalizePathReturns("", nil)
351+
mock.GSUtilOutputReturns(gcsVersion, nil)
352+
},
353+
version: "v1.24.1",
354+
gcsVersion: "v1.24.0",
355+
needsUpdate: true,
356+
shouldError: false,
357+
},
358+
{ // success version < gcsVersion (patch)
359+
prepare: func(mock *releasefakes.FakePublisherClient, gcsVersion string) {
360+
mock.NormalizePathReturns("", nil)
361+
mock.GSUtilOutputReturns(gcsVersion, nil)
362+
},
363+
version: "v1.24.0",
364+
gcsVersion: "v1.24.1",
365+
needsUpdate: false,
366+
shouldError: false,
367+
},
368+
369+
{ // success version > gcsVersion (minor)
370+
prepare: func(mock *releasefakes.FakePublisherClient, gcsVersion string) {
371+
mock.NormalizePathReturns("", nil)
372+
mock.GSUtilOutputReturns(gcsVersion, nil)
373+
},
374+
version: "v1.25.0",
375+
gcsVersion: "v1.24.0",
376+
needsUpdate: true,
377+
shouldError: false,
378+
},
379+
{ // success version < gcsVersion (minor)
380+
prepare: func(mock *releasefakes.FakePublisherClient, gcsVersion string) {
381+
mock.NormalizePathReturns("", nil)
382+
mock.GSUtilOutputReturns(gcsVersion, nil)
383+
},
384+
version: "v1.23.0",
385+
gcsVersion: "v1.24.0",
386+
needsUpdate: false,
387+
shouldError: false,
388+
},
389+
390+
{ // success version = gcsVersion (with build version)
391+
prepare: func(mock *releasefakes.FakePublisherClient, gcsVersion string) {
392+
mock.NormalizePathReturns("", nil)
393+
mock.GSUtilOutputReturns(gcsVersion, nil)
394+
},
395+
version: "v1.28.0-7+c4e17abb04728e",
396+
gcsVersion: "v1.28.0-7+c4e17abb04728e",
397+
needsUpdate: false,
398+
shouldError: false,
399+
},
400+
{ // success version > gcsVersion (with build version)
401+
prepare: func(mock *releasefakes.FakePublisherClient, gcsVersion string) {
402+
mock.NormalizePathReturns("", nil)
403+
mock.GSUtilOutputReturns(gcsVersion, nil)
404+
},
405+
version: "v1.28.0-9+aaaaaabb04728e",
406+
gcsVersion: "v1.28.0-7+c4e17abb04728e",
407+
needsUpdate: true,
408+
shouldError: false,
409+
},
410+
{ // success version < gcsVersion (with build version)
411+
prepare: func(mock *releasefakes.FakePublisherClient, gcsVersion string) {
412+
mock.NormalizePathReturns("", nil)
413+
mock.GSUtilOutputReturns(gcsVersion, nil)
414+
},
415+
version: "v1.28.0-7+c4e17abb04728e",
416+
gcsVersion: "v1.28.0-9+aaaaaabb04728e",
417+
needsUpdate: false,
418+
shouldError: false,
419+
},
420+
{ // success version > gcsVersion (with build version)
421+
prepare: func(mock *releasefakes.FakePublisherClient, gcsVersion string) {
422+
mock.NormalizePathReturns("", nil)
423+
mock.GSUtilOutputReturns(gcsVersion, nil)
424+
},
425+
version: "v1.28.1-1+aaaaaabb04728e",
426+
gcsVersion: "v1.28.0-7+c4e17abb04728e",
427+
needsUpdate: true,
428+
shouldError: false,
429+
},
430+
{ // success version = gcsVersion (with build version, prerelease)
431+
prepare: func(mock *releasefakes.FakePublisherClient, gcsVersion string) {
432+
mock.NormalizePathReturns("", nil)
433+
mock.GSUtilOutputReturns(gcsVersion, nil)
434+
},
435+
version: "v1.28.0-rc.1.9+3fb5377b25ec51",
436+
gcsVersion: "v1.28.0-rc.1.9+3fb5377b25ec51",
437+
needsUpdate: false,
438+
shouldError: false,
439+
},
440+
{ // success version > gcsVersion (with build version, prerelease)
441+
prepare: func(mock *releasefakes.FakePublisherClient, gcsVersion string) {
442+
mock.NormalizePathReturns("", nil)
443+
mock.GSUtilOutputReturns(gcsVersion, nil)
444+
},
445+
version: "v1.28.0-rc.1.10+3fb5377b25ec51",
446+
gcsVersion: "v1.28.0-rc.1.9+3fb5377b25ec51",
447+
needsUpdate: true,
448+
shouldError: false,
449+
},
450+
{ // success version < gcsVersion (with build version, prerelease)
451+
prepare: func(mock *releasefakes.FakePublisherClient, gcsVersion string) {
452+
mock.NormalizePathReturns("", nil)
453+
mock.GSUtilOutputReturns(gcsVersion, nil)
454+
},
455+
version: "v1.28.0-rc.1.9+3fb5377b25ec51",
456+
gcsVersion: "v1.28.0-rc.1.10+3fb5377b25ec51",
457+
needsUpdate: false,
458+
shouldError: false,
459+
},
460+
{ // success version < gcsVersion (with build version, prerelease)
461+
prepare: func(mock *releasefakes.FakePublisherClient, gcsVersion string) {
462+
mock.NormalizePathReturns("", nil)
463+
mock.GSUtilOutputReturns(gcsVersion, nil)
464+
},
465+
version: "v1.28.0-beta.1.9+3fb5377b25ec51",
466+
gcsVersion: "v1.28.0-rc.1.9+3fb5377b25ec51",
467+
needsUpdate: false,
468+
shouldError: false,
469+
},
470+
{ // success version > gcsVersion (with build version, prerelease)
471+
prepare: func(mock *releasefakes.FakePublisherClient, gcsVersion string) {
472+
mock.NormalizePathReturns("", nil)
473+
mock.GSUtilOutputReturns(gcsVersion, nil)
474+
},
475+
version: "v1.28.0-rc.1.9+3fb5377b25ec51",
476+
gcsVersion: "v1.28.0-beta.1.9+3fb5377b25ec51",
477+
needsUpdate: true,
478+
shouldError: false,
479+
},
480+
{ // success version > gcsVersion (with build version, stable and prerelease)
481+
prepare: func(mock *releasefakes.FakePublisherClient, gcsVersion string) {
482+
mock.NormalizePathReturns("", nil)
483+
mock.GSUtilOutputReturns(gcsVersion, nil)
484+
},
485+
version: "v1.28.0-7+c4e17abb04728e",
486+
gcsVersion: "v1.28.0-rc.1.9+3fb5377b25ec51",
487+
needsUpdate: true,
488+
shouldError: false,
489+
},
490+
} {
491+
sut := release.NewPublisher()
492+
clientMock := &releasefakes.FakePublisherClient{}
493+
sut.SetClient(clientMock)
494+
tc.prepare(clientMock, tc.gcsVersion)
495+
496+
needsUpdate, err := sut.VerifyLatestUpdate("", "", tc.version)
497+
if tc.shouldError {
498+
require.NotNil(t, err)
499+
} else {
500+
require.Nil(t, err)
501+
require.Equal(t, tc.needsUpdate, needsUpdate)
502+
}
503+
}
504+
}

0 commit comments

Comments
 (0)