From 16ff7f6ac972eb01577fc96d37dce3d2eb8febe7 Mon Sep 17 00:00:00 2001 From: TheiLLeniumStudios <104288623+TheiLLeniumStudios@users.noreply.github.com> Date: Fri, 9 Jan 2026 15:27:19 +0100 Subject: [PATCH] fix: Default reconcile metric result to error for panic safety --- internal/pkg/handler/create.go | 33 +++++++++++++------------- internal/pkg/handler/delete.go | 33 +++++++++++++------------- internal/pkg/handler/update.go | 42 +++++++++++++++++----------------- 3 files changed, 53 insertions(+), 55 deletions(-) diff --git a/internal/pkg/handler/create.go b/internal/pkg/handler/create.go index 5fd3014..d676610 100644 --- a/internal/pkg/handler/create.go +++ b/internal/pkg/handler/create.go @@ -27,7 +27,7 @@ func (r ResourceCreatedHandler) GetEnqueueTime() time.Time { // Handle processes the newly created resource func (r ResourceCreatedHandler) Handle() error { startTime := time.Now() - result := "success" + result := "error" defer func() { r.Collectors.RecordReconcile(result, time.Since(startTime)) @@ -35,25 +35,24 @@ func (r ResourceCreatedHandler) Handle() error { if r.Resource == nil { logrus.Errorf("Resource creation handler received nil resource") - result = "error" - } else { - config, _ := r.GetConfig() - // Send webhook - if options.WebhookUrl != "" { - err := sendUpgradeWebhook(config, options.WebhookUrl) - if err != nil { - result = "error" - } - return err - } - // process resource based on its type - err := doRollingUpgrade(config, r.Collectors, r.Recorder, invokeReloadStrategy) - if err != nil { - result = "error" + return nil + } + + config, _ := r.GetConfig() + // Send webhook + if options.WebhookUrl != "" { + err := sendUpgradeWebhook(config, options.WebhookUrl) + if err == nil { + result = "success" } return err } - return nil + // process resource based on its type + err := doRollingUpgrade(config, r.Collectors, r.Recorder, invokeReloadStrategy) + if err == nil { + result = "success" + } + return err } // GetConfig gets configurations containing SHA, annotations, namespace and resource name diff --git a/internal/pkg/handler/delete.go b/internal/pkg/handler/delete.go index 243602c..34e032b 100644 --- a/internal/pkg/handler/delete.go +++ b/internal/pkg/handler/delete.go @@ -35,7 +35,7 @@ func (r ResourceDeleteHandler) GetEnqueueTime() time.Time { // Handle processes resources being deleted func (r ResourceDeleteHandler) Handle() error { startTime := time.Now() - result := "success" + result := "error" defer func() { r.Collectors.RecordReconcile(result, time.Since(startTime)) @@ -43,25 +43,24 @@ func (r ResourceDeleteHandler) Handle() error { if r.Resource == nil { logrus.Errorf("Resource delete handler received nil resource") - result = "error" - } else { - config, _ := r.GetConfig() - // Send webhook - if options.WebhookUrl != "" { - err := sendUpgradeWebhook(config, options.WebhookUrl) - if err != nil { - result = "error" - } - return err - } - // process resource based on its type - err := doRollingUpgrade(config, r.Collectors, r.Recorder, invokeDeleteStrategy) - if err != nil { - result = "error" + return nil + } + + config, _ := r.GetConfig() + // Send webhook + if options.WebhookUrl != "" { + err := sendUpgradeWebhook(config, options.WebhookUrl) + if err == nil { + result = "success" } return err } - return nil + // process resource based on its type + err := doRollingUpgrade(config, r.Collectors, r.Recorder, invokeDeleteStrategy) + if err == nil { + result = "success" + } + return err } // GetConfig gets configurations containing SHA, annotations, namespace and resource name diff --git a/internal/pkg/handler/update.go b/internal/pkg/handler/update.go index 1d0403b..3fde98e 100644 --- a/internal/pkg/handler/update.go +++ b/internal/pkg/handler/update.go @@ -30,7 +30,7 @@ func (r ResourceUpdatedHandler) GetEnqueueTime() time.Time { // Handle processes the updated resource func (r ResourceUpdatedHandler) Handle() error { startTime := time.Now() - result := "success" + result := "error" defer func() { r.Collectors.RecordReconcile(result, time.Since(startTime)) @@ -38,30 +38,30 @@ func (r ResourceUpdatedHandler) Handle() error { if r.Resource == nil || r.OldResource == nil { logrus.Errorf("Resource update handler received nil resource") - result = "error" - } else { - config, oldSHAData := r.GetConfig() - if config.SHAValue != oldSHAData { - // Send a webhook if update - if options.WebhookUrl != "" { - err := sendUpgradeWebhook(config, options.WebhookUrl) - if err != nil { - result = "error" - } - return err - } - // process resource based on its type - err := doRollingUpgrade(config, r.Collectors, r.Recorder, invokeReloadStrategy) - if err != nil { - result = "error" + return nil + } + + config, oldSHAData := r.GetConfig() + if config.SHAValue != oldSHAData { + // Send a webhook if update + if options.WebhookUrl != "" { + err := sendUpgradeWebhook(config, options.WebhookUrl) + if err == nil { + result = "success" } return err - } else { - // No data change - skip - result = "skipped" - r.Collectors.RecordSkipped("no_data_change") } + // process resource based on its type + err := doRollingUpgrade(config, r.Collectors, r.Recorder, invokeReloadStrategy) + if err == nil { + result = "success" + } + return err } + + // No data change - skip + result = "skipped" + r.Collectors.RecordSkipped("no_data_change") return nil }