diff --git a/pkg/apiserver/rest/dualwriter_mode1.go b/pkg/apiserver/rest/dualwriter_mode1.go index 0caf3a7aed2..06f7edb823e 100644 --- a/pkg/apiserver/rest/dualwriter_mode1.go +++ b/pkg/apiserver/rest/dualwriter_mode1.go @@ -31,7 +31,7 @@ func (d *DualWriterMode1) Create(ctx context.Context, obj runtime.Object, create return legacy.Create(ctx, obj, createValidation, options) } -// Get overrides the default behavior of the DualWriter and reads only to LegacyStorage. +// Get overrides the behavior of the generic DualWriter and reads only from LegacyStorage. func (d *DualWriterMode1) Get(ctx context.Context, name string, options *metav1.GetOptions) (runtime.Object, error) { return d.Legacy.Get(ctx, name, options) } diff --git a/pkg/apiserver/rest/dualwriter_mode1_test.go b/pkg/apiserver/rest/dualwriter_mode1_test.go index 2f0e58510bc..9177e37caaa 100644 --- a/pkg/apiserver/rest/dualwriter_mode1_test.go +++ b/pkg/apiserver/rest/dualwriter_mode1_test.go @@ -7,6 +7,7 @@ import ( "github.com/stretchr/testify/assert" metainternalversion "k8s.io/apimachinery/pkg/apis/meta/internalversion" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" ) const kind = "dummy" @@ -19,8 +20,14 @@ func TestMode1(t *testing.T) { dw := NewDualWriterMode1(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, 0, sSpy.Counts("Storage.Create")) + // Get: it should use the Legacy Get implementation - _, err := dw.Get(context.Background(), kind, &metav1.GetOptions{}) + _, 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")) diff --git a/pkg/apiserver/rest/dualwriter_mode2.go b/pkg/apiserver/rest/dualwriter_mode2.go index cb5c5a5879d..0073c3c8b32 100644 --- a/pkg/apiserver/rest/dualwriter_mode2.go +++ b/pkg/apiserver/rest/dualwriter_mode2.go @@ -3,6 +3,7 @@ package rest import ( "context" + 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" @@ -55,8 +56,19 @@ func (d *DualWriterMode2) Create(ctx context.Context, obj runtime.Object, create return rsp, err } -// Get overrides the behavior of the generic DualWriter and retrieves an object from LegacyStorage. +// Get overrides the behavior of the generic DualWriter. +// 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 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{}) } diff --git a/pkg/apiserver/rest/dualwriter_mode2_test.go b/pkg/apiserver/rest/dualwriter_mode2_test.go index 12fa392deb5..ce4cdc7f54e 100644 --- a/pkg/apiserver/rest/dualwriter_mode2_test.go +++ b/pkg/apiserver/rest/dualwriter_mode2_test.go @@ -8,6 +8,7 @@ import ( "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 TestMode2(t *testing.T) { @@ -18,19 +19,26 @@ func TestMode2(t *testing.T) { dw := NewDualWriterMode2(lsSpy, sSpy) - // Get: it should use the Legacy Get implementation - _, err := dw.Get(context.Background(), kind, &metav1.GetOptions{}) + // 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.Get")) - assert.Equal(t, 0, sSpy.Counts("Storage.Get")) + 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 - res, err := dw.List(context.Background(), &metainternalversion.ListOptions{}) + 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(res) + resList, err := meta.ExtractList(l) assert.NoError(t, err) expectedItems := map[string]string{ diff --git a/pkg/apiserver/rest/dualwriter_mode3.go b/pkg/apiserver/rest/dualwriter_mode3.go index 911d0f8668f..a0132013c13 100644 --- a/pkg/apiserver/rest/dualwriter_mode3.go +++ b/pkg/apiserver/rest/dualwriter_mode3.go @@ -2,9 +2,7 @@ package rest import ( "context" - "fmt" - apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apiserver/pkg/registry/rest" @@ -40,23 +38,7 @@ func (d *DualWriterMode3) Create(ctx context.Context, obj runtime.Object, create return created, nil } -// Get overrides the default behavior of the Storage and retrieves an object from Unified Storage -// the object is still fetched from Legacy Storage if it's not found in Unified Storage +// Get overrides the behavior of the generic DualWriter and retrieves an object from Storage. func (d *DualWriterMode3) Get(ctx context.Context, name string, options *metav1.GetOptions) (runtime.Object, error) { - legacy, ok := d.Legacy.(rest.Getter) - if !ok { - return nil, fmt.Errorf("legacy storage rest.Getter is missing") - } - - s, err := d.Storage.Get(ctx, name, &metav1.GetOptions{}) - if err == nil { - return s, err - } - if !apierrors.IsNotFound(err) { - return nil, err - } - - klog.Info("object not found in unified storage. Getting it from legacy", "name", name) - - return legacy.Get(ctx, name, &metav1.GetOptions{}) + return d.Storage.Get(ctx, name, &metav1.GetOptions{}) } diff --git a/pkg/apiserver/rest/dualwriter_mode3_test.go b/pkg/apiserver/rest/dualwriter_mode3_test.go index bfd79723804..dbb481727f8 100644 --- a/pkg/apiserver/rest/dualwriter_mode3_test.go +++ b/pkg/apiserver/rest/dualwriter_mode3_test.go @@ -7,6 +7,7 @@ import ( "github.com/stretchr/testify/assert" metainternalversion "k8s.io/apimachinery/pkg/apis/meta/internalversion" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" ) func TestMode3(t *testing.T) { @@ -17,8 +18,14 @@ func TestMode3(t *testing.T) { dw := NewDualWriterMode3(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 use the Storage Get implementation - _, err := dw.Get(context.Background(), kind, &metav1.GetOptions{}) + _, 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")) diff --git a/pkg/apiserver/rest/dualwriter_mode4.go b/pkg/apiserver/rest/dualwriter_mode4.go index fe62b988261..7e56970acb4 100644 --- a/pkg/apiserver/rest/dualwriter_mode4.go +++ b/pkg/apiserver/rest/dualwriter_mode4.go @@ -25,7 +25,7 @@ func (d *DualWriterMode4) Create(ctx context.Context, obj runtime.Object, create return d.Storage.Create(ctx, obj, createValidation, options) } -// Get overrides the behavior of the generic DualWriter and retrieves an object from Unified Storage. +// 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) { return d.Storage.Get(ctx, name, &metav1.GetOptions{}) } diff --git a/pkg/apiserver/rest/dualwriter_mode4_test.go b/pkg/apiserver/rest/dualwriter_mode4_test.go index 09c9882b853..2815741fb77 100644 --- a/pkg/apiserver/rest/dualwriter_mode4_test.go +++ b/pkg/apiserver/rest/dualwriter_mode4_test.go @@ -7,6 +7,7 @@ import ( "github.com/stretchr/testify/assert" metainternalversion "k8s.io/apimachinery/pkg/apis/meta/internalversion" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" ) func TestMode4(t *testing.T) { @@ -17,8 +18,14 @@ func TestMode4(t *testing.T) { dw := NewDualWriterMode4(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, 0, lsSpy.Counts("LegacyStorage.Create")) + assert.Equal(t, 1, sSpy.Counts("Storage.Create")) + // Get: it should use the Storage Get implementation - _, err := dw.Get(context.Background(), kind, &metav1.GetOptions{}) + _, 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")) diff --git a/pkg/apiserver/rest/spy_client.go b/pkg/apiserver/rest/spy_client.go index 9c80350c584..af9cb808b43 100644 --- a/pkg/apiserver/rest/spy_client.go +++ b/pkg/apiserver/rest/spy_client.go @@ -59,7 +59,7 @@ type spyLegacyStorageShim struct { func (c *spyStorageClient) Create(ctx context.Context, obj runtime.Object, valitation rest.ValidateObjectFunc, options *metav1.CreateOptions) (runtime.Object, error) { c.spy.record("Storage.Create") klog.Info("method: Storage.Create") - return nil, nil + return &dummyObject{}, nil } func (c *spyStorageClient) Get(ctx context.Context, name string, options *metav1.GetOptions) (runtime.Object, error) { @@ -137,7 +137,7 @@ func (c *spyLegacyStorageClient) Counts(method string) int { func (c *spyLegacyStorageClient) Create(ctx context.Context, obj runtime.Object, valitation rest.ValidateObjectFunc, options *metav1.CreateOptions) (runtime.Object, error) { c.spy.record("LegacyStorage.Create") klog.Info("method: LegacyStorage.Create") - return nil, nil + return &dummyObject{}, nil } func (c *spyLegacyStorageClient) Get(ctx context.Context, name string, options *metav1.GetOptions) (runtime.Object, error) {