From ed3375e076b7c363649f67b80c3b6220351d5fde Mon Sep 17 00:00:00 2001 From: Marek Siarkowicz Date: Sun, 19 Nov 2023 15:36:01 +0100 Subject: [PATCH] Remove v2 apply logic 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 --- server/etcdserver/apply_v2.go | 3 + server/etcdserver/server_test.go | 243 -------------------------- tests/e2e/v2store_deprecation_test.go | 28 +++ 3 files changed, 31 insertions(+), 243 deletions(-) diff --git a/server/etcdserver/apply_v2.go b/server/etcdserver/apply_v2.go index c9e4c3e87b0..d29e857d923 100644 --- a/server/etcdserver/apply_v2.go +++ b/server/etcdserver/apply_v2.go @@ -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) }, diff --git a/server/etcdserver/server_test.go b/server/etcdserver/server_test.go index 4ff2e6f8916..b967af0da31 100644 --- a/server/etcdserver/server_test.go +++ b/server/etcdserver/server_test.go @@ -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) { diff --git a/tests/e2e/v2store_deprecation_test.go b/tests/e2e/v2store_deprecation_test.go index da3b15668b0..99628cd54e3 100644 --- a/tests/e2e/v2store_deprecation_test.go +++ b/tests/e2e/v2store_deprecation_test.go @@ -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()