diff --git a/internal/operator-controller/controllers/clusterextensionrevision_controller.go b/internal/operator-controller/controllers/clusterextensionrevision_controller.go index ec035eee7..9d4b07817 100644 --- a/internal/operator-controller/controllers/clusterextensionrevision_controller.go +++ b/internal/operator-controller/controllers/clusterextensionrevision_controller.go @@ -34,6 +34,7 @@ import ( ocv1 "github.com/operator-framework/operator-controller/api/v1" "github.com/operator-framework/operator-controller/internal/operator-controller/labels" + "github.com/operator-framework/operator-controller/internal/operator-controller/util" ) const ( @@ -158,12 +159,14 @@ func (c *ClusterExtensionRevisionReconciler) reconcile(ctx context.Context, rev rres, err := c.RevisionEngine.Reconcile(ctx, *revision, opts...) if err != nil { + // Generate human-readable summary for logging if rres != nil { - l.Error(err, "revision reconcile failed") - l.V(1).Info("reconcile failure report", "report", rres.String()) + summary := util.SummarizeRevisionResult(rres) + l.Error(err, "revision reconcile failed", "summary", summary) } else { l.Error(err, "revision reconcile failed") } + meta.SetStatusCondition(&rev.Status.Conditions, metav1.Condition{ Type: ocv1.ClusterExtensionRevisionTypeAvailable, Status: metav1.ConditionFalse, @@ -320,9 +323,10 @@ func (c *ClusterExtensionRevisionReconciler) teardown(ctx context.Context, rev * tres, err := c.RevisionEngine.Teardown(ctx, *revision) if err != nil { + // Generate human-readable summary for logging if tres != nil { - l.Error(err, "revision teardown failed") - l.V(1).Info("teardown failure report", "report", tres.String()) + summary := util.SummarizeRevisionTeardownResult(tres) + l.Error(err, "revision teardown failed", "summary", summary) } else { l.Error(err, "revision teardown failed") } @@ -337,7 +341,10 @@ func (c *ClusterExtensionRevisionReconciler) teardown(ctx context.Context, rev * } // Log detailed teardown reports only in debug mode (V(1)) to reduce verbosity. - l.V(1).Info("teardown report", "report", tres.String()) + summary := util.SummarizeRevisionTeardownResult(tres) + if summary != "" { + l.Info("teardown report", "report", summary) + } if !tres.IsComplete() { // TODO: If it is not complete, it seems like it would be good to update // the status in some way to tell the user that the teardown is still diff --git a/internal/operator-controller/util/boxcutter_report.go b/internal/operator-controller/util/boxcutter_report.go new file mode 100644 index 000000000..5acb459d2 --- /dev/null +++ b/internal/operator-controller/util/boxcutter_report.go @@ -0,0 +1,271 @@ +package util + +import ( + "fmt" + "strings" + + "pkg.package-operator.run/boxcutter/machinery" +) + +// SummarizeRevisionResult creates a concise, human-readable summary of a boxcutter +// RevisionResult, extracting key information without the verbose details of String(). +// This is similar to how crdupgradesafety.conciseUnhandledMessage works for CRD diffs. +func SummarizeRevisionResult(result machinery.RevisionResult) string { + if result == nil { + return "" + } + + var parts []string + + // Check for validation errors first (using error interface) + if verr := result.GetValidationError(); verr != nil { + parts = append(parts, fmt.Sprintf("validation error: %s", verr.Error())) + } + + // Summarize phase information + phases := result.GetPhases() + if len(phases) > 0 { + phaseSummary := summarizePhases(phases) + if phaseSummary != "" { + parts = append(parts, phaseSummary) + } + } + + // Add completion status + if !result.IsComplete() { + if result.InTransistion() { + parts = append(parts, "status: in transition") + } else { + parts = append(parts, "status: incomplete") + } + } + + if len(parts) == 0 { + return "reconcile completed successfully" + } + + return strings.Join(parts, "; ") +} + +// summarizePhases creates a summary of phase results, focusing on problems +func summarizePhases(phases []machinery.PhaseResult) string { + var problemPhases []string + var successfulPhases []string + + for _, phase := range phases { + phaseName := phase.GetName() + if phaseName == "" { + phaseName = "unnamed" + } + + // Check for validation errors (using error interface) + if verr := phase.GetValidationError(); verr != nil { + problemPhases = append(problemPhases, fmt.Sprintf("%s: validation error", phaseName)) + continue + } + + // Check for object issues + objects := phase.GetObjects() + if len(objects) > 0 { + objectSummary := summarizeObjects(objects) + if objectSummary.hasIssues { + problemPhases = append(problemPhases, fmt.Sprintf("%s: %s", phaseName, objectSummary.summary)) + } else if phase.IsComplete() { + successfulPhases = append(successfulPhases, phaseName) + } + } + + // Check phase completion status + if !phase.IsComplete() && len(objects) == 0 { + problemPhases = append(problemPhases, fmt.Sprintf("%s: incomplete", phaseName)) + } + } + + var parts []string + if len(problemPhases) > 0 { + parts = append(parts, fmt.Sprintf("phases with issues: %s", strings.Join(problemPhases, ", "))) + } + if len(successfulPhases) > 0 && len(problemPhases) == 0 { + parts = append(parts, fmt.Sprintf("%d phase(s) successful", len(successfulPhases))) + } + + return strings.Join(parts, "; ") +} + +type objectSummary struct { + hasIssues bool + summary string +} + +// summarizeObjects creates a summary of object results +func summarizeObjects(objects []machinery.ObjectResult) objectSummary { + var collisions []string + var failures []string + var probeFailures []string + successCount := 0 + + for _, obj := range objects { + action := obj.Action() + success := obj.Success() + objInfo := getObjectInfo(obj.Object()) + + switch action { + case machinery.ActionCollision: + collisions = append(collisions, objInfo) + default: + if !success { + failures = append(failures, fmt.Sprintf("%s (action: %s)", objInfo, action)) + } else { + // Check probe results + probes := obj.Probes() + for probeName, probeResult := range probes { + if !probeResult.Success { + // Only include the probe name and status, as probeResult.Message doesn't exist + probeFailures = append(probeFailures, fmt.Sprintf("%s probe '%s' failed", objInfo, probeName)) + } + } + if len(probes) == 0 || allProbesSuccessful(probes) { + successCount++ + } + } + } + } + + var parts []string + if len(collisions) > 0 { + // Limit to first 3 collisions to avoid verbose output + displayed := collisions + if len(collisions) > 3 { + displayed = collisions[:3] + parts = append(parts, fmt.Sprintf("%d collision(s) [showing first 3: %s]", len(collisions), strings.Join(displayed, ", "))) + } else { + parts = append(parts, fmt.Sprintf("%d collision(s): %s", len(collisions), strings.Join(displayed, ", "))) + } + } + if len(failures) > 0 { + // Limit to first 3 failures + displayed := failures + if len(failures) > 3 { + displayed = failures[:3] + parts = append(parts, fmt.Sprintf("%d failed object(s) [showing first 3: %s]", len(failures), strings.Join(displayed, ", "))) + } else { + parts = append(parts, fmt.Sprintf("%d failed object(s): %s", len(failures), strings.Join(displayed, ", "))) + } + } + if len(probeFailures) > 0 { + // Limit to first 3 probe failures + displayed := probeFailures + if len(probeFailures) > 3 { + displayed = probeFailures[:3] + parts = append(parts, fmt.Sprintf("%d probe failure(s) [showing first 3: %s]", len(probeFailures), strings.Join(displayed, ", "))) + } else { + parts = append(parts, fmt.Sprintf("%d probe failure(s): %s", len(probeFailures), strings.Join(displayed, ", "))) + } + } + + hasIssues := len(collisions) > 0 || len(failures) > 0 || len(probeFailures) > 0 + summary := strings.Join(parts, "; ") + + if !hasIssues && successCount > 0 { + summary = fmt.Sprintf("%d object(s) applied successfully", successCount) + } + + return objectSummary{ + hasIssues: hasIssues, + summary: summary, + } +} + +// getObjectInfo extracts a human-readable identifier from an object +func getObjectInfo(obj machinery.Object) string { + if obj == nil { + return "unknown object" + } + + gvk := obj.GetObjectKind().GroupVersionKind() + name := obj.GetName() + namespace := obj.GetNamespace() + + kind := gvk.Kind + if kind == "" { + kind = "unknown" + } + + if namespace != "" { + return fmt.Sprintf("%s %s/%s", kind, namespace, name) + } + return fmt.Sprintf("%s %s", kind, name) +} + +// allProbesSuccessful checks if all probes passed +func allProbesSuccessful(probes map[string]machinery.ObjectProbeResult) bool { + for _, result := range probes { + if !result.Success { + return false + } + } + return true +} + +// SummarizeRevisionTeardownResult creates a concise summary of a teardown result +func SummarizeRevisionTeardownResult(result machinery.RevisionTeardownResult) string { + if result == nil { + return "" + } + + if result.IsComplete() { + return "teardown completed successfully" + } + + var parts []string + + // Check waiting phases + waitingPhases := result.GetWaitingPhaseNames() + if len(waitingPhases) > 0 { + parts = append(parts, fmt.Sprintf("waiting on phases: %s", strings.Join(waitingPhases, ", "))) + } + + // Summarize phase teardown + phases := result.GetPhases() + if len(phases) > 0 { + phaseSummary := summarizeTeardownPhases(phases) + if phaseSummary != "" { + parts = append(parts, phaseSummary) + } + } + + if len(parts) == 0 { + return "teardown in progress" + } + + return strings.Join(parts, "; ") +} + +// summarizeTeardownPhases creates a summary of phase teardown results +func summarizeTeardownPhases(phases []machinery.PhaseTeardownResult) string { + var incompletePhases []string + completedCount := 0 + + for _, phase := range phases { + phaseName := phase.GetName() + if phaseName == "" { + phaseName = "unnamed" + } + + if !phase.IsComplete() { + incompletePhases = append(incompletePhases, phaseName) + } else { + completedCount++ + } + } + + var parts []string + if len(incompletePhases) > 0 { + parts = append(parts, fmt.Sprintf("incomplete phases: %s", strings.Join(incompletePhases, ", "))) + } + if completedCount > 0 { + parts = append(parts, fmt.Sprintf("%d phase(s) completed", completedCount)) + } + + return strings.Join(parts, "; ") +} diff --git a/internal/operator-controller/util/boxcutter_report_test.go b/internal/operator-controller/util/boxcutter_report_test.go new file mode 100644 index 000000000..fa2ee1f05 --- /dev/null +++ b/internal/operator-controller/util/boxcutter_report_test.go @@ -0,0 +1,393 @@ +package util + +import ( + "testing" + + "github.com/stretchr/testify/assert" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" + "pkg.package-operator.run/boxcutter/machinery" + machinerytypes "pkg.package-operator.run/boxcutter/machinery/types" + "pkg.package-operator.run/boxcutter/validation" +) + +// Mock implementations for testing +type mockRevisionResult struct { + validationError *validation.RevisionValidationError + phases []machinery.PhaseResult + inTransition bool + isComplete bool + hasProgressed bool +} + +func (m mockRevisionResult) GetValidationError() *validation.RevisionValidationError { + return m.validationError +} + +func (m mockRevisionResult) GetPhases() []machinery.PhaseResult { + return m.phases +} + +func (m mockRevisionResult) InTransistion() bool { + return m.inTransition +} + +func (m mockRevisionResult) IsComplete() bool { + return m.isComplete +} + +func (m mockRevisionResult) HasProgressed() bool { + return m.hasProgressed +} + +func (m mockRevisionResult) String() string { + return "verbose full report..." +} + +type mockPhaseResult struct { + name string + validationError *validation.PhaseValidationError + objects []machinery.ObjectResult + inTransition bool + isComplete bool + hasProgressed bool +} + +func (m mockPhaseResult) GetName() string { + return m.name +} + +func (m mockPhaseResult) GetValidationError() *validation.PhaseValidationError { + return m.validationError +} + +func (m mockPhaseResult) GetObjects() []machinery.ObjectResult { + return m.objects +} + +func (m mockPhaseResult) InTransistion() bool { + return m.inTransition +} + +func (m mockPhaseResult) IsComplete() bool { + return m.isComplete +} + +func (m mockPhaseResult) HasProgressed() bool { + return m.hasProgressed +} + +func (m mockPhaseResult) String() string { + return "verbose phase report..." +} + +type mockObjectResult struct { + action machinery.Action + object machinery.Object + success bool + probes map[string]machinery.ObjectProbeResult +} + +func (m mockObjectResult) Action() machinery.Action { + return m.action +} + +func (m mockObjectResult) Object() machinery.Object { + return m.object +} + +func (m mockObjectResult) Success() bool { + return m.success +} + +func (m mockObjectResult) Probes() map[string]machinery.ObjectProbeResult { + return m.probes +} + +func (m mockObjectResult) String() string { + return "verbose object report..." +} + +type mockRevisionTeardownResult struct { + phases []machinery.PhaseTeardownResult + isComplete bool + waitingPhaseNames []string +} + +func (m mockRevisionTeardownResult) GetPhases() []machinery.PhaseTeardownResult { + return m.phases +} + +func (m mockRevisionTeardownResult) IsComplete() bool { + return m.isComplete +} + +func (m mockRevisionTeardownResult) GetWaitingPhaseNames() []string { + return m.waitingPhaseNames +} + +func (m mockRevisionTeardownResult) GetActivePhaseName() (string, bool) { + return "", false +} + +func (m mockRevisionTeardownResult) GetGonePhaseNames() []string { + return nil +} + +func (m mockRevisionTeardownResult) String() string { + return "verbose teardown report..." +} + +type mockPhaseTeardownResult struct { + name string + isComplete bool +} + +func (m mockPhaseTeardownResult) GetName() string { + return m.name +} + +func (m mockPhaseTeardownResult) IsComplete() bool { + return m.isComplete +} + +func (m mockPhaseTeardownResult) Gone() []machinerytypes.ObjectRef { + if m.isComplete { + return []machinerytypes.ObjectRef{} + } + return nil +} + +func (m mockPhaseTeardownResult) Waiting() []machinerytypes.ObjectRef { + if !m.isComplete { + return []machinerytypes.ObjectRef{} + } + return nil +} + +func (m mockPhaseTeardownResult) String() string { + return "verbose phase teardown report..." +} + +func TestSummarizeRevisionResult_Nil(t *testing.T) { + result := SummarizeRevisionResult(nil) + assert.Empty(t, result) +} + +func TestSummarizeRevisionResult_Success(t *testing.T) { + result := SummarizeRevisionResult(mockRevisionResult{ + isComplete: true, + phases: []machinery.PhaseResult{ + mockPhaseResult{ + name: "deploy", + isComplete: true, + }, + }, + }) + assert.Equal(t, "reconcile completed successfully", result) +} + +func TestSummarizeRevisionResult_ValidationError(t *testing.T) { + verr := &validation.RevisionValidationError{ + RevisionName: "test", + } + result := SummarizeRevisionResult(mockRevisionResult{ + validationError: verr, + }) + assert.Contains(t, result, "validation error") +} + +func TestSummarizeRevisionResult_Collision(t *testing.T) { + cm := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cm", + Namespace: "default", + }, + } + cm.SetGroupVersionKind(schema.GroupVersionKind{ + Group: "", + Version: "v1", + Kind: "ConfigMap", + }) + + result := SummarizeRevisionResult(mockRevisionResult{ + isComplete: false, + phases: []machinery.PhaseResult{ + mockPhaseResult{ + name: "deploy", + isComplete: false, + objects: []machinery.ObjectResult{ + mockObjectResult{ + action: machinery.ActionCollision, + object: cm, + success: false, + }, + }, + }, + }, + }) + + assert.Contains(t, result, "collision") + assert.Contains(t, result, "ConfigMap") + assert.Contains(t, result, "default/test-cm") +} + +func TestSummarizeRevisionResult_ProbeFailure(t *testing.T) { + cm := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cm", + Namespace: "default", + }, + } + cm.SetGroupVersionKind(schema.GroupVersionKind{ + Group: "", + Version: "v1", + Kind: "ConfigMap", + }) + + result := SummarizeRevisionResult(mockRevisionResult{ + isComplete: false, + phases: []machinery.PhaseResult{ + mockPhaseResult{ + name: "deploy", + isComplete: false, + objects: []machinery.ObjectResult{ + mockObjectResult{ + action: "apply", + object: cm, + success: true, + probes: map[string]machinery.ObjectProbeResult{ + "progress": { + Success: false, + Messages: []string{"not ready"}, + }, + }, + }, + }, + }, + }, + }) + + assert.Contains(t, result, "probe failure") + assert.Contains(t, result, "progress") +} + +func TestSummarizeRevisionResult_MultipleCollisions(t *testing.T) { + objects := []machinery.ObjectResult{} + for i := 0; i < 5; i++ { + cm := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cm", + Namespace: "default", + }, + } + cm.SetGroupVersionKind(schema.GroupVersionKind{ + Group: "", + Version: "v1", + Kind: "ConfigMap", + }) + objects = append(objects, mockObjectResult{ + action: machinery.ActionCollision, + object: cm, + success: false, + }) + } + + result := SummarizeRevisionResult(mockRevisionResult{ + isComplete: false, + phases: []machinery.PhaseResult{ + mockPhaseResult{ + name: "deploy", + isComplete: false, + objects: objects, + }, + }, + }) + + // Should limit to first 3 and show total count + assert.Contains(t, result, "5 collision(s)") + assert.Contains(t, result, "showing first 3") +} + +func TestSummarizeRevisionResult_InTransition(t *testing.T) { + result := SummarizeRevisionResult(mockRevisionResult{ + inTransition: true, + isComplete: false, + }) + assert.Contains(t, result, "in transition") +} + +func TestSummarizeRevisionTeardownResult_Complete(t *testing.T) { + result := SummarizeRevisionTeardownResult(mockRevisionTeardownResult{ + isComplete: true, + }) + assert.Equal(t, "teardown completed successfully", result) +} + +func TestSummarizeRevisionTeardownResult_WaitingPhases(t *testing.T) { + result := SummarizeRevisionTeardownResult(mockRevisionTeardownResult{ + isComplete: false, + waitingPhaseNames: []string{"deploy", "configure"}, + }) + assert.Contains(t, result, "waiting on phases") + assert.Contains(t, result, "deploy") + assert.Contains(t, result, "configure") +} + +func TestSummarizeRevisionTeardownResult_IncompletePhases(t *testing.T) { + result := SummarizeRevisionTeardownResult(mockRevisionTeardownResult{ + isComplete: false, + phases: []machinery.PhaseTeardownResult{ + mockPhaseTeardownResult{ + name: "deploy", + isComplete: false, + }, + mockPhaseTeardownResult{ + name: "configure", + isComplete: true, + }, + }, + }) + assert.Contains(t, result, "incomplete phases") + assert.Contains(t, result, "deploy") + assert.Contains(t, result, "1 phase(s) completed") +} + +func TestGetObjectInfo_WithNamespace(t *testing.T) { + cm := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cm", + Namespace: "test-ns", + }, + } + cm.SetGroupVersionKind(schema.GroupVersionKind{ + Group: "", + Version: "v1", + Kind: "ConfigMap", + }) + + info := getObjectInfo(cm) + assert.Equal(t, "ConfigMap test-ns/test-cm", info) +} + +func TestGetObjectInfo_ClusterScoped(t *testing.T) { + cm := &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cm", + }, + } + cm.SetGroupVersionKind(schema.GroupVersionKind{ + Group: "", + Version: "v1", + Kind: "ConfigMap", + }) + + info := getObjectInfo(cm) + assert.Equal(t, "ConfigMap test-cm", info) +} + +func TestGetObjectInfo_Nil(t *testing.T) { + info := getObjectInfo(nil) + assert.Equal(t, "unknown object", info) +}