Merge pull request #965 from jpeeler/existing-installplan

Bug 1714140: fix(catalog): re-install resources in existing installplan

Author: openshift-merge-robot

pkg/controller/operators/catalog/operator.go

@@ -881,6 +881,26 @@ func (o *Operator) ensureInstallPlan(logger *logrus.Entry, namespace string, sub
 	for _, installPlan := range installPlans {
 		if installPlan.Status.CSVManifestsMatch(steps) {
 			logger.Infof("found InstallPlan with matching manifests: %s", installPlan.GetName())
+
+			ownerWasAdded := false
+			for _, sub := range subs {
+				ownerWasAdded = ownerWasAdded || !ownerutil.EnsureOwner(installPlan, sub)
+			}
+			if ownerWasAdded {
+				_, err := o.client.OperatorsV1alpha1().InstallPlans(installPlan.GetNamespace()).Update(installPlan)
+				if err != nil {
+					return nil, err
+				}
+			}
+
+			installPlan.Status.Phase = v1alpha1.InstallPlanPhaseInstalling
+			for _, step := range installPlan.Status.Plan {
+				step.Status = v1alpha1.StepStatusUnknown
+			}
+			_, err = o.client.OperatorsV1alpha1().InstallPlans(namespace).UpdateStatus(installPlan)
+			if err != nil {
+				return nil, err
+			}
 			return reference.GetReference(installPlan)
 		}
 	}

pkg/controller/operators/catalog/operator_test.go

@@ -704,7 +704,7 @@ func withFakeClientOptions(options ...clientfake.Option) fakeOperatorOption {
 func NewFakeOperator(ctx context.Context, namespace string, watchedNamespaces []string, fakeOptions ...fakeOperatorOption) (*Operator, error) {
 	// Apply options to default config
 	config := &fakeOperatorConfig{
-		logger:   logrus.New(),
+		logger:   logrus.StandardLogger(),
 		clock:    utilclock.RealClock{},
 		resolver: &fakes.FakeResolver{},
 	}

pkg/controller/operators/catalog/subscriptions_test.go

@@ -3,6 +3,7 @@ package catalog
 import (
 	"context"
 	"fmt"
+	"reflect"
 	"testing"
 	"time"
 
@@ -17,6 +18,7 @@ import (
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/resolver"
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/fakes"
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/clientfake"
+	"github.com/operator-framework/operator-lifecycle-manager/pkg/lib/ownerutil"
 )
 
 func TestSyncSubscriptions(t *testing.T) {
@@ -638,6 +640,163 @@ func TestSyncSubscriptions(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "Status/HaveCurrentCSV/SameManifests",
+			fields: fields{
+				clientOptions: []clientfake.Option{clientfake.WithSelfLinks(t)},
+				existingOLMObjs: []runtime.Object{
+					&v1alpha1.ClusterServiceVersion{
+						ObjectMeta: metav1.ObjectMeta{
+							Name:      "csv.v.1",
+							Namespace: testNamespace,
+						},
+						Status: v1alpha1.ClusterServiceVersionStatus{
+							Phase: v1alpha1.CSVPhaseSucceeded,
+						},
+					},
+					&v1alpha1.Subscription{
+						ObjectMeta: metav1.ObjectMeta{
+							Name:      "sub",
+							Namespace: testNamespace,
+						},
+						Spec: &v1alpha1.SubscriptionSpec{
+							CatalogSource:          "src",
+							CatalogSourceNamespace: testNamespace,
+						},
+						Status: v1alpha1.SubscriptionStatus{
+							CurrentCSV: "",
+							State:      "",
+						},
+					},
+					&v1alpha1.InstallPlan{
+						ObjectMeta: metav1.ObjectMeta{
+							Name:      "install-ip123",
+							Namespace: testNamespace,
+							SelfLink:  fmt.Sprintf("/apis/%s/namespaces/%s/%s/%s", v1alpha1.SchemeGroupVersion.String(), "installplans", testNamespace, "install-ip123"),
+						},
+						Spec: v1alpha1.InstallPlanSpec{
+							ClusterServiceVersionNames: []string{
+								"csv.v.1",
+							},
+							Approval: v1alpha1.ApprovalAutomatic,
+							Approved: true,
+						},
+						Status: v1alpha1.InstallPlanStatus{
+							Phase: v1alpha1.InstallPlanPhaseComplete,
+							CatalogSources: []string{
+								"src",
+							},
+							Plan: []*v1alpha1.Step{
+								{
+									Resolving: "csv.v.1",
+									Resource: v1alpha1.StepResource{
+										CatalogSource:          "src",
+										CatalogSourceNamespace: testNamespace,
+										Group:                  v1alpha1.GroupName,
+										Version:                v1alpha1.GroupVersion,
+										Kind:                   v1alpha1.ClusterServiceVersionKind,
+										Name:                   "csv.v.1",
+										Manifest:               "{}",
+									},
+								},
+							},
+						},
+					},
+				},
+				resolveSteps: []*v1alpha1.Step{
+					{
+						Resolving: "csv.v.1",
+						Resource: v1alpha1.StepResource{
+							CatalogSource:          "src",
+							CatalogSourceNamespace: testNamespace,
+							Group:                  v1alpha1.GroupName,
+							Version:                v1alpha1.GroupVersion,
+							Kind:                   v1alpha1.ClusterServiceVersionKind,
+							Name:                   "csv.v.1",
+							Manifest:               "{}",
+						},
+					},
+				},
+				resolveSubs: []*v1alpha1.Subscription{
+					{
+						TypeMeta: metav1.TypeMeta{
+							Kind:       v1alpha1.SubscriptionKind,
+							APIVersion: v1alpha1.SubscriptionCRDAPIVersion,
+						},
+						ObjectMeta: metav1.ObjectMeta{
+							Name:      "sub",
+							Namespace: testNamespace,
+						},
+						Spec: &v1alpha1.SubscriptionSpec{
+							CatalogSource:          "src",
+							CatalogSourceNamespace: testNamespace,
+						},
+						Status: v1alpha1.SubscriptionStatus{
+							CurrentCSV: "csv.v.1",
+							State:      "SubscriptionStateAtLatest",
+						},
+					},
+				},
+			},
+			args: args{
+				obj: &v1alpha1.Subscription{
+					ObjectMeta: metav1.ObjectMeta{
+						Name:      "sub",
+						Namespace: testNamespace,
+					},
+					Spec: &v1alpha1.SubscriptionSpec{
+						CatalogSource:          "src",
+						CatalogSourceNamespace: testNamespace,
+					},
+					Status: v1alpha1.SubscriptionStatus{
+						CurrentCSV: "",
+						State:      "",
+					},
+				},
+			},
+			wantInstallPlan: &v1alpha1.InstallPlan{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "install-ip123",
+					Namespace: testNamespace,
+					SelfLink:  fmt.Sprintf("/apis/%s/namespaces/%s/%s/%s", v1alpha1.SchemeGroupVersion.String(), "installplans", testNamespace, "install-ip123"),
+					OwnerReferences: []metav1.OwnerReference{{
+						APIVersion:         v1alpha1.SubscriptionCRDAPIVersion,
+						Kind:               v1alpha1.SubscriptionKind,
+						Name:               "sub",
+						Controller:         &ownerutil.NotController,
+						BlockOwnerDeletion: &ownerutil.DontBlockOwnerDeletion,
+					}},
+				},
+				Spec: v1alpha1.InstallPlanSpec{
+					ClusterServiceVersionNames: []string{
+						"csv.v.1",
+					},
+					Approval: v1alpha1.ApprovalAutomatic,
+					Approved: true,
+				},
+				Status: v1alpha1.InstallPlanStatus{
+					Phase: v1alpha1.InstallPlanPhaseInstalling,
+					CatalogSources: []string{
+						"src",
+					},
+					Plan: []*v1alpha1.Step{
+						{
+							Resolving: "csv.v.1",
+							Resource: v1alpha1.StepResource{
+								CatalogSource:          "src",
+								CatalogSourceNamespace: testNamespace,
+								Group:                  v1alpha1.GroupName,
+								Version:                v1alpha1.GroupVersion,
+								Kind:                   v1alpha1.ClusterServiceVersionKind,
+								Name:                   "csv.v.1",
+								Manifest:               "{}",
+							},
+							Status: v1alpha1.StepStatusUnknown,
+						},
+					},
+				},
+			},
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -686,6 +845,9 @@ func TestSyncSubscriptions(t *testing.T) {
 				require.NoError(t, err)
 				require.Equal(t, 1, len(installPlans.Items))
 				ip := installPlans.Items[0]
+				if (!reflect.DeepEqual(tt.wantInstallPlan.ObjectMeta, metav1.ObjectMeta{})) {
+					require.Equal(t, tt.wantInstallPlan.ObjectMeta, ip.ObjectMeta)
+				}
 				require.Equal(t, tt.wantInstallPlan.Spec, ip.Spec)
 				require.Equal(t, tt.wantInstallPlan.Status, ip.Status)
 			}