Merge pull request #9668 from rebeccaalpert/operatorhub

Bug 1986699: Differentiate between operators for install
openshift · Aug 25, 2021 · f8031b3 · f8031b3
2 parents 162d931 + 49647b7
commit f8031b3
Show file tree

Hide file tree

Showing 4 changed files with 77 additions and 26 deletions.
diff --git a/frontend/packages/operator-lifecycle-manager/src/components/operator-group.spec.tsx b/frontend/packages/operator-lifecycle-manager/src/components/operator-group.spec.tsx
@@ -1,7 +1,12 @@
 import * as React from 'react';
 import { shallow } from 'enzyme';
 import * as _ from 'lodash';
-import { testOperatorGroup, testSubscription } from '../../mocks';
+import {
+  testOperatorGroup,
+  testSubscription,
+  testPackageManifest,
+  dummyPackageManifest,
+} from '../../mocks';
 import { OperatorGroupKind, SubscriptionKind, InstallModeType } from '../types';
 import {
   requireOperatorGroup,
@@ -43,7 +48,7 @@ describe('requireOperatorGroup', () => {
 });
 
 describe('subscriptionFor', () => {
-  const pkgName = testSubscription.spec.name;
+  const pkg = testPackageManifest;
   const ns = testSubscription.metadata.namespace;
   let subscriptions: SubscriptionKind[];
   let operatorGroups: OperatorGroupKind[];
@@ -57,42 +62,66 @@ describe('subscriptionFor', () => {
     subscriptions = [testSubscription];
     operatorGroups = [{ ...testOperatorGroup, status: { namespaces: [ns], lastUpdated: null } }];
 
-    expect(subscriptionFor(subscriptions)(operatorGroups)('new-operator')(ns)).toBeUndefined();
+    expect(
+      subscriptionFor(subscriptions)(operatorGroups)(dummyPackageManifest)(ns),
+    ).toBeUndefined();
   });
 
-  it('returns noting if no `OperatorGroups` target the given namespace', () => {
+  it('returns nothing if no `OperatorGroups` target the given namespace', () => {
     subscriptions = [testSubscription];
     operatorGroups = [
       { ...testOperatorGroup, status: { namespaces: ['prod-a', 'prod-b'], lastUpdated: null } },
     ];
 
-    expect(subscriptionFor(subscriptions)(operatorGroups)(pkgName)(ns)).toBeUndefined();
+    expect(subscriptionFor(subscriptions)(operatorGroups)(pkg)(ns)).toBeUndefined();
+  });
+
+  it('returns nothing if no `Subscriptions` share the package namespace', () => {
+    subscriptions = [testSubscription];
+    operatorGroups = [
+      { ...testOperatorGroup, status: { namespaces: ['prod-a', 'prod-b'], lastUpdated: null } },
+    ];
+
+    expect(
+      subscriptionFor(subscriptions)(operatorGroups)(dummyPackageManifest)(ns),
+    ).toBeUndefined();
+  });
+
+  it('returns nothing if no `Subscriptions` share the package catalog source', () => {
+    subscriptions = [testSubscription];
+    operatorGroups = [
+      { ...testOperatorGroup, status: { namespaces: ['prod-a', 'prod-b'], lastUpdated: null } },
+    ];
+
+    expect(
+      subscriptionFor(subscriptions)(operatorGroups)(dummyPackageManifest)(ns),
+    ).toBeUndefined();
   });
 
   it('returns nothing if checking for `all-namespaces`', () => {
     subscriptions = [testSubscription];
     operatorGroups = [{ ...testOperatorGroup, status: { namespaces: [ns], lastUpdated: null } }];
 
-    expect(subscriptionFor(subscriptions)(operatorGroups)(pkgName)('')).toBeUndefined();
+    expect(subscriptionFor(subscriptions)(operatorGroups)(pkg)('')).toBeUndefined();
   });
 
   it('returns `Subscription` when it exists in the "global" `OperatorGroup`', () => {
     subscriptions = [testSubscription];
     operatorGroups = [{ ...testOperatorGroup, status: { namespaces: [''], lastUpdated: null } }];
 
-    expect(subscriptionFor(subscriptions)(operatorGroups)(pkgName)(ns)).toEqual(testSubscription);
+    expect(subscriptionFor(subscriptions)(operatorGroups)(pkg)(ns)).toEqual(testSubscription);
   });
 
   it('returns `Subscription` when it exists in an `OperatorGroup` that targets given namespace', () => {
     subscriptions = [testSubscription];
     operatorGroups = [{ ...testOperatorGroup, status: { namespaces: [ns], lastUpdated: null } }];
 
-    expect(subscriptionFor(subscriptions)(operatorGroups)(pkgName)(ns)).toEqual(testSubscription);
+    expect(subscriptionFor(subscriptions)(operatorGroups)(pkg)(ns)).toEqual(testSubscription);
   });
 });
 
 describe('installedFor', () => {
-  const pkgName = testSubscription.spec.name;
+  const pkg = testPackageManifest;
   const ns = testSubscription.metadata.namespace;
   let subscriptions: SubscriptionKind[];
   let operatorGroups: OperatorGroupKind[];
@@ -106,7 +135,7 @@ describe('installedFor', () => {
     subscriptions = [testSubscription];
     operatorGroups = [{ ...testOperatorGroup, status: { namespaces: [ns], lastUpdated: null } }];
 
-    expect(installedFor(subscriptions)(operatorGroups)('new-operator')(ns)).toBe(false);
+    expect(installedFor(subscriptions)(operatorGroups)(dummyPackageManifest)(ns)).toBe(false);
   });
 
   it('returns false if no `OperatorGroups` target the given namespace', () => {
@@ -115,28 +144,42 @@ describe('installedFor', () => {
       { ...testOperatorGroup, status: { namespaces: ['prod-a', 'prod-b'], lastUpdated: null } },
     ];
 
-    expect(installedFor(subscriptions)(operatorGroups)(pkgName)(ns)).toBe(false);
+    expect(installedFor(subscriptions)(operatorGroups)(pkg)(ns)).toBe(false);
   });
 
   it('returns false if checking for `all-namespaces`', () => {
     subscriptions = [testSubscription];
     operatorGroups = [{ ...testOperatorGroup, status: { namespaces: [ns], lastUpdated: null } }];
 
-    expect(installedFor(subscriptions)(operatorGroups)(pkgName)('')).toBe(false);
+    expect(installedFor(subscriptions)(operatorGroups)(pkg)('')).toBe(false);
+  });
+
+  it('returns false if `Subscription` is in a different namespace than the given package', () => {
+    subscriptions = [testSubscription];
+    operatorGroups = [{ ...testOperatorGroup, status: { namespaces: [ns], lastUpdated: null } }];
+
+    expect(installedFor(subscriptions)(operatorGroups)(dummyPackageManifest)(ns)).toBe(false);
+  });
+
+  it('returns false if `Subscription` is from a different catalog source than the given package', () => {
+    subscriptions = [testSubscription];
+    operatorGroups = [{ ...testOperatorGroup, status: { namespaces: [ns], lastUpdated: null } }];
+
+    expect(installedFor(subscriptions)(operatorGroups)(dummyPackageManifest)(ns)).toBe(false);
   });
 
   it('returns true if `Subscription` exists in the "global" `OperatorGroup`', () => {
     subscriptions = [testSubscription];
     operatorGroups = [{ ...testOperatorGroup, status: { namespaces: [''], lastUpdated: null } }];
 
-    expect(installedFor(subscriptions)(operatorGroups)(pkgName)(ns)).toBe(true);
+    expect(installedFor(subscriptions)(operatorGroups)(pkg)(ns)).toBe(true);
   });
 
   it('returns true if `Subscription` exists in an `OperatorGroup` that targets given namespace', () => {
     subscriptions = [testSubscription];
     operatorGroups = [{ ...testOperatorGroup, status: { namespaces: [ns], lastUpdated: null } }];
 
-    expect(installedFor(subscriptions)(operatorGroups)(pkgName)(ns)).toBe(true);
+    expect(installedFor(subscriptions)(operatorGroups)(pkg)(ns)).toBe(true);
   });
 });
 

diff --git a/frontend/packages/operator-lifecycle-manager/src/components/operator-group.tsx b/frontend/packages/operator-lifecycle-manager/src/components/operator-group.tsx
@@ -12,7 +12,12 @@ import {
   referenceForGroupVersionKind,
 } from '@console/internal/module/k8s';
 import { OperatorGroupModel } from '../models';
-import { OperatorGroupKind, SubscriptionKind, InstallModeType } from '../types';
+import {
+  OperatorGroupKind,
+  SubscriptionKind,
+  InstallModeType,
+  PackageManifestKind,
+} from '../types';
 
 export const targetNamespacesFor = (obj: K8sResourceKind) =>
   obj?.metadata?.annotations?.['olm.targetNamespaces'];
@@ -136,9 +141,14 @@ export const isSingle = (obj: OperatorGroupKind) =>
  */
 export const subscriptionFor = (allSubscriptions: SubscriptionKind[] = []) => (
   allGroups: OperatorGroupKind[] = [],
-) => (pkgName: string) => (ns = '') => {
+) => (pkg: PackageManifestKind) => (ns = '') => {
   return allSubscriptions
-    .filter((sub) => sub.spec.name === pkgName)
+    .filter(
+      (sub) =>
+        sub.spec.name === pkg.status.packageName &&
+        sub.spec.source === pkg.status.catalogSource &&
+        sub.spec.sourceNamespace === pkg.status.catalogSourceNamespace,
+    )
     .find((sub) =>
       allGroups.some(
         (og) =>
@@ -150,8 +160,8 @@ export const subscriptionFor = (allSubscriptions: SubscriptionKind[] = []) => (
 
 export const installedFor = (allSubscriptions: SubscriptionKind[] = []) => (
   allGroups: OperatorGroupKind[] = [],
-) => (pkgName: string) => (ns = '') => {
-  return !_.isNil(subscriptionFor(allSubscriptions)(allGroups)(pkgName)(ns));
+) => (pkg: PackageManifestKind) => (ns = '') => {
+  return !_.isNil(subscriptionFor(allSubscriptions)(allGroups)(pkg)(ns));
 };
 
 export const providedAPIsForOperatorGroup = (og: OperatorGroupKind) =>

diff --git a/...end/packages/operator-lifecycle-manager/src/components/operator-hub/operator-hub-page.tsx b/...end/packages/operator-lifecycle-manager/src/components/operator-hub/operator-hub-page.tsx
@@ -116,10 +116,7 @@ export const OperatorHubList: React.FC<OperatorHubListProps> = ({
           } = currentCSVAnnotations;
 
           const subscription =
-            loaded &&
-            subscriptionFor(subscriptions?.data)(operatorGroups?.data)(pkg.status.packageName)(
-              namespace,
-            );
+            loaded && subscriptionFor(subscriptions?.data)(operatorGroups?.data)(pkg)(namespace);
 
           const clusterServiceVersion =
             loaded &&
@@ -139,6 +136,7 @@ export const OperatorHubList: React.FC<OperatorHubListProps> = ({
             isInstalling:
               loaded &&
               !_.isNil(subscription) &&
+              !_.isNil(clusterServiceVersion?.status?.phase) &&
               clusterServiceVersion?.status?.phase !== 'Succeeded',
             subscription,
             installState: installed ? InstalledState.Installed : InstalledState.NotInstalled,

diff --git a/...ackages/operator-lifecycle-manager/src/components/operator-hub/operator-hub-subscribe.tsx b/...ackages/operator-lifecycle-manager/src/components/operator-hub/operator-hub-subscribe.tsx
@@ -246,9 +246,9 @@ export const OperatorHubSubscribeForm: React.FC<OperatorHubSubscribeFormProps> =
     return t('olm~This mode is not supported by this Operator');
   };
   const subscriptionExists = (ns: string) =>
-    installedFor(props.subscription.data)(props.operatorGroup.data)(
-      props.packageManifest.data[0].status.packageName,
-    )(ns);
+    installedFor(props.subscription.data)(props.operatorGroup.data)(props.packageManifest.data[0])(
+      ns,
+    );
   const namespaceSupports = (ns: string) => (mode: InstallModeType) => {
     const operatorGroup = props.operatorGroup.data.find((og) => og.metadata.namespace === ns);
     if (!operatorGroup || !ns) {