Repository: operator-framework/operator-lifecycle-manager
Pull request #2268 (2021-07-19 16:17:02)

Update extractMessage
When building the subscription condition message,
check if the status of the InstallPlan is available
before iterating through bundle lookup conditions.


openshift-ci[bot]: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/operator-framework/operator-lifecycle-manager/pull/2268#" title="Author self-approved">awgreene</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=operator-framework%2Foperator-lifecycle-manager).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/operator-framework/operator-lifecycle-manager/blob/master/OWNERS)~~ [awgreene]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
openshift-ci[bot]: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/operator-framework/operator-lifecycle-manager/pull/2268#pullrequestreview-709758410" title="LGTM">anik120</a>*, *<a href="https://github.com/operator-framework/operator-lifecycle-manager/pull/2268#" title="Author self-approved">awgreene</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=operator-framework%2Foperator-lifecycle-manager).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/operator-framework/operator-lifecycle-manager/blob/master/OWNERS)~~ [awgreene]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->
awgreene: /hold
openshift-ci[bot]: New changes are detected. LGTM label has been removed.
anik120: /lgtm
awgreene: /unhold
openshift-ci[bot]: New changes are detected. LGTM label has been removed.

Patch:
diff --git a/pkg/controller/operators/catalog/subscription/state.go b/pkg/controller/operators/catalog/subscription/state.go
index e7280e39b..024cb3dd9 100644
--- a/pkg/controller/operators/catalog/subscription/state.go
+++ b/pkg/controller/operators/catalog/subscription/state.go
@@ -511,23 +511,22 @@ func (i *installPlanReferencedState) CheckInstallPlanStatus(now *metav1.Time, cl
 }
 
 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 && cond.Message != "" {
+		return cond.Message
 	}
-	if cond := status.GetCondition(v1alpha1.InstallPlanInstalled); cond.Status != corev1.ConditionUnknown {
-		if cond.Message != "" {
-			str = cond.Message
+
+	var b bytes.Buffer
+	for _, lookup := range status.BundleLookups {
+		if cond := lookup.GetCondition(v1alpha1.BundleLookupPending); cond.Status != corev1.ConditionUnknown && cond.Message != "" {
+			if b.Len() != 0 {
+				b.WriteString(" ")
+			}
+			b.WriteString(cond.Message)
+			b.WriteString(".")
 		}
 	}
-	return str
+
+	return b.String()
 }
 
 type installPlanKnownState struct {


Test patch:
diff --git a/pkg/controller/operators/catalog/subscription/state_test.go b/pkg/controller/operators/catalog/subscription/state_test.go
index 422821649..afd0f576b 100644
--- a/pkg/controller/operators/catalog/subscription/state_test.go
+++ b/pkg/controller/operators/catalog/subscription/state_test.go
@@ -1571,6 +1571,75 @@ func TestCheckInstallPlanStatus(t *testing.T) {
 				}),
 			},
 		},
