diff --git a/pkg/cvo/reconciliation_issues.go b/pkg/cvo/reconciliation_issues.go index 144cc619ef..87640125ba 100644 --- a/pkg/cvo/reconciliation_issues.go +++ b/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 +} diff --git a/pkg/cvo/reconciliation_issues_test.go b/pkg/cvo/reconciliation_issues_test.go new file mode 100644 index 0000000000..5c2f4681bb --- /dev/null +++ b/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) + } +} diff --git a/pkg/cvo/status.go b/pkg/cvo/status.go index 36ec476c22..45bdca5304 100644 --- a/pkg/cvo/status.go +++ b/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 { diff --git a/pkg/cvo/status_test.go b/pkg/cvo/status_test.go index c98d8985db..48488ca3ee 100644 --- a/pkg/cvo/status_test.go +++ b/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"}`, }, }, {