Add better logging to the dual writer (#85594)

* Make Legacy a public field * Remove duplicated Create method * Add logger to dualwriter * Use klog * Add comment about selecting the dual writer * Update pkg/apiserver/rest/dualwriter_mode1.go Co-authored-by: Arati R. <[email protected]> * Update pkg/apiserver/rest/dualwriter_mode2.go Co-authored-by: Arati R. <[email protected]> * Update pkg/apiserver/rest/dualwriter_mode3.go Co-authored-by: Arati R. <[email protected]> * Update pkg/apiserver/rest/dualwriter_mode3.go Co-authored-by: Arati R. <[email protected]> * Update pkg/apiserver/rest/dualwriter_mode2.go Co-authored-by: Arati R. <[email protected]> * Create error var * Lint --------- Co-authored-by: Arati R. <[email protected]>
tigergraph · Apr 9, 2024 · 8d75dce · 8d75dce
1 parent 4c12d77
commit 8d75dce
Show file tree

Hide file tree

Showing 4 changed files with 25 additions and 15 deletions.
diff --git a/pkg/apiserver/rest/dualwriter.go b/pkg/apiserver/rest/dualwriter.go
@@ -62,7 +62,7 @@ type LegacyStorage interface {
 // - rest.CollectionDeleter
 type DualWriter struct {
 	Storage
-	legacy LegacyStorage
+	Legacy LegacyStorage
 }
 
 type DualWriterMode int
@@ -80,15 +80,17 @@ var CurrentMode = Mode2
 
 // NewDualWriter returns a new DualWriter.
 func NewDualWriter(legacy LegacyStorage, storage Storage) *DualWriter {
+	//TODO: replace this with
+	// SelectDualWriter(CurrentMode, legacy, storage)
 	return &DualWriter{
 		Storage: storage,
-		legacy:  legacy,
+		Legacy:  legacy,
 	}
 }
 
 // Create overrides the default behavior of the Storage and writes to both the LegacyStorage and Storage.
 func (d *DualWriter) Create(ctx context.Context, obj runtime.Object, createValidation rest.ValidateObjectFunc, options *metav1.CreateOptions) (runtime.Object, error) {
-	if legacy, ok := d.legacy.(rest.Creater); ok {
+	if legacy, ok := d.Legacy.(rest.Creater); ok {
 		created, err := legacy.Create(ctx, obj, createValidation, options)
 		if err != nil {
 			return nil, err
@@ -113,7 +115,7 @@ func (d *DualWriter) Create(ctx context.Context, obj runtime.Object, createValid
 
 // Update overrides the default behavior of the Storage and writes to both the LegacyStorage and Storage.
 func (d *DualWriter) Update(ctx context.Context, name string, objInfo rest.UpdatedObjectInfo, createValidation rest.ValidateObjectFunc, updateValidation rest.ValidateObjectUpdateFunc, forceAllowCreate bool, options *metav1.UpdateOptions) (runtime.Object, bool, error) {
-	if legacy, ok := d.legacy.(rest.Updater); ok {
+	if legacy, ok := d.Legacy.(rest.Updater); ok {
 		// Get the previous version from k8s storage (the one)
 		old, err := d.Get(ctx, name, &metav1.GetOptions{})
 		if err != nil {
@@ -168,7 +170,7 @@ func (d *DualWriter) Delete(ctx context.Context, name string, deleteValidation r
 	// Delete from storage *first* so the item is still exists if a failure happens
 	obj, async, err := d.Storage.Delete(ctx, name, deleteValidation, options)
 	if err == nil {
-		if legacy, ok := d.legacy.(rest.GracefulDeleter); ok {
+		if legacy, ok := d.Legacy.(rest.GracefulDeleter); ok {
 			obj, async, err = legacy.Delete(ctx, name, deleteValidation, options)
 		}
 	}
@@ -179,7 +181,7 @@ func (d *DualWriter) Delete(ctx context.Context, name string, deleteValidation r
 func (d *DualWriter) DeleteCollection(ctx context.Context, deleteValidation rest.ValidateObjectFunc, options *metav1.DeleteOptions, listOptions *metainternalversion.ListOptions) (runtime.Object, error) {
 	out, err := d.Storage.DeleteCollection(ctx, deleteValidation, options, listOptions)
 	if err == nil {
-		if legacy, ok := d.legacy.(rest.CollectionDeleter); ok {
+		if legacy, ok := d.Legacy.(rest.CollectionDeleter); ok {
 			out, err = legacy.DeleteCollection(ctx, deleteValidation, options, listOptions)
 		}
 	}

diff --git a/pkg/apiserver/rest/dualwriter_mode1.go b/pkg/apiserver/rest/dualwriter_mode1.go
@@ -2,17 +2,20 @@ package rest
 
 import (
 	"context"
-	"fmt"
+	"errors"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apiserver/pkg/registry/rest"
+	"k8s.io/klog/v2"
 )
 
 type DualWriterMode1 struct {
 	DualWriter
 }
 
+var errNoCreaterMethod = errors.New("legacy storage rest.Creater is missing")
+
 // NewDualWriterMode1 returns a new DualWriter in mode 1.
 // Mode 1 represents writing to and reading from LegacyStorage.
 func NewDualWriterMode1(legacy LegacyStorage, storage Storage) *DualWriterMode1 {
@@ -21,9 +24,10 @@ func NewDualWriterMode1(legacy LegacyStorage, storage Storage) *DualWriterMode1
 
 // Create overrides the default behavior of the DualWriter and writes only to LegacyStorage.
 func (d *DualWriterMode1) Create(ctx context.Context, obj runtime.Object, createValidation rest.ValidateObjectFunc, options *metav1.CreateOptions) (runtime.Object, error) {
-	legacy, ok := d.legacy.(rest.Creater)
+	legacy, ok := d.Legacy.(rest.Creater)
 	if !ok {
-		return nil, fmt.Errorf("legacy storage rest.Creater is missing")
+		klog.FromContext(ctx).Error(errNoCreaterMethod, "legacy storage rest.Creater is missing")
+		return nil, errNoCreaterMethod
 	}
 
 	return legacy.Create(ctx, obj, createValidation, options)

diff --git a/pkg/apiserver/rest/dualwriter_mode2.go b/pkg/apiserver/rest/dualwriter_mode2.go
@@ -8,7 +8,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apiserver/pkg/registry/rest"
-	"k8s.io/klog"
+	"k8s.io/klog/v2"
 )
 
 type DualWriterMode2 struct {
@@ -23,13 +23,14 @@ func NewDualWriterMode2(legacy LegacyStorage, storage Storage) *DualWriterMode2
 
 // Create overrides the default behavior of the DualWriter and writes to LegacyStorage and Storage.
 func (d *DualWriterMode2) Create(ctx context.Context, obj runtime.Object, createValidation rest.ValidateObjectFunc, options *metav1.CreateOptions) (runtime.Object, error) {
-	legacy, ok := d.legacy.(rest.Creater)
+	legacy, ok := d.Legacy.(rest.Creater)
 	if !ok {
 		return nil, fmt.Errorf("legacy storage rest.Creater is missing")
 	}
 
 	created, err := legacy.Create(ctx, obj, createValidation, options)
 	if err != nil {
+		klog.FromContext(ctx).Error(err, "unable to create object in legacy storage", "mode", 2)
 		return created, err
 	}
 
@@ -42,12 +43,14 @@ func (d *DualWriterMode2) Create(ctx context.Context, obj runtime.Object, create
 	if err != nil {
 		return created, err
 	}
+
+	// create method expects an empty resource version
 	accessor.SetResourceVersion("")
 	accessor.SetUID("")
 
 	rsp, err := d.Storage.Create(ctx, c, createValidation, options)
 	if err != nil {
-		klog.Error("unable to create object in duplicate storage", "error", err, "mode", Mode2)
+		klog.FromContext(ctx).Error(err, "unable to create object in Storage", "mode", 2)
 	}
 	return rsp, err
 }

diff --git a/pkg/apiserver/rest/dualwriter_mode3.go b/pkg/apiserver/rest/dualwriter_mode3.go
@@ -7,7 +7,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apiserver/pkg/registry/rest"
-	"k8s.io/klog"
+	"k8s.io/klog/v2"
 )
 
 type DualWriterMode3 struct {
@@ -22,18 +22,19 @@ func NewDualWriterMode3(legacy LegacyStorage, storage Storage) *DualWriterMode3
 
 // Create overrides the default behavior of the DualWriter and writes to LegacyStorage and Storage.
 func (d *DualWriterMode3) Create(ctx context.Context, obj runtime.Object, createValidation rest.ValidateObjectFunc, options *metav1.CreateOptions) (runtime.Object, error) {
-	legacy, ok := d.legacy.(rest.Creater)
+	legacy, ok := d.Legacy.(rest.Creater)
 	if !ok {
 		return nil, fmt.Errorf("legacy storage rest.Creater is missing")
 	}
 
 	created, err := d.Storage.Create(ctx, obj, createValidation, options)
 	if err != nil {
+		klog.FromContext(ctx).Error(err, "unable to create object in Storage", "mode", 3)
 		return created, err
 	}
 
 	if _, err := legacy.Create(ctx, obj, createValidation, options); err != nil {
-		klog.Error("unable to create object in legacy storage", "error", err)
+		klog.FromContext(ctx).Error(err, "unable to create object in legacy storage", "mode", 3)
 	}
 	return created, nil
 }