From 7f94f54f53373d40992ca9325886cab1c282fd6f Mon Sep 17 00:00:00 2001 From: Max Jonas Werner Date: Fri, 20 May 2022 16:37:56 +0200 Subject: [PATCH 1/2] log when the OCI temp credentials file can't be deleted Signed-off-by: Max Jonas Werner --- controllers/helmchart_controller.go | 10 +++++++++- controllers/helmrepository_controller_oci.go | 16 +++++++++++++--- 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/controllers/helmchart_controller.go b/controllers/helmchart_controller.go index a294c8cba..5913c9aa4 100644 --- a/controllers/helmchart_controller.go +++ b/controllers/helmchart_controller.go @@ -527,7 +527,15 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj * if file != "" { defer func() { - os.Remove(file) + if err := os.Remove(file); err != nil { + r.eventLogf( + ctx, + obj, + corev1.EventTypeWarning, + meta.FailedReason, + "failed to delete temporary credentials file: %s", + err) + } }() } diff --git a/controllers/helmrepository_controller_oci.go b/controllers/helmrepository_controller_oci.go index ba2d356d6..8d8e39660 100644 --- a/controllers/helmrepository_controller_oci.go +++ b/controllers/helmrepository_controller_oci.go @@ -171,7 +171,7 @@ func (r *HelmRepositoryOCIReconciler) Reconcile(ctx context.Context, req ctrl.Re r.Metrics.RecordDuration(ctx, obj, start) }() - // Add finalizer first if not exist to avoid the race condition + // Add finalizer first if it doesn't exist to avoid the race condition // between init and delete if !controllerutil.ContainsFinalizer(obj, sourcev1.SourceFinalizer) { controllerutil.AddFinalizer(obj, sourcev1.SourceFinalizer) @@ -303,7 +303,7 @@ func (r *HelmRepositoryOCIReconciler) validateSource(ctx context.Context, obj *s registryClient, file, err := r.RegistryClientGenerator(logOpts != nil) if err != nil { e := &serror.Stalling{ - Err: fmt.Errorf("failed to create registry client:: %w", err), + Err: fmt.Errorf("failed to create registry client: %w", err), Reason: meta.FailedReason, } conditions.MarkFalse(obj, meta.ReadyCondition, e.Reason, e.Err.Error()) @@ -312,7 +312,17 @@ func (r *HelmRepositoryOCIReconciler) validateSource(ctx context.Context, obj *s if file != "" { defer func() { - os.Remove(file) + if err := os.Remove(file); err != nil { + log := ctrl.LoggerFrom(ctx) + log.Error(err, "failed to delete temporary credentials file") + r.Eventf( + obj, + corev1.EventTypeWarning, + meta.FailedReason, + "failed to delete temporary credentials file: %s", + err, + ) + } }() } From 1e899521022e801f0babee32070494052364f033 Mon Sep 17 00:00:00 2001 From: Max Jonas Werner Date: Mon, 23 May 2022 17:32:31 +0200 Subject: [PATCH 2/2] introduce eventLogf to HelmRepositoryOCIReconciler; fix formatting Signed-off-by: Max Jonas Werner --- controllers/helmchart_controller.go | 9 ++---- controllers/helmrepository_controller_oci.go | 29 ++++++++++++++------ 2 files changed, 22 insertions(+), 16 deletions(-) diff --git a/controllers/helmchart_controller.go b/controllers/helmchart_controller.go index 5913c9aa4..1198adb3c 100644 --- a/controllers/helmchart_controller.go +++ b/controllers/helmchart_controller.go @@ -528,13 +528,8 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj * if file != "" { defer func() { if err := os.Remove(file); err != nil { - r.eventLogf( - ctx, - obj, - corev1.EventTypeWarning, - meta.FailedReason, - "failed to delete temporary credentials file: %s", - err) + r.eventLogf(ctx, obj, corev1.EventTypeWarning, meta.FailedReason, + "failed to delete temporary credentials file: %s", err) } }() } diff --git a/controllers/helmrepository_controller_oci.go b/controllers/helmrepository_controller_oci.go index 8d8e39660..ec4330afb 100644 --- a/controllers/helmrepository_controller_oci.go +++ b/controllers/helmrepository_controller_oci.go @@ -18,6 +18,7 @@ package controllers import ( "context" + "errors" "fmt" "os" "strings" @@ -38,6 +39,7 @@ import ( helmgetter "helm.sh/helm/v3/pkg/getter" helmreg "helm.sh/helm/v3/pkg/registry" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" kuberecorder "k8s.io/client-go/tools/record" ctrl "sigs.k8s.io/controller-runtime" @@ -313,15 +315,8 @@ func (r *HelmRepositoryOCIReconciler) validateSource(ctx context.Context, obj *s if file != "" { defer func() { if err := os.Remove(file); err != nil { - log := ctrl.LoggerFrom(ctx) - log.Error(err, "failed to delete temporary credentials file") - r.Eventf( - obj, - corev1.EventTypeWarning, - meta.FailedReason, - "failed to delete temporary credentials file: %s", - err, - ) + r.eventLogf(ctx, obj, corev1.EventTypeWarning, meta.FailedReason, + "failed to delete temporary credentials file: %s", err) } }() } @@ -362,3 +357,19 @@ func (r *HelmRepositoryOCIReconciler) validateSource(ctx context.Context, obj *s return sreconcile.ResultSuccess, nil } + +// eventLogf records events, and logs at the same time. +// +// This log is different from the debug log in the EventRecorder, in the sense +// that this is a simple log. While the debug log contains complete details +// about the event. +func (r *HelmRepositoryOCIReconciler) eventLogf(ctx context.Context, obj runtime.Object, eventType string, reason string, messageFmt string, args ...interface{}) { + msg := fmt.Sprintf(messageFmt, args...) + // Log and emit event. + if eventType == corev1.EventTypeWarning { + ctrl.LoggerFrom(ctx).Error(errors.New(reason), msg) + } else { + ctrl.LoggerFrom(ctx).Info(msg) + } + r.Eventf(obj, eventType, reason, msg) +}