+		{
+			description: "InstallPlanReferencedState/NoConditions/Failed/ToInstallPlanFailedState/Update/InstallPlanReasonComponentFailed/MultipleBundleConditions",
+			fields: fields{
+				existingObjs: existingObjs{
+					clientObjs: []runtime.Object{
+						&v1alpha1.Subscription{
+							ObjectMeta: metav1.ObjectMeta{
+								Name:      "sub",
+								Namespace: "ns",
+							},
+						},
+					},
+				},
+				namespace: "ns",
+				state: newInstallPlanReferencedState(&v1alpha1.Subscription{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "sub",
+						Namespace: "ns",
+					},
+				}),
+			},
+			args: args{
+				now: &now,
+				status: &v1alpha1.InstallPlanStatus{
+					Phase: v1alpha1.InstallPlanPhaseFailed,
+					Conditions: []v1alpha1.InstallPlanCondition{
+						{
+							Type:   v1alpha1.InstallPlanInstalled,
+							Status: corev1.ConditionFalse,
+							Reason: v1alpha1.InstallPlanReasonComponentFailed,
+						},
+					},
+					BundleLookups: []v1alpha1.BundleLookup{
+						{
+							Conditions: []v1alpha1.BundleLookupCondition{
+								{
+									Type:    v1alpha1.BundleLookupPending,
+									Status:  corev1.ConditionTrue,
+									Message: "encountered issue foo",
+								},
+							},
+						},
+						{
+							Conditions: []v1alpha1.BundleLookupCondition{
+								{
+									Type:    v1alpha1.BundleLookupPending,
+									Status:  corev1.ConditionTrue,
+									Message: "encountered issue bar",
+								},
+							},
+						},
+					},
+				},
+			},
+			want: want{
+				transitioned: newInstallPlanFailedState(&v1alpha1.Subscription{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "sub",
+						Namespace: "ns",
+					},
+					Status: v1alpha1.SubscriptionStatus{
+						Conditions: []v1alpha1.SubscriptionCondition{
+							planFailedCondition(corev1.ConditionTrue, string(v1alpha1.InstallPlanReasonComponentFailed), "encountered issue foo. encountered issue bar.", &now),
+						},
+						LastUpdated: now,
+					},
+				}),
+			},
+		},
 		{
 			description: "InstallPlanReferencedState/Conditions/Failed/ToInstallPlanFailedState/NoUpdate/InstallPlanReasonComponentFailed",
 			fields: fields{
diff --git a/pkg/controller/registry/resolver/resolver_test.go b/pkg/controller/registry/resolver/resolver_test.go
index b95442593..aa288d094 100644
--- a/pkg/controller/registry/resolver/resolver_test.go
+++ b/pkg/controller/registry/resolver/resolver_test.go
@@ -59,35 +59,6 @@ func TestSolveOperators(t *testing.T) {
 	require.EqualValues(t, expected, operators)
 }
 
-func TestDisjointChannelGraph(t *testing.T) {
-	const namespace = "test-namespace"
-	catalog := registry.CatalogKey{Name: "test-catalog", Namespace: namespace}
-
-	newSub := newSub(namespace, "packageA", "alpha", catalog)
-	subs := []*v1alpha1.Subscription{newSub}
-
-	fakeNamespacedOperatorCache := NamespacedOperatorCache{
-		snapshots: map[registry.CatalogKey]*CatalogSnapshot{
-			catalog: {
-				key: catalog,
-				operators: []*Operator{
-					genOperator("packageA.side1.v1", "0.0.1", "", "packageA", "alpha", catalog.Name, catalog.Namespace, nil, nil, nil, "", false),
-					genOperator("packageA.side1.v2", "0.0.2", "packageA.side1.v1", "packageA", "alpha", catalog.Name, catalog.Namespace, nil, nil, nil, "", false),
-					genOperator("packageA.side2.v1", "1.0.0", "", "packageA", "alpha", catalog.Name, catalog.Namespace, nil, nil, nil, "", false),
-					genOperator("packageA.side2.v2", "2.0.0", "packageA.side2.v1", "packageA", "alpha", catalog.Name, catalog.Namespace, nil, nil, nil, "", false),
-				},
-			},
-		},
-	}
-	satResolver := SatResolver{
-		cache: getFakeOperatorCache(fakeNamespacedOperatorCache),
-		log:   logrus.New(),
-	}
-
-	_, err := satResolver.SolveOperators([]string{namespace}, nil, subs)
-	require.Error(t, err, "a unique replacement chain within a channel is required to determine the relative order between channel entries, but 2 replacement chains were found in channel \"alpha\" of package \"packageA\": packageA.side1.v2...packageA.side1.v1, packageA.side2.v2...packageA.side2.v1")
-}
-
 func TestPropertiesAnnotationHonored(t *testing.T) {
 	const (
 		namespace = "olm"