From 303608dbe3c9cbfe4679bafc43a6acafea9f7e9e Mon Sep 17 00:00:00 2001
From: Anik Bhattacharjee <anikbhattacharya93@gmail.com>
Date: Fri, 16 Jul 2021 16:21:38 -0400
Subject: [PATCH] feat(sub): Include IP failure condition message in sub status
 condition

Currently when an InstallPlan fails, due to bundle unpacking error
is pending due to errors like invalid operatorgroup, or pending bundle unpacking job
the reason is propagated to the Subscription that owns it. The message for the failure is
however missing from the Subscription condition.

eg

```
kind: Subscription
status:
conditions:
    - lastTransitionTime: "2021-07-07T17:55:20Z"
      reason: Installing
      status: "True"
      type: InstallPlanPending
```

This PR propagates the message assosiated with the reason in the InstallPlan
condition when an InstallPlan is either pending, or has failed permanantly.

eg

```
kind: Subscription
status:
conditions:
    - lastTransitionTime: "2021-07-07T17:55:20Z"
      message: no operator group found that is managing this namespace
      reason: Installing
      status: "True"
      type: InstallPlanPending
```

Signed-off-by: Anik Bhattacharjee <anikbhattacharya93@gmail.com>
---
 .../operators/catalog/subscription/state.go   | 24 ++++++++++++++++++-
 .../catalog/subscription/state_test.go        | 23 ++++++++++++++++--
 2 files changed, 44 insertions(+), 3 deletions(-)

diff --git a/pkg/controller/operators/catalog/subscription/state.go b/pkg/controller/operators/catalog/subscription/state.go
index f5da96614a..e7280e39b2 100644
--- a/pkg/controller/operators/catalog/subscription/state.go
+++ b/pkg/controller/operators/catalog/subscription/state.go
@@ -1,6 +1,7 @@
 package subscription
 
 import (
+	"bytes"
 	"context"
 	"fmt"
 
@@ -452,7 +453,7 @@ func (i *installPlanReferencedState) CheckInstallPlanStatus(now *metav1.Time, cl
 		if cond.Reason == "" {
 			cond.Reason = string(phase)
 		}
-
+		cond.Message = extractMessage(status)
 		cond.Type = v1alpha1.SubscriptionInstallPlanPending
 		cond.Status = corev1.ConditionTrue
 		out.Status.SetCondition(cond)
@@ -472,6 +473,7 @@ func (i *installPlanReferencedState) CheckInstallPlanStatus(now *metav1.Time, cl
 		}
 
 		cond.Type = v1alpha1.SubscriptionInstallPlanFailed
+		cond.Message = extractMessage(status)
 		cond.Status = corev1.ConditionTrue
 		out.Status.SetCondition(cond)
 
@@ -508,6 +510,26 @@ func (i *installPlanReferencedState) CheckInstallPlanStatus(now *metav1.Time, cl
 	return known, nil
 }
 
+func extractMessage(status *v1alpha1.InstallPlanStatus) string {
+	str := ""
+	if len(status.BundleLookups) > 0 {
+		var b bytes.Buffer
+		for _, lookup := range status.BundleLookups {
+			if cond := lookup.GetCondition(v1alpha1.BundleLookupPending); cond.Status != corev1.ConditionUnknown {
+				b.WriteString(cond.Message)
+				b.WriteString(".")
+			}
+		}
+		str = b.String()
+	}
+	if cond := status.GetCondition(v1alpha1.InstallPlanInstalled); cond.Status != corev1.ConditionUnknown {
+		if cond.Message != "" {
+			str = cond.Message
+		}
+	}
+	return str
+}
+
 type installPlanKnownState struct {
 	InstallPlanReferencedState
 }
diff --git a/pkg/controller/operators/catalog/subscription/state_test.go b/pkg/controller/operators/catalog/subscription/state_test.go
index 3ba23f4fdf..4228216499 100644
--- a/pkg/controller/operators/catalog/subscription/state_test.go
+++ b/pkg/controller/operators/catalog/subscription/state_test.go
@@ -1336,6 +1336,14 @@ func TestCheckInstallPlanStatus(t *testing.T) {
 				now: &now,
 				status: &v1alpha1.InstallPlanStatus{
 					Phase: v1alpha1.InstallPlanPhaseInstalling,
+					Conditions: []v1alpha1.InstallPlanCondition{
+						{
+							Type:    v1alpha1.InstallPlanInstalled,
+							Message: "no operatorgroup found that is managing this namespace",
+							Reason:  v1alpha1.InstallPlanConditionReason("Installing"),
+							Status:  corev1.ConditionFalse,
+						},
+					},
 				},
 			},
 			want: want{
@@ -1346,7 +1354,7 @@ func TestCheckInstallPlanStatus(t *testing.T) {
 					},
 					Status: v1alpha1.SubscriptionStatus{
 						Conditions: []v1alpha1.SubscriptionCondition{
-							planPendingCondition(corev1.ConditionTrue, string(v1alpha1.InstallPlanPhaseInstalling), "", &now),
+							planPendingCondition(corev1.ConditionTrue, string(v1alpha1.InstallPlanPhaseInstalling), "no operatorgroup found that is managing this namespace", &now),
 						},
 						LastUpdated: now,
 					},
@@ -1535,6 +1543,17 @@ func TestCheckInstallPlanStatus(t *testing.T) {
 							Reason: v1alpha1.InstallPlanReasonComponentFailed,
 						},
 					},
+					BundleLookups: []v1alpha1.BundleLookup{
+						{
+							Conditions: []v1alpha1.BundleLookupCondition{
+								{
+									Type:    v1alpha1.BundleLookupPending,
+									Status:  corev1.ConditionTrue,
+									Message: "unpack job not completed: Unpack pod(olm/c5a4) container(pull) is pending. Reason: ImagePullBackOff, Message: Back-off pulling image",
+								},
+							},
+						},
+					},
 				},
 			},
 			want: want{
@@ -1545,7 +1564,7 @@ func TestCheckInstallPlanStatus(t *testing.T) {
 					},
 					Status: v1alpha1.SubscriptionStatus{
 						Conditions: []v1alpha1.SubscriptionCondition{
-							planFailedCondition(corev1.ConditionTrue, string(v1alpha1.InstallPlanReasonComponentFailed), "", &now),
+							planFailedCondition(corev1.ConditionTrue, string(v1alpha1.InstallPlanReasonComponentFailed), "unpack job not completed: Unpack pod(olm/c5a4) container(pull) is pending. Reason: ImagePullBackOff, Message: Back-off pulling image.", &now),
 						},
 						LastUpdated: now,
 					},