Skip to content

Commit

Permalink
fix: message formatting not passed in (#432)
Browse files Browse the repository at this point in the history
## Description

<!-- 
Please do not leave this blank 
This PR [adds/removes/fixes/replaces] the [feature/bug/etc]. 
-->

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

<!-- 
Please use this format link issue numbers: Fixes #123

https://docs.github.com/en/free-pro-team@latest/github/managing-your-work-on-github/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword
-->
- Related Issue # (issue)
- Closes # (issue)
- Fixes # (issue)
> Remove if not applicable

## Screenshots

<!-- Visual changes require 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
  • Loading branch information
Skarlso authored May 27, 2024
1 parent 212aa6e commit 422354c
Show file tree
Hide file tree
Showing 8 changed files with 18 additions and 22 deletions.
9 changes: 5 additions & 4 deletions controllers/componentversion_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
Expand Down
7 changes: 3 additions & 4 deletions controllers/fluxdeployer_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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()

Expand Down
2 changes: 1 addition & 1 deletion controllers/resource_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
3 changes: 1 addition & 2 deletions controllers/snapshot_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions pkg/event/event.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
}
Expand All @@ -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...)
}
2 changes: 1 addition & 1 deletion pkg/event/event_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
6 changes: 3 additions & 3 deletions pkg/status/mutate_condition_status.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,20 +12,20 @@ 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`.
func MarkAsStalled(recorder kuberecorder.EventRecorder, obj conditions.Setter, reason, msg string) {
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...)
}
7 changes: 2 additions & 5 deletions pkg/status/register_status_defer.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package status

import (
"context"
"fmt"
"time"

eventv1 "github.com/fluxcd/pkg/apis/event/v1beta1"
Expand All @@ -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.
Expand Down

0 comments on commit 422354c

Please sign in to comment.