CNV19658: Add migrations table #759

avivtur · 2022-07-13T15:24:41Z

Signed-off-by: Aviv Turgeman aturgema@redhat.com

📝 Description

Adding a table for migrations
more info:
UX design: https://docs.google.com/document/d/1VWGGOe1UhmhEqGMCN6kE6zl4-kzmd-orpnDzvh7hG6M/edit#heading=h.zi7ooe3oqbb4
sub task: https://issues.redhat.com/browse/CNV-19658
issue: https://issues.redhat.com/browse/CNV-18220?page=com.docminer.jira.issue-links:com.kintosoft.jira.links.tab-panel

🎥 Demo

migrations_table.mp4

glekner · 2022-07-17T15:39:21Z

/lgtm

openshift-bot · 2022-07-17T18:51:28Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-07-17T19:03:27Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-07-17T19:15:28Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-07-17T19:27:29Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-07-17T19:39:28Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-07-17T19:51:28Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-07-17T20:03:29Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-07-17T20:15:27Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-07-17T20:27:28Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-07-17T20:39:29Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-07-17T20:51:28Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-07-17T21:03:29Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-07-17T21:15:28Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-07-17T21:27:29Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-07-17T21:39:28Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-07-17T21:51:28Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-07-18T04:03:29Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-07-18T04:15:29Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-07-18T04:27:29Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-07-18T04:39:28Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-07-18T04:51:27Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

openshift-bot · 2022-07-18T05:03:29Z

/retest-required

Please review the full test history for this PR and help us cut down flakes.

avivtur · 2022-07-18T05:14:17Z

/hold

Signed-off-by: Aviv Turgeman <aturgema@redhat.com>

avivtur · 2022-07-18T06:41:25Z

/unhold

glekner · 2022-07-18T06:46:07Z

/lgtm

openshift-ci · 2022-07-18T06:46:15Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: avivtur, glekner

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [avivtur,glekner]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

metalice · 2022-07-18T07:02:59Z

tsconfig.json

