Skip to content

Commit

Permalink
etcdserver: separate function isValidDowngrade from mustDetectDowngra…
Browse files Browse the repository at this point in the history 
…de; remove new-added plog
  • Loading branch information
@YoyinZyc
YoyinZyc committed Nov 25, 2019
1 parent ddd44c8 commit d7b0c14
Show file tree
Hide file tree
Showing 7 changed files with 220 additions and 82 deletions.
80 changes: 38 additions & 42 deletions etcdserver/api/membership/cluster.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -258,10 +258,14 @@ func (c *RaftCluster) Recover(onSet func(*zap.Logger, *semver.Version)) {
defer c.Unlock()

c.members, c.removed = membersFromStore(c.lg, c.v2store)
// recover cluster version from backend
c.version = clusterVersionFromBackend(c.be)
if c.be != nil {
// recover cluster version from backend
c.version = clusterVersionFromBackend(c.be)
} else {
c.version = clusterVersionFromStore(c.lg, c.v2store)
}

c.downgradeInfo = downgradeFromBackend(c.be)
c.downgradeInfo = downgradeFromBackend(c.lg, c.be)
var d *DowngradeInfo
if c.downgradeInfo == nil {
d = &DowngradeInfo{Enabled: false}
Expand Down Expand Up @@ -811,57 +815,52 @@ func mustDetectDowngrade(lg *zap.Logger, cv *semver.Version, d *DowngradeInfo) {
// 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.Enabled && d.TargetVersion != nil {
oneMinorHigher := &semver.Version{Major: d.TargetVersion.Major, Minor: d.TargetVersion.Minor + 1}
if !lv.Equal(*d.TargetVersion) && !lv.Equal(*oneMinorHigher) {
if lg != nil {
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.String()),
)
} else {
plog.Fatalf("invalid downgrade; server version is not allowed to join when downgrade is enabled "+
"(current version: %s, target cluster version: %s).", version.Version, d.TargetVersion.String())
}
return
}
}

if cv != nil {
if d.Enabled && d.TargetVersion != nil {
if lv.Equal(*d.TargetVersion) || lv.Equal(*cv) {
if isValidDowngrade(d.TargetVersion, lv) {
if cv != nil {
if lg != nil {
lg.Info(
"cluster is downgrading to target version",
zap.String("target-cluster-version", d.TargetVersion.String()),
zap.String("determined-cluster-version", version.Cluster(cv.String())),
zap.String("current-server-version", version.Version),
)
} else {
plog.Infof("cluster is downgrading to target version: %s, "+
"determined cluster version: %s, current-server-version: %s).",
d.TargetVersion.String(), version.Cluster(cv.String()), version.Version)
}
return
}
return
}
if lg != nil {
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.String()),
)
}
}

if !d.Enabled && lv.LessThan(*cv) {
if lg != nil {
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())),
)
} else {
plog.Fatalf("invalid downgrade; server version is lower than determined cluster version (current version: %s, determined cluster version: %s).",
version.Version, version.Cluster(cv.String()))
}
// 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 lg != nil {
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())),
)
}
}
}

// isValidDowngrade checks whether joining local server is a valid downgrade when cluster enables downgrade.
// the validation passes when
// 1. local version is target version
// 2. local version is one minor version higher then target version
func isValidDowngrade(tv *semver.Version, lv *semver.Version) bool {
oneMinorHigher := &semver.Version{Major: tv.Major, Minor: tv.Minor + 1}
return lv.Equal(*tv) || lv.Equal(*oneMinorHigher)
}

