diff --git a/server/etcdserver/api/capability.go b/server/etcdserver/api/capability.go index 4cd9aca84..c417d4ca1 100644 --- a/server/etcdserver/api/capability.go +++ b/server/etcdserver/api/capability.go @@ -64,7 +64,7 @@ func UpdateCapability(lg *zap.Logger, v *semver.Version) { return } enableMapMu.Lock() - if curVersion != nil && !serverversion.IsValidVersionChange(v, curVersion) { + if curVersion != nil && !serverversion.IsValidClusterVersionChange(v, curVersion) { enableMapMu.Unlock() return } diff --git a/server/etcdserver/version/downgrade.go b/server/etcdserver/version/downgrade.go index efad36705..f2c6e1194 100644 --- a/server/etcdserver/version/downgrade.go +++ b/server/etcdserver/version/downgrade.go @@ -59,17 +59,17 @@ func allowedDowngradeVersion(ver *semver.Version) *semver.Version { return &semver.Version{Major: ver.Major, Minor: ver.Minor - 1} } -// IsValidVersionChange checks the two scenario when version is valid to change: +// IsValidClusterVersionChange checks the two scenario when version is valid to change: // 1. Downgrade: cluster version is 1 minor version higher than local version, // cluster version should change. // 2. Cluster start: when not all members version are available, cluster version // is set to MinVersion(3.0), when all members are at higher version, cluster version -// is lower than local version, cluster version should change -func IsValidVersionChange(cv *semver.Version, lv *semver.Version) bool { - cv = &semver.Version{Major: cv.Major, Minor: cv.Minor} - lv = &semver.Version{Major: lv.Major, Minor: lv.Minor} +// is lower than minimal server version, cluster version should change +func IsValidClusterVersionChange(verFrom *semver.Version, verTo *semver.Version) bool { + verFrom = &semver.Version{Major: verFrom.Major, Minor: verFrom.Minor} + verTo = &semver.Version{Major: verTo.Major, Minor: verTo.Minor} - if isValidDowngrade(cv, lv) || (cv.Major == lv.Major && cv.LessThan(*lv)) { + if isValidDowngrade(verFrom, verTo) || (verFrom.Major == verTo.Major && verFrom.LessThan(*verTo)) { return true } return false diff --git a/server/etcdserver/version/downgrade_test.go b/server/etcdserver/version/downgrade_test.go index 83c8afead..252fcb57c 100644 --- a/server/etcdserver/version/downgrade_test.go +++ b/server/etcdserver/version/downgrade_test.go @@ -19,6 +19,7 @@ import ( "testing" "github.com/coreos/go-semver/semver" + "github.com/stretchr/testify/assert" "go.uber.org/zap" "go.uber.org/zap/zaptest" @@ -120,73 +121,68 @@ func TestIsValidDowngrade(t *testing.T) { } func TestIsVersionChangable(t *testing.T) { - v0 := semver.Must(semver.NewVersion("2.4.0")) - v1 := semver.Must(semver.NewVersion("3.4.0")) - v2 := semver.Must(semver.NewVersion("3.5.0")) - v3 := semver.Must(semver.NewVersion("3.5.1")) - v4 := semver.Must(semver.NewVersion("3.6.0")) - tests := []struct { name string - currentVersion *semver.Version - localVersion *semver.Version + verFrom string + verTo string expectedResult bool }{ { name: "When local version is one minor lower than cluster version", - currentVersion: v2, - localVersion: v1, + verFrom: "3.5.0", + verTo: "3.4.0", expectedResult: true, }, { name: "When local version is one minor and one patch lower than cluster version", - currentVersion: v3, - localVersion: v1, + verFrom: "3.5.1", + verTo: "3.4.0", expectedResult: true, }, { name: "When local version is one minor higher than cluster version", - currentVersion: v1, - localVersion: v2, + verFrom: "3.4.0", + verTo: "3.5.0", expectedResult: true, }, { name: "When local version is two minor higher than cluster version", - currentVersion: v1, - localVersion: v4, + verFrom: "3.4.0", + verTo: "3.6.0", expectedResult: true, }, { name: "When local version is one major higher than cluster version", - currentVersion: v0, - localVersion: v1, + verFrom: "2.4.0", + verTo: "3.4.0", expectedResult: false, }, { name: "When local version is equal to cluster version", - currentVersion: v1, - localVersion: v1, + verFrom: "3.4.0", + verTo: "3.4.0", expectedResult: false, }, { name: "When local version is one patch higher than cluster version", - currentVersion: v2, - localVersion: v3, + verFrom: "3.5.0", + verTo: "3.5.1", expectedResult: false, }, { name: "When local version is two minor lower than cluster version", - currentVersion: v4, - localVersion: v1, + verFrom: "3.6.0", + verTo: "3.4.0", expectedResult: false, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if ret := IsValidVersionChange(tt.currentVersion, tt.localVersion); ret != tt.expectedResult { - t.Errorf("Expected %v; Got %v", tt.expectedResult, ret) - } + verFrom := semver.Must(semver.NewVersion(tt.verFrom)) + verTo := semver.Must(semver.NewVersion(tt.verTo)) + ret := IsValidClusterVersionChange(verFrom, verTo) + assert.Equal(t, tt.expectedResult, ret) }) } } diff --git a/server/etcdserver/version/monitor.go b/server/etcdserver/version/monitor.go index 7ecc19709..15697e960 100644 --- a/server/etcdserver/version/monitor.go +++ b/server/etcdserver/version/monitor.go @@ -97,7 +97,7 @@ func (m *Monitor) decideClusterVersion() (*semver.Version, error) { } return downgrade.GetTargetVersion(), nil } - if clusterVersion.LessThan(*minimalServerVersion) && IsValidVersionChange(clusterVersion, minimalServerVersion) { + if clusterVersion.LessThan(*minimalServerVersion) && IsValidClusterVersionChange(clusterVersion, minimalServerVersion) { return minimalServerVersion, nil } return nil, nil