From d18a27d189a63f9310677dea6bb8c5eb1cb4a05e Mon Sep 17 00:00:00 2001 From: Pradyot Ranjan <99216956+prady0t@users.noreply.github.com> Date: Wed, 19 Jul 2023 14:45:08 +0530 Subject: [PATCH] Added log message for API call failures (#7834) * Added error message to deferred loader on API call failure Signed-off-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com> * Small change in error message Signed-off-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com> --------- Signed-off-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com> Co-authored-by: Pradyot Ranjan <99216956+pradyotRanjan@users.noreply.github.com> Co-authored-by: Chip Zoller Co-authored-by: shuting Co-authored-by: Jim Bugwadia --- pkg/engine/context/deferred.go | 11 +++++++++-- pkg/engine/context/deferred_test.go | 2 +- pkg/engine/factories/contextloaderfactory.go | 16 ++++++++-------- 3 files changed, 18 insertions(+), 11 deletions(-) diff --git a/pkg/engine/context/deferred.go b/pkg/engine/context/deferred.go index 6545b21128cf..fc862381c044 100644 --- a/pkg/engine/context/deferred.go +++ b/pkg/engine/context/deferred.go @@ -2,15 +2,18 @@ package context import ( "regexp" + + "github.com/go-logr/logr" ) type deferredLoader struct { name string matcher regexp.Regexp loader Loader + logger logr.Logger } -func NewDeferredLoader(name string, loader Loader) (DeferredLoader, error) { +func NewDeferredLoader(name string, loader Loader, logger logr.Logger) (DeferredLoader, error) { // match on ASCII word boundaries except do not allow starting with a `.` // this allows `x` to match `x.y` but not `y.x` or `y.x.z` matcher, err := regexp.Compile(`(?:\A|\z|\s|[^.0-9A-Za-z])` + name + `\b`) @@ -22,6 +25,7 @@ func NewDeferredLoader(name string, loader Loader) (DeferredLoader, error) { name: name, matcher: *matcher, loader: loader, + logger: logger, }, nil } @@ -34,7 +38,10 @@ func (dl *deferredLoader) HasLoaded() bool { } func (dl *deferredLoader) LoadData() error { - return dl.loader.LoadData() + if err := dl.loader.LoadData(); err != nil { + dl.logger.Error(err, "failed to load data", "name", dl.name) + } + return nil } func (d *deferredLoader) Matches(query string) bool { diff --git a/pkg/engine/context/deferred_test.go b/pkg/engine/context/deferred_test.go index b9efd2cfa939..7a08c8d76714 100644 --- a/pkg/engine/context/deferred_test.go +++ b/pkg/engine/context/deferred_test.go @@ -174,7 +174,7 @@ func addDeferredWithQuery(ctx *context, name string, value interface{}, query st query: query, } - d, err := NewDeferredLoader(name, loader) + d, err := NewDeferredLoader(name, loader, logger) if err != nil { return loader, err } diff --git a/pkg/engine/factories/contextloaderfactory.go b/pkg/engine/factories/contextloaderfactory.go index b288c8225f1f..12b8b47ae83b 100644 --- a/pkg/engine/factories/contextloaderfactory.go +++ b/pkg/engine/factories/contextloaderfactory.go @@ -84,31 +84,31 @@ func (l *contextLoader) newLoader( ) (enginecontext.DeferredLoader, error) { if entry.ConfigMap != nil { if l.cmResolver != nil { - l := loaders.NewConfigMapLoader(ctx, l.logger, entry, l.cmResolver, jsonContext) - return enginecontext.NewDeferredLoader(entry.Name, l) + ldr := loaders.NewConfigMapLoader(ctx, l.logger, entry, l.cmResolver, jsonContext) + return enginecontext.NewDeferredLoader(entry.Name, ldr, l.logger) } else { l.logger.Info("disabled loading of ConfigMap context entry %s", entry.Name) return nil, nil } } else if entry.APICall != nil { if client != nil { - l := loaders.NewAPILoader(ctx, l.logger, entry, jsonContext, jp, client) - return enginecontext.NewDeferredLoader(entry.Name, l) + ldr := loaders.NewAPILoader(ctx, l.logger, entry, jsonContext, jp, client) + return enginecontext.NewDeferredLoader(entry.Name, ldr, l.logger) } else { l.logger.Info("disabled loading of APICall context entry %s", entry.Name) return nil, nil } } else if entry.ImageRegistry != nil { if rclientFactory != nil { - l := loaders.NewImageDataLoader(ctx, l.logger, entry, jsonContext, jp, rclientFactory) - return enginecontext.NewDeferredLoader(entry.Name, l) + ldr := loaders.NewImageDataLoader(ctx, l.logger, entry, jsonContext, jp, rclientFactory) + return enginecontext.NewDeferredLoader(entry.Name, ldr, l.logger) } else { l.logger.Info("disabled loading of ImageRegistry context entry %s", entry.Name) return nil, nil } } else if entry.Variable != nil { - l := loaders.NewVariableLoader(l.logger, entry, jsonContext, jp) - return enginecontext.NewDeferredLoader(entry.Name, l) + ldr := loaders.NewVariableLoader(l.logger, entry, jsonContext, jp) + return enginecontext.NewDeferredLoader(entry.Name, ldr, l.logger) } return nil, fmt.Errorf("missing ConfigMap|APICall|ImageRegistry|Variable in context entry %s", entry.Name) }