From 2541b0bba96a4b6ed9c2b3782234b99092176979 Mon Sep 17 00:00:00 2001 From: yoyinzyc Date: Wed, 24 Jun 2020 11:06:29 -0700 Subject: [PATCH] etcdserver: apply downgrade policy to server. --- etcdserver/api/capability.go | 3 +- etcdserver/api/membership/cluster.go | 40 ++-- etcdserver/api/membership/cluster_test.go | 73 ++++++++ etcdserver/api/membership/downgrade.go | 80 ++++++++ etcdserver/api/membership/downgrade_test.go | 194 ++++++++++++++++++++ etcdserver/cluster_util.go | 35 ++-- etcdserver/cluster_util_test.go | 39 ++++ etcdserver/server.go | 4 +- etcdserver/v3_server.go | 2 +- 9 files changed, 434 insertions(+), 36 deletions(-) create mode 100644 etcdserver/api/membership/downgrade.go create mode 100644 etcdserver/api/membership/downgrade_test.go diff --git a/etcdserver/api/capability.go b/etcdserver/api/capability.go index c97d6e1aff4..a0edd5b4e65 100644 --- a/etcdserver/api/capability.go +++ b/etcdserver/api/capability.go @@ -17,6 +17,7 @@ package api import ( "sync" + "go.etcd.io/etcd/v3/etcdserver/api/membership" "go.etcd.io/etcd/v3/version" "go.uber.org/zap" @@ -62,7 +63,7 @@ func UpdateCapability(lg *zap.Logger, v *semver.Version) { return } enableMapMu.Lock() - if curVersion != nil && !curVersion.LessThan(*v) { + if curVersion != nil && !membership.IsValidVersionChange(v, curVersion) { enableMapMu.Unlock() return } diff --git a/etcdserver/api/membership/cluster.go b/etcdserver/api/membership/cluster.go index f5e668bf966..b0939131b52 100644 --- a/etcdserver/api/membership/cluster.go +++ b/etcdserver/api/membership/cluster.go @@ -63,14 +63,6 @@ type RaftCluster struct { downgradeInfo *DowngradeInfo } -type DowngradeInfo struct { - // TargetVersion is the target downgrade version, if the cluster is not under downgrading, - // the targetVersion will be an empty string - TargetVersion string `json:"target-version"` - // Enabled indicates whether the cluster is enabled to downgrade - Enabled bool `json:"enabled"` -} - // ConfigChangeContext represents a context for confChange. type ConfigChangeContext struct { Member @@ -263,7 +255,14 @@ func (c *RaftCluster) Recover(onSet func(*zap.Logger, *semver.Version)) { c.version = clusterVersionFromStore(c.lg, c.v2store) } - mustDetectDowngrade(c.lg, c.version) + if c.be != nil { + c.downgradeInfo = downgradeInfoFromBackend(c.lg, c.be) + } + d := &DowngradeInfo{Enabled: false} + if c.downgradeInfo != nil { + d = &DowngradeInfo{Enabled: c.downgradeInfo.Enabled, TargetVersion: c.downgradeInfo.TargetVersion} + } + mustDetectDowngrade(c.lg, c.version, d) onSet(c.lg, c.version) for _, m := range c.members { @@ -530,7 +529,7 @@ func (c *RaftCluster) SetVersion(ver *semver.Version, onSet func(*zap.Logger, *s } oldVer := c.version c.version = ver - mustDetectDowngrade(c.lg, c.version) + mustDetectDowngrade(c.lg, c.version, c.downgradeInfo) if c.v2store != nil { mustSaveClusterVersionToStore(c.lg, c.v2store, ver) } @@ -768,17 +767,20 @@ func ValidateClusterAndAssignIDs(lg *zap.Logger, local *RaftCluster, existing *R return nil } -func mustDetectDowngrade(lg *zap.Logger, cv *semver.Version) { - lv := semver.Must(semver.NewVersion(version.Version)) - // only keep major.minor version for comparison against cluster version +// IsValidVersionChange 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} - if cv != nil && lv.LessThan(*cv) { - lg.Fatal( - "invalid downgrade; server version is lower than determined cluster version", - zap.String("current-server-version", version.Version), - zap.String("determined-cluster-version", version.Cluster(cv.String())), - ) + + if isValidDowngrade(cv, lv) || (cv.Major == lv.Major && cv.LessThan(*lv)) { + return true } + return false } // IsLocalMemberLearner returns if the local member is raft learner diff --git a/etcdserver/api/membership/cluster_test.go b/etcdserver/api/membership/cluster_test.go index 2a1e8b8d499..dade2c0b6c6 100644 --- a/etcdserver/api/membership/cluster_test.go +++ b/etcdserver/api/membership/cluster_test.go @@ -17,6 +17,7 @@ package membership import ( "encoding/json" "fmt" + "github.com/coreos/go-semver/semver" "path" "reflect" "testing" @@ -944,3 +945,75 @@ func TestIsReadyToPromoteMember(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 + expectedResult bool + }{ + { + name: "When local version is one minor lower than cluster version", + currentVersion: v2, + localVersion: v1, + expectedResult: true, + }, + { + name: "When local version is one minor and one patch lower than cluster version", + currentVersion: v3, + localVersion: v1, + expectedResult: true, + }, + { + name: "When local version is one minor higher than cluster version", + currentVersion: v1, + localVersion: v2, + expectedResult: true, + }, + { + name: "When local version is two minor higher than cluster version", + currentVersion: v1, + localVersion: v4, + expectedResult: true, + }, + { + name: "When local version is one major higher than cluster version", + currentVersion: v0, + localVersion: v1, + expectedResult: false, + }, + { + name: "When local version is equal to cluster version", + currentVersion: v1, + localVersion: v1, + expectedResult: false, + }, + { + name: "When local version is one patch higher than cluster version", + currentVersion: v2, + localVersion: v3, + expectedResult: false, + }, + { + name: "When local version is two minor lower than cluster version", + currentVersion: v4, + localVersion: v1, + 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) + } + }) + } +} diff --git a/etcdserver/api/membership/downgrade.go b/etcdserver/api/membership/downgrade.go new file mode 100644 index 00000000000..2ec2363c9cf --- /dev/null +++ b/etcdserver/api/membership/downgrade.go @@ -0,0 +1,80 @@ +// Copyright 2020 The etcd Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package membership + +import ( + "github.com/coreos/go-semver/semver" + "go.etcd.io/etcd/v3/version" + "go.uber.org/zap" +) + +type DowngradeInfo struct { + // TargetVersion is the target downgrade version, if the cluster is not under downgrading, + // the targetVersion will be an empty string + TargetVersion string `json:"target-version"` + // Enabled indicates whether the cluster is enabled to downgrade + Enabled bool `json:"enabled"` +} + +func (d *DowngradeInfo) GetTargetVersion() *semver.Version { + return semver.Must(semver.NewVersion(d.TargetVersion)) +} + +// 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)) +} + +// 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)) + // only keep major.minor version for comparison against cluster version + lv = &semver.Version{Major: lv.Major, Minor: lv.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 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), + ) + } + return + } + lg.Fatal( + "invalid downgrade; server version is not allowed to join when downgrade is enabled", + zap.String("current-server-version", version.Version), + 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) { + lg.Fatal( + "invalid downgrade; server version is lower than determined cluster version", + zap.String("current-server-version", version.Version), + zap.String("determined-cluster-version", version.Cluster(cv.String())), + ) + } +} + +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/etcdserver/api/membership/downgrade_test.go b/etcdserver/api/membership/downgrade_test.go new file mode 100644 index 00000000000..8163a2ec3e2 --- /dev/null +++ b/etcdserver/api/membership/downgrade_test.go @@ -0,0 +1,194 @@ +// Copyright 2020 The etcd Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package membership + +import ( + "bytes" + "fmt" + "github.com/coreos/go-semver/semver" + "go.etcd.io/etcd/v3/version" + "go.uber.org/zap" + "io/ioutil" + "os" + "os/exec" + "path/filepath" + "strconv" + "testing" +) + +func TestMustDetectDowngrade(t *testing.T) { + lv := semver.Must(semver.NewVersion(version.Version)) + lv = &semver.Version{Major: lv.Major, Minor: lv.Minor} + oneMinorHigher := &semver.Version{Major: lv.Major, Minor: lv.Minor + 1} + oneMinorLower := &semver.Version{Major: lv.Major, Minor: lv.Minor - 1} + downgradeEnabledHigherVersion := &DowngradeInfo{Enabled: true, TargetVersion: oneMinorHigher.String()} + downgradeEnabledEqualVersion := &DowngradeInfo{Enabled: true, TargetVersion: lv.String()} + downgradeEnabledLowerVersion := &DowngradeInfo{Enabled: true, TargetVersion: oneMinorLower.String()} + downgradeDisabled := &DowngradeInfo{Enabled: false} + + tests := []struct { + name string + clusterVersion *semver.Version + downgrade *DowngradeInfo + success bool + message string + }{ + { + "Succeeded when downgrade is disabled and cluster version is nil", + nil, + downgradeDisabled, + true, + "", + }, + { + "Succeeded when downgrade is disabled and cluster version is one minor lower", + oneMinorLower, + downgradeDisabled, + true, + "", + }, + { + "Succeeded when downgrade is disabled and cluster version is server version", + lv, + downgradeDisabled, + true, + "", + }, + { + "Failed when downgrade is disabled and server version is lower than determined cluster version ", + oneMinorHigher, + downgradeDisabled, + false, + "invalid downgrade; server version is lower than determined cluster version", + }, + { + "Succeeded when downgrade is enabled and cluster version is nil", + nil, + downgradeEnabledEqualVersion, + true, + "", + }, + { + "Failed when downgrade is enabled and server version is target version", + lv, + downgradeEnabledEqualVersion, + true, + "cluster is downgrading to target version", + }, + { + "Succeeded when downgrade to lower version and server version is cluster version ", + lv, + downgradeEnabledLowerVersion, + false, + "invalid downgrade; server version is not allowed to join when downgrade is enabled", + }, + { + "Failed when downgrade is enabled and local version is out of range and cluster version is nil", + nil, + downgradeEnabledHigherVersion, + false, + "invalid downgrade; server version is not allowed to join when downgrade is enabled", + }, + + { + "Failed when downgrade is enabled and local version is out of range", + lv, + downgradeEnabledHigherVersion, + false, + "invalid downgrade; server version is not allowed to join when downgrade is enabled", + }, + } + + if os.Getenv("DETECT_DOWNGRADE") != "" { + i := os.Getenv("DETECT_DOWNGRADE") + iint, _ := strconv.Atoi(i) + logPath := filepath.Join(os.TempDir(), fmt.Sprintf("test-log-must-detect-downgrade-%v", iint)) + + lcfg := zap.NewProductionConfig() + lcfg.OutputPaths = []string{logPath} + lcfg.ErrorOutputPaths = []string{logPath} + lg, _ := lcfg.Build() + + mustDetectDowngrade(lg, tests[iint].clusterVersion, tests[iint].downgrade) + return + } + + for i, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + logPath := filepath.Join(os.TempDir(), fmt.Sprintf("test-log-must-detect-downgrade-%d", i)) + t.Log(logPath) + defer os.RemoveAll(logPath) + + cmd := exec.Command(os.Args[0], "-test.run=TestMustDetectDowngrade") + cmd.Env = append(os.Environ(), fmt.Sprintf("DETECT_DOWNGRADE=%d", i)) + if err := cmd.Start(); err != nil { + t.Fatal(err) + } + + errCmd := cmd.Wait() + + data, err := ioutil.ReadFile(logPath) + if err == nil { + if !bytes.Contains(data, []byte(tt.message)) { + t.Errorf("Expected to find %v in log", tt.message) + } + } else { + t.Fatal(err) + } + + if !tt.success { + e, ok := errCmd.(*exec.ExitError) + if !ok || e.Success() { + t.Errorf("Expected exit with status 1; Got %v", err) + } + } + + if tt.success && errCmd != nil { + t.Errorf("Expected not failure; Got %v", errCmd) + } + }) + } +} + +func TestIsValidDowngrade(t *testing.T) { + tests := []struct { + name string + verFrom string + verTo string + result bool + }{ + { + "Valid downgrade", + "3.5.0", + "3.4.0", + true, + }, + { + "Invalid downgrade", + "3.5.2", + "3.3.0", + false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + res := isValidDowngrade( + semver.Must(semver.NewVersion(tt.verFrom)), semver.Must(semver.NewVersion(tt.verTo))) + if res != tt.result { + t.Errorf("Expected downgrade valid is %v; Got %v", tt.result, res) + } + }) + } +} diff --git a/etcdserver/cluster_util.go b/etcdserver/cluster_util.go index 7990a31d58c..fc09e1a398f 100644 --- a/etcdserver/cluster_util.go +++ b/etcdserver/cluster_util.go @@ -198,20 +198,31 @@ func decideClusterVersion(lg *zap.Logger, vers map[string]*version.Versions) *se return cv } +// allowedVersionRange decides the available version range of the cluster that local server can join in; +// if the downgrade enabled status is true, the version window is [oneMinorHigher, oneMinorHigher] +// if the downgrade is not enabled, the version window is [MinClusterVersion, localVersion] +func allowedVersionRange(downgradeEnabled bool) (minV *semver.Version, maxV *semver.Version) { + minV = semver.Must(semver.NewVersion(version.MinClusterVersion)) + maxV = semver.Must(semver.NewVersion(version.Version)) + maxV = &semver.Version{Major: maxV.Major, Minor: maxV.Minor} + + if downgradeEnabled { + // Todo: handle the case that downgrading from higher major version(e.g. downgrade from v4.0 to v3.x) + maxV.Minor = maxV.Minor + 1 + minV = &semver.Version{Major: maxV.Major, Minor: maxV.Minor} + } + return minV, maxV +} + // isCompatibleWithCluster return true if the local member has a compatible version with // the current running cluster. // The version is considered as compatible when at least one of the other members in the cluster has a -// cluster version in the range of [MinClusterVersion, Version] and no known members has a cluster version +// cluster version in the range of [MinV, MaxV] and no known members has a cluster version // out of the range. // We set this rule since when the local member joins, another member might be offline. func isCompatibleWithCluster(lg *zap.Logger, cl *membership.RaftCluster, local types.ID, rt http.RoundTripper) bool { vers := getVersions(lg, cl, local, rt) - minV := semver.Must(semver.NewVersion(version.MinClusterVersion)) - maxV := semver.Must(semver.NewVersion(version.Version)) - maxV = &semver.Version{ - Major: maxV.Major, - Minor: maxV.Minor, - } + minV, maxV := allowedVersionRange(getDowngradeEnabledFromRemotePeers(lg, cl, local, rt)) return isCompatibleWithVers(lg, vers, local, minV, maxV) } @@ -356,6 +367,11 @@ func promoteMemberHTTP(ctx context.Context, url string, id uint64, peerRt http.R return membs, nil } +// getDowngradeEnabledFromRemotePeers will get the downgrade enabled status of the cluster. +func getDowngradeEnabledFromRemotePeers(lg *zap.Logger, cl *membership.RaftCluster, local types.ID, rt http.RoundTripper) bool { + return false +} + func convertToClusterVersion(v string) (*semver.Version, error) { ver, err := semver.NewVersion(v) if err != nil { @@ -369,8 +385,3 @@ func convertToClusterVersion(v string) (*semver.Version, error) { ver = &semver.Version{Major: ver.Major, Minor: ver.Minor} return ver, nil } - -// Todo: handle the case that downgrading from higher major version(e.g. downgrade from v4.0 to v3.x) -func allowedDowngradeVersion(ver *semver.Version) *semver.Version { - return &semver.Version{Major: ver.Major, Minor: ver.Minor - 1} -} diff --git a/etcdserver/cluster_util_test.go b/etcdserver/cluster_util_test.go index 04b9925ff95..29ba9f90dc8 100644 --- a/etcdserver/cluster_util_test.go +++ b/etcdserver/cluster_util_test.go @@ -176,3 +176,42 @@ func TestConvertToClusterVersion(t *testing.T) { }) } } + +func TestDecideAllowedVersionRange(t *testing.T) { + minClusterV := semver.Must(semver.NewVersion(version.MinClusterVersion)) + localV := semver.Must(semver.NewVersion(version.Version)) + localV = &semver.Version{Major: localV.Major, Minor: localV.Minor} + + tests := []struct { + name string + downgradeEnabled bool + expectedMinV *semver.Version + expectedMaxV *semver.Version + }{ + { + "When cluster enables downgrade", + true, + &semver.Version{Major: localV.Major, Minor: localV.Minor + 1}, + &semver.Version{Major: localV.Major, Minor: localV.Minor + 1}, + }, + { + "When cluster disables downgrade", + false, + minClusterV, + localV, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + minV, maxV := allowedVersionRange(tt.downgradeEnabled) + if !minV.Equal(*tt.expectedMinV) { + t.Errorf("Expected minV is %v; Got %v", tt.expectedMinV.String(), minV.String()) + } + + if !maxV.Equal(*tt.expectedMaxV) { + t.Errorf("Expected maxV is %v; Got %v", tt.expectedMaxV.String(), maxV.String()) + } + }) + } +} diff --git a/etcdserver/server.go b/etcdserver/server.go index e3192a360fa..40f556dd0b9 100644 --- a/etcdserver/server.go +++ b/etcdserver/server.go @@ -2272,9 +2272,7 @@ func (s *EtcdServer) monitorVersions() { continue } - // update cluster version only if the decided version is greater than - // the current cluster version - if v != nil && s.cluster.Version().LessThan(*v) { + if v != nil && membership.IsValidVersionChange(s.cluster.Version(), v) { s.goAttach(func() { s.updateClusterVersion(v.String()) }) } } diff --git a/etcdserver/v3_server.go b/etcdserver/v3_server.go index 92d8fb5348e..b580f4334cb 100644 --- a/etcdserver/v3_server.go +++ b/etcdserver/v3_server.go @@ -840,7 +840,7 @@ func (s *EtcdServer) downgradeValidate(ctx context.Context, v string) (*pb.Downg } resp.Version = cv.String() - allowedTargetVersion := allowedDowngradeVersion(cv) + allowedTargetVersion := membership.AllowedDowngradeVersion(cv) if !targetVersion.Equal(*allowedTargetVersion) { return nil, ErrInvalidDowngradeTargetVersion }