Bug 2067708: Support source VMs, networks and datastores referenced e…

…ither by id or name in plan and mapping objects (#948) * Bug 2067708: Support source VMs, networks and datastores referenced either by id or name in plan and mapping objects * Adjust mock data to test changes
kubev2v · Apr 19, 2022 · 545418c · 545418c
1 parent 4141385
commit 545418c
Show file tree

Hide file tree

Showing 14 changed files with 82 additions and 51 deletions.
diff --git a/pkg/web/src/app/Mappings/components/MappingBuilder/helpers.ts b/pkg/web/src/app/Mappings/components/MappingBuilder/helpers.ts
@@ -15,7 +15,7 @@ import {
   SourceInventoryProvider,
 } from '@app/queries/types';
 import { IMappingBuilderItem } from './MappingBuilder';
-import { getMappingSourceById, getMappingTargetByRef } from '../helpers';
+import { getMappingSourceByRef, getMappingTargetByRef } from '../helpers';
 import { CLUSTER_API_VERSION, META, ProviderType } from '@app/common/constants';
 import { nameAndNamespace } from '@app/queries/helpers';
 import { filterSourcesBySelectedVMs } from '@app/Plans/components/Wizard/helpers';
@@ -31,7 +31,7 @@ export const getBuilderItemsFromMappingItems = (
   items
     ? (items
         .map((item: MappingItem): IMappingBuilderItem | null => {
-          const source = getMappingSourceById(allSources, item.source.id);
+          const source = getMappingSourceByRef(allSources, item.source);
           const target = getMappingTargetByRef(allTargets, item.destination, mappingType);
           if (source) {
             return { source, target };

diff --git a/pkg/web/src/app/Mappings/components/MappingDetailView/MappingDetailView.tsx b/pkg/web/src/app/Mappings/components/MappingDetailView/MappingDetailView.tsx
@@ -8,7 +8,7 @@ import { LineArrow } from '@app/common/components/LineArrow';
 import { useResourceQueriesForMapping } from '@app/queries';
 import { TruncatedText } from '@app/common/components/TruncatedText';
 import { ResolvedQueries } from '@app/common/components/ResolvedQuery';
-import { getMappingSourceById, getMappingSourceTitle, getMappingTargetTitle } from '../helpers';
+import { getMappingSourceByRef, getMappingSourceTitle, getMappingTargetTitle } from '../helpers';
 import { getMappingItemTargetName, groupMappingItemsByTarget } from './helpers';
 
 import './MappingDetailView.css';
@@ -68,9 +68,9 @@ export const MappingDetailView: React.FunctionComponent<IMappingDetailViewProps>
               <GridItem span={5} className={`mapping-view-box ${spacing.pSm}`}>
                 <ul>
                   {items.map((item, itemIndex) => {
-                    const source = getMappingSourceById(
+                    const source = getMappingSourceByRef(
                       mappingResourceQueries.availableSources,
-                      item.source.id
+                      item.source
                     );
                     const sourceName = source ? source.name : '';
                     return (

diff --git a/pkg/web/src/app/Mappings/components/helpers.ts b/pkg/web/src/app/Mappings/components/helpers.ts
@@ -1,6 +1,7 @@
 import * as React from 'react';
 import { isSameResource } from '@app/queries/helpers';
 import {
+  IdOrNameRef,
   IMetaObjectMeta,
   INetworkMappingItem,
   IOpenShiftProvider,
@@ -20,8 +21,11 @@ import { getBuilderItemsFromMapping } from './MappingBuilder/helpers';
 import { ProviderType } from '@app/common/constants';
 import { getStorageTitle } from '@app/common/helpers';
 
-export const getMappingSourceById = (sources: MappingSource[], id: string): MappingSource | null =>
-  sources.find((source) => source.id === id) || null;
+export const getMappingSourceByRef = (
+  sources: MappingSource[],
+  ref: IdOrNameRef
+): MappingSource | null =>
+  sources.find((source) => (ref.id ? source.id === ref.id : source.name === ref.name)) || null;
 
 export const getMappingTargetByRef = (
   targets: MappingTarget[],

diff --git a/pkg/web/src/app/Plans/components/PlanDetailsModal.tsx b/pkg/web/src/app/Plans/components/PlanDetailsModal.tsx
@@ -51,7 +51,7 @@ export const PlanDetailsModal: React.FunctionComponent<IPlanDetailsModalProps> =
     allProviders.find((provider) => isSameResource(provider, plan.spec.provider.source)) || null;
 
   const vmsQuery = useSourceVMsQuery(provider);
-  const selectedVMs = vmsQuery.data?.findVMsByIds(plan.spec.vms.map(({ id }) => id)) || [];
+  const selectedVMs = vmsQuery.data?.findVMsByRefs(plan.spec.vms) || [];
 
   const hooksQuery = useHooksQuery();
   const selectedHooks =

diff --git a/pkg/web/src/app/Plans/components/VMMigrationDetails.tsx b/pkg/web/src/app/Plans/components/VMMigrationDetails.tsx
@@ -95,7 +95,7 @@ export const VMMigrationDetails: React.FunctionComponent = () => {
 
   const vmsQuery = useSourceVMsQuery(sourceProvider);
   const getVMName = (vmStatus: IVMStatus) => {
-    const nameFromInventory = vmsQuery.data?.vmsById[vmStatus.id]?.name || null;
+    const nameFromInventory = vmsQuery.data?.findVMByRef(vmStatus)?.name || null;
     return nameFromInventory || vmStatus.name;
   };
 
@@ -112,9 +112,9 @@ export const VMMigrationDetails: React.FunctionComponent = () => {
 
   const vmStatuses: IVMStatus[] = planStarted
     ? plan?.status?.migration?.vms || []
-    : plan?.spec.vms.map(({ id }) => ({
-        id,
-        name: vmsQuery.data?.vmsById[id]?.name || '',
+    : plan?.spec.vms.map((vm) => ({
+        id: vmsQuery.data?.findVMByRef(vm)?.id || '',
+        name: vmsQuery.data?.findVMByRef(vm)?.name || '',
         pipeline: [],
         phase: '',
       })) || [];
@@ -280,7 +280,7 @@ export const VMMigrationDetails: React.FunctionComponent = () => {
     const isExpanded = isVMExpanded(vmStatus);
     const ratio = getTotalCopiedRatio(vmStatus);
     const isCanceled = isVMCanceled(vmStatus);
-    const vm = vmsQuery.data?.vmsById[vmStatus.id];
+    const vm = vmsQuery.data?.findVMByRef(vmStatus);
     rows.push({
       meta: { vmStatus },
       selected: isItemSelected(vmStatus),
@@ -429,7 +429,7 @@ export const VMMigrationDetails: React.FunctionComponent = () => {
         isOpen={isCancelModalOpen}
         toggleOpen={toggleCancelModal}
         mutateFn={() => {
-          const vmsToCancel = vmsQuery.data?.findVMsByIds(selectedItems.map(({ id }) => id)) || [];
+          const vmsToCancel = vmsQuery.data?.findVMsByRefs(selectedItems) || [];
           cancelVMsMutation.mutate(vmsToCancel);
         }}
         mutateResult={cancelVMsMutation}

diff --git a/pkg/web/src/app/Plans/components/Wizard/PlanWizard.tsx b/pkg/web/src/app/Plans/components/Wizard/PlanWizard.tsx
@@ -251,7 +251,8 @@ export const PlanWizard: React.FunctionComponent = () => {
     // eslint-disable-next-line react-hooks/exhaustive-deps
   }, [mutationStatus]);
 
-  const selectedVMs = vmsQuery.data?.findVMsByIds(forms.selectVMs.values.selectedVMIds) || [];
+  const selectedVMs =
+    vmsQuery.data?.findVMsByRefs(forms.selectVMs.values.selectedVMIds.map((id) => ({ id }))) || [];
 
   const clusterTreeQuery = useInventoryTreeQuery(
     forms.general.values.sourceProvider,

diff --git a/pkg/web/src/app/Plans/components/Wizard/helpers.tsx b/pkg/web/src/app/Plans/components/Wizard/helpers.tsx
@@ -553,7 +553,7 @@ export const getSelectedVMsFromPlan = (
   indexedVMs: IndexedSourceVMs | undefined
 ): SourceVM[] => {
   if (!planBeingPrefilled || !indexedVMs) return [];
-  return indexedVMs.findVMsByIds(planBeingPrefilled?.spec.vms.map(({ id }) => id));
+  return indexedVMs.findVMsByRefs(planBeingPrefilled?.spec.vms);
 };
 
 interface IPlanWizardPrefillResults {

diff --git a/pkg/web/src/app/queries/__tests__/vms.test.ts b/pkg/web/src/app/queries/__tests__/vms.test.ts
@@ -14,15 +14,6 @@ describe('indexVMs', () => {
     ]);
   });
 
-  it('indexes VMs by id', () => {
-    const { vmsById } = indexedVMs;
-    expect(vmsById['vm-1630']?.name).toEqual('fdupont-test-migration');
-    expect(vmsById['vm-2844']?.name).toEqual('fdupont-test');
-    expect(vmsById['vm-1008']?.name).toEqual('fdupont-test-migration-centos');
-    expect(vmsById['vm-2685']?.name).toEqual('pemcg-discovery01');
-    expect(vmsById['vm-431']?.name).toEqual('pemcg-iscsi-target');
-  });
-
   it('indexes VMs by selfLink', () => {
     const { vmsBySelfLink } = indexedVMs;
     expect(vmsBySelfLink['/providers/vsphere/test/vms/vm-1630']?.name).toEqual(
@@ -37,7 +28,20 @@ describe('indexVMs', () => {
   });
 
   it('finds multiple VMs by ids correctly, ignoring invalid ids', () => {
-    const foundVMs = indexedVMs.findVMsByIds(['vm-2844', 'vm-something-invalid', 'vm-1630']);
+    const foundVMs = indexedVMs.findVMsByRefs([
+      { id: 'vm-2844' },
+      { id: 'vm-something-invalid' },
+      { id: 'vm-1630' },
+    ]);
+    expect(foundVMs.map((vm) => vm.name)).toEqual(['fdupont-test', 'fdupont-test-migration']);
+  });
+
+  it('finds multiple VMs by names correctly, ignoring invalid names', () => {
+    const foundVMs = indexedVMs.findVMsByRefs([
+      { name: 'fdupont-test' },
+      { name: 'vm-something-invalid' },
+      { name: 'fdupont-test-migration' },
+    ]);
     expect(foundVMs.map((vm) => vm.name)).toEqual(['fdupont-test', 'fdupont-test-migration']);
   });
 

diff --git a/pkg/web/src/app/queries/mocks/mappings.mock.ts b/pkg/web/src/app/queries/mocks/mappings.mock.ts
@@ -70,7 +70,7 @@ if (process.env.NODE_ENV === 'test' || process.env.DATA_SOURCE === 'mock') {
       map: [
         {
           source: {
-            id: MOCK_VMWARE_DATASTORES[1].id,
+            name: MOCK_VMWARE_DATASTORES[1].name,
           },
           destination: {
             storageClass: MOCK_STORAGE_CLASSES_BY_PROVIDER['ocpv-1'][1].name,
@@ -174,7 +174,7 @@ if (process.env.NODE_ENV === 'test' || process.env.DATA_SOURCE === 'mock') {
       map: [
         {
           source: {
-            id: MOCK_VMWARE_NETWORKS[1].id,
+            name: MOCK_VMWARE_NETWORKS[1].name,
           },
           destination: {
             ...nameAndNamespace(MOCK_OPENSHIFT_NETWORKS[1]),

diff --git a/pkg/web/src/app/queries/mocks/plans.mock.ts b/pkg/web/src/app/queries/mocks/plans.mock.ts
@@ -1,4 +1,4 @@
-import { IPlan, IPlanVM, IVMStatus } from '../types';
+import { IPlan, IVMStatus } from '../types';
 import { MOCK_INVENTORY_PROVIDERS } from '@app/queries/mocks/providers.mock';
 import { CLUSTER_API_VERSION, META } from '@app/common/constants';
 import { nameAndNamespace } from '../helpers';
@@ -9,20 +9,21 @@ import { MOCK_HOOKS } from './hooks.mock';
 export let MOCK_PLANS: IPlan[];
 
 if (process.env.NODE_ENV === 'test' || process.env.DATA_SOURCE === 'mock') {
-  const vm1: IPlanVM = {
+  const vm1 = {
     id: 'vm-1630', // fdupont-test-migration
   };
 
-  const vm2: IPlanVM = {
+  const vm2 = {
     id: 'vm-2844', // fdupont-test
   };
 
-  const vm3: IPlanVM = {
+  const vm3 = {
     id: 'vm-1008', // fdupont-test-migration-centos
   };
 
-  const vm4: IPlanVM = {
-    id: 'vm-2685', // pemcg-discovery01
+  const vm4 = {
+    // id: 'vm-2685'
+    name: 'pemcg-discovery01',
   };
 
   const vmStatus1: IVMStatus = {
@@ -113,8 +114,8 @@ if (process.env.NODE_ENV === 'test' || process.env.DATA_SOURCE === 'mock') {
   };
 
   const vmStatus4: IVMStatus = {
-    id: vm4.id,
-    name: 'pemcg-discovery01',
+    id: 'vm-2685',
+    name: vm4.name,
     pipeline: [
       {
         name: 'DiskTransfer',

diff --git a/pkg/web/src/app/queries/types/common.types.ts b/pkg/web/src/app/queries/types/common.types.ts
@@ -56,3 +56,8 @@ export interface INameNamespaceRef {
   name: string;
   namespace: string;
 }
+
+export interface IdOrNameRef {
+  id?: string;
+  name?: string;
+}
diff --git a/pkg/web/src/app/queries/types/mappings.types.ts b/pkg/web/src/app/queries/types/mappings.types.ts
@@ -1,7 +1,12 @@
 import { ISourceNetwork, IOpenShiftNetwork } from './networks.types';
 import { ISourceStorage } from './storages.types';
 import { IAnnotatedStorageClass } from './storages.types';
-import { IMetaObjectGenerateName, IMetaObjectMeta, IMetaTypeMeta } from './common.types';
+import {
+  IdOrNameRef,
+  IMetaObjectGenerateName,
+  IMetaObjectMeta,
+  IMetaTypeMeta,
+} from './common.types';
 import { ISrcDestRefs } from './providers.types';
 
 export enum MappingType {
@@ -10,9 +15,7 @@ export enum MappingType {
 }
 
 export interface INetworkMappingItem {
-  source: {
-    id: string;
-  };
+  source: IdOrNameRef;
   destination:
     | {
         name: string;
@@ -23,9 +26,7 @@ export interface INetworkMappingItem {
 }
 
 export interface IStorageMappingItem {
-  source: {
-    id: string;
-  };
+  source: IdOrNameRef;
   destination: {
     storageClass: string;
   };

diff --git a/pkg/web/src/app/queries/types/plans.types.ts b/pkg/web/src/app/queries/types/plans.types.ts
@@ -1,4 +1,10 @@
-import { ICR, IMetaObjectMeta, INameNamespaceRef, IStatusCondition } from '../types/common.types';
+import {
+  ICR,
+  IdOrNameRef,
+  IMetaObjectMeta,
+  INameNamespaceRef,
+  IStatusCondition,
+} from '../types/common.types';
 import { ISrcDestRefs } from './providers.types';
 
 export interface IProgress {
@@ -55,8 +61,7 @@ export interface IPlanVMHook {
   step: HookStep;
 }
 
-export interface IPlanVM {
-  id: string;
+export interface IPlanVM extends IdOrNameRef {
   hooks?: IPlanVMHook[];
 }
 

diff --git a/pkg/web/src/app/queries/vms.ts b/pkg/web/src/app/queries/vms.ts
@@ -4,16 +4,16 @@ import { UseQueryResult } from 'react-query';
 import { useAuthorizedFetch } from './fetchHelpers';
 import { useMockableQuery, getInventoryApiUrl, sortByName } from './helpers';
 import { MOCK_RHV_VMS, MOCK_VMWARE_VMS } from './mocks/vms.mock';
-import { SourceInventoryProvider } from './types';
+import { IdOrNameRef, SourceInventoryProvider } from './types';
 import { SourceVM, IVMwareVM } from './types/vms.types';
 
 type SourceVMsRecord = Record<string, SourceVM | undefined>;
 
 export interface IndexedSourceVMs {
   vms: SourceVM[];
-  vmsById: SourceVMsRecord;
   vmsBySelfLink: SourceVMsRecord;
-  findVMsByIds: (ids: string[]) => SourceVM[];
+  findVMByRef: (ref: IdOrNameRef) => SourceVM | undefined;
+  findVMsByRefs: (refs: IdOrNameRef[]) => SourceVM[];
   findVMsBySelfLinks: (selfLinks: string[]) => SourceVM[];
 }
 
@@ -23,16 +23,26 @@ const findVMsInRecord = (record: SourceVMsRecord, keys: string[]) =>
 export const indexVMs = (vms: SourceVM[]): IndexedSourceVMs => {
   const sortedVMs = sortByName(vms.filter((vm) => !(vm as IVMwareVM).isTemplate));
   const vmsById: SourceVMsRecord = {};
+  const vmsByName: SourceVMsRecord = {};
   const vmsBySelfLink: SourceVMsRecord = {};
   sortedVMs.forEach((vm) => {
     vmsById[vm.id] = vm;
+    vmsByName[vm.name] = vm;
     vmsBySelfLink[vm.selfLink] = vm;
   });
+  const findVMByRef = (ref: IdOrNameRef): SourceVM | undefined => {
+    const record = ref.id ? vmsById : vmsByName;
+    return record[ref.id ? ref.id : ref.name || ''];
+  };
   return {
     vms: sortedVMs,
-    vmsById,
     vmsBySelfLink,
-    findVMsByIds: (ids) => findVMsInRecord(vmsById, ids),
+    findVMByRef,
+    findVMsByRefs: (refs) =>
+      refs.flatMap((ref) => {
+        const vm = findVMByRef(ref);
+        return (vm ? [vm] : []) as SourceVM[];
+      }),
     findVMsBySelfLinks: (selfLinks) => findVMsInRecord(vmsBySelfLink, selfLinks),
   };
 };