Skip to content

Make cluster operator diff messages shorter for events #784

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
mfojtik wants to merge 1 commit into from
Closed

Make cluster operator diff messages shorter for events #784

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 16 additions & 4 deletions pkg/config/clusteroperator/v1helpers/status.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,20 +62,28 @@ func FindStatusCondition(conditions []configv1.ClusterOperatorStatusCondition, c
}

// GetStatusDiff returns a string representing change in condition status in human readable form.
func GetStatusDiff(oldStatus configv1.ClusterOperatorStatus, newStatus configv1.ClusterOperatorStatus) string {
func GetStatusDiff(oldStatus configv1.ClusterOperatorStatus, newStatus configv1.ClusterOperatorStatus, forEvent bool) string {
messages := []string{}
for _, newCondition := range newStatus.Conditions {
existingStatusCondition := FindStatusCondition(oldStatus.Conditions, newCondition.Type)
if existingStatusCondition == nil {
messages = append(messages, fmt.Sprintf("%s set to %s (%q)", newCondition.Type, newCondition.Status, newCondition.Message))
condMessage := ""
if !forEvent {
condMessage = fmt.Sprintf(" (%s)", newCondition.Message)
}
messages = append(messages, fmt.Sprintf("%s set to %s%s", newCondition.Type, newCondition.Status, condMessage))
continue
}
if existingStatusCondition.Status != newCondition.Status {
messages = append(messages, fmt.Sprintf("%s changed from %s to %s (%q)", existingStatusCondition.Type, existingStatusCondition.Status, newCondition.Status, newCondition.Message))
if forEvent {
messages = append(messages, fmt.Sprintf("%s changed from %s to %s", existingStatusCondition.Type, existingStatusCondition.Status, newCondition.Status))
continue
}
messages = append(messages, fmt.Sprintf("%s changed from %s (%s) to %s (%s)", existingStatusCondition.Type, existingStatusCondition.Status, existingStatusCondition.Message, newCondition.Status, newCondition.Message))
continue
}
if existingStatusCondition.Message != newCondition.Message {
messages = append(messages, fmt.Sprintf("%s message changed from %q to %q", existingStatusCondition.Type, existingStatusCondition.Message, newCondition.Message))
messages = append(messages, fmt.Sprintf("%s message changed to %q", existingStatusCondition.Type, newCondition.Message))
}
}
for _, oldCondition := range oldStatus.Conditions {
Expand All @@ -101,6 +109,10 @@ func GetStatusDiff(oldStatus configv1.ClusterOperatorStatus, newStatus configv1.
messages = append(messages, diff.StringDiff(originalJSON.String(), newJSON.String()))
}

if !forEvent {
return strings.Join(messages, ",\n")
}

return strings.Join(messages, ",")
}

Expand Down
82 changes: 77 additions & 5 deletions pkg/config/clusteroperator/v1helpers/status_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ func TestGetStatusConditionDiff(t *testing.T) {
newConditions []configv1.ClusterOperatorStatusCondition
oldConditions []configv1.ClusterOperatorStatusCondition
expectedMessages []string
forEvent bool
}{
{
name: "new condition",
Expand All @@ -24,7 +25,37 @@ func TestGetStatusConditionDiff(t *testing.T) {
Message: "test",
},
},
expectedMessages: []string{`RetrievedUpdates set to True ("test")`},
expectedMessages: []string{`RetrievedUpdates set to True`},
forEvent: true,
},
{
name: "new multiple condition",
newConditions: []configv1.ClusterOperatorStatusCondition{
{
Type: configv1.RetrievedUpdates,
Status: configv1.ConditionTrue,
Message: "test",
},
{
Type: "AnotherDegraded",
Status: configv1.ConditionTrue,
Message: "bar",
},
},
expectedMessages: []string{"RetrievedUpdates set to True (test)", "\nAnotherDegraded set to True (bar)"},
forEvent: false,
},
{
name: "new condition for log",
newConditions: []configv1.ClusterOperatorStatusCondition{
{
Type: configv1.RetrievedUpdates,
Status: configv1.ConditionTrue,
Message: "test",
},
},
expectedMessages: []string{`RetrievedUpdates set to True (test)`},
forEvent: false,
},
{
name: "condition status change",
Expand All @@ -42,7 +73,27 @@ func TestGetStatusConditionDiff(t *testing.T) {
Message: "test",
},
},
expectedMessages: []string{`RetrievedUpdates changed from True to False ("test")`},
expectedMessages: []string{`RetrievedUpdates changed from True to False`},
forEvent: true,
},
{
name: "condition status change for log",
newConditions: []configv1.ClusterOperatorStatusCondition{
{
Type: configv1.RetrievedUpdates,
Status: configv1.ConditionFalse,
Message: "test",
},
},
oldConditions: []configv1.ClusterOperatorStatusCondition{
{
Type: configv1.RetrievedUpdates,
Status: configv1.ConditionTrue,
Message: "test",
},
},
expectedMessages: []string{`RetrievedUpdates changed from True (test) to False (test)`},
forEvent: false,
},
{
name: "condition message change",
Expand All @@ -60,7 +111,27 @@ func TestGetStatusConditionDiff(t *testing.T) {
Message: "bar",
},
},
expectedMessages: []string{`RetrievedUpdates message changed from "bar" to "foo"`},
expectedMessages: []string{`RetrievedUpdates message changed to "foo"`},
forEvent: true,
},
{
name: "condition message change for log",
newConditions: []configv1.ClusterOperatorStatusCondition{
{
Type: configv1.RetrievedUpdates,
Status: configv1.ConditionTrue,
Message: "foo",
},
},
oldConditions: []configv1.ClusterOperatorStatusCondition{
{
Type: configv1.RetrievedUpdates,
Status: configv1.ConditionTrue,
Message: "bar",
},
},
expectedMessages: []string{`RetrievedUpdates message changed to "foo"`},
forEvent: false,
},
{
name: "condition message deleted",
Expand All @@ -72,13 +143,14 @@ func TestGetStatusConditionDiff(t *testing.T) {
},
},
expectedMessages: []string{"RetrievedUpdates was removed"},
forEvent: true,
},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
result := GetStatusDiff(configv1.ClusterOperatorStatus{Conditions: test.oldConditions}, configv1.ClusterOperatorStatus{Conditions: test.newConditions})
result := GetStatusDiff(configv1.ClusterOperatorStatus{Conditions: test.oldConditions}, configv1.ClusterOperatorStatus{Conditions: test.newConditions}, test.forEvent)
if !reflect.DeepEqual(test.expectedMessages, strings.Split(result, ",")) {
t.Errorf("expected %#v, got %#v", test.expectedMessages, result)
t.Errorf("expected %#v, got %#v", test.expectedMessages, strings.Split(result, ","))
}
})
}
Expand Down
6 changes: 4 additions & 2 deletions pkg/operator/status/status_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,8 @@ func (c StatusSyncer) Sync(ctx context.Context, syncCtx factory.SyncContext) err
if _, updateErr := c.clusterOperatorClient.ClusterOperators().UpdateStatus(ctx, clusterOperatorObj, metav1.UpdateOptions{}); err != nil {
return updateErr
}
syncCtx.Recorder().Eventf("OperatorStatusChanged", "Status for operator %s changed: %s", c.clusterOperatorName, configv1helpers.GetStatusDiff(originalClusterOperatorObj.Status, clusterOperatorObj.Status))
syncCtx.Recorder().Eventf("OperatorStatusChanged", "Status for operator %s changed: %s", c.clusterOperatorName, configv1helpers.GetStatusDiff(originalClusterOperatorObj.Status, clusterOperatorObj.Status, true))
klog.Infof("Status for operator %s changed: %s", c.clusterOperatorName, configv1helpers.GetStatusDiff(originalClusterOperatorObj.Status, clusterOperatorObj.Status, false))
return nil
}

Expand Down Expand Up @@ -174,7 +175,8 @@ func (c StatusSyncer) Sync(ctx context.Context, syncCtx factory.SyncContext) err
if _, updateErr := c.clusterOperatorClient.ClusterOperators().UpdateStatus(ctx, clusterOperatorObj, metav1.UpdateOptions{}); err != nil {
return updateErr
}
syncCtx.Recorder().Eventf("OperatorStatusChanged", "Status for clusteroperator/%s changed: %s", c.clusterOperatorName, configv1helpers.GetStatusDiff(originalClusterOperatorObj.Status, clusterOperatorObj.Status))
syncCtx.Recorder().Eventf("OperatorStatusChanged", "Status for clusteroperator/%s changed: %s", c.clusterOperatorName, configv1helpers.GetStatusDiff(originalClusterOperatorObj.Status, clusterOperatorObj.Status, true))
klog.Infof("Status for clusteroperator/%s changed: %s", c.clusterOperatorName, configv1helpers.GetStatusDiff(originalClusterOperatorObj.Status, clusterOperatorObj.Status, true))
return nil
}

Expand Down