Skip to content

Commit

Permalink
Remove v2 apply logic
Browse files Browse the repository at this point in the history
v2 store is no longer available in v3.6.
We can remove apply logic for it as they will never be used.

Only v2 PUT is neeeded as it applies to v3 storage and etcd v3.5 uses it for setting member
attributes and cluster version.

Signed-off-by: Marek Siarkowicz <siarkowicz@google.com>
  • Loading branch information
serathius committed Nov 23, 2023
1 parent ccd711a commit ed3375e
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 243 deletions.
3 changes: 3 additions & 0 deletions server/etcdserver/apply_v2.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,9 @@ func (a *applierV2store) Sync(r *RequestV2) Response {
// applyV2Request interprets r as a call to v2store.X
// and returns a Response interpreted from v2store.Event
func (s *EtcdServer) applyV2Request(r *RequestV2, shouldApplyV3 membership.ShouldApplyV3) (resp Response) {
if r.Method != "PUT" || (!storeMemberAttributeRegexp.MatchString(r.Path) && r.Path != membership.StoreClusterVersionKey()) {
s.lg.Panic("detected disallowed v2 WAL for stage --v2-deprecation=write-only", zap.String("method", r.Method))
}
stringer := panicAlternativeStringer{
stringer: r,
alternative: func() string { return fmt.Sprintf("id:%d,method:%s,path:%s", r.ID, r.Method, r.Path) },
Expand Down
243 changes: 0 additions & 243 deletions server/etcdserver/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,249 +147,6 @@ func (uberApplierMock) Apply(r *pb.InternalRaftRequest, shouldApplyV3 membership
return &apply2.Result{}
}

func TestApplyRequest(t *testing.T) {
tests := []struct {
req pb.Request

wresp Response
wactions []testutil.Action
}{
// POST ==> Create
{
pb.Request{Method: "POST", ID: 1},
Response{Event: &v2store.Event{}},
[]testutil.Action{
{
Name: "Create",
Params: []any{"", false, "", true, v2store.TTLOptionSet{ExpireTime: time.Time{}}},
},
},
},
// POST ==> Create, with expiration
{
pb.Request{Method: "POST", ID: 1, Expiration: 1337},
Response{Event: &v2store.Event{}},
[]testutil.Action{
{
Name: "Create",
Params: []any{"", false, "", true, v2store.TTLOptionSet{ExpireTime: time.Unix(0, 1337)}},
},
},
},
// POST ==> Create, with dir
{
pb.Request{Method: "POST", ID: 1, Dir: true},
Response{Event: &v2store.Event{}},
[]testutil.Action{
{
Name: "Create",
Params: []any{"", true, "", true, v2store.TTLOptionSet{ExpireTime: time.Time{}}},
},
},
},
// PUT ==> Set
{
pb.Request{Method: "PUT", ID: 1},
Response{Event: &v2store.Event{}},
[]testutil.Action{
{
Name: "Set",
Params: []any{"", false, "", v2store.TTLOptionSet{ExpireTime: time.Time{}}},
},
},
},
// PUT ==> Set, with dir
{
pb.Request{Method: "PUT", ID: 1, Dir: true},
Response{Event: &v2store.Event{}},
[]testutil.Action{
{
Name: "Set",
Params: []any{"", true, "", v2store.TTLOptionSet{ExpireTime: time.Time{}}},
},
},
},
// PUT with PrevExist=true ==> Update
{
pb.Request{Method: "PUT", ID: 1, PrevExist: pbutil.Boolp(true)},
Response{Event: &v2store.Event{}},
[]testutil.Action{
{
Name: "Update",
Params: []any{"", "", v2store.TTLOptionSet{ExpireTime: time.Time{}}},
},
},
},
// PUT with PrevExist=false ==> Create
{
pb.Request{Method: "PUT", ID: 1, PrevExist: pbutil.Boolp(false)},
Response{Event: &v2store.Event{}},
[]testutil.Action{
{
Name: "Create",
Params: []any{"", false, "", false, v2store.TTLOptionSet{ExpireTime: time.Time{}}},
},
},
},
// PUT with PrevExist=true *and* PrevIndex set ==> CompareAndSwap
{
pb.Request{Method: "PUT", ID: 1, PrevExist: pbutil.Boolp(true), PrevIndex: 1},
Response{Event: &v2store.Event{}},
[]testutil.Action{
{
Name: "CompareAndSwap",
Params: []any{"", "", uint64(1), "", v2store.TTLOptionSet{ExpireTime: time.Time{}}},
},
},
},
// PUT with PrevExist=false *and* PrevIndex set ==> Create
{
pb.Request{Method: "PUT", ID: 1, PrevExist: pbutil.Boolp(false), PrevIndex: 1},
Response{Event: &v2store.Event{}},
[]testutil.Action{
{
Name: "Create",
Params: []any{"", false, "", false, v2store.TTLOptionSet{ExpireTime: time.Time{}}},
},
},
},
// PUT with PrevIndex set ==> CompareAndSwap
{
pb.Request{Method: "PUT", ID: 1, PrevIndex: 1},
Response{Event: &v2store.Event{}},
[]testutil.Action{
{
Name: "CompareAndSwap",
Params: []any{"", "", uint64(1), "", v2store.TTLOptionSet{ExpireTime: time.Time{}}},
},
},
},
// PUT with PrevValue set ==> CompareAndSwap
{
pb.Request{Method: "PUT", ID: 1, PrevValue: "bar"},
Response{Event: &v2store.Event{}},
[]testutil.Action{
{
Name: "CompareAndSwap",
Params: []any{"", "bar", uint64(0), "", v2store.TTLOptionSet{ExpireTime: time.Time{}}},
},
},
},
// PUT with PrevIndex and PrevValue set ==> CompareAndSwap
{
pb.Request{Method: "PUT", ID: 1, PrevIndex: 1, PrevValue: "bar"},
Response{Event: &v2store.Event{}},
[]testutil.Action{
{
Name: "CompareAndSwap",
Params: []any{"", "bar", uint64(1), "", v2store.TTLOptionSet{ExpireTime: time.Time{}}},
},
},
},
// DELETE ==> Delete
{
pb.Request{Method: "DELETE", ID: 1},
Response{Event: &v2store.Event{}},
[]testutil.Action{
{
Name: "Delete",
Params: []any{"", false, false},
},
},
},
// DELETE with PrevIndex set ==> CompareAndDelete
{
pb.Request{Method: "DELETE", ID: 1, PrevIndex: 1},
Response{Event: &v2store.Event{}},
[]testutil.Action{
{
Name: "CompareAndDelete",
Params: []any{"", "", uint64(1)},
},
},
},
// DELETE with PrevValue set ==> CompareAndDelete
{
pb.Request{Method: "DELETE", ID: 1, PrevValue: "bar"},
Response{Event: &v2store.Event{}},
[]testutil.Action{
{
Name: "CompareAndDelete",
Params: []any{"", "bar", uint64(0)},
},
},
},
// DELETE with PrevIndex *and* PrevValue set ==> CompareAndDelete
{
pb.Request{Method: "DELETE", ID: 1, PrevIndex: 5, PrevValue: "bar"},
Response{Event: &v2store.Event{}},
[]testutil.Action{
{
Name: "CompareAndDelete",
Params: []any{"", "bar", uint64(5)},
},
},
},
// QGET ==> Get
{
pb.Request{Method: "QGET", ID: 1},
Response{Event: &v2store.Event{}},
[]testutil.Action{
{
Name: "Get",
Params: []any{"", false, false},
},
},
},
// SYNC ==> DeleteExpiredKeys
{
pb.Request{Method: "SYNC", ID: 1},
Response{},
[]testutil.Action{
{
Name: "DeleteExpiredKeys",
Params: []any{time.Unix(0, 0)},
},
},
},
{
pb.Request{Method: "SYNC", ID: 1, Time: 12345},
Response{},
[]testutil.Action{
{
Name: "DeleteExpiredKeys",
Params: []any{time.Unix(0, 12345)},
},
},
},
// Unknown method - error
{
pb.Request{Method: "BADMETHOD", ID: 1},
Response{Err: errors.ErrUnknownMethod},
[]testutil.Action{},
},
}

for i, tt := range tests {
st := mockstore.NewRecorder()
srv := &EtcdServer{
lgMu: new(sync.RWMutex),
lg: zaptest.NewLogger(t),
v2store: st,
}
srv.applyV2 = &applierV2store{store: srv.v2store, cluster: srv.cluster}
resp := srv.applyV2Request((*RequestV2)(&tt.req), membership.ApplyBoth)

if !reflect.DeepEqual(resp, tt.wresp) {
t.Errorf("#%d: resp = %+v, want %+v", i, resp, tt.wresp)
}
gaction := st.Action()
if !reflect.DeepEqual(gaction, tt.wactions) {
t.Errorf("#%d: action = %#v, want %#v", i, gaction, tt.wactions)
}
}
}

// TestV2SetMemberAttributes validates support of hybrid v3.5 cluster which still uses v2 request.
// TODO: Remove in v3.7
func TestV2SetMemberAttributes(t *testing.T) {
Expand Down
28 changes: 28 additions & 0 deletions tests/e2e/v2store_deprecation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,34 @@ func TestV2DeprecationNotYet(t *testing.T) {
assert.NoError(t, err)
}

func TestV2DeprecationWriteOnlyWAL(t *testing.T) {
e2e.BeforeTest(t)
dataDirPath := t.TempDir()

if !fileutil.Exist(e2e.BinPath.EtcdLastRelease) {
t.Skipf("%q does not exist", e2e.BinPath.EtcdLastRelease)
}
cfg := e2e.ConfigStandalone(*e2e.NewConfig(
e2e.WithVersion(e2e.LastVersion),
e2e.WithEnableV2(true),
e2e.WithDataDirPath(dataDirPath),
))
epc, err := e2e.NewEtcdProcessCluster(context.TODO(), t, e2e.WithConfig(cfg))
assert.NoError(t, err)
memberDataDir := epc.Procs[0].Config().DataDirPath

writeCustomV2Data(t, epc, 1)

assert.NoError(t, epc.Stop())

t.Log("Verify its infeasible to start etcd with --v2-deprecation=write-only mode")
proc, err := e2e.SpawnCmd([]string{e2e.BinPath.Etcd, "--v2-deprecation=write-only", "--data-dir=" + memberDataDir}, nil)
assert.NoError(t, err)

_, err = proc.Expect("detected disallowed v2 WAL for stage --v2-deprecation=write-only")
assert.NoError(t, err)
}

func TestV2DeprecationWriteOnlySnapshot(t *testing.T) {
e2e.BeforeTest(t)
dataDirPath := t.TempDir()
Expand Down

0 comments on commit ed3375e

Please sign in to comment.