OTA-1159: pkg/cvo: Render status.Failure in ReconciliationIssues

pkg/cvo/reconciliation_issues.go

@@ -1,13 +1,139 @@
 package cvo
 
-import v1 "github.com/openshift/api/config/v1"
+import (
+	"encoding/json"
+	"fmt"
+	"sort"
+
+	configv1 "github.com/openshift/api/config/v1"
+	"github.com/openshift/cluster-version-operator/pkg/payload"
+)
 
 const (
-	reconciliationIssuesConditionType v1.ClusterStatusConditionType = "ReconciliationIssues"
+	reconciliationIssuesConditionType configv1.ClusterStatusConditionType = "ReconciliationIssues"
 
 	noReconciliationIssuesReason  string = "NoIssues"
 	noReconciliationIssuesMessage string = "No issues found during reconciliation"
 
-	reconciliationIssuesFoundReason  string = "IssuesFound"
-	reconciliationIssuesFoundMessage string = "Issues found during reconciliation"
+	reconciliationIssuesFoundReason string = "IssuesFound"
 )
+
+// errorWalkCallback processes an error.  It returns an error to fail the walk.
+type errorWalkCallback func(err error, depth int) error
+
+// errorWalk walks an error depth-first via Unwrap(), and calls the
+// callback on each error until the callback returns false for continuing
+// or an error.
+func errorWalk(err error, depth int, fn errorWalkCallback) error {
+	if err == nil {
+		return nil
+	}
+
+	if err2 := fn(err, depth); err2 != nil {
+		return err2
+	}
+
+	switch errType := err.(type) {
+	case interface{ Unwrap() error }:
+		return errorWalk(errType.Unwrap(), depth+1, fn)
+	case interface{ Unwrap() []error }:
+		for _, err := range errType.Unwrap() {
+			err := errorWalk(err, depth+1, fn)
+			if err != nil {
+				return err
+			}
+		}
+		return nil
+	case interface{ Errors() []error }: // k8s.io/apimachinery/pkg/util/errors hasn't caught up with Unwrap() []error
+		for _, err := range errType.Errors() {
+			err := errorWalk(err, depth+1, fn)
+			if err != nil {
+				return err
+			}
+		}
+		return nil
+	default:
+		return nil
+	}
+}
+
+type reconciliationIssue struct {
+	Message  string                 `json:"message"`
+	Children []*reconciliationIssue `json:"children,omitempty"`
+
+	// The following properties export portions of payload.UpdateError
+
+	Effect   string              `json:"effect,omitempty"`
+	Manifest *reconciledManifest `json:"manifest,omitempty"`
+}
+
+type reconciledManifest struct {
+	OriginalFilename string `json:"originalFilename,omitempty"`
+	Group            string `json:"group"`
+	Kind             string `json:"kind"`
+	Namespace        string `json:"namespace,omitempty"`
+	Name             string `json:"name"`
+}
+
+func reconciliationIssueFromError(err error) (string, error) {
+	root := &reconciliationIssue{}
+
+	if err := errorWalk(err, 0, func(err error, depth int) error {
+		parent := root
+		var entry *reconciliationIssue
+		if depth == 0 {
+			entry = root
+		} else {
+			for i := 0; i < depth-1; i++ {
+				parent = parent.Children[len(parent.Children)-1]
+			}
+			entry = &reconciliationIssue{}
+			parent.Children = append(parent.Children, entry)
+		}
+
+		entry.Message = err.Error()
+		if updateErr, ok := err.(*payload.UpdateError); ok {
+			entry.Effect = string(updateErr.UpdateEffect)
+			if updateErr.Task != nil && updateErr.Task.Manifest != nil {
+				entry.Manifest = &reconciledManifest{
+					OriginalFilename: updateErr.Task.Manifest.OriginalFilename,
+					Group:            updateErr.Task.Manifest.GVK.Group,
+					Kind:             updateErr.Task.Manifest.GVK.Kind,
+				}
+				if updateErr.Task.Manifest.Obj != nil {
+					entry.Manifest.Namespace = updateErr.Task.Manifest.Obj.GetNamespace()
+					entry.Manifest.Name = updateErr.Task.Manifest.Obj.GetName()
+				}
+			}
+		}
+
+		sort.Slice(parent.Children, func(i, j int) bool {
+			if parent.Children[i].Manifest == nil && parent.Children[j].Manifest == nil {
+				return parent.Children[i].Message < parent.Children[j].Message
+			} else if parent.Children[i].Manifest == nil {
+				return true
+			} else if parent.Children[j].Manifest == nil {
+				return false
+			}
+			return parent.Children[i].Manifest.OriginalFilename < parent.Children[j].Manifest.OriginalFilename
+		})
+
+		return nil
+	}); err != nil {
+		return "", err
+	}
+
+	bytes, err := json.Marshal(root)
+	if err != nil {
+		return string(bytes), err
+	}
+
+	if len(bytes) > 32768 {
+		root.Children = []*reconciliationIssue{{
+			Message: fmt.Sprintf("truncated children due to overly long JSON: %d bytes > 32768", len(bytes)),
+		}}
+		bytes, err = json.Marshal(root)
+	}
+
+	return string(bytes), err
+}

pkg/cvo/reconciliation_issues_test.go