// IsVersionChangable checks the two scenario when version is changable:
// 1. Downgrade: cluster version is 1 minor version higher than local version,
// cluster version should change.
Expand Down Expand Up @@ -934,7 +933,7 @@ func (c *RaftCluster) SetDowngradeInfo(d *DowngradeInfo) {
defer c.Unlock()

if c.be != nil {
mustSaveDowngradeToBackend(c.be, d)
mustSaveDowngradeToBackend(c.lg, c.be, d)
}

c.downgradeInfo = d
Expand All @@ -944,9 +943,6 @@ func (c *RaftCluster) SetDowngradeInfo(d *DowngradeInfo) {
c.lg.Info("The server is ready to downgrade",
zap.String("target-version", d.TargetVersion.String()),
zap.String("server-version", version.Version))
} else {
plog.Info("The server is ready to downgrade from current server version %v to target version %v",
version.Version, d.TargetVersion.String())
}
}
}
60 changes: 56 additions & 4 deletions etcdserver/api/membership/cluster_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1012,54 +1012,63 @@ func TestIsVersionChangable(t *testing.T) {
v4 := semver.Must(semver.NewVersion("3.6.0"))

tests := []struct {
name string
currentVersion *semver.Version
localVersion *semver.Version
expectedResult bool
}{
{
name: "Fail when local version is one major higher than cluster version",
currentVersion: v0,
localVersion: v1,
expectedResult: false,
},
{
name: "Fail when local version is equal to cluster version",
currentVersion: v1,
localVersion: v1,
expectedResult: false,
},
{
name: "Succeed when local version is one minor higher than cluster version",
currentVersion: v1,
localVersion: v2,
expectedResult: true,
},
{
name: "Succeed when local version is two minor higher than cluster version",
currentVersion: v1,
localVersion: v4,
expectedResult: true,
},
{
name: "Fail when local version is one patch higher than cluster version",
currentVersion: v2,
localVersion: v3,
expectedResult: false,
},
{
name: "Succeed when local version is one minor lower than cluster version",
currentVersion: v2,
localVersion: v1,
expectedResult: true,
},
{
name: "Succeed when local version is one minor and one patch lower than cluster version",
currentVersion: v3,
localVersion: v1,
expectedResult: true,
},
{
name: "Fail when local version is two minor lower than cluster version",
currentVersion: v4,
localVersion: v1,
expectedResult: false,
},
}

for i, tt := range tests {
t.Run(string(i), func(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if ret := IsVersionChangable(tt.currentVersion, tt.localVersion); ret != tt.expectedResult {
t.Errorf("Expected %v; Got %v", tt.expectedResult, ret)
}
Expand All @@ -1069,29 +1078,33 @@ func TestIsVersionChangable(t *testing.T) {

func TestGetDowngrade(t *testing.T) {
tests := []struct {
name string
cluster *RaftCluster
expectedEnabled bool
expectedTargetVersion *semver.Version
}{

{
"When downgradeInfo is empty",
&RaftCluster{},
false,
nil,
},
{
"When downgrade is disabled",
&RaftCluster{downgradeInfo: &DowngradeInfo{Enabled: false}},
false,
nil,
},
{
"When downgrade is enabled",
&RaftCluster{downgradeInfo: &DowngradeInfo{Enabled: true, TargetVersion: semver.Must(semver.NewVersion("3.4.0"))}},
true,
semver.Must(semver.NewVersion("3.4.0")),
},
}
for i, tt := range tests {
t.Run(string(i), func(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
d := tt.cluster.DowngradeInfo()
if d.Enabled != tt.expectedEnabled {
t.Errorf("Expected %v; Got %v", tt.expectedEnabled, d.Enabled)
Expand All @@ -1109,3 +1122,42 @@ func TestGetDowngrade(t *testing.T) {
})
}
}

func TestIsValidDowngrade(t *testing.T) {
lv := semver.Must(semver.NewVersion(version.Version))
lv = &semver.Version{Major: lv.Major, Minor: lv.Minor}
tv := &semver.Version{Major: lv.Major, Minor: lv.Minor - 1}
tests := []struct {
name string
targetVersion *semver.Version
localVersion *semver.Version
expectedResult bool
}{
{
name: "When local version is equal to target version",
targetVersion: tv,
localVersion: lv,
expectedResult: true,
},
{
name: "When local version is one minor version higher than target version",
targetVersion: tv,
localVersion: lv,
expectedResult: true,
},
{
name: "When local version is lower than target version",
targetVersion: tv,
localVersion: &semver.Version{Major: tv.Major, Minor: tv.Minor - 1},
expectedResult: false,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if ret := isValidDowngrade(tt.targetVersion, tt.localVersion); ret != tt.expectedResult {
t.Errorf("Expected %v; Got %v", tt.expectedResult, ret)
}
})
}
}
19 changes: 10 additions & 9 deletions etcdserver/api/membership/store.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"go.etcd.io/etcd/etcdserver/api/v2store"
"go.etcd.io/etcd/mvcc/backend"
"go.etcd.io/etcd/pkg/types"
"go.uber.org/zap"

"github.com/coreos/go-semver/semver"
)
Expand Down Expand Up @@ -75,20 +76,21 @@ func mustSaveClusterVersionToBackend(be backend.Backend, ver *semver.Version) {
tx.UnsafePut(clusterBucketName, ckey, []byte(ver.String()))
}

func mustSaveDowngradeToBackend(be backend.Backend, downgrade *DowngradeInfo) {
func mustSaveDowngradeToBackend(lg *zap.Logger, be backend.Backend, downgrade *DowngradeInfo) {
dkey := backendDowngradeKey()
dvalue, err := json.Marshal(downgrade)
if err != nil {
plog.Panicf("marshal raftAttributes should never fail: %v", err)
if lg != nil {
lg.Panic("failed to marshal downgrade information", zap.Error(err))
}
}

tx := be.BatchTx()
tx.Lock()
defer tx.Unlock()
tx.UnsafePut(clusterBucketName, dkey, dvalue)
}

func downgradeFromBackend(be backend.Backend) *DowngradeInfo {
func downgradeFromBackend(lg *zap.Logger, be backend.Backend) *DowngradeInfo {
dkey := backendDowngradeKey()
if be != nil {
tx := be.BatchTx()
Expand All @@ -99,7 +101,9 @@ func downgradeFromBackend(be backend.Backend) *DowngradeInfo {
if len(vs) != 0 {
var d DowngradeInfo
if err := json.Unmarshal(vs[0], &d); err != nil {
plog.Panicf("fail to unmarshal downgrade: %v", err)
if lg != nil {
lg.Panic("failed to unmarshal downgrade information", zap.Error(err))
}
}
return &d
}
Expand All @@ -115,12 +119,9 @@ func clusterVersionFromBackend(be backend.Backend) *semver.Version {
defer tx.Unlock()

_, vs := tx.UnsafeRange(clusterBucketName, ckey, nil, 0)

if len(vs) != 0 {
v := string(vs[0])
if sv, err := semver.NewVersion(v); err == nil {
return sv
}
return semver.Must(semver.NewVersion(v))
}
}
return nil
Expand Down
10 changes: 0 additions & 10 deletions etcdserver/cluster_util.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -204,8 +204,6 @@ func decideDowngradeEnabled(lg *zap.Logger, cl *membership.RaftCluster, local ty
if err != nil {
if lg != nil {
lg.Warn("failed to get downgrade enabled status", zap.String("remote-member-id", m.ID.String()), zap.Error(err))
} else {
plog.Warningf("cannot get the downgrade enabled status of member %s (%v)", m.ID, err)
}
} else {
// Since the "/downgrade/enabled" serves linearized data,
Expand Down Expand Up @@ -238,8 +236,6 @@ func getDowngradeEnabled(lg *zap.Logger, m *membership.Member, rt http.RoundTrip
zap.String("remote-member-id", m.ID.String()),
zap.Error(err),
)
} else {
plog.Warningf("failed to reach the peerURL(%s) of member %s (%v)", u, m.ID, err)
}
continue
}
Expand All @@ -254,8 +250,6 @@ func getDowngradeEnabled(lg *zap.Logger, m *membership.Member, rt http.RoundTrip
zap.String("remote-member-id", m.ID.String()),
zap.Error(err),
)
} else {
plog.Warningf("failed to read out the response body from the peerURL(%s) of member %s (%v)", u, m.ID, err)
}
continue
}
Expand All @@ -268,8 +262,6 @@ func getDowngradeEnabled(lg *zap.Logger, m *membership.Member, rt http.RoundTrip
zap.String("remote-member-id", m.ID.String()),
zap.Error(err),
)
} else {
plog.Warningf("failed to unmarshal the response body got from the peerURL(%s) of member %s (%v)", u, m.ID, err)
}
continue
}
Expand Down Expand Up @@ -341,8 +333,6 @@ func isDowngradeFinished(lg *zap.Logger, targetVersion *semver.Version, vers map
zap.String("remote-member-version", ver.Server),
zap.Error(err),
)
} else {
plog.Errorf("cannot understand the version of member %s (%v)", mid, err)
}
return false
}
Expand Down
Loading

0 comments on commit d7b0c14

Please sign in to comment.