From 72154aa668f002119ec5128df68a2eb38f9c130e Mon Sep 17 00:00:00 2001 From: Jakob Borg Date: Wed, 25 May 2016 14:01:52 +0000 Subject: [PATCH] lib/upgrade: Prefer a minor upgrade over a major (fixes #3163) GitHub-Pull-Request: https://github.com/syncthing/syncthing/pull/3184 --- lib/upgrade/upgrade_supported.go | 31 ++++++++++++++-- lib/upgrade/upgrade_test.go | 63 +++++++++++++++++++++++++++----- 2 files changed, 82 insertions(+), 12 deletions(-) diff --git a/lib/upgrade/upgrade_supported.go b/lib/upgrade/upgrade_supported.go index d1ccabcfe..5ecb98ea8 100644 --- a/lib/upgrade/upgrade_supported.go +++ b/lib/upgrade/upgrade_supported.go @@ -129,11 +129,31 @@ func SelectLatestRelease(version string, rels []Release) (Release, error) { return Release{}, ErrNoVersionToSelect } - sort.Sort(SortByRelease(rels)) + // Sort the releases, lowest version number first + sort.Sort(sort.Reverse(SortByRelease(rels))) // Check for a beta build beta := strings.Contains(version, "-") + var selected Release for _, rel := range rels { + switch CompareVersions(rel.Tag, version) { + case Older, MajorOlder: + // This is older than what we're already running + continue + + case MajorNewer: + // We've found a new major version. That's fine, but if we've + // already found a minor upgrade that is acceptable we should go + // with that one first and then revisit in the future. + if selected.Tag != "" && CompareVersions(selected.Tag, version) == Newer { + return selected, nil + } + // else it may be viable, do the needful below + + default: + // New minor release, do the usual processing + } + if rel.Prerelease && !beta { continue } @@ -144,11 +164,16 @@ func SelectLatestRelease(version string, rels []Release) (Release, error) { l.Debugf("expected release asset %q", expectedRelease) l.Debugln("considering release", assetName) if strings.HasPrefix(assetName, expectedRelease) { - return rel, nil + selected = rel } } } - return Release{}, ErrNoReleaseDownload + + if selected.Tag == "" { + return Release{}, ErrNoReleaseDownload + } + + return selected, nil } // Upgrade to the given release, saving the previous binary with a ".old" extension. diff --git a/lib/upgrade/upgrade_test.go b/lib/upgrade/upgrade_test.go index e1aa8f1c9..2c7c7e779 100644 --- a/lib/upgrade/upgrade_test.go +++ b/lib/upgrade/upgrade_test.go @@ -11,6 +11,7 @@ package upgrade import ( "encoding/json" "os" + "strings" "testing" ) @@ -58,16 +59,15 @@ func TestCompareVersions(t *testing.T) { } } -var upgrades = map[string]string{ - "v0.10.21": "v0.10.30", - "v0.10.29": "v0.10.30", - "v0.10.31": "v0.10.30", - "v0.10.0-alpha": "v0.11.0-beta0", - "v0.10.0-beta": "v0.11.0-beta0", - "v0.11.0-beta0+40-g53cb66e-dirty": "v0.11.0-beta0", -} - func TestGithubRelease(t *testing.T) { + var upgrades = map[string]string{ + "v0.10.21": "v0.10.30", + "v0.10.29": "v0.10.30", + "v0.10.0-alpha": "v0.10.30", + "v0.10.0-beta": "v0.10.30", + "v0.11.0-beta0+40-g53cb66e-dirty": "v0.11.0-beta0", + } + fd, err := os.Open("testdata/github-releases.json") if err != nil { t.Errorf("Missing github-release test data") @@ -94,3 +94,48 @@ func TestErrorRelease(t *testing.T) { t.Error("Should return an error when no release were available") } } + +func TestSelectedRelease(t *testing.T) { + testcases := []struct { + current string + candidates []string + selected string + }{ + // Within the same "major" (minor, in this case) select the newest + {"v0.12.24", []string{"v0.12.23", "v0.12.24", "v0.12.25", "v0.12.26"}, "v0.12.26"}, + // Do no select beta versions when we are not a beta + {"v0.12.24", []string{"v0.12.26", "v0.12.27-beta.42"}, "v0.12.26"}, + // Do select beta versions when we are a beta + {"v0.12.24-beta.0", []string{"v0.12.26", "v0.12.27-beta.42"}, "v0.12.27-beta.42"}, + // Select the best within the current major when there is a minor upgrade available + {"v0.12.24", []string{"v0.12.23", "v0.12.24", "v0.12.25", "v0.13.0"}, "v0.12.25"}, + {"v1.12.24", []string{"v1.12.23", "v1.12.24", "v1.14.2", "v2.0.0"}, "v1.14.2"}, + // Select the next major when we are at the best minor + {"v0.12.25", []string{"v0.12.23", "v0.12.24", "v0.12.25", "v0.13.0"}, "v0.13.0"}, + {"v1.14.2", []string{"v0.12.23", "v0.12.24", "v1.14.2", "v2.0.0"}, "v2.0.0"}, + } + + for i, tc := range testcases { + // Prepare a list of candidate releases + var rels []Release + for _, c := range tc.candidates { + rels = append(rels, Release{ + Tag: c, + Prerelease: strings.Contains(c, "-"), + Assets: []Asset{ + // There must be a matching asset or it will not get selected + {Name: releaseName(c)}, + }, + }) + } + + // Check the selection + sel, err := SelectLatestRelease(tc.current, rels) + if err != nil { + t.Fatal("Unexpected error:", err) + } + if sel.Tag != tc.selected { + t.Errorf("Test case %d: expected %s to be selected, but got %s", i, tc.selected, sel.Tag) + } + } +}