diff --git a/pkg/apiserver/rest/dualwriter.go b/pkg/apiserver/rest/dualwriter.go index 89541d6c18d..2bf3c59cfd3 100644 --- a/pkg/apiserver/rest/dualwriter.go +++ b/pkg/apiserver/rest/dualwriter.go @@ -111,12 +111,12 @@ func NewDualWriter( storage Storage, reg prometheus.Registerer, resource string, -) DualWriter { +) Storage { metrics := &dualWriterMetrics{} metrics.init(reg) switch mode { - // It is not possible to initialize a mode 0 dual writer. Mode 0 represents - // writing to legacy storage without Unified Storage enabled. + case Mode0: + return legacy case Mode1: // read and write only from legacy storage return newDualWriterMode1(legacy, storage, metrics, resource) @@ -127,8 +127,7 @@ func NewDualWriter( // write to both, read from storage only return newDualWriterMode3(legacy, storage, metrics, resource) case Mode4: - // read and write only from storage - return newDualWriterMode4(legacy, storage, metrics, resource) + return storage default: return newDualWriterMode1(legacy, storage, metrics, resource) } diff --git a/pkg/apiserver/rest/dualwriter_mode1.go b/pkg/apiserver/rest/dualwriter_mode1.go index 63883573100..2aa92ca61f5 100644 --- a/pkg/apiserver/rest/dualwriter_mode1.go +++ b/pkg/apiserver/rest/dualwriter_mode1.go @@ -9,7 +9,6 @@ import ( metainternalversion "k8s.io/apimachinery/pkg/apis/meta/internalversion" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/watch" "k8s.io/apiserver/pkg/registry/rest" "k8s.io/klog/v2" ) @@ -282,12 +281,6 @@ func (d *DualWriterMode1) Destroy() { d.Legacy.Destroy() } -func (d *DualWriterMode1) Watch(ctx context.Context, options *metainternalversion.ListOptions) (watch.Interface, error) { - err := errors.New("Watch not implemented in mode 1") - d.Log.Error(err, err.Error()) - return nil, err -} - func (d *DualWriterMode1) GetSingularName() string { return d.Legacy.GetSingularName() } diff --git a/pkg/apiserver/rest/dualwriter_mode2.go b/pkg/apiserver/rest/dualwriter_mode2.go index c1c70c40dc0..c1356c5de16 100644 --- a/pkg/apiserver/rest/dualwriter_mode2.go +++ b/pkg/apiserver/rest/dualwriter_mode2.go @@ -2,7 +2,6 @@ package rest import ( "context" - "errors" "fmt" "time" @@ -12,7 +11,6 @@ import ( metainternalversion "k8s.io/apimachinery/pkg/apis/meta/internalversion" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/watch" "k8s.io/apiserver/pkg/endpoints/request" "k8s.io/apiserver/pkg/registry/rest" "k8s.io/klog/v2" @@ -330,12 +328,6 @@ func (d *DualWriterMode2) Update(ctx context.Context, name string, objInfo rest. return res, created, err } -func (d *DualWriterMode2) Watch(ctx context.Context, options *metainternalversion.ListOptions) (watch.Interface, error) { - err := errors.New("Watch not implemented in mode 2") - d.Log.Error(err, err.Error()) - return nil, err -} - func (d *DualWriterMode2) Destroy() { d.Storage.Destroy() d.Legacy.Destroy() diff --git a/pkg/apiserver/rest/dualwriter_mode4.go b/pkg/apiserver/rest/dualwriter_mode4.go deleted file mode 100644 index 5779e96f022..00000000000 --- a/pkg/apiserver/rest/dualwriter_mode4.go +++ /dev/null @@ -1,161 +0,0 @@ -package rest - -import ( - "context" - "time" - - metainternalversion "k8s.io/apimachinery/pkg/apis/meta/internalversion" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" - "k8s.io/apimachinery/pkg/watch" - "k8s.io/apiserver/pkg/registry/rest" - "k8s.io/klog/v2" -) - -type DualWriterMode4 struct { - Legacy LegacyStorage - Storage Storage - watchImp rest.Watcher // watch is only available in mode 3 and 4 - *dualWriterMetrics - resource string - Log klog.Logger -} - -const mode4Str = "4" - -// newDualWriterMode4 returns a new DualWriter in mode 4. -// Mode 4 represents writing and reading from Storage. -func newDualWriterMode4(legacy LegacyStorage, storage Storage, dwm *dualWriterMetrics, resource string) *DualWriterMode4 { - return &DualWriterMode4{ - Legacy: legacy, - Storage: storage, - Log: klog.NewKlogr().WithName("DualWriterMode4").WithValues("mode", mode4Str, "resource", resource), - dualWriterMetrics: dwm, - resource: resource, - } -} - -// Mode returns the mode of the dual writer. -func (d *DualWriterMode4) Mode() DualWriterMode { - return Mode4 -} - -// #TODO remove all DualWriterMode4 methods once we remove the generic DualWriter implementation - -// Create overrides the behavior of the generic DualWriter and writes only to Storage. -func (d *DualWriterMode4) Create(ctx context.Context, obj runtime.Object, createValidation rest.ValidateObjectFunc, options *metav1.CreateOptions) (runtime.Object, error) { - var method = "create" - log := d.Log.WithValues("method", method) - ctx = klog.NewContext(ctx, log) - - startStorage := time.Now() - res, err := d.Storage.Create(ctx, obj, createValidation, options) - if err != nil { - log.Error(err, "unable to create object in storage") - } - d.recordStorageDuration(err != nil, mode4Str, d.resource, method, startStorage) - return res, err -} - -// Get overrides the behavior of the generic DualWriter and retrieves an object from Storage. -func (d *DualWriterMode4) Get(ctx context.Context, name string, options *metav1.GetOptions) (runtime.Object, error) { - var method = "get" - log := d.Log.WithValues("method", method) - ctx = klog.NewContext(ctx, log) - - startStorage := time.Now() - res, err := d.Storage.Get(ctx, name, options) - if err != nil { - log.Error(err, "unable to create object in storage") - } - d.recordStorageDuration(err != nil, mode4Str, d.resource, method, startStorage) - return res, err -} - -func (d *DualWriterMode4) 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) - ctx = klog.NewContext(ctx, log) - - startStorage := time.Now() - res, async, err := d.Storage.Delete(ctx, name, deleteValidation, options) - if err != nil { - log.Error(err, "unable to delete object in storage") - } - d.recordStorageDuration(err != nil, mode4Str, d.resource, method, startStorage) - return res, async, err -} - -// DeleteCollection overrides the behavior of the generic DualWriter and deletes only from Storage. -func (d *DualWriterMode4) DeleteCollection(ctx context.Context, deleteValidation rest.ValidateObjectFunc, options *metav1.DeleteOptions, listOptions *metainternalversion.ListOptions) (runtime.Object, error) { - var method = "delete-collection" - log := d.Log.WithValues("resourceVersion", listOptions.ResourceVersion, "method", method, "mode", mode4Str) - ctx = klog.NewContext(ctx, log) - - startStorage := time.Now() - res, err := d.Storage.DeleteCollection(ctx, deleteValidation, options, listOptions) - if err != nil { - log.Error(err, "unable to delete collection in storage") - } - d.recordStorageDuration(err != nil, mode4Str, d.resource, method, startStorage) - return res, err -} - -// Update overrides the generic behavior of the Storage and writes only to US. -func (d *DualWriterMode4) Update(ctx context.Context, name string, objInfo rest.UpdatedObjectInfo, createValidation rest.ValidateObjectFunc, updateValidation rest.ValidateObjectUpdateFunc, forceAllowCreate bool, options *metav1.UpdateOptions) (runtime.Object, bool, error) { - var method = "update" - log := d.Log.WithValues("name", name, "resource", d.resource, "method", method) - ctx = klog.NewContext(ctx, log) - - startStorage := time.Now() - res, async, err := d.Storage.Update(ctx, name, objInfo, createValidation, updateValidation, forceAllowCreate, options) - if err != nil { - log.Error(err, "unable to update object in storage") - } - d.recordStorageDuration(err != nil, mode4Str, d.resource, method, startStorage) - return res, async, err -} - -func (d *DualWriterMode4) List(ctx context.Context, options *metainternalversion.ListOptions) (runtime.Object, error) { - var method = "list" - log := d.Log.WithValues("resourceVersion", options.ResourceVersion, "method", method) - ctx = klog.NewContext(ctx, log) - - startStorage := time.Now() - res, err := d.Storage.List(ctx, options) - if err != nil { - log.Error(err, "unable to list objects in storage") - } - d.recordStorageDuration(err != nil, mode4Str, d.resource, method, startStorage) - return res, err -} - -func (d *DualWriterMode4) Watch(ctx context.Context, options *metainternalversion.ListOptions) (watch.Interface, error) { - var method = "watch" - d.Log.WithValues("method", method, "mode", mode4Str).Info("starting to watch") - return d.watchImp.Watch(ctx, options) -} - -func (d *DualWriterMode4) Destroy() { - d.Storage.Destroy() -} - -func (d *DualWriterMode4) GetSingularName() string { - return d.Storage.GetSingularName() -} - -func (d *DualWriterMode4) NamespaceScoped() bool { - return d.Storage.NamespaceScoped() -} - -func (d *DualWriterMode4) New() runtime.Object { - return d.Storage.New() -} - -func (d *DualWriterMode4) NewList() runtime.Object { - return d.Storage.NewList() -} - -func (d *DualWriterMode4) ConvertToTable(ctx context.Context, object runtime.Object, tableOptions runtime.Object) (*metav1.Table, error) { - return d.Storage.ConvertToTable(ctx, object, tableOptions) -} diff --git a/pkg/apiserver/rest/dualwriter_mode4_test.go b/pkg/apiserver/rest/dualwriter_mode4_test.go deleted file mode 100644 index 8a334239a4a..00000000000 --- a/pkg/apiserver/rest/dualwriter_mode4_test.go +++ /dev/null @@ -1,346 +0,0 @@ -package rest - -import ( - "context" - "errors" - "testing" - - "github.com/prometheus/client_golang/prometheus" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/mock" - "k8s.io/apimachinery/pkg/api/meta" - metainternalversion "k8s.io/apimachinery/pkg/apis/meta/internalversion" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" -) - -func TestMode4_Create(t *testing.T) { - type testCase struct { - input runtime.Object - setupStorageFn func(m *mock.Mock, input runtime.Object) - name string - wantErr bool - } - tests := - []testCase{ - { - name: "creating an object only in the unified store", - input: exampleObj, - setupStorageFn: func(m *mock.Mock, input runtime.Object) { - m.On("Create", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(exampleObj, nil) - }, - }, - { - name: "error when creating object in the unified store fails", - input: failingObj, - setupStorageFn: func(m *mock.Mock, input runtime.Object) { - m.On("Create", mock.Anything, input, mock.Anything, mock.Anything).Return(nil, errors.New("error")) - }, - wantErr: true, - }, - } - - 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.setupStorageFn != nil { - tt.setupStorageFn(m, tt.input) - } - - dw := NewDualWriter(Mode4, ls, us, p, kind) - - obj, err := dw.Create(context.Background(), tt.input, func(context.Context, runtime.Object) error { return nil }, &metav1.CreateOptions{}) - - if tt.wantErr { - assert.Error(t, err) - return - } - - acc, err := meta.Accessor(obj) - assert.NoError(t, err) - assert.Equal(t, acc.GetResourceVersion(), "") - assert.NotEqual(t, obj, anotherObj) - }) - } -} - -func TestMode4_Get(t *testing.T) { - type testCase struct { - setupStorageFn func(m *mock.Mock, name string) - name string - input string - wantErr bool - } - tests := - []testCase{ - { - name: "get an object only in unified store", - input: "foo", - setupStorageFn: func(m *mock.Mock, name string) { - m.On("Get", mock.Anything, name, mock.Anything).Return(exampleObj, nil) - }, - }, - { - name: "error when getting an object in the inified store fails", - input: "object-fail", - setupStorageFn: func(m *mock.Mock, name string) { - m.On("Get", mock.Anything, name, mock.Anything).Return(nil, errors.New("error")) - }, - wantErr: true, - }, - } - - 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.setupStorageFn != nil { - tt.setupStorageFn(m, tt.input) - } - - p := prometheus.NewRegistry() - dw := NewDualWriter(Mode4, ls, us, p, kind) - - obj, err := dw.Get(context.Background(), tt.input, &metav1.GetOptions{}) - - if tt.wantErr { - assert.Error(t, err) - return - } - - us.AssertNotCalled(t, "Get", context.Background(), tt.name, &metav1.GetOptions{}) - - assert.Equal(t, obj, exampleObj) - assert.NotEqual(t, obj, anotherObj) - }) - } -} - -func TestMode4_List(t *testing.T) { - type testCase struct { - setupStorageFn func(m *mock.Mock, options *metainternalversion.ListOptions) - name string - options *metainternalversion.ListOptions - wantErr bool - } - tests := - []testCase{ - { - name: "error when listing an object in the unified store is not implemented", - options: &metainternalversion.ListOptions{TypeMeta: metav1.TypeMeta{Kind: "fail"}}, - setupStorageFn: func(m *mock.Mock, options *metainternalversion.ListOptions) { - m.On("List", mock.Anything, options).Return(nil, errors.New("error")) - }, - wantErr: true, - }, - { - name: "list objects in the unified store", - options: &metainternalversion.ListOptions{TypeMeta: metav1.TypeMeta{Kind: "foo"}}, - setupStorageFn: func(m *mock.Mock, options *metainternalversion.ListOptions) { - m.On("List", mock.Anything, options).Return(exampleList, 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.setupStorageFn != nil { - tt.setupStorageFn(m, tt.options) - } - - dw := NewDualWriter(Mode4, ls, us, p, kind) - - res, err := dw.List(context.Background(), tt.options) - - if tt.wantErr { - assert.Error(t, err) - return - } - - assert.Equal(t, exampleList, res) - assert.NotEqual(t, anotherList, res) - }) - } -} - -func TestMode4_Delete(t *testing.T) { - type testCase struct { - setupStorageFn func(m *mock.Mock, name string) - name string - input string - wantErr bool - } - tests := - []testCase{ - { - name: "deleting an object in the unified store", - input: "foo", - setupStorageFn: func(m *mock.Mock, name string) { - m.On("Delete", mock.Anything, name, mock.Anything, mock.Anything).Return(exampleObj, false, nil) - }, - }, - { - name: "error when deleting an object in the unified store", - input: "object-fail", - setupStorageFn: func(m *mock.Mock, name string) { - m.On("Delete", mock.Anything, name, mock.Anything, mock.Anything).Return(nil, false, errors.New("error")) - }, - wantErr: true, - }, - } - - 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.setupStorageFn != nil { - tt.setupStorageFn(m, tt.input) - } - - dw := NewDualWriter(Mode4, ls, us, p, kind) - - obj, _, err := dw.Delete(context.Background(), tt.input, func(ctx context.Context, obj runtime.Object) error { return nil }, &metav1.DeleteOptions{}) - - if tt.wantErr { - assert.Error(t, err) - return - } - - us.AssertNotCalled(t, "Delete", context.Background(), tt.input, func(ctx context.Context, obj runtime.Object) error { return nil }, &metav1.DeleteOptions{}) - assert.Equal(t, obj, exampleObj) - assert.NotEqual(t, obj, anotherObj) - }) - } -} - -func TestMode4_DeleteCollection(t *testing.T) { - type testCase struct { - input *metav1.DeleteOptions - setupStorageFn func(m *mock.Mock, input *metav1.DeleteOptions) - name string - wantErr bool - } - tests := - []testCase{ - { - name: "deleting a collection in the unified store", - input: &metav1.DeleteOptions{TypeMeta: metav1.TypeMeta{Kind: "foo"}}, - setupStorageFn: func(m *mock.Mock, input *metav1.DeleteOptions) { - m.On("DeleteCollection", mock.Anything, mock.Anything, input, mock.Anything).Return(exampleObj, nil) - }, - }, - { - name: "error deleting a collection in the unified store", - input: &metav1.DeleteOptions{TypeMeta: metav1.TypeMeta{Kind: "fail"}}, - setupStorageFn: func(m *mock.Mock, input *metav1.DeleteOptions) { - m.On("DeleteCollection", mock.Anything, mock.Anything, input, mock.Anything).Return(nil, errors.New("error")) - }, - wantErr: true, - }, - } - - 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.setupStorageFn != nil { - tt.setupStorageFn(m, tt.input) - } - - dw := NewDualWriter(Mode4, ls, us, p, kind) - - obj, err := dw.DeleteCollection(context.Background(), func(ctx context.Context, obj runtime.Object) error { return nil }, tt.input, &metainternalversion.ListOptions{}) - - if tt.wantErr { - assert.Error(t, err) - return - } - - us.AssertNotCalled(t, "DeleteCollection", context.Background(), tt.input, func(ctx context.Context, obj runtime.Object) error { return nil }, &metav1.DeleteOptions{}) - assert.Equal(t, obj, exampleObj) - assert.NotEqual(t, obj, anotherObj) - }) - } -} - -func TestMode4_Update(t *testing.T) { - type testCase struct { - setupStorageFn func(m *mock.Mock, input string) - name string - input string - wantErr bool - } - tests := - []testCase{ - { - name: "update an object in unified store", - input: "foo", - setupStorageFn: func(m *mock.Mock, input string) { - m.On("Update", mock.Anything, input, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(exampleObj, false, nil) - }, - }, - { - name: "error updating an object in unified store", - input: "object-fail", - setupStorageFn: func(m *mock.Mock, input string) { - m.On("Update", mock.Anything, input, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil, false, errors.New("error")) - }, - wantErr: true, - }, - } - - 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.setupStorageFn != nil { - tt.setupStorageFn(m, tt.input) - } - - dw := NewDualWriter(Mode4, ls, us, p, kind) - - obj, _, err := dw.Update(context.Background(), tt.input, updatedObjInfoObj{}, func(ctx context.Context, obj runtime.Object) error { return nil }, func(ctx context.Context, obj, old runtime.Object) error { return nil }, false, &metav1.UpdateOptions{}) - - if tt.wantErr { - assert.Error(t, err) - return - } - - assert.Equal(t, obj, exampleObj) - assert.NotEqual(t, obj, anotherObj) - }) - } -}