diff --git a/server/etcdserver/api/membership/cluster.go b/server/etcdserver/api/membership/cluster.go index f022d29c44e..87dc170dc70 100644 --- a/server/etcdserver/api/membership/cluster.go +++ b/server/etcdserver/api/membership/cluster.go @@ -263,9 +263,6 @@ func (c *RaftCluster) Recover(onSet func(*zap.Logger, *semver.Version)) { if c.be != nil { c.version = c.be.ClusterVersionFromBackend() c.members, c.removed = c.be.MustReadMembersFromBackend() - } else { - c.version = clusterVersionFromStore(c.lg, c.v2store) - c.members, c.removed = membersFromStore(c.lg, c.v2store) } c.buildMembershipMetric() @@ -303,9 +300,13 @@ func (c *RaftCluster) Recover(onSet func(*zap.Logger, *semver.Version)) { // ValidateConfigurationChange takes a proposed ConfChange and // ensures that it is still valid. -func (c *RaftCluster) ValidateConfigurationChange(cc raftpb.ConfChange) error { - // TODO: this must be switched to backend as well. - membersMap, removedMap := membersFromStore(c.lg, c.v2store) +func (c *RaftCluster) ValidateConfigurationChange(cc raftpb.ConfChange, shouldApplyV3 ShouldApplyV3) error { + // It makes no sense to validate a confChange if we don't apply it. + if !shouldApplyV3 { + return nil + } + membersMap, removedMap := c.be.MustReadMembersFromBackend() + id := types.ID(cc.NodeID) if removedMap[id] { return ErrIDRemoved @@ -390,9 +391,6 @@ func (c *RaftCluster) ValidateConfigurationChange(cc raftpb.ConfChange) error { func (c *RaftCluster) AddMember(m *Member, shouldApplyV3 ShouldApplyV3) { c.Lock() defer c.Unlock() - if c.v2store != nil { - mustSaveMemberToStore(c.lg, c.v2store, m) - } if c.be != nil && shouldApplyV3 { c.be.MustSaveMemberToBackend(m) @@ -423,9 +421,6 @@ func (c *RaftCluster) AddMember(m *Member, shouldApplyV3 ShouldApplyV3) { func (c *RaftCluster) RemoveMember(id types.ID, shouldApplyV3 ShouldApplyV3) { c.Lock() defer c.Unlock() - if c.v2store != nil { - mustDeleteMemberFromStore(c.lg, c.v2store, id) - } if c.be != nil && shouldApplyV3 { c.be.MustDeleteMemberFromBackend(id) @@ -467,9 +462,6 @@ func (c *RaftCluster) UpdateAttributes(id types.ID, attr Attributes, shouldApply if m, ok := c.members[id]; ok { m.Attributes = attr - if c.v2store != nil { - mustUpdateMemberAttrInStore(c.lg, c.v2store, m) - } if c.be != nil && shouldApplyV3 { c.be.MustSaveMemberToBackend(m) } @@ -499,12 +491,6 @@ func (c *RaftCluster) PromoteMember(id types.ID, shouldApplyV3 ShouldApplyV3) { c.Lock() defer c.Unlock() - if c.v2store != nil { - m := *(c.members[id]) - m.RaftAttributes.IsLearner = false - mustUpdateMemberInStore(c.lg, c.v2store, &m) - } - if c.be != nil && shouldApplyV3 { c.members[id].RaftAttributes.IsLearner = false c.updateMembershipMetric(id, true) @@ -528,11 +514,6 @@ func (c *RaftCluster) UpdateRaftAttributes(id types.ID, raftAttr RaftAttributes, c.Lock() defer c.Unlock() - if c.v2store != nil { - m := *(c.members[id]) - m.RaftAttributes = raftAttr - mustUpdateMemberInStore(c.lg, c.v2store, &m) - } if c.be != nil && shouldApplyV3 { c.members[id].RaftAttributes = raftAttr c.be.MustSaveMemberToBackend(c.members[id]) @@ -589,9 +570,6 @@ func (c *RaftCluster) SetVersion(ver *semver.Version, onSet func(*zap.Logger, *s c.version = ver sv := semver.Must(semver.NewVersion(version.Version)) serverversion.MustDetectDowngrade(c.lg, sv, c.version) - if c.v2store != nil { - mustSaveClusterVersionToStore(c.lg, c.v2store, ver) - } if c.be != nil && shouldApplyV3 { c.be.MustSaveClusterVersionToBackend(ver) } diff --git a/server/etcdserver/api/membership/cluster_test.go b/server/etcdserver/api/membership/cluster_test.go index b06d3302475..175ef1a5953 100644 --- a/server/etcdserver/api/membership/cluster_test.go +++ b/server/etcdserver/api/membership/cluster_test.go @@ -17,14 +17,12 @@ package membership import ( "encoding/json" "fmt" - "path" "reflect" "testing" "github.com/stretchr/testify/assert" "go.uber.org/zap/zaptest" - "go.etcd.io/etcd/client/pkg/v3/testutil" "go.etcd.io/etcd/client/pkg/v3/types" "go.etcd.io/etcd/server/v3/etcdserver/api/v2store" "go.etcd.io/etcd/server/v3/mock/mockstore" @@ -278,6 +276,8 @@ func TestClusterValidateAndAssignIDs(t *testing.T) { func TestClusterValidateConfigurationChange(t *testing.T) { cl := NewCluster(zaptest.NewLogger(t), WithMaxLearners(1)) + be := newMembershipBackend() + cl.SetBackend(be) cl.SetStore(v2store.New()) for i := 1; i <= 4; i++ { var isLearner bool @@ -455,7 +455,7 @@ func TestClusterValidateConfigurationChange(t *testing.T) { }, } for i, tt := range tests { - err := cl.ValidateConfigurationChange(tt.cc) + err := cl.ValidateConfigurationChange(tt.cc, true) if err != tt.werr { t.Errorf("#%d: validateConfigurationChange error = %v, want %v", i, err, tt.werr) } @@ -517,52 +517,6 @@ func TestNodeToMemberBad(t *testing.T) { } } -func TestClusterAddMember(t *testing.T) { - st := mockstore.NewRecorder() - c := newTestCluster(t, nil) - c.SetStore(st) - c.AddMember(newTestMember(1, nil, "node1", nil), true) - - wactions := []testutil.Action{ - { - Name: "Create", - Params: []any{ - path.Join(StoreMembersPrefix, "1", "raftAttributes"), - false, - `{"peerURLs":null}`, - false, - v2store.TTLOptionSet{ExpireTime: v2store.Permanent}, - }, - }, - } - if g := st.Action(); !reflect.DeepEqual(g, wactions) { - t.Errorf("actions = %v, want %v", g, wactions) - } -} - -func TestClusterAddMemberAsLearner(t *testing.T) { - st := mockstore.NewRecorder() - c := newTestCluster(t, nil) - c.SetStore(st) - c.AddMember(newTestMemberAsLearner(1, []string{}, "node1", []string{"http://node1"}), true) - - wactions := []testutil.Action{ - { - Name: "Create", - Params: []any{ - path.Join(StoreMembersPrefix, "1", "raftAttributes"), - false, - `{"peerURLs":[],"isLearner":true}`, - false, - v2store.TTLOptionSet{ExpireTime: v2store.Permanent}, - }, - }, - } - if g := st.Action(); !reflect.DeepEqual(g, wactions) { - t.Errorf("actions = %v, want %v", g, wactions) - } -} - func TestClusterMembers(t *testing.T) { cls := newTestCluster(t, []*Member{ {ID: 1}, @@ -583,21 +537,6 @@ func TestClusterMembers(t *testing.T) { } } -func TestClusterRemoveMember(t *testing.T) { - st := mockstore.NewRecorder() - c := newTestCluster(t, nil) - c.SetStore(st) - c.RemoveMember(1, true) - - wactions := []testutil.Action{ - {Name: "Delete", Params: []any{MemberStoreKey(1), true, true}}, - {Name: "Create", Params: []any{RemovedMemberStoreKey(1), false, "", false, v2store.TTLOptionSet{ExpireTime: v2store.Permanent}}}, - } - if !reflect.DeepEqual(st.Action(), wactions) { - t.Errorf("actions = %v, want %v", st.Action(), wactions) - } -} - func TestClusterUpdateAttributes(t *testing.T) { name := "etcd" clientURLs := []string{"http://127.0.0.1:4001"} @@ -903,7 +842,7 @@ func TestIsReadyToPromoteMember(t *testing.T) { // 1/1 members ready, should succeed (quorum = 1, new quorum = 2) []*Member{ newTestMember(1, nil, "1", nil), - newTestMemberAsLearner(2, nil, "2", nil), + newTestMemberAsLearner(2, []string{}, "2", []string{}), }, 2, true, diff --git a/server/etcdserver/api/membership/storev2.go b/server/etcdserver/api/membership/storev2.go index 4a1e702aa58..438ce766232 100644 --- a/server/etcdserver/api/membership/storev2.go +++ b/server/etcdserver/api/membership/storev2.go @@ -78,18 +78,6 @@ func mustSaveMemberToStore(lg *zap.Logger, s v2store.Store, m *Member) { } } -func mustDeleteMemberFromStore(lg *zap.Logger, s v2store.Store, id types.ID) { - if _, err := s.Delete(MemberStoreKey(id), true, true); err != nil { - lg.Panic( - "failed to delete member from store", - zap.String("path", MemberStoreKey(id)), - zap.Error(err), - ) - } - - mustAddToRemovedMembersInStore(lg, s, id) -} - func mustAddToRemovedMembersInStore(lg *zap.Logger, s v2store.Store, id types.ID) { if _, err := s.Create(RemovedMemberStoreKey(id), false, "", false, v2store.TTLOptionSet{ExpireTime: v2store.Permanent}); err != nil { @@ -101,21 +89,6 @@ func mustAddToRemovedMembersInStore(lg *zap.Logger, s v2store.Store, id types.ID } } -func mustUpdateMemberInStore(lg *zap.Logger, s v2store.Store, m *Member) { - b, err := json.Marshal(m.RaftAttributes) - if err != nil { - lg.Panic("failed to marshal raftAttributes", zap.Error(err)) - } - p := path.Join(MemberStoreKey(m.ID), raftAttributesSuffix) - if _, err := s.Update(p, string(b), v2store.TTLOptionSet{ExpireTime: v2store.Permanent}); err != nil { - lg.Panic( - "failed to update raftAttributes", - zap.String("path", p), - zap.Error(err), - ) - } -} - func mustUpdateMemberAttrInStore(lg *zap.Logger, s v2store.Store, m *Member) { b, err := json.Marshal(m.Attributes) if err != nil { @@ -184,18 +157,3 @@ func MemberStoreKey(id types.ID) string { func MemberAttributesStorePath(id types.ID) string { return path.Join(MemberStoreKey(id), attributesSuffix) } - -func clusterVersionFromStore(lg *zap.Logger, st v2store.Store) *semver.Version { - e, err := st.Get(path.Join(storePrefix, "version"), false, false) - if err != nil { - if isKeyNotFound(err) { - return nil - } - lg.Panic( - "failed to get cluster version from store", - zap.String("path", path.Join(storePrefix, "version")), - zap.Error(err), - ) - } - return semver.Must(semver.NewVersion(*e.Node.Value)) -} diff --git a/server/etcdserver/server.go b/server/etcdserver/server.go index 4ad4539c6e2..a4a8161c208 100644 --- a/server/etcdserver/server.go +++ b/server/etcdserver/server.go @@ -1935,7 +1935,7 @@ func removeNeedlessRangeReqs(txn *pb.TxnRequest) { // applyConfChange applies a ConfChange to the server. It is only // invoked with a ConfChange that has already passed through Raft func (s *EtcdServer) applyConfChange(cc raftpb.ConfChange, confState *raftpb.ConfState, shouldApplyV3 membership.ShouldApplyV3) (bool, error) { - if err := s.cluster.ValidateConfigurationChange(cc); err != nil { + if err := s.cluster.ValidateConfigurationChange(cc, shouldApplyV3); err != nil { cc.NodeID = raft.None s.r.ApplyConfChange(cc)