From 8d75dcec042339c353a90cbf296d4109031430ff Mon Sep 17 00:00:00 2001 From: Leonor Oliveira <9090754+leonorfmartins@users.noreply.github.com> Date: Tue, 9 Apr 2024 09:54:09 +0100 Subject: [PATCH] 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. <33031346+suntala@users.noreply.github.com> * Update pkg/apiserver/rest/dualwriter_mode2.go Co-authored-by: Arati R. <33031346+suntala@users.noreply.github.com> * Update pkg/apiserver/rest/dualwriter_mode3.go Co-authored-by: Arati R. <33031346+suntala@users.noreply.github.com> * Update pkg/apiserver/rest/dualwriter_mode3.go Co-authored-by: Arati R. <33031346+suntala@users.noreply.github.com> * Update pkg/apiserver/rest/dualwriter_mode2.go Co-authored-by: Arati R. <33031346+suntala@users.noreply.github.com> * Create error var * Lint --------- Co-authored-by: Arati R. <33031346+suntala@users.noreply.github.com> --- pkg/apiserver/rest/dualwriter.go | 14 ++++++++------ pkg/apiserver/rest/dualwriter_mode1.go | 10 +++++++--- pkg/apiserver/rest/dualwriter_mode2.go | 9 ++++++--- pkg/apiserver/rest/dualwriter_mode3.go | 7 ++++--- 4 files changed, 25 insertions(+), 15 deletions(-) diff --git a/pkg/apiserver/rest/dualwriter.go b/pkg/apiserver/rest/dualwriter.go index a2d5d215b95..61e5e6d3348 100644 --- 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 index 2dd7c235dc0..e153cfb95b4 100644 --- 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 index d3dbd586cc5..151c22a7f3c 100644 --- 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 index a4f02bc48e9..a6ea69b85f7 100644 --- 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 }