@@ -0,0 +1,114 @@
+package cvo
+
+import (
+	"encoding/json"
+	"errors"
+	"strings"
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	configv1 "github.com/openshift/api/config/v1"
+	"github.com/openshift/cluster-version-operator/pkg/payload"
+	"github.com/openshift/library-go/pkg/manifest"
+)
+
+func Test_reconciliationIssueFromError(t *testing.T) {
+	err := summarizeTaskGraphErrors([]error{
+		&payload.UpdateError{
+			Name:                "etcd",
+			UpdateEffect:        payload.UpdateEffectNone,
+			Reason:              "ClusterOperatorUpdating",
+			PluralReason:        "ClusterOperatorsUpdating",
+			Message:             "Cluster operator etcd is updating versions",
+			PluralMessageFormat: "Cluster operators %s are updating versions",
+			Nested:              errors.New("cluster operator etcd is available and not degraded but has not finished updating to target version"),
+			Task: &payload.Task{
+				Index: 2,
+				Total: 10,
+				Manifest: &manifest.Manifest{
+					OriginalFilename: "etcd-cluster-operator.yaml",
+					GVK:              configv1.GroupVersion.WithKind("ClusterOperator"),
+				},
+			},
+		},
+		&payload.UpdateError{
+			Name:                "kube-apiserver",
+			UpdateEffect:        payload.UpdateEffectNone,
+			Reason:              "ClusterOperatorUpdating",
+			PluralReason:        "ClusterOperatorsUpdating",
+			Message:             "Cluster operator kube-apiserver is updating versions",
+			PluralMessageFormat: "Cluster operators %s are updating versions",
+			Nested:              errors.New("cluster operator kube-apiserver is available and not degraded but has not finished updating to target version"),
+			Task: &payload.Task{
+				Index: 4,
+				Total: 10,
+				Manifest: &manifest.Manifest{
+					OriginalFilename: "kube-apiserver-cluster-operator.yaml",
+					GVK:              configv1.GroupVersion.WithKind("ClusterOperator"),
+				},
+			},
+		},
+	})
+
+	message, err := reconciliationIssueFromError(err)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	var data interface{}
+	if err := json.Unmarshal([]byte(message), &data); err != nil {
+		t.Fatal(err)
+	}
+
+	indentedMessage, err := json.MarshalIndent(data, "", "  ")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	expected := `{
+  "children": [
+    {
+      "children": [
+        {
+          "children": [
+            {
+              "message": "cluster operator etcd is available and not degraded but has not finished updating to target version"
+            }
+          ],
+          "effect": "None",
+          "manifest": {
+            "group": "config.openshift.io",
+            "kind": "ClusterOperator",
+            "name": "",
+            "originalFilename": "etcd-cluster-operator.yaml"
+          },
+          "message": "Cluster operator etcd is updating versions"
+        },
+        {
+          "children": [
+            {
+              "message": "cluster operator kube-apiserver is available and not degraded but has not finished updating to target version"
+            }
+          ],
+          "effect": "None",
+          "manifest": {
+            "group": "config.openshift.io",
+            "kind": "ClusterOperator",
+            "name": "",
+            "originalFilename": "kube-apiserver-cluster-operator.yaml"
+          },
+          "message": "Cluster operator kube-apiserver is updating versions"
+        }
+      ],
+      "message": "[Cluster operator etcd is updating versions, Cluster operator kube-apiserver is updating versions]"
+    }
+  ],
+  "effect": "None",
+  "message": "Cluster operators etcd, kube-apiserver are updating versions"
+}`
+
+	diff := cmp.Diff(strings.Split(expected, "\n"), strings.Split(string(indentedMessage), "\n"))
+	if diff != "" {
+		t.Fatalf("unexpected output (-want, +got):\n%s", diff)
+	}
+}

pkg/cvo/status.go

@@ -390,9 +390,13 @@ func updateClusterVersionStatus(cvStatus *configv1.ClusterVersionStatus, status
 			Message: noReconciliationIssuesMessage,
 		}
 		if status.Failure != nil {
+			message, err := reconciliationIssueFromError(status.Failure)
+			if err != nil {
+				message = err.Error()
+			}
 			riCondition.Status = configv1.ConditionTrue
 			riCondition.Reason = reconciliationIssuesFoundReason
-			riCondition.Message = fmt.Sprintf("%s: %s", reconciliationIssuesFoundMessage, status.Failure.Error())
+			riCondition.Message = message
 		}
 		resourcemerge.SetOperatorStatusCondition(&cvStatus.Conditions, riCondition)
 	} else if oldRiCondition != nil {

pkg/cvo/status_test.go

@@ -246,7 +246,7 @@ func TestUpdateClusterVersionStatus_UnknownVersionAndReconciliationIssues(t *tes
 				Type:    reconciliationIssuesConditionType,
 				Status:  configv1.ConditionTrue,
 				Reason:  reconciliationIssuesFoundReason,
-				Message: "Issues found during reconciliation: Something happened",
+				Message: `{"message":"Something happened"}`,
 			},
 		},
 		{
@@ -313,7 +313,7 @@ func TestUpdateClusterVersionStatus_ReconciliationIssues(t *testing.T) {
 				Type:    reconciliationIssuesConditionType,
 				Status:  configv1.ConditionTrue,
 				Reason:  reconciliationIssuesFoundReason,
-				Message: "Issues found during reconciliation: Something happened",
+				Message: `{"message":"Something happened"}`,
 			},
 		},
 		{