From 6af67197a779f431e9e3e056cae0a3a8e58f10f4 Mon Sep 17 00:00:00 2001 From: Todd Treece <360020+toddtreece@users.noreply.github.com> Date: Fri, 7 Feb 2025 09:17:53 -0500 Subject: [PATCH] Dual Writer: Remove list comparisons in mode 2 & 3 (#100215) --- pkg/apiserver/rest/dualwriter_mode2.go | 45 ------------------ pkg/apiserver/rest/dualwriter_mode3.go | 28 ----------- pkg/apiserver/rest/dualwriter_mode3_test.go | 52 --------------------- 3 files changed, 125 deletions(-) diff --git a/pkg/apiserver/rest/dualwriter_mode2.go b/pkg/apiserver/rest/dualwriter_mode2.go index 105f450da4d..cc064ff4d5c 100644 --- a/pkg/apiserver/rest/dualwriter_mode2.go +++ b/pkg/apiserver/rest/dualwriter_mode2.go @@ -156,51 +156,6 @@ func (d *DualWriterMode2) List(ctx context.Context, options *metainternalversion return ll, err } d.recordLegacyDuration(false, mode2Str, d.resource, method, startLegacy) - - legacyList, err := meta.ExtractList(ll) - if err != nil { - log.Error(err, "unable to extract list from legacy storage") - return nil, err - } - - // Record the index of each LegacyStorage object so it can later be replaced by - // an equivalent Storage object if it exists. - legacyNames, err := parseList(legacyList) - if err != nil { - return nil, err - } - - startStorage := time.Now() - sl, err := d.Storage.List(ctx, options) - if err != nil { - log.Error(err, "unable to list objects from storage") - d.recordStorageDuration(true, mode2Str, d.resource, method, startStorage) - return sl, err - } - d.recordStorageDuration(false, mode2Str, d.resource, method, startStorage) - - storageList, err := meta.ExtractList(sl) - if err != nil { - log.Error(err, "unable to extract list from storage") - return nil, err - } - - for _, obj := range storageList { - name := getName(obj) - if i, ok := legacyNames[name]; ok { - areEqual := Compare(obj, legacyList[i]) - d.recordOutcome(mode2Str, name, areEqual, method) - if !areEqual { - log.WithValues("name", name).Info("object from legacy and storage are not equal") - } - } - } - - if err = meta.SetList(ll, legacyList); err != nil { - return nil, err - } - - // always return the list from legacy storage return ll, nil } diff --git a/pkg/apiserver/rest/dualwriter_mode3.go b/pkg/apiserver/rest/dualwriter_mode3.go index 1e7358c62e2..8b68c748769 100644 --- a/pkg/apiserver/rest/dualwriter_mode3.go +++ b/pkg/apiserver/rest/dualwriter_mode3.go @@ -162,37 +162,9 @@ func (d *DualWriterMode3) List(ctx context.Context, options *metainternalversion if err != nil { log.Error(err, "unable to list object in storage") } - - //nolint:errcheck - go d.listFromLegacyStorage(ctx, options, objFromStorage) - return objFromStorage, err } -func (d *DualWriterMode3) listFromLegacyStorage(ctx context.Context, options *metainternalversion.ListOptions, objFromStorage runtime.Object) error { - var method = "list" - log := d.Log.WithValues("resourceVersion", options.ResourceVersion, "method", method) - startLegacy := time.Now() - - ctx, cancel := context.WithTimeoutCause(context.WithoutCancel(ctx), time.Second*10, errors.New("legacy list timeout")) - defer cancel() - - objFromLegacy, err := d.Legacy.List(ctx, options) - d.recordLegacyDuration(err != nil, mode3Str, d.resource, method, startLegacy) - if err != nil { - log.Error(err, "unable to list object in legacy storage") - cancel() - } - - areEqual := Compare(objFromStorage, objFromLegacy) - d.recordOutcome(mode3Str, getName(objFromStorage), areEqual, method) - if !areEqual { - log.WithValues("name", getName(objFromStorage)).Info("object from legacy and storage are not equal") - } - - return err -} - func (d *DualWriterMode3) Delete(ctx context.Context, name string, deleteValidation rest.ValidateObjectFunc, options *metav1.DeleteOptions) (runtime.Object, bool, error) { var method = "delete" log := d.Log.WithValues("name", name, "method", method) diff --git a/pkg/apiserver/rest/dualwriter_mode3_test.go b/pkg/apiserver/rest/dualwriter_mode3_test.go index ee1bcb888dd..d5ddbe04a88 100644 --- a/pkg/apiserver/rest/dualwriter_mode3_test.go +++ b/pkg/apiserver/rest/dualwriter_mode3_test.go @@ -250,58 +250,6 @@ func TestMode3_List(t *testing.T) { } } -func TestMode1_ListFromLegacyStorage(t *testing.T) { - ctxCanceled, cancel := context.WithCancel(context.TODO()) - cancel() - - type testCase struct { - ctx *context.Context - name string - setupLegacyFn func(m *mock.Mock) - } - tests := - []testCase{ - { - name: "list from legacy storage", - setupLegacyFn: func(m *mock.Mock) { - m.On("List", mock.Anything, mock.Anything).Return(anotherList, nil) - }, - }, - { - name: "list from legacy storage works even if parent context is canceled", - ctx: &ctxCanceled, - setupLegacyFn: func(m *mock.Mock) { - m.On("List", mock.Anything, mock.Anything).Return(anotherList, nil) - }, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - l := (LegacyStorage)(nil) - s := (Storage)(nil) - m := &mock.Mock{} - - ls := legacyStoreMock{m, l} - us := storageMock{m, s} - - if tt.setupLegacyFn != nil { - tt.setupLegacyFn(m) - } - - ctx := context.TODO() - if tt.ctx != nil { - ctx = *tt.ctx - } - - dw := NewDualWriter(Mode3, ls, us, p, kind) - - err := dw.(*DualWriterMode3).listFromLegacyStorage(ctx, &metainternalversion.ListOptions{}, anotherList) - assert.NoError(t, err) - }) - } -} - func TestMode3_Delete(t *testing.T) { type testCase struct { setupLegacyFn func(m *mock.Mock, input string)