@@ -16,7 +16,8 @@
    "paths": {
      "@kubevirt-utils": ["src/utils"],
      "@kubevirt-utils/*": ["src/utils/*"],
-      "@catalog/*": ["src/views/catalog/*"]
+      "@catalog/*": ["src/views/catalog/*"],
+      "@virtualmachines/*": ["src/views/virtualmachines/*"]


why we need it? virtualmachines isnt sharable. we just saving some ../../../ ?

yes its to avoid ../../......../.. pattern

metalice · 2022-07-18T07:04:07Z

...iews/clusteroverview/MonitoringTab/migrations-card/components/MigrationsTable/utils/utils.ts

+    const mpObj = vmiObj?.status?.migrationState?.migrationPolicyName
+      ? (mps || []).find(
+          (mp) => mp?.metadata?.name === vmiObj?.status?.migrationState?.migrationPolicyName,


consider naming migrationPolicy instead of mps, or mp

metalice · 2022-07-18T07:05:02Z

...iews/clusteroverview/MonitoringTab/migrations-card/components/MigrationsTable/utils/utils.ts

+const ONE_SECOND_IN_MS = 1000;
+const ONE_MINUTE_IN_MS = 60 * ONE_SECOND_IN_MS;
+const FIVE_MINUTES_IN_MS = 5 * ONE_MINUTE_IN_MS;
+const FIFTEEN_MINUTES_IN_MS = 15 * ONE_MINUTE_IN_MS;
+const THIRTY_MINUTES_IN_MS = 30 * ONE_MINUTE_IN_MS;
+const ONE_HOUR_IN_MS = 60 * ONE_MINUTE_IN_MS;
+const THREE_HOURS_IN_MS = 3 * ONE_HOUR_IN_MS;
+const SIX_HOURS_IN_MS = 6 * ONE_HOUR_IN_MS;
+const TWELVE_HOURS_IN_MS = 12 * ONE_HOUR_IN_MS;
+const ONE_DAY_IN_MS = 24 * ONE_HOUR_IN_MS;
+const TWO_DAYS_IN_MS = 2 * ONE_DAY_IN_MS;
+const ONE_WEEK_IN_MS = 7 * ONE_DAY_IN_MS;
+
+const valueToMSMapper = {
+  '5m': FIVE_MINUTES_IN_MS,
+  '15m': FIFTEEN_MINUTES_IN_MS,
+  '30m': THIRTY_MINUTES_IN_MS,
+  '1h': ONE_HOUR_IN_MS,
+  '3h': THREE_HOURS_IN_MS,
+  '6h': SIX_HOURS_IN_MS,
+  '12h': TWELVE_HOURS_IN_MS,
+  '1d': ONE_DAY_IN_MS,
+  '2d': TWO_DAYS_IN_MS,
+  '1w': ONE_WEEK_IN_MS,
+};


Please export it to a more shareable place, other components I'm sure will love to use it too.

suggestion? @metalice

metalice · 2022-07-18T07:05:53Z

...s/clusteroverview/MonitoringTab/migrations-card/components/MigrationsTable/utils/statuses.ts

+export const iconMapper = {
+  Pending: InProgressIcon,
+  Scheduling: InProgressIcon,
+  Scheduled: InProgressIcon,
+  PreparingTarget: InProgressIcon,
+  TargetReady: InProgressIcon,
+  Paused: PausedIcon,
+  Running: SyncAltIcon,
+  Succeeded: GreenCheckCircleIcon,
+  Failed: RedExclamationCircleIcon,
+};


Default icon for non-existing status?

metalice · 2022-07-18T07:06:26Z

...ws/clusteroverview/MonitoringTab/migrations-card/components/MigrationsTable/utils/filters.ts

+      filter: (selectedNodes, obj) => {
+        const nodeName = vmis?.find((vmi) => vmi.metadata?.name === obj?.vmiObj?.metadata?.name)
+          ?.status?.migrationState?.targetNode;
+        return selectedNodes.selected?.length === 0 || selectedNodes.selected?.includes(nodeName);


metalice · 2022-07-18T07:07:27Z

...ws/clusteroverview/MonitoringTab/migrations-card/components/MigrationsTable/utils/filters.ts

+import { RowFilter } from '@openshift-console/dynamic-plugin-sdk';
+
+import { vmimStatuses } from './statuses';
+


please check for all those functions that receive t as argument if you can simply import the default T , not the one from the hook, directly into this file

we can add a comment to utils/i18n.ts and remove this parameter, but I don't like that approach

metalice · 2022-07-18T07:09:49Z

...clusteroverview/MonitoringTab/migrations-card/components/MigrationsTable/MigrationsTable.tsx

+          rowFilters={filters}
+          onFilterChange={onFilterChange}
+        />
+        <VirtualizedTable


missing type?

metalice · 2022-07-18T07:10:34Z

...clusteroverview/MonitoringTab/migrations-card/components/MigrationsTable/MigrationsTable.tsx

+    ...getStatusFilter(t),
+    ...getSourceNodeFilter(vmis, t),
+    ...getTargetNodeFilter(vmis, t),
+  ];


is this a "heavy action"? should we consider memo here?

what do you mean by heavy action?

metalice · 2022-07-18T07:13:11Z

...clusteroverview/MonitoringTab/migrations-card/components/MigrationsTable/MigrationsTable.tsx

+  });
+
+  const [mps] = useK8sWatchResource<V1alpha1MigrationPolicy[]>({
+    groupVersionKind: {


can we use modelToGroupVersionKind or something similar here?

metalice · 2022-07-18T07:13:35Z

...s/clusteroverview/MonitoringTab/migrations-card/components/MigrationsTable/MigrationsRow.tsx

+            }
+          >
+            <ResourceLink
+              groupVersionKind={{


can we use modelToGroupVersionKind or something similar here?

we dont have migrationpolicymodel?

I've send a PR to add this model to kubevirt-api: kubevirt-ui/kubevirt-api#23
but we need to build a new release for it

metalice · 2022-07-18T07:15:09Z

...s/clusteroverview/MonitoringTab/migrations-card/components/MigrationsTable/MigrationsRow.tsx

+
+const MigrationsRow: React.FC<RowProps<MigrationTableDataLayout>> = ({ obj, activeColumnIDs }) => {
+  const { t } = useKubevirtTranslation();
+  const StatusIcon = iconMapper[obj?.vmim?.status?.phase];


iconMapper?.

metalice · 2022-07-18T07:16:15Z

...erview/MonitoringTab/migrations-card/components/MigrationsTable/MigrationActionsDropdown.tsx

+}) => {
+  const { t } = useKubevirtTranslation();
+
+  const [isOpen, setIsOpen] = React.useState(false);


missing type

openshift-ci bot requested review from glekner and pcbailey July 13, 2022 15:24

openshift-ci bot added the approved This issue is something we want to fix label Jul 13, 2022

avivtur force-pushed the migration_page_table branch from 80e865a to 1c5a477 Compare July 13, 2022 15:39

avivtur changed the title ~~Add migrations table~~ [WIP] CNV19658: Add migrations table Jul 14, 2022

openshift-ci bot added the do-not-merge/work-in-progress label Jul 14, 2022

avivtur force-pushed the migration_page_table branch from 1c5a477 to 2c157a8 Compare July 14, 2022 16:56

avivtur changed the title ~~[WIP] CNV19658: Add migrations table~~ CNV19658: Add migrations table Jul 14, 2022

openshift-ci bot removed the do-not-merge/work-in-progress label Jul 14, 2022

avivtur force-pushed the migration_page_table branch 2 times, most recently from 49d0a3f to b49966f Compare July 17, 2022 15:37

openshift-ci bot assigned glekner Jul 17, 2022

openshift-ci bot added the lgtm Passed code review, ready for merge label Jul 17, 2022

openshift-ci bot added the do-not-merge/hold label Jul 18, 2022

avivtur added 2 commits July 18, 2022 05:15

Add migrations table

52884bc

Signed-off-by: Aviv Turgeman <aturgema@redhat.com>

Adding tooltips and actions

2afd37f

Signed-off-by: Aviv Turgeman <aturgema@redhat.com>

avivtur force-pushed the migration_page_table branch from b49966f to 2afd37f Compare July 18, 2022 05:16

openshift-ci bot removed the lgtm Passed code review, ready for merge label Jul 18, 2022

openshift-ci bot removed the do-not-merge/hold label Jul 18, 2022

openshift-ci bot added the lgtm Passed code review, ready for merge label Jul 18, 2022

openshift-ci bot merged commit 844e576 into kubevirt-ui:main Jul 18, 2022

metalice reviewed Jul 18, 2022

View reviewed changes

		import { RowFilter } from '@openshift-console/dynamic-plugin-sdk';

		import { vmimStatuses } from './statuses';

CNV19658: Add migrations table #759

CNV19658: Add migrations table #759

Conversation

avivtur commented Jul 13, 2022 • edited Loading

📝 Description

🎥 Demo

glekner commented Jul 17, 2022

openshift-bot commented Jul 17, 2022

openshift-bot commented Jul 17, 2022

openshift-bot commented Jul 17, 2022

openshift-bot commented Jul 17, 2022

openshift-bot commented Jul 17, 2022

openshift-bot commented Jul 17, 2022

openshift-bot commented Jul 17, 2022

openshift-bot commented Jul 17, 2022

openshift-bot commented Jul 17, 2022

openshift-bot commented Jul 17, 2022

openshift-bot commented Jul 17, 2022

openshift-bot commented Jul 17, 2022

openshift-bot commented Jul 17, 2022

openshift-bot commented Jul 17, 2022

openshift-bot commented Jul 17, 2022

openshift-bot commented Jul 17, 2022

openshift-bot commented Jul 18, 2022

openshift-bot commented Jul 18, 2022

openshift-bot commented Jul 18, 2022

openshift-bot commented Jul 18, 2022

openshift-bot commented Jul 18, 2022

openshift-bot commented Jul 18, 2022

avivtur commented Jul 18, 2022

avivtur commented Jul 18, 2022

glekner commented Jul 18, 2022

openshift-ci bot commented Jul 18, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

avivtur commented Jul 13, 2022 •

edited

Loading