From 422354c874d28662a05f41a312420bb36bc9a502 Mon Sep 17 00:00:00 2001 From: Gergely Brautigam <182850+Skarlso@users.noreply.github.com> Date: Mon, 27 May 2024 10:32:44 +0200 Subject: [PATCH] fix: message formatting not passed in (#432) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description Please include a summary of the changes and the related issue. Please also include relevant motivation and context. List any dependencies that are required for this change. ## What type of PR is this? (check all applicable) - [ ] 🍕 Feature - [ ] 🐛 Bug Fix - [ ] 📝 Documentation Update - [ ] 🎨 Style - [ ] 🧑‍💻 Code Refactor - [ ] 🔥 Performance Improvements - [ ] ✅ Test - [ ] 🤖 Build - [ ] 🔁 CI - [ ] 📦 Chore (Release) - [ ] ⏩ Revert ## Related Tickets & Documents - Related Issue # (issue) - Closes # (issue) - Fixes # (issue) > Remove if not applicable ## Screenshots ## Added tests? - [ ] 👍 yes - [ ] 🙅 no, because they aren't needed - [ ] 🙋 no, because I need help - [ ] Separate ticket for tests # (issue/pr) Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration ## Added to documentation? - [ ] 📜 README.md - [ ] 🙅 no documentation needed ## Checklist: - [ ] My code follows the style guidelines of this project - [ ] I have performed a self-review of my code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [ ] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] New and existing unit tests pass locally with my changes - [ ] Any dependent changes have been merged and published in downstream modules --- controllers/componentversion_controller.go | 9 +++++---- controllers/fluxdeployer_controller.go | 7 +++---- controllers/resource_controller.go | 2 +- controllers/snapshot_controller.go | 3 +-- pkg/event/event.go | 4 ++-- pkg/event/event_test.go | 2 +- pkg/status/mutate_condition_status.go | 6 +++--- pkg/status/register_status_defer.go | 7 ++----- 8 files changed, 18 insertions(+), 22 deletions(-) diff --git a/controllers/componentversion_controller.go b/controllers/componentversion_controller.go index 70377e8f..a9486192 100644 --- a/controllers/componentversion_controller.go +++ b/controllers/componentversion_controller.go @@ -189,7 +189,7 @@ func (r *ComponentVersionReconciler) Reconcile(ctx context.Context, req ctrl.Req } if !update { - status.MarkReady(r.EventRecorder, obj, fmt.Sprintf("Applied version: %s", version)) + status.MarkReady(r.EventRecorder, obj, "Applied version: %s", version) return ctrl.Result{ RequeueAfter: obj.GetRequeueAfter(), @@ -339,7 +339,7 @@ func (r *ComponentVersionReconciler) reconcile( metrics.MPASComponentVersionReconciledStatus.WithLabelValues(product, mh.MPASStatusSuccess).Inc() } - status.MarkReady(r.EventRecorder, obj, fmt.Sprintf("Applied version: %s", version)) + status.MarkReady(r.EventRecorder, obj, "Applied version: %s", version) return ctrl.Result{RequeueAfter: obj.GetRequeueAfter()}, nil } @@ -377,9 +377,10 @@ func (r *ComponentVersionReconciler) checkVersion(ctx context.Context, octx ocm. event.New( r.EventRecorder, obj, - eventv1.EventSeverityInfo, - fmt.Sprintf("Version check succeeded, found latest version: %s", latest), nil, + eventv1.EventSeverityInfo, + "Version check succeeded, found latest version: %s", + latest, ) return true, latest, nil diff --git a/controllers/fluxdeployer_controller.go b/controllers/fluxdeployer_controller.go index c8af444a..a469d14a 100644 --- a/controllers/fluxdeployer_controller.go +++ b/controllers/fluxdeployer_controller.go @@ -202,7 +202,7 @@ func (r *FluxDeployerReconciler) reconcile( v1alpha1.CreateOrUpdateKustomizationFailedReason, err.Error(), ) - event.New(r.EventRecorder, obj, eventv1.EventSeverityError, msg, nil) + event.New(r.EventRecorder, obj, nil, eventv1.EventSeverityError, msg) return ctrl.Result{}, err } @@ -228,14 +228,13 @@ func (r *FluxDeployerReconciler) reconcile( err.Error(), ) conditions.MarkStalled(obj, v1alpha1.CreateOrUpdateHelmFailedReason, err.Error()) - event.New(r.EventRecorder, obj, eventv1.EventSeverityError, msg, nil) + event.New(r.EventRecorder, obj, nil, eventv1.EventSeverityError, msg) return ctrl.Result{}, err } } - msg := fmt.Sprintf("FluxDeployer '%s' is ready", obj.Name) - status.MarkReady(r.EventRecorder, obj, msg) + status.MarkReady(r.EventRecorder, obj, "FluxDeployer '%s' is ready", obj.Name) metrics.FluxDeployerReconcileSuccess.WithLabelValues(obj.Name).Inc() diff --git a/controllers/resource_controller.go b/controllers/resource_controller.go index 1fc543c7..2d9a63f3 100644 --- a/controllers/resource_controller.go +++ b/controllers/resource_controller.go @@ -284,7 +284,7 @@ func (r *ResourceReconciler) reconcile( metrics.MPASResourceReconciledStatus.WithLabelValues(product, mh.MPASStatusSuccess).Inc() } - status.MarkReady(r.EventRecorder, obj, fmt.Sprintf("Applied version: %s", obj.Status.LastAppliedComponentVersion)) + status.MarkReady(r.EventRecorder, obj, "Applied version: %s", obj.Status.LastAppliedComponentVersion) return ctrl.Result{RequeueAfter: obj.GetRequeueAfter()}, nil } diff --git a/controllers/snapshot_controller.go b/controllers/snapshot_controller.go index 6437992d..7c524320 100644 --- a/controllers/snapshot_controller.go +++ b/controllers/snapshot_controller.go @@ -128,8 +128,7 @@ func (r *SnapshotReconciler) Reconcile(ctx context.Context, req ctrl.Request) (r } obj.Status.RepositoryURL = fmt.Sprintf("%s://%s/%s", scheme, r.RegistryServiceName, name) - msg := fmt.Sprintf("Snapshot with name '%s' is ready", obj.Name) - status.MarkReady(r.EventRecorder, obj, msg) + status.MarkReady(r.EventRecorder, obj, "Snapshot with name '%s' is ready", obj.Name) metrics.SnapshotReconcileSuccess.WithLabelValues(obj.Name).Inc() return ctrl.Result{}, nil diff --git a/pkg/event/event.go b/pkg/event/event.go index 76dc4fac..735c94a4 100644 --- a/pkg/event/event.go +++ b/pkg/event/event.go @@ -13,7 +13,7 @@ import ( kuberecorder "k8s.io/client-go/tools/record" ) -func New(recorder kuberecorder.EventRecorder, obj conditions.Getter, severity, msg string, metadata map[string]string) { +func New(recorder kuberecorder.EventRecorder, obj conditions.Getter, metadata map[string]string, severity, msg string, args ...any) { if metadata == nil { metadata = map[string]string{} } @@ -28,5 +28,5 @@ func New(recorder kuberecorder.EventRecorder, obj conditions.Getter, severity, m eventType = corev1.EventTypeWarning } - recorder.AnnotatedEventf(obj, metadata, eventType, reason, msg) + recorder.AnnotatedEventf(obj, metadata, eventType, reason, msg, args...) } diff --git a/pkg/event/event_test.go b/pkg/event/event_test.go index e1c747c4..5eda9eed 100644 --- a/pkg/event/event_test.go +++ b/pkg/event/event_test.go @@ -42,7 +42,7 @@ func TestNewEvent(t *testing.T) { conditions.MarkStalled(obj, v1alpha1.CheckVersionFailedReason, "err") conditions.MarkFalse(obj, meta.ReadyCondition, v1alpha1.CheckVersionFailedReason, "err") - New(recorder, obj, tt.severity, "msg", nil) + New(recorder, obj, nil, tt.severity, "msg") close(recorder.Events) for e := range recorder.Events { diff --git a/pkg/status/mutate_condition_status.go b/pkg/status/mutate_condition_status.go index d2f419bb..ca4039fc 100644 --- a/pkg/status/mutate_condition_status.go +++ b/pkg/status/mutate_condition_status.go @@ -12,7 +12,7 @@ import ( func MarkNotReady(recorder kuberecorder.EventRecorder, obj conditions.Setter, reason, msg string) { conditions.Delete(obj, meta.ReconcilingCondition) conditions.MarkFalse(obj, meta.ReadyCondition, reason, msg) - event.New(recorder, obj, eventv1.EventSeverityError, msg, nil) + event.New(recorder, obj, nil, eventv1.EventSeverityError, msg) } // MarkAsStalled sets the condition status of an Object to `Stalled`. @@ -20,12 +20,12 @@ func MarkAsStalled(recorder kuberecorder.EventRecorder, obj conditions.Setter, r conditions.Delete(obj, meta.ReconcilingCondition) conditions.MarkFalse(obj, meta.ReadyCondition, reason, msg) conditions.MarkStalled(obj, reason, msg) - event.New(recorder, obj, eventv1.EventSeverityError, msg, nil) + event.New(recorder, obj, nil, eventv1.EventSeverityError, msg) } // MarkReady sets the condition status of an Object to `Ready`. func MarkReady(recorder kuberecorder.EventRecorder, obj conditions.Setter, msg string, messageArgs ...any) { conditions.MarkTrue(obj, meta.ReadyCondition, meta.SucceededReason, msg, messageArgs...) conditions.Delete(obj, meta.ReconcilingCondition) - event.New(recorder, obj, eventv1.EventSeverityInfo, msg, nil) + event.New(recorder, obj, nil, eventv1.EventSeverityInfo, msg, messageArgs...) } diff --git a/pkg/status/register_status_defer.go b/pkg/status/register_status_defer.go index 3b48406c..3c20a3f2 100644 --- a/pkg/status/register_status_defer.go +++ b/pkg/status/register_status_defer.go @@ -2,7 +2,6 @@ package status import ( "context" - "fmt" "time" eventv1 "github.com/fluxcd/pkg/apis/event/v1beta1" @@ -27,15 +26,13 @@ func UpdateStatus( reconciling := conditions.Get(obj, meta.ReconcilingCondition) reconciling.Reason = meta.ProgressingWithRetryReason conditions.Set(obj, reconciling) - msg := fmt.Sprintf("Reconciliation did not succeed, retrying in %s", requeue) - event.New(recorder, obj, eventv1.EventSeverityError, msg, obj.GetVID()) + event.New(recorder, obj, obj.GetVID(), eventv1.EventSeverityError, "Reconciliation did not succeed, retrying in %s", requeue) } // Set status observed generation option if the component is ready. if conditions.IsReady(obj) { obj.SetObservedGeneration(obj.GetGeneration()) - msg := fmt.Sprintf("Reconciliation finished, next run in %s", requeue) - event.New(recorder, obj, eventv1.EventSeverityInfo, msg, obj.GetVID()) + event.New(recorder, obj, obj.GetVID(), eventv1.EventSeverityInfo, "Reconciliation finished, next run in %s", requeue) } // Update the object.