[POC] store v2 deprecation

etcdutl/etcdutl/backup_command.go

@@ -15,6 +15,7 @@
 package etcdutl
 
 import (
+	"fmt"
 	"os"
 	"path"
 	"regexp"
@@ -28,9 +29,9 @@ import (
 	"go.etcd.io/etcd/client/pkg/v3/types"
 	"go.etcd.io/etcd/pkg/v3/idutil"
 	"go.etcd.io/etcd/pkg/v3/pbutil"
+	"go.etcd.io/etcd/server/v3/etcdserver"
 	"go.etcd.io/etcd/server/v3/etcdserver/api/membership"
 	"go.etcd.io/etcd/server/v3/etcdserver/api/snap"
-	"go.etcd.io/etcd/server/v3/etcdserver/api/v2store"
 	"go.etcd.io/etcd/server/v3/storage/backend"
 	"go.etcd.io/etcd/server/v3/storage/datadir"
 	"go.etcd.io/etcd/server/v3/storage/schema"
@@ -178,21 +179,11 @@ func saveSnap(lg *zap.Logger, destSnap, srcSnap string, desired *desiredCluster)
 // mustTranslateV2store processes storeData such that they match 'desiredCluster'.
 // In particular the method overrides membership information.
 func mustTranslateV2store(lg *zap.Logger, storeData []byte, desired *desiredCluster) []byte {
-	st := v2store.New()
-	if err := st.Recovery(storeData); err != nil {
-		lg.Panic("cannot translate v2store", zap.Error(err))
-	}
-
 	raftCluster := membership.NewClusterFromMembers(lg, desired.clusterId, desired.members)
 	raftCluster.SetID(desired.nodeId, desired.clusterId)
-	raftCluster.SetStore(st)
-	raftCluster.PushMembershipToStorage()
-
-	outputData, err := st.Save()
-	if err != nil {
-		lg.Panic("cannot save v2store", zap.Error(err))
-	}
-	return outputData
+	d := etcdserver.GetMembershipInfoInV2Format(lg, raftCluster)
+	fmt.Printf("storeData = %v d = %v\n", storeData, d)
+	return d
 }
 
 func translateWAL(lg *zap.Logger, srcWAL string, walsnap walpb.Snapshot) (etcdserverpb.Metadata, raftpb.HardState, []raftpb.Entry) {

etcdutl/snapshot/v3_snapshot.go

@@ -38,7 +38,6 @@ import (
 	"go.etcd.io/etcd/server/v3/etcdserver"
 	"go.etcd.io/etcd/server/v3/etcdserver/api/membership"
 	"go.etcd.io/etcd/server/v3/etcdserver/api/snap"
-	"go.etcd.io/etcd/server/v3/etcdserver/api/v2store"
 	"go.etcd.io/etcd/server/v3/etcdserver/cindex"
 	"go.etcd.io/etcd/server/v3/storage/backend"
 	"go.etcd.io/etcd/server/v3/storage/schema"
@@ -396,8 +395,6 @@ func (s *v3Manager) saveWALAndSnap() (*raftpb.HardState, error) {
 	}
 
 	// add members again to persist them to the store we create.
-	st := v2store.New(etcdserver.StoreClusterPrefix, etcdserver.StoreKeysPrefix)
-	s.cl.SetStore(st)
 	be := backend.NewDefaultBackend(s.lg, s.outDbPath())
 	defer be.Close()
 	s.cl.SetBackend(schema.NewMembershipBackend(s.lg, be))
@@ -457,15 +454,11 @@ func (s *v3Manager) saveWALAndSnap() (*raftpb.HardState, error) {
 		return nil, err
 	}
 
-	b, berr := st.Save()
-	if berr != nil {
-		return nil, berr
-	}
 	confState := raftpb.ConfState{
 		Voters: nodeIDs,
 	}
 	raftSnap := raftpb.Snapshot{
-		Data: b,
+		Data: etcdserver.GetMembershipInfoInV2Format(s.lg, s.cl),
 		Metadata: raftpb.SnapshotMetadata{
 			Index:     commit,
 			Term:      term,

server/config/v2_deprecation.go

@@ -24,10 +24,10 @@ const (
 	V2_DEPR_1_WRITE_ONLY = V2DeprecationEnum("write-only")
 	// V2store is WIPED if found !!!
 	V2_DEPR_1_WRITE_ONLY_DROP = V2DeprecationEnum("write-only-drop-data")
-	// V2store is neither written nor read. Usage of this configuration is blocking
-	// ability to rollback to etcd v3.5.
+	// V2store is neither written nor read for 3.6. v2snapshot is published only for backward compatibility
 	V2_DEPR_2_GONE = V2DeprecationEnum("gone")
 
+	//TODO geetasg does thie default need to change
 	V2_DEPR_DEFAULT = V2_DEPR_1_WRITE_ONLY
 )
 

server/etcdserver/api/membership/cluster.go

@@ -46,8 +46,7 @@ type RaftCluster struct {
 	localID types.ID
 	cid     types.ID
 
-	v2store v2store.Store
-	be      MembershipBackend
+	be MembershipBackend
 
 	sync.Mutex // guards the fields below
 	version    *semver.Version
@@ -245,8 +244,6 @@ func (c *RaftCluster) SetID(localID, cid types.ID) {
 	c.buildMembershipMetric()
 }
 
-func (c *RaftCluster) SetStore(st v2store.Store) { c.v2store = st }
-
 func (c *RaftCluster) SetBackend(be MembershipBackend) {
 	c.be = be
 	c.be.MustCreateBackendBuckets()
@@ -260,13 +257,8 @@ func (c *RaftCluster) Recover(onSet func(*zap.Logger, *semver.Version)) {
 	c.Lock()
 	defer c.Unlock()
 
-	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.version = c.be.ClusterVersionFromBackend()
+	c.members, c.removed = c.be.MustReadMembersFromBackend()
 	c.buildMembershipMetric()
 
 	if c.be != nil {
@@ -303,9 +295,16 @@ 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 {
+
+	var membersMap map[types.ID]*Member
+	var removedMap map[types.ID]bool
+	membersMap, removedMap = c.be.MustReadMembersFromBackend()
+	if !shouldApplyV3 {
+		//TODO geetasg - RaftCluster purely based on backend cannot validate an entry older than ci
+		return nil
+	}
+
 	id := types.ID(cc.NodeID)
 	if removedMap[id] {
 		return ErrIDRemoved
@@ -390,10 +389,7 @@ 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 {
+	if shouldApplyV3 {
 		c.be.MustSaveMemberToBackend(m)
 	}
 
@@ -415,10 +411,7 @@ 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 {
+	if shouldApplyV3 {
 		c.be.MustDeleteMemberFromBackend(id)
 	}
 
@@ -452,10 +445,7 @@ 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 {
+		if shouldApplyV3 {
 			c.be.MustSaveMemberToBackend(m)
 		}
 		return
@@ -486,10 +476,7 @@ func (c *RaftCluster) PromoteMember(id types.ID, shouldApplyV3 ShouldApplyV3) {
 
 	c.members[id].RaftAttributes.IsLearner = false
 	c.updateMembershipMetric(id, true)
-	if c.v2store != nil {
-		mustUpdateMemberInStore(c.lg, c.v2store, c.members[id])
-	}
-	if c.be != nil && shouldApplyV3 {
+	if shouldApplyV3 {
 		c.be.MustSaveMemberToBackend(c.members[id])
 	}
 
@@ -505,10 +492,7 @@ func (c *RaftCluster) UpdateRaftAttributes(id types.ID, raftAttr RaftAttributes,
 	defer c.Unlock()
 
 	c.members[id].RaftAttributes = raftAttr
-	if c.v2store != nil {
-		mustUpdateMemberInStore(c.lg, c.v2store, c.members[id])
-	}
-	if c.be != nil && shouldApplyV3 {
+	if shouldApplyV3 {
 		c.be.MustSaveMemberToBackend(c.members[id])
 	}
 
@@ -554,10 +538,7 @@ 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 {
+	if shouldApplyV3 {
 		c.be.MustSaveClusterVersionToBackend(ver)
 	}
 	if oldVer != nil {
@@ -791,17 +772,9 @@ func (c *RaftCluster) VotingMemberIDs() []types.ID {
 // PushMembershipToStorage is overriding storage information about cluster's
 // members, such that they fully reflect internal RaftCluster's storage.
 func (c *RaftCluster) PushMembershipToStorage() {
-	if c.be != nil {
-		c.be.TrimMembershipFromBackend()
-		for _, m := range c.members {
-			c.be.MustSaveMemberToBackend(m)
-		}
-	}
-	if c.v2store != nil {
-		TrimMembershipFromV2Store(c.lg, c.v2store)
-		for _, m := range c.members {
-			mustSaveMemberToStore(c.lg, c.v2store, m)
-		}
+	c.be.TrimMembershipFromBackend()
+	for _, m := range c.members {
+		c.be.MustSaveMemberToBackend(m)
 	}
 }
 
@@ -854,3 +827,31 @@ func ValidateMaxLearnerConfig(maxLearners int, members []*Member, scaleUpLearner
 
 	return nil
 }
+
+func (c *RaftCluster) Store(store v2store.Store) {
+	c.Lock()
+	defer c.Unlock()
+	for _, m := range c.members {
+		mustSaveMemberToStore(c.lg, store, m)
+		if m.ClientURLs != nil {
+			mustUpdateMemberAttrInStore(c.lg, store, m)
+		}
+		c.lg.Info(
+			"snapshot storing member",
+			zap.String("id", m.ID.String()),
+			zap.Strings("peer-urls", m.PeerURLs),
+			zap.Bool("is-learner", m.IsLearner),
+		)
+	}
+	for id, _ := range c.removed {
+		mustDeleteMemberFromStore(c.lg, store, id)
+	}
+	if c.version != nil {
+		mustSaveClusterVersionToStore(c.lg, store, c.version)
+	}
+}
+
+func (c *RaftCluster) RecoverMembersFromStore(st v2store.Store) {
+	c.members, c.removed = membersFromStore(c.lg, st)
+	c.buildMembershipMetric()
+}

server/etcdserver/api/membership/cluster_test.go

@@ -277,7 +277,9 @@ func TestClusterValidateAndAssignIDs(t *testing.T) {
 
 func TestClusterValidateConfigurationChange(t *testing.T) {
 	cl := NewCluster(zaptest.NewLogger(t), WithMaxLearners(1))
-	cl.SetStore(v2store.New())
+	be := newMembershipBackend()
+	cl.SetBackend(be)
+
 	for i := 1; i <= 4; i++ {
 		var isLearner bool
 		if i == 1 {
@@ -454,7 +456,7 @@ func TestClusterValidateConfigurationChange(t *testing.T) {
 		},
 	}
 	for i, tt := range tests {
-		err := cl.ValidateConfigurationChange(tt.cc)
+		err := cl.ValidateConfigurationChange(tt.cc, ApplyBoth)
 		if err != tt.werr {
 			t.Errorf("#%d: validateConfigurationChange error = %v, want %v", i, err, tt.werr)
 		}
@@ -473,7 +475,6 @@ func TestClusterGenID(t *testing.T) {
 	}
 	previd := cs.ID()
 
-	cs.SetStore(mockstore.NewNop())
 	cs.AddMember(newTestMember(3, nil, "", nil), true)
 	cs.genID()
 	if cs.ID() == previd {
@@ -516,8 +517,8 @@ 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)
+	c.Store(st)
 
 	wactions := []testutil.Action{
 		{
@@ -539,8 +540,8 @@ func TestClusterAddMember(t *testing.T) {
 func TestClusterAddMemberAsLearner(t *testing.T) {
 	st := mockstore.NewRecorder()
 	c := newTestCluster(t, nil)
-	c.SetStore(st)
 	c.AddMember(newTestMemberAsLearner(1, nil, "node1", nil), true)
+	c.Store(st)
 
 	wactions := []testutil.Action{
 		{
@@ -582,8 +583,8 @@ func TestClusterMembers(t *testing.T) {
 func TestClusterRemoveMember(t *testing.T) {
 	st := mockstore.NewRecorder()
 	c := newTestCluster(t, nil)
-	c.SetStore(st)
 	c.RemoveMember(1, true)
+	c.Store(st)
 
 	wactions := []testutil.Action{
 		{Name: "Delete", Params: []interface{}{MemberStoreKey(1), true, true}},
@@ -647,8 +648,10 @@ func TestNodeToMember(t *testing.T) {
 
 func newTestCluster(t testing.TB, membs []*Member) *RaftCluster {
 	c := &RaftCluster{lg: zaptest.NewLogger(t), members: make(map[types.ID]*Member), removed: make(map[types.ID]bool)}
+	be := newMembershipBackend()
+	c.SetBackend(be)
 	for _, m := range membs {
-		c.members[m.ID] = m
+		c.AddMember(m, true)
 	}
 	return c
 }
@@ -975,3 +978,48 @@ func TestIsReadyToPromoteMember(t *testing.T) {
 		}
 	}
 }
+
+/*
+covered by TestV2DeprecationSnapshotMatches
+// TestMembershipStore tests code path used by snapshot
+func TestMembershipStore(t *testing.T) {
+	name := "etcd"
+	clientURLs := []string{"http://127.0.0.1:4001"}
+	tests := []struct {
+		mems    []*Member
+		removed map[types.ID]bool
+	}{
+		// update attributes of existing member
+		{
+			[]*Member{
+				newTestMember(2, nil, name, clientURLs),
+			},
+			map[types.ID]bool{types.ID(1): true},
+		},
+	}
+	for i, tt := range tests {
+		c := newTestCluster(t, tt.mems)
+		for id, _ := range tt.removed {
+			c.RemoveMember(id, true)
+		}
+
+		//snapshot
+		st := v2store.New("/0", "/1")
+		c.Store(st)
+		d, _ := st.SaveNoCopy()
+
+		//3.5 recover from snapshot
+		rst := v2store.New("/0", "/1")
+		rst.Recovery(d)
+		rc := &RaftCluster{lg: zaptest.NewLogger(t), members: make(map[types.ID]*Member), removed: make(map[types.ID]bool)}
+		rc.SetBackend(c.be)
+		//rc.SetStore(rst)
+		rc.Recover(func(lg *zap.Logger, v *semver.Version) { return })
+
+		//membership should match
+		if g := rc.Members(); !reflect.DeepEqual(g, tt.mems) {
+			t.Errorf("#%d: members = %+v, want %+v", i, g, tt.mems)
+		}
+	}
+}
+*/