From f92b4f9a2827e5d2bc3ef96f4385dc957c2531de Mon Sep 17 00:00:00 2001 From: Marek Siarkowicz Date: Tue, 5 Oct 2021 17:21:20 +0200 Subject: [PATCH] server: Integrate version validation logic into tests --- server/etcdserver/api/membership/cluster.go | 6 ++++-- server/etcdserver/version/downgrade.go | 19 +++++++++---------- server/etcdserver/version/downgrade_test.go | 7 ++++--- server/etcdserver/version/version.go | 2 +- server/etcdserver/version/version_test.go | 15 +++++++++------ 5 files changed, 27 insertions(+), 22 deletions(-) diff --git a/server/etcdserver/api/membership/cluster.go b/server/etcdserver/api/membership/cluster.go index ad6633ebd..f1b41ee20 100644 --- a/server/etcdserver/api/membership/cluster.go +++ b/server/etcdserver/api/membership/cluster.go @@ -273,7 +273,8 @@ func (c *RaftCluster) Recover(onSet func(*zap.Logger, *semver.Version)) { if c.downgradeInfo != nil { d = &serverversion.DowngradeInfo{Enabled: c.downgradeInfo.Enabled, TargetVersion: c.downgradeInfo.TargetVersion} } - serverversion.MustDetectDowngrade(c.lg, c.version, d) + sv := semver.Must(semver.NewVersion(version.Version)) + serverversion.MustDetectDowngrade(c.lg, sv, c.version, d) onSet(c.lg, c.version) for _, m := range c.members { @@ -541,7 +542,8 @@ func (c *RaftCluster) SetVersion(ver *semver.Version, onSet func(*zap.Logger, *s } oldVer := c.version c.version = ver - serverversion.MustDetectDowngrade(c.lg, c.version, c.downgradeInfo) + sv := semver.Must(semver.NewVersion(version.Version)) + serverversion.MustDetectDowngrade(c.lg, sv, c.version, c.downgradeInfo) if c.v2store != nil { mustSaveClusterVersionToStore(c.lg, c.v2store, ver) } diff --git a/server/etcdserver/version/downgrade.go b/server/etcdserver/version/downgrade.go index f43370455..d70fd63ac 100644 --- a/server/etcdserver/version/downgrade.go +++ b/server/etcdserver/version/downgrade.go @@ -34,47 +34,46 @@ func (d *DowngradeInfo) GetTargetVersion() *semver.Version { // isValidDowngrade verifies whether the cluster can be downgraded from verFrom to verTo func isValidDowngrade(verFrom *semver.Version, verTo *semver.Version) bool { - return verTo.Equal(*AllowedDowngradeVersion(verFrom)) + return verTo.Equal(*allowedDowngradeVersion(verFrom)) } // MustDetectDowngrade will detect unexpected downgrade when the local server is recovered. -func MustDetectDowngrade(lg *zap.Logger, cv *semver.Version, d *DowngradeInfo) { - lv := semver.Must(semver.NewVersion(version.Version)) +func MustDetectDowngrade(lg *zap.Logger, sv, cv *semver.Version, d *DowngradeInfo) { // only keep major.minor version for comparison against cluster version - lv = &semver.Version{Major: lv.Major, Minor: lv.Minor} + sv = &semver.Version{Major: sv.Major, Minor: sv.Minor} // if the cluster enables downgrade, check local version against downgrade target version. if d != nil && d.Enabled && d.TargetVersion != "" { - if lv.Equal(*d.GetTargetVersion()) { + if sv.Equal(*d.GetTargetVersion()) { if cv != nil { lg.Info( "cluster is downgrading to target version", zap.String("target-cluster-version", d.TargetVersion), zap.String("determined-cluster-version", version.Cluster(cv.String())), - zap.String("current-server-version", version.Version), + zap.String("current-server-version", sv.String()), ) } return } lg.Panic( "invalid downgrade; server version is not allowed to join when downgrade is enabled", - zap.String("current-server-version", version.Version), + zap.String("current-server-version", sv.String()), zap.String("target-cluster-version", d.TargetVersion), ) } // if the cluster disables downgrade, check local version against determined cluster version. // the validation passes when local version is not less than cluster version - if cv != nil && lv.LessThan(*cv) { + if cv != nil && sv.LessThan(*cv) { lg.Panic( "invalid downgrade; server version is lower than determined cluster version", - zap.String("current-server-version", version.Version), + zap.String("current-server-version", sv.String()), zap.String("determined-cluster-version", version.Cluster(cv.String())), ) } } -func AllowedDowngradeVersion(ver *semver.Version) *semver.Version { +func allowedDowngradeVersion(ver *semver.Version) *semver.Version { // Todo: handle the case that downgrading from higher major version(e.g. downgrade from v4.0 to v3.x) return &semver.Version{Major: ver.Major, Minor: ver.Minor - 1} } diff --git a/server/etcdserver/version/downgrade_test.go b/server/etcdserver/version/downgrade_test.go index e3ba69963..97c8d2125 100644 --- a/server/etcdserver/version/downgrade_test.go +++ b/server/etcdserver/version/downgrade_test.go @@ -110,7 +110,8 @@ func TestMustDetectDowngrade(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { lg := zaptest.NewLogger(t) - err := tryMustDetectDowngrade(lg, tt.clusterVersion, tt.downgrade) + sv := semver.Must(semver.NewVersion(version.Version)) + err := tryMustDetectDowngrade(lg, sv, tt.clusterVersion, tt.downgrade) if tt.success != (err == nil) { t.Errorf("Unexpected status, got %q, wanted: %v", err, tt.success) @@ -123,11 +124,11 @@ func TestMustDetectDowngrade(t *testing.T) { } } -func tryMustDetectDowngrade(lg *zap.Logger, cv *semver.Version, d *DowngradeInfo) (err interface{}) { +func tryMustDetectDowngrade(lg *zap.Logger, sv, cv *semver.Version, d *DowngradeInfo) (err interface{}) { defer func() { err = recover() }() - MustDetectDowngrade(lg, cv, d) + MustDetectDowngrade(lg, sv, cv, d) return err } diff --git a/server/etcdserver/version/version.go b/server/etcdserver/version/version.go index b2c62b15c..eb9a370c1 100644 --- a/server/etcdserver/version/version.go +++ b/server/etcdserver/version/version.go @@ -44,7 +44,7 @@ func (m *Manager) DowngradeValidate(ctx context.Context, targetVersion *semver.V return err } cv := m.s.GetClusterVersion() - allowedTargetVersion := AllowedDowngradeVersion(cv) + allowedTargetVersion := allowedDowngradeVersion(cv) if !targetVersion.Equal(*allowedTargetVersion) { return ErrInvalidDowngradeTargetVersion } diff --git a/server/etcdserver/version/version_test.go b/server/etcdserver/version/version_test.go index 5dd01d01e..6beaec0b3 100644 --- a/server/etcdserver/version/version_test.go +++ b/server/etcdserver/version/version_test.go @@ -64,6 +64,7 @@ func TestUpgradeThreeNodes(t *testing.T) { func newCluster(lg *zap.Logger, memberCount int, ver semver.Version) *clusterMock { cluster := &clusterMock{ + lg: lg, clusterVersion: ver, members: make([]*memberMock, 0, memberCount), } @@ -99,13 +100,14 @@ func (c *clusterMock) StepMonitors() { } type clusterMock struct { + lg *zap.Logger clusterVersion semver.Version downgradeInfo *DowngradeInfo members []*memberMock } -func (c *clusterMock) DowngradeEnable(ver semver.Version) { - c.downgradeInfo = &DowngradeInfo{TargetVersion: ver.String(), Enabled: true} +func (c *clusterMock) Version() *Manager { + return NewManager(c.lg, c.members[0]) } func (c *clusterMock) MembersVersions() map[string]*version.Versions { @@ -120,9 +122,7 @@ func (c *clusterMock) MembersVersions() map[string]*version.Versions { } func (c *clusterMock) ReplaceMemberBinary(mid int, newServerVersion semver.Version) { - if newServerVersion.LessThan(c.clusterVersion) { - panic("Members cannot join clusters with higher version") - } + MustDetectDowngrade(c.lg, &c.members[mid].serverVersion, &c.clusterVersion, c.downgradeInfo) c.members[mid].serverVersion = newServerVersion } @@ -146,7 +146,10 @@ func (m *memberMock) LinearizableReadNotify(ctx context.Context) error { } func (m *memberMock) DowngradeEnable(ctx context.Context, targetVersion *semver.Version) error { - m.cluster.downgradeInfo = nil + m.cluster.downgradeInfo = &DowngradeInfo{ + TargetVersion: targetVersion.String(), + Enabled: true, + } return nil }