diff --git a/pkg/apiserver/rest/dualwriter_mode1_test.go b/pkg/apiserver/rest/dualwriter_mode1_test.go index 04c0b79bd5b..01f7b2a519f 100644 --- a/pkg/apiserver/rest/dualwriter_mode1_test.go +++ b/pkg/apiserver/rest/dualwriter_mode1_test.go @@ -2,57 +2,301 @@ package rest import ( "context" + "errors" "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" metainternalversion "k8s.io/apimachinery/pkg/apis/meta/internalversion" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apiserver/pkg/apis/example" ) const kind = "dummy" -func TestMode1(t *testing.T) { - var ls = (LegacyStorage)(nil) - var s = (Storage)(nil) - lsSpy := NewLegacyStorageSpyClient(ls) - sSpy := NewStorageSpyClient(s) +var exampleObj = &example.Pod{TypeMeta: metav1.TypeMeta{}, ObjectMeta: metav1.ObjectMeta{Name: "foo", ResourceVersion: "1"}, Spec: example.PodSpec{}, Status: example.PodStatus{}} +var anotherObj = &example.Pod{TypeMeta: metav1.TypeMeta{}, ObjectMeta: metav1.ObjectMeta{Name: "bar", ResourceVersion: "2"}, Spec: example.PodSpec{}, Status: example.PodStatus{}} +var failingObj = &example.Pod{TypeMeta: metav1.TypeMeta{}, ObjectMeta: metav1.ObjectMeta{Name: "object-fail", ResourceVersion: "2"}, Spec: example.PodSpec{}, Status: example.PodStatus{}} - dw := NewDualWriterMode1(lsSpy, sSpy) +func TestMode1_Create(t *testing.T) { + type testCase struct { + name string + input runtime.Object + setupLegacyFn func(m *mock.Mock, input runtime.Object) + setupStorageFn func(m *mock.Mock, input runtime.Object) + wantErr bool + } + tests := + []testCase{ + { + name: "creating an object only in the legacy store", + input: exampleObj, + setupLegacyFn: func(m *mock.Mock, input runtime.Object) { + m.On("Create", context.Background(), input, mock.Anything, mock.Anything).Return(exampleObj, nil) + }, + setupStorageFn: func(m *mock.Mock, input runtime.Object) { + m.On("Create", context.Background(), anotherObj, mock.Anything, mock.Anything).Return(anotherObj, nil) + }, + }, + { + name: "error when creating object in the legacy store fails", + input: failingObj, + setupLegacyFn: func(m *mock.Mock, input runtime.Object) { + m.On("Create", context.Background(), failingObj, mock.Anything, mock.Anything).Return(nil, errors.New("error")) + }, + wantErr: true, + }, + } - // Create: it should use the Legacy Create implementation - _, err := dw.Create(context.Background(), &dummyObject{}, func(context.Context, runtime.Object) error { return nil }, &metav1.CreateOptions{}) - assert.NoError(t, err) - assert.Equal(t, 1, lsSpy.Counts("LegacyStorage.Create")) - assert.Equal(t, 0, sSpy.Counts("Storage.Create")) + for _, tt := range tests { + l := (LegacyStorage)(nil) + s := (Storage)(nil) + m := &mock.Mock{} - // Get: it should use the Legacy Get implementation - _, err = dw.Get(context.Background(), kind, &metav1.GetOptions{}) - assert.NoError(t, err) - assert.Equal(t, 1, lsSpy.Counts("LegacyStorage.Get")) - assert.Equal(t, 0, sSpy.Counts("Storage.Get")) + ls := legacyStoreMock{m, l} + us := storageMock{m, s} - // List: it should use the Legacy List implementation - _, err = dw.List(context.Background(), &metainternalversion.ListOptions{}) - assert.NoError(t, err) - assert.Equal(t, 1, lsSpy.Counts("LegacyStorage.List")) - assert.Equal(t, 0, sSpy.Counts("Storage.List")) + if tt.setupLegacyFn != nil { + tt.setupLegacyFn(m, tt.input) + } + if tt.setupStorageFn != nil { + tt.setupStorageFn(m, tt.input) + } - // Delete: it should use the Legacy Delete implementation - var deleteValidation = func(ctx context.Context, obj runtime.Object) error { return nil } - _, _, err = dw.Delete(context.Background(), kind, deleteValidation, &metav1.DeleteOptions{}) - assert.NoError(t, err) - assert.Equal(t, 1, lsSpy.Counts("LegacyStorage.Delete")) - assert.Equal(t, 0, sSpy.Counts("Storage.Delete")) + dw := SelectDualWriter(Mode1, ls, us) - // DeleteCollection: it should use the Legacy DeleteCollection implementation - _, err = dw.DeleteCollection( - context.Background(), - func(context.Context, runtime.Object) error { return nil }, - &metav1.DeleteOptions{}, - &metainternalversion.ListOptions{}, - ) - assert.NoError(t, err) - assert.Equal(t, 1, lsSpy.Counts("LegacyStorage.DeleteCollection")) - assert.Equal(t, 0, sSpy.Counts("Storage.DeleteCollection")) + 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) + continue + } + + us.AssertNotCalled(t, "Create", context.Background(), tt.input, func(context.Context, runtime.Object) error { return nil }, &metav1.CreateOptions{}) + + assert.Equal(t, obj, exampleObj) + assert.NotEqual(t, obj, anotherObj) + } +} + +func TestMode1_Get(t *testing.T) { + type testCase struct { + name string + input string + setupLegacyFn func(m *mock.Mock, name string) + setupStorageFn func(m *mock.Mock, name string) + wantErr bool + } + tests := + []testCase{ + { + name: "get an object only in the legacy store", + input: "foo", + setupLegacyFn: func(m *mock.Mock, name string) { + m.On("Get", context.Background(), name, mock.Anything).Return(exampleObj, nil) + }, + setupStorageFn: func(m *mock.Mock, name string) { + m.On("Get", context.Background(), name, mock.Anything).Return(anotherObj, nil) + }, + }, + { + name: "error when getting an object in the legacy store fails", + input: "object-fail", + setupLegacyFn: func(m *mock.Mock, name string) { + m.On("Get", context.Background(), name, mock.Anything).Return(nil, errors.New("error")) + }, + wantErr: true, + }, + } + + for _, tt := range tests { + l := (LegacyStorage)(nil) + s := (Storage)(nil) + m := &mock.Mock{} + + ls := legacyStoreMock{m, l} + us := storageMock{m, s} + + if tt.setupLegacyFn != nil { + tt.setupLegacyFn(m, tt.input) + } + if tt.setupStorageFn != nil { + tt.setupStorageFn(m, tt.input) + } + + dw := SelectDualWriter(Mode1, ls, us) + + obj, err := dw.Get(context.Background(), tt.input, &metav1.GetOptions{}) + + if tt.wantErr { + assert.Error(t, err) + continue + } + + us.AssertNotCalled(t, "Get", context.Background(), tt.name, &metav1.GetOptions{}) + + assert.Equal(t, obj, exampleObj) + assert.NotEqual(t, obj, anotherObj) + } +} + +func TestMode1_List(t *testing.T) { + type testCase struct { + name string + setupLegacyFn func(m *mock.Mock) + setupStorageFn func(m *mock.Mock) + wantErr bool + } + tests := + []testCase{ + { + name: "error when listing an object in the legacy store is not implemented", + setupLegacyFn: func(m *mock.Mock) { + m.On("List", context.Background(), mock.Anything).Return(nil, errors.New("error")) + }, + }, + // TODO: legacy list is missing + } + + for _, tt := range tests { + l := (LegacyStorage)(nil) + s := (Storage)(nil) + m := &mock.Mock{} + + ls := legacyStoreMock{m, l} + us := storageMock{m, s} + + if tt.setupLegacyFn != nil { + tt.setupLegacyFn(m) + } + if tt.setupStorageFn != nil { + tt.setupStorageFn(m) + } + + dw := SelectDualWriter(Mode1, ls, us) + + _, err := dw.List(context.Background(), &metainternalversion.ListOptions{}) + + if tt.wantErr { + assert.Error(t, err) + continue + } + + us.AssertNotCalled(t, "List", context.Background(), &metainternalversion.ListOptions{}) + } +} + +func TestMode1_Delete(t *testing.T) { + type testCase struct { + name string + input string + setupLegacyFn func(m *mock.Mock, name string) + setupStorageFn func(m *mock.Mock, name string) + wantErr bool + } + tests := + []testCase{ + { + name: "deleting an object in the legacy store", + input: "foo", + setupLegacyFn: func(m *mock.Mock, name string) { + m.On("Delete", context.Background(), name, mock.Anything, mock.Anything).Return(exampleObj, false, nil) + }, + }, + { + name: "error when deleting an object in the legacy store", + input: "object-fail", + setupLegacyFn: func(m *mock.Mock, name string) { + m.On("Delete", context.Background(), name, mock.Anything, mock.Anything).Return(nil, false, errors.New("error")) + }, + wantErr: true, + }, + } + + for _, tt := range tests { + l := (LegacyStorage)(nil) + s := (Storage)(nil) + m := &mock.Mock{} + + ls := legacyStoreMock{m, l} + us := storageMock{m, s} + + if tt.setupLegacyFn != nil { + tt.setupLegacyFn(m, tt.input) + } + if tt.setupStorageFn != nil { + tt.setupStorageFn(m, tt.input) + } + + dw := SelectDualWriter(Mode1, ls, us) + + 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) + continue + } + + 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 TestMode1_DeleteCollection(t *testing.T) { + type testCase struct { + name string + input *metav1.DeleteOptions + setupLegacyFn func(m *mock.Mock, input *metav1.DeleteOptions) + setupStorageFn func(m *mock.Mock, input *metav1.DeleteOptions) + wantErr bool + } + tests := + []testCase{ + { + name: "deleting a collection in the legacy store", + input: &metav1.DeleteOptions{TypeMeta: metav1.TypeMeta{Kind: "foo"}}, + setupLegacyFn: func(m *mock.Mock, input *metav1.DeleteOptions) { + m.On("DeleteCollection", context.Background(), mock.Anything, input, mock.Anything).Return(exampleObj, nil) + }, + }, + { + name: "error deleting a collection in the legacy store", + input: &metav1.DeleteOptions{TypeMeta: metav1.TypeMeta{Kind: "fail"}}, + setupLegacyFn: func(m *mock.Mock, input *metav1.DeleteOptions) { + m.On("DeleteCollection", context.Background(), mock.Anything, input, mock.Anything).Return(nil, errors.New("error")) + }, + wantErr: true, + }, + } + + for _, tt := range tests { + l := (LegacyStorage)(nil) + s := (Storage)(nil) + m := &mock.Mock{} + + ls := legacyStoreMock{m, l} + us := storageMock{m, s} + + if tt.setupLegacyFn != nil { + tt.setupLegacyFn(m, tt.input) + } + if tt.setupStorageFn != nil { + tt.setupStorageFn(m, tt.input) + } + + dw := SelectDualWriter(Mode1, ls, us) + + 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) + continue + } + + 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) + } } diff --git a/pkg/apiserver/rest/dualwriter_mode2.go b/pkg/apiserver/rest/dualwriter_mode2.go index efef61de2a1..ac61d5ef214 100644 --- a/pkg/apiserver/rest/dualwriter_mode2.go +++ b/pkg/apiserver/rest/dualwriter_mode2.go @@ -60,16 +60,15 @@ func (d *DualWriterMode2) Create(ctx context.Context, obj runtime.Object, create // It retrieves an object from Storage if possible, and if not it falls back to LegacyStorage. func (d *DualWriterMode2) Get(ctx context.Context, name string, options *metav1.GetOptions) (runtime.Object, error) { s, err := d.Storage.Get(ctx, name, &metav1.GetOptions{}) - if err == nil { - return s, err + if err != nil { + if apierrors.IsNotFound(err) { + klog.Info("object not found in storage", "name", name) + return d.Legacy.Get(ctx, name, &metav1.GetOptions{}) + } + klog.Error("unable to fetch object from storage", "error", err, "name", name) + return d.Legacy.Get(ctx, name, &metav1.GetOptions{}) } - if apierrors.IsNotFound(err) { - klog.Info("object not found in duplicate storage", "name", name) - } else { - klog.Error("unable to fetch object from duplicate storage", "error", err, "name", name) - } - - return d.Legacy.Get(ctx, name, &metav1.GetOptions{}) + return s, nil } // List overrides the behavior of the generic DualWriter. diff --git a/pkg/apiserver/rest/dualwriter_mode2_test.go b/pkg/apiserver/rest/dualwriter_mode2_test.go index 76dd25dc7e8..000cc595328 100644 --- a/pkg/apiserver/rest/dualwriter_mode2_test.go +++ b/pkg/apiserver/rest/dualwriter_mode2_test.go @@ -2,81 +2,374 @@ package rest import ( "context" + "errors" "testing" - "github.com/zeebo/assert" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + apierrors "k8s.io/apimachinery/pkg/api/errors" "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" + "k8s.io/apimachinery/pkg/runtime/schema" ) -func TestMode2(t *testing.T) { - var ls = (LegacyStorage)(nil) - var s = (Storage)(nil) - lsSpy := NewLegacyStorageSpyClient(ls) - sSpy := NewStorageSpyClient(s) - - dw := NewDualWriterMode2(lsSpy, sSpy) - - // Create: it should use the Legacy Create implementation - _, err := dw.Create(context.Background(), &dummyObject{}, func(context.Context, runtime.Object) error { return nil }, &metav1.CreateOptions{}) - assert.NoError(t, err) - assert.Equal(t, 1, lsSpy.Counts("LegacyStorage.Create")) - assert.Equal(t, 1, sSpy.Counts("Storage.Create")) - - // Get: it should read from Storage with LegacyStorage as a fallback - // #TODO: Currently only testing the happy path. Refactor testing to more easily test other cases. - _, err = dw.Get(context.Background(), kind, &metav1.GetOptions{}) - assert.NoError(t, err) - assert.Equal(t, 0, lsSpy.Counts("LegacyStorage.Get")) - assert.Equal(t, 1, sSpy.Counts("Storage.Get")) - - // List: it should use call both Legacy and Storage List methods - l, err := dw.List(context.Background(), &metainternalversion.ListOptions{}) - assert.NoError(t, err) - assert.Equal(t, 1, lsSpy.Counts("LegacyStorage.List")) - assert.Equal(t, 1, sSpy.Counts("Storage.List")) - - resList, err := meta.ExtractList(l) - assert.NoError(t, err) - - expectedItems := map[string]string{ - // Item 1: Storage should override Legacy - "Item 1": "Storage field 1", - // Item 2 shouldn't be included because it's not in Storage - // Item 3 should because it's in Legacy - "Item 3": "Legacy field 3", - } - - assert.Equal(t, len(expectedItems), len(resList)) - - for _, obj := range resList { - v, ok := obj.(*dummyObject) - assert.True(t, ok) - accessor, err := meta.Accessor(v) - assert.NoError(t, err) - - k, ok := expectedItems[accessor.GetName()] - assert.True(t, ok) - assert.Equal(t, k, v.Foo) - } - - // Delete: it should use call both Legacy and Storage Delete methods - var deleteValidation = func(ctx context.Context, obj runtime.Object) error { return nil } - _, _, err = dw.Delete(context.Background(), kind, deleteValidation, &metav1.DeleteOptions{}) - assert.NoError(t, err) - assert.Equal(t, 1, lsSpy.Counts("LegacyStorage.Delete")) - assert.Equal(t, 1, sSpy.Counts("Storage.Delete")) - - // DeleteCollection: it should delete from both LegacyStorage and Storage - _, err = dw.DeleteCollection( - context.Background(), - func(context.Context, runtime.Object) error { return nil }, - &metav1.DeleteOptions{}, - &metainternalversion.ListOptions{}, - ) - assert.NoError(t, err) - assert.Equal(t, 1, lsSpy.Counts("LegacyStorage.DeleteCollection")) - assert.Equal(t, 1, sSpy.Counts("Storage.DeleteCollection")) +var createFn = func(context.Context, runtime.Object) error { return nil } +var exampleOption = &metainternalversion.ListOptions{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "v1", + Kind: "foo", + }, +} + +func TestMode2_Create(t *testing.T) { + type testCase struct { + name string + input runtime.Object + setupLegacyFn func(m *mock.Mock, input runtime.Object) + setupStorageFn func(m *mock.Mock, input runtime.Object) + wantErr bool + } + tests := + []testCase{ + { + name: "creating an object in both the LegacyStorage and Storage", + input: exampleObj, + setupLegacyFn: func(m *mock.Mock, input runtime.Object) { + m.On("Create", context.Background(), input, mock.Anything, mock.Anything).Return(exampleObj, nil) + }, + setupStorageFn: func(m *mock.Mock, input runtime.Object) { + m.On("Create", context.Background(), input, mock.Anything, mock.Anything).Return(exampleObj, nil) + }, + }, + { + name: "error when creating object in the legacy store fails", + input: failingObj, + setupLegacyFn: func(m *mock.Mock, input runtime.Object) { + m.On("Create", context.Background(), input, mock.Anything, mock.Anything).Return(nil, errors.New("error")) + }, + wantErr: true, + }, + } + + for _, tt := range tests { + l := (LegacyStorage)(nil) + s := (Storage)(nil) + m := &mock.Mock{} + + ls := legacyStoreMock{m, l} + us := storageMock{m, s} + + if tt.setupLegacyFn != nil { + tt.setupLegacyFn(m, tt.input) + } + if tt.setupStorageFn != nil { + tt.setupStorageFn(m, tt.input) + } + + dw := SelectDualWriter(Mode2, ls, us) + + obj, err := dw.Create(context.Background(), tt.input, createFn, &metav1.CreateOptions{}) + + if tt.wantErr { + assert.Error(t, err) + continue + } + + assert.Equal(t, obj, exampleObj) + accessor, err := meta.Accessor(obj) + assert.NoError(t, err) + assert.Equal(t, accessor.GetResourceVersion(), "") + } +} + +func TestMode2_Get(t *testing.T) { + type testCase struct { + name string + input string + setupLegacyFn func(m *mock.Mock, input string) + setupStorageFn func(m *mock.Mock, input string) + wantErr bool + } + tests := + []testCase{ + { + name: "getting an object from storage", + input: "foo", + setupLegacyFn: func(m *mock.Mock, input string) { + m.On("Get", context.Background(), input, mock.Anything).Return(exampleObj, nil) + }, + setupStorageFn: func(m *mock.Mock, input string) { + m.On("Get", context.Background(), input, mock.Anything).Return(anotherObj, nil) + }, + }, + { + name: "object not present in storage but present in legacy store", + input: "foo", + setupLegacyFn: func(m *mock.Mock, input string) { + m.On("Get", context.Background(), input, mock.Anything).Return(exampleObj, nil) + }, + setupStorageFn: func(m *mock.Mock, input string) { + m.On("Get", context.Background(), input, mock.Anything).Return(nil, errors.New("error")) + }, + }, + { + name: "error when getting object in both stores fails", + input: "object-fail", + setupLegacyFn: func(m *mock.Mock, input string) { + m.On("Get", context.Background(), input, mock.Anything).Return(nil, errors.New("error")) + }, + setupStorageFn: func(m *mock.Mock, input string) { + m.On("Get", context.Background(), input, mock.Anything).Return(nil, errors.New("error")) + }, + wantErr: true, + }, + } + + for _, tt := range tests { + l := (LegacyStorage)(nil) + s := (Storage)(nil) + m := &mock.Mock{} + + ls := legacyStoreMock{m, l} + us := storageMock{m, s} + + if tt.setupLegacyFn != nil { + tt.setupLegacyFn(m, tt.input) + } + if tt.setupStorageFn != nil { + tt.setupStorageFn(m, tt.input) + } + + dw := SelectDualWriter(Mode2, ls, us) + + obj, err := dw.Get(context.Background(), tt.input, &metav1.GetOptions{}) + + if tt.wantErr { + assert.Error(t, err) + continue + } + + assert.Equal(t, obj, exampleObj) + assert.NotEqual(t, obj, anotherObj) + } +} + +func TestMode2_List(t *testing.T) { + type testCase struct { + name string + input *metainternalversion.ListOptions + setupLegacyFn func(m *mock.Mock, input *metainternalversion.ListOptions) + setupStorageFn func(m *mock.Mock, input *metainternalversion.ListOptions) + wantErr bool + } + tests := + []testCase{ + { + name: "error when legacy list is not implmented", + input: exampleOption, + setupLegacyFn: func(m *mock.Mock, input *metainternalversion.ListOptions) { + m.On("List", context.Background(), input).Return(exampleObj, nil) + }, + setupStorageFn: func(m *mock.Mock, input *metainternalversion.ListOptions) { + m.On("List", context.Background(), input).Return(exampleObj, nil) + }, + wantErr: true, + }, + } + + for _, tt := range tests { + l := (LegacyStorage)(nil) + s := (Storage)(nil) + m := &mock.Mock{} + + ls := legacyStoreMock{m, l} + us := storageMock{m, s} + + if tt.setupLegacyFn != nil { + tt.setupLegacyFn(m, tt.input) + } + if tt.setupStorageFn != nil { + tt.setupStorageFn(m, tt.input) + } + + dw := SelectDualWriter(Mode2, ls, us) + + obj, err := dw.List(context.Background(), exampleOption) + + if tt.wantErr { + assert.Error(t, err) + continue + } + + assert.Equal(t, obj, exampleObj) + } +} + +func TestMode2_Delete(t *testing.T) { + type testCase struct { + name string + input string + setupLegacyFn func(m *mock.Mock, input string) + setupStorageFn func(m *mock.Mock, input string) + wantErr bool + } + tests := + []testCase{ + { + name: "delete in legacy and storage", + input: "foo", + setupLegacyFn: func(m *mock.Mock, input string) { + m.On("Delete", context.Background(), input, mock.Anything, mock.Anything).Return(exampleObj, false, nil) + }, + setupStorageFn: func(m *mock.Mock, input string) { + m.On("Delete", context.Background(), input, mock.Anything, mock.Anything).Return(exampleObj, false, nil) + }, + }, + { + name: "object delete in legacy not found, but found in storage", + input: "foo", + setupLegacyFn: func(m *mock.Mock, input string) { + m.On("Delete", context.Background(), "not-found-legacy", mock.Anything, mock.Anything).Return(nil, false, apierrors.NewNotFound(schema.GroupResource{Group: "", Resource: "pods"}, "not-found")) + }, + setupStorageFn: func(m *mock.Mock, input string) { + m.On("Delete", context.Background(), input, mock.Anything, mock.Anything).Return(exampleObj, false, nil) + }, + }, + { + name: " object delete in storage not found, but found in legacy", + input: "foo", + setupLegacyFn: func(m *mock.Mock, input string) { + m.On("Delete", context.Background(), input, mock.Anything, mock.Anything).Return(exampleObj, false, nil) + }, + setupStorageFn: func(m *mock.Mock, input string) { + m.On("Delete", context.Background(), "not-found-storage", mock.Anything, mock.Anything).Return(nil, false, apierrors.NewNotFound(schema.GroupResource{Group: "", Resource: "pods"}, "not-found")) + }, + }, + { + name: " object not found in both", + input: "object-fail", + setupLegacyFn: func(m *mock.Mock, input string) { + m.On("Delete", context.Background(), input, mock.Anything, mock.Anything).Return(nil, false, apierrors.NewNotFound(schema.GroupResource{Group: "", Resource: "pods"}, "not-found")) + }, + setupStorageFn: func(m *mock.Mock, input string) { + m.On("Delete", context.Background(), input, mock.Anything, mock.Anything).Return(nil, false, apierrors.NewNotFound(schema.GroupResource{Group: "", Resource: "pods"}, "not-found")) + }, + wantErr: true, + }, + { + name: " object delete error", + input: "object-fail", + setupLegacyFn: func(m *mock.Mock, input string) { + m.On("Delete", context.Background(), input, mock.Anything, mock.Anything).Return(nil, false, errors.New("error")) + }, + setupStorageFn: func(m *mock.Mock, input string) { + m.On("Delete", context.Background(), input, mock.Anything, mock.Anything).Return(nil, false, errors.New("error")) + }, + wantErr: true, + }, + } + + for _, tt := range tests { + l := (LegacyStorage)(nil) + s := (Storage)(nil) + m := &mock.Mock{} + + ls := legacyStoreMock{m, l} + us := storageMock{m, s} + + if tt.setupLegacyFn != nil { + tt.setupLegacyFn(m, tt.input) + } + if tt.setupStorageFn != nil { + tt.setupStorageFn(m, tt.input) + } + + dw := SelectDualWriter(Mode2, ls, us) + + obj, _, err := dw.Delete(context.Background(), tt.input, func(context.Context, runtime.Object) error { return nil }, &metav1.DeleteOptions{}) + + if tt.wantErr { + assert.Error(t, err) + continue + } + + assert.Equal(t, obj, exampleObj) + assert.NotEqual(t, obj, anotherObj) + } +} + +func TestMode2_DeleteCollection(t *testing.T) { + type testCase struct { + name string + input *metav1.DeleteOptions + setupLegacyFn func(m *mock.Mock, input *metav1.DeleteOptions) + setupStorageFn func(m *mock.Mock, input *metav1.DeleteOptions) + wantErr bool + } + tests := + []testCase{ + { + name: "deleting a collection in both stores", + input: &metav1.DeleteOptions{TypeMeta: metav1.TypeMeta{Kind: "foo"}}, + setupLegacyFn: func(m *mock.Mock, input *metav1.DeleteOptions) { + m.On("DeleteCollection", context.Background(), mock.Anything, input, mock.Anything).Return(exampleObj, nil) + }, + setupStorageFn: func(m *mock.Mock, input *metav1.DeleteOptions) { + m.On("DeleteCollection", context.Background(), mock.Anything, input, mock.Anything).Return(exampleObj, nil) + }, + }, + { + name: "error deleting a collection in the storage when legacy store is successful", + input: &metav1.DeleteOptions{TypeMeta: metav1.TypeMeta{Kind: "fail"}}, + setupLegacyFn: func(m *mock.Mock, input *metav1.DeleteOptions) { + m.On("DeleteCollection", context.Background(), mock.Anything, &metav1.DeleteOptions{TypeMeta: metav1.TypeMeta{Kind: "foo"}}, mock.Anything).Return(exampleObj, nil) + }, + setupStorageFn: func(m *mock.Mock, input *metav1.DeleteOptions) { + m.On("DeleteCollection", context.Background(), mock.Anything, input, mock.Anything).Return(nil, errors.New("error")) + }, + wantErr: true, + }, + { + name: "error deleting a collection when error in both stores", + input: &metav1.DeleteOptions{TypeMeta: metav1.TypeMeta{Kind: "fail"}}, + setupLegacyFn: func(m *mock.Mock, input *metav1.DeleteOptions) { + m.On("DeleteCollection", context.Background(), mock.Anything, input, mock.Anything).Return(nil, errors.New("error")) + }, + setupStorageFn: func(m *mock.Mock, input *metav1.DeleteOptions) { + m.On("DeleteCollection", context.Background(), mock.Anything, input, mock.Anything).Return(nil, errors.New("error")) + }, + wantErr: true, + }, + } + + for _, tt := range tests { + l := (LegacyStorage)(nil) + s := (Storage)(nil) + m := &mock.Mock{} + + ls := legacyStoreMock{m, l} + us := storageMock{m, s} + + if tt.setupLegacyFn != nil { + tt.setupLegacyFn(m, tt.input) + } + if tt.setupStorageFn != nil { + tt.setupStorageFn(m, tt.input) + } + + dw := SelectDualWriter(Mode2, ls, us) + + 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) + continue + } + + 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) + } } diff --git a/pkg/apiserver/rest/dualwriter_mode3.go b/pkg/apiserver/rest/dualwriter_mode3.go index 89c868c14e3..b04a6a387ea 100644 --- a/pkg/apiserver/rest/dualwriter_mode3.go +++ b/pkg/apiserver/rest/dualwriter_mode3.go @@ -55,6 +55,7 @@ func (d *DualWriterMode3) Delete(ctx context.Context, name string, deleteValidat if err != nil { if !apierrors.IsNotFound(err) { klog.FromContext(ctx).Error(err, "could not delete from unified store", "mode", Mode3) + return deleted, async, err } } diff --git a/pkg/apiserver/rest/storage_mocks_test.go b/pkg/apiserver/rest/storage_mocks_test.go new file mode 100644 index 00000000000..a3ad87f9ece --- /dev/null +++ b/pkg/apiserver/rest/storage_mocks_test.go @@ -0,0 +1,126 @@ +package rest + +import ( + "context" + + "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" + "k8s.io/apiserver/pkg/registry/rest" +) + +type legacyStoreMock struct { + *mock.Mock + LegacyStorage +} + +type storageMock struct { + *mock.Mock + Storage +} + +func (m legacyStoreMock) Get(ctx context.Context, name string, options *metav1.GetOptions) (runtime.Object, error) { + args := m.Called(ctx, name, options) + if name == "object-fail" { + return nil, args.Error(1) + } + return args.Get(0).(runtime.Object), args.Error(1) +} + +func (m legacyStoreMock) Create(ctx context.Context, obj runtime.Object, createValidation rest.ValidateObjectFunc, options *metav1.CreateOptions) (runtime.Object, error) { + args := m.Called(ctx, obj, createValidation, options) + acc, err := meta.Accessor(obj) + if err != nil { + return nil, args.Error(1) + } + name := acc.GetName() + if name == "object-fail" { + return nil, args.Error(1) + } + return args.Get(0).(runtime.Object), args.Error(1) +} + +func (m legacyStoreMock) List(ctx context.Context, options *metainternalversion.ListOptions) (runtime.Object, error) { + args := m.Called(ctx, options) + if options.Kind == "fail" { + return nil, args.Error(1) + } + return args.Get(0).(runtime.Object), args.Error(1) +} + +func (m legacyStoreMock) Update(ctx context.Context, name string, objInfo rest.UpdatedObjectInfo, createValidation rest.ValidateObjectFunc, updateValidation rest.ValidateObjectUpdateFunc, forceAllowCreate bool, options *metav1.UpdateOptions) (runtime.Object, bool, error) { + args := m.Called(ctx, name, objInfo, createValidation, updateValidation, forceAllowCreate, options) + return args.Get(0).(runtime.Object), args.Bool(1), args.Error(2) +} + +func (m legacyStoreMock) Delete(ctx context.Context, name string, deleteValidation rest.ValidateObjectFunc, options *metav1.DeleteOptions) (runtime.Object, bool, error) { + args := m.Called(ctx, name, deleteValidation, options) + if name == "object-fail" { + return nil, false, args.Error(2) + } + if name == "not-found-legacy" { + return nil, false, args.Error(2) + } + return args.Get(0).(runtime.Object), args.Bool(1), args.Error(2) +} + +func (m legacyStoreMock) DeleteCollection(ctx context.Context, deleteValidation rest.ValidateObjectFunc, options *metav1.DeleteOptions, listOptions *metainternalversion.ListOptions) (runtime.Object, error) { + args := m.Called(ctx, deleteValidation, options, listOptions) + if options.Kind == "fail" { + return nil, args.Error(1) + } + return args.Get(0).(runtime.Object), args.Error(1) +} + +// Unified Store +func (m storageMock) Get(ctx context.Context, name string, options *metav1.GetOptions) (runtime.Object, error) { + args := m.Called(ctx, name, options) + if name == "object-fail" { + return nil, args.Error(1) + } + return args.Get(0).(runtime.Object), args.Error(1) +} + +func (m storageMock) Create(ctx context.Context, obj runtime.Object, createValidation rest.ValidateObjectFunc, options *metav1.CreateOptions) (runtime.Object, error) { + args := m.Called(ctx, obj, createValidation, options) + acc, err := meta.Accessor(obj) + if err != nil { + return nil, args.Error(1) + } + name := acc.GetName() + if name == "object-fail" { + return nil, args.Error(1) + } + return args.Get(0).(runtime.Object), args.Error(1) +} + +func (m storageMock) List(ctx context.Context, options *metainternalversion.ListOptions) (runtime.Object, error) { + args := m.Called(ctx, options) + if options.Kind == "fail" { + return nil, args.Error(1) + } + return args.Get(0).(runtime.Object), args.Error(1) +} + +func (m storageMock) Update(ctx context.Context, name string, objInfo rest.UpdatedObjectInfo, createValidation rest.ValidateObjectFunc, updateValidation rest.ValidateObjectUpdateFunc, forceAllowCreate bool, options *metav1.UpdateOptions) (runtime.Object, bool, error) { + args := m.Called(ctx, name, objInfo, createValidation, updateValidation, forceAllowCreate, options) + return args.Get(0).(runtime.Object), args.Bool(1), args.Error(2) +} + +func (m storageMock) Delete(ctx context.Context, name string, deleteValidation rest.ValidateObjectFunc, options *metav1.DeleteOptions) (runtime.Object, bool, error) { + args := m.Called(ctx, name, deleteValidation, options) + if name == "object-fail" { + return nil, false, args.Error(2) + } + return args.Get(0).(runtime.Object), args.Bool(1), args.Error(2) +} + +func (m storageMock) DeleteCollection(ctx context.Context, deleteValidation rest.ValidateObjectFunc, options *metav1.DeleteOptions, listOptions *metainternalversion.ListOptions) (runtime.Object, error) { + args := m.Called(ctx, deleteValidation, options, listOptions) + if options.Kind == "fail" { + return nil, args.Error(1) + } + return args.Get(0).(runtime.Object), args.Error(1) +}