Bug 1869620: kubevirt support only e1000e and virtio #6479

yaacov · 2020-08-31T11:40:05Z

The correct list of supported models is: e1000e and virtio

Screenshots:
Before:

After:

openshift-ci-robot · 2020-08-31T11:40:13Z

@yaacov: This pull request references Bugzilla bug 1869620, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.6.0) matches configured target release for branch (4.6.0)
bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

Bug 1869620: kubevirt support only e1000e and virtio

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

yaacov · 2020-08-31T11:40:54Z

@suomiy @pcbailey @irosenzw @glekner please review

openshift-ci-robot · 2020-08-31T11:41:52Z

@yaacov: This pull request references Bugzilla bug 1869620, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.6.0) matches configured target release for branch (4.6.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

Bug 1869620: kubevirt support only e1000e and virtio

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

atiratree · 2020-08-31T12:20:32Z

frontend/packages/kubevirt-plugin/src/constants/vm/network/network-interface-model.ts

@@ -4,11 +4,7 @@ import { READABLE_VIRTIO } from '../constants';

 export class NetworkInterfaceModel extends ObjectEnum<string> {
  static readonly VIRTIO = new NetworkInterfaceModel('virtio');
-  static readonly E1000 = new NetworkInterfaceModel('e1000');


they are still valid values (you can still have a VM with these models). Can you please just mark them as not supported? Similar to what we do in DiskType enum? And filter them in places where we use them.

using the deprecation logic like in disks :-)

atiratree · 2020-08-31T12:24:49Z

...end/packages/kubevirt-plugin/src/constants/v2v-import/ovirt/ovirt-network-interface-model.ts

@@ -3,18 +3,15 @@ import { ObjectEnum } from '../../object-enum';
 import { NetworkInterfaceModel } from '../../vm/network';

 export class OvirtNetworkInterfaceModel extends ObjectEnum<string> {
-  static readonly E1000 = new OvirtNetworkInterfaceModel('e1000', NetworkInterfaceModel.E1000);
+  static readonly E1000 = new OvirtNetworkInterfaceModel('e1000', NetworkInterfaceModel.E1000E);
  static readonly PCI_PASSTHROUGH = new OvirtNetworkInterfaceModel(


let's wait first until discussion in https://bugzilla.redhat.com/show_bug.cgi?id=1869208 is resolved

https://bugzilla.redhat.com/show_bug.cgi?id=1869208 is resolved as wont fix.
a. the API will continue to accept the unsupported cards,
b. Once Import operator will merge (or not) a fix that will map unsupported network cards, we will could fix our mapping accordingly.

atiratree · 2020-08-31T12:25:21Z

frontend/packages/kubevirt-plugin/integration-tests/tests/utils/constants/vm.ts

@@ -50,11 +50,7 @@ export enum VM_STATUS {
 // Network
 export enum NIC_MODEL {
  VirtIO = 'VirtIO',
-  e1000 = 'e1000',


I suppose we don't need it for testing, but it doesn't hurt to have it here

openshift-ci-robot · 2020-08-31T13:34:41Z

@yaacov: This pull request references Bugzilla bug 1869620, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.6.0) matches configured target release for branch (4.6.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

Bug 1869620: kubevirt support only e1000e and virtio

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

yaacov · 2020-08-31T13:44:01Z

/test e2e-gcp-console

yaacov · 2020-08-31T14:08:24Z

/retest

atiratree · 2020-09-01T08:09:29Z

frontend/packages/kubevirt-plugin/src/constants/vm/network/network-interface-model.ts

@@ -27,10 +37,16 @@ export class NetworkInterfaceModel extends ObjectEnum<string> {
  static fromString = (model: string): NetworkInterfaceModel =>
    NetworkInterfaceModel.stringMapper[model];

+  isDeprecated = () => this.deprecated;


can we rather call it isSupported since the value still can be used (but shouldn't in the UI)? This is a probably different semantic than floppy.

atiratree · 2020-09-02T12:32:12Z

frontend/packages/kubevirt-plugin/src/components/modals/nic-modal/nic-modal.tsx

-                );
-              })}
+              {NetworkInterfaceModel.getAll()
+                .filter((ifaceModel) => ifaceModel.isSupported())


we should show the unsupported one if it is already selected in the beginning

added a filter exception 👍

atiratree · 2020-09-03T09:51:15Z

frontend/packages/kubevirt-plugin/src/components/modals/nic-modal/nic-modal.tsx

+              {NetworkInterfaceModel.getAll()
+                .filter(
+                  (ifaceModel) =>
+                    ifaceModel.isSupported() || ifaceModel.toString() === model.toString(),


Suggested change

ifaceModel.isSupported() || ifaceModel.toString() === model.toString(),

ifaceModel.isSupported() || ifaceModel === model,

you can just compare - plus no NPE will occur

atiratree · 2020-09-03T11:29:32Z

/lgtm

openshift-ci-robot · 2020-09-03T11:29:48Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: suomiy, yaacov

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:

~~frontend/packages/kubevirt-plugin/OWNERS~~ [suomiy,yaacov]

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

openshift-bot · 2020-09-03T15:41:38Z

/retest

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

openshift-bot · 2020-09-03T16:07:48Z

/retest

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

openshift-ci-robot · 2020-09-03T16:54:27Z

@yaacov: All pull requests linked via external trackers have merged:

openshift/console#6479

Bugzilla bug 1869620 has been moved to the MODIFIED state.

In response to this:

Bug 1869620: kubevirt support only e1000e and virtio

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

yaacov · 2020-09-15T08:30:46Z

/cherrypick release-4.5

openshift-cherrypick-robot · 2020-09-15T08:31:03Z

@yaacov: new pull request created: #6622

In response to this:

/cherrypick release-4.5

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-ci-robot added bugzilla/severity-high Referenced Bugzilla bug's severity is high for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. labels Aug 31, 2020

openshift-ci-robot added the component/kubevirt Related to kubevirt-plugin label Aug 31, 2020

openshift-ci-robot requested review from jelkosz and rawagner August 31, 2020 11:40

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 31, 2020

atiratree reviewed Aug 31, 2020

View reviewed changes

deprecate none suported network cards

a43cde8

yaacov force-pushed the kubevirt-support-only-e1000e-and-virtio branch from e57bb3a to a43cde8 Compare August 31, 2020 13:32

atiratree reviewed Sep 1, 2020

View reviewed changes

use supported instead of deprecated

870c038

atiratree reviewed Sep 2, 2020

View reviewed changes

show the unsupprted network card if vm use it

55addbd

atiratree suggested changes Sep 3, 2020

View reviewed changes

compare intreface directly

6691a12

openshift-ci-robot assigned atiratree Sep 3, 2020

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Sep 3, 2020

openshift-merge-robot merged commit 20c1c18 into openshift:master Sep 3, 2020

spadgett added this to the v4.6 milestone Sep 11, 2020

openshift-cherrypick-robot mentioned this pull request Sep 15, 2020

[release-4.5] Bug 1879016: kubevirt support only e1000e and virtio #6622

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bug 1869620: kubevirt support only e1000e and virtio #6479

Bug 1869620: kubevirt support only e1000e and virtio #6479

yaacov commented Aug 31, 2020 •

edited

openshift-ci-robot commented Aug 31, 2020

yaacov commented Aug 31, 2020

openshift-ci-robot commented Aug 31, 2020

atiratree Aug 31, 2020

yaacov Aug 31, 2020

atiratree Aug 31, 2020

yaacov Aug 31, 2020

yaacov Sep 2, 2020

atiratree Aug 31, 2020

yaacov Aug 31, 2020

openshift-ci-robot commented Aug 31, 2020

yaacov commented Aug 31, 2020

yaacov commented Aug 31, 2020

atiratree Sep 1, 2020

yaacov Sep 1, 2020

atiratree Sep 2, 2020

yaacov Sep 2, 2020

atiratree Sep 3, 2020

atiratree commented Sep 3, 2020

openshift-ci-robot commented Sep 3, 2020

openshift-bot commented Sep 3, 2020

openshift-bot commented Sep 3, 2020

openshift-ci-robot commented Sep 3, 2020

yaacov commented Sep 15, 2020

openshift-cherrypick-robot commented Sep 15, 2020

	ifaceModel.isSupported() \|\| ifaceModel.toString() === model.toString(),
	ifaceModel.isSupported() \|\| ifaceModel === model,

Bug 1869620: kubevirt support only e1000e and virtio #6479

Bug 1869620: kubevirt support only e1000e and virtio #6479

Conversation

yaacov commented Aug 31, 2020 • edited

openshift-ci-robot commented Aug 31, 2020

yaacov commented Aug 31, 2020

openshift-ci-robot commented Aug 31, 2020

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

openshift-ci-robot commented Aug 31, 2020

yaacov commented Aug 31, 2020

yaacov commented Aug 31, 2020

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

atiratree commented Sep 3, 2020

openshift-ci-robot commented Sep 3, 2020

openshift-bot commented Sep 3, 2020

openshift-bot commented Sep 3, 2020

openshift-ci-robot commented Sep 3, 2020

yaacov commented Sep 15, 2020

openshift-cherrypick-robot commented Sep 15, 2020

yaacov commented Aug 31, 2020 •

edited