From aed5bf931a3b7808e632e04dab0ecd111addb11c Mon Sep 17 00:00:00 2001 From: Jeff Peeler Date: Wed, 24 Jul 2019 15:29:25 -0400 Subject: [PATCH] fix(catalog): re-install resources in existing installplan This specifically fixes the scenario where a subscription is deleted and recreated in a namespace with an additional subscription. When the subscription of interest is deleted, the associated installplan remains due to the other subscription's owner reference. Because the existing installplan manifests match, no additional work is done. This is a problem if resources that were originally installed have been deleted. Now after a subscription is recreated, the proper owner references are re-added onto the install plan as well as all the resources are checked that they are installed. --- pkg/controller/operators/catalog/operator.go | 20 +++ .../operators/catalog/subscriptions_test.go | 162 ++++++++++++++++++ 2 files changed, 182 insertions(+) diff --git a/pkg/controller/operators/catalog/operator.go b/pkg/controller/operators/catalog/operator.go index c5cb6ae480..831cfcce26 100644 --- a/pkg/controller/operators/catalog/operator.go +++ b/pkg/controller/operators/catalog/operator.go @@ -844,6 +844,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 operators.GetReference(installPlan) } } diff --git a/pkg/controller/operators/catalog/subscriptions_test.go b/pkg/controller/operators/catalog/subscriptions_test.go index 90b9b09e96..b18e1998e1 100644 --- a/pkg/controller/operators/catalog/subscriptions_test.go +++ b/pkg/controller/operators/catalog/subscriptions_test.go @@ -2,6 +2,7 @@ package catalog import ( "fmt" + "reflect" "testing" "time" @@ -15,6 +16,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) { @@ -509,6 +511,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) { @@ -556,6 +715,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) }