WIP: kubernetes: add VMs and VM/VMInstance details #8948

atiratree · 2018-04-05T16:43:47Z

discussed in #8895

functionality:

listing offline vms and their specifications
start, stop, restart, delete actions

Todo:

vm instance (old vm) detail kubernetes: virtual machines - add Vm Detail page #9466
vm instance tests
port code to use new terminology (vm / vm instance)
add support for vms (old ovm) - with new design
add vm detail
blocked on VMs fail to launch with Insufficient devices.kubevirt.io/tun kubevirt/kubevirt#1316
fix tests
tests

atiratree · 2018-04-05T16:47:31Z

First version:

Althought I would prefer offline vm and vm be together in one row and switch between the views, because they are tied logically together.

@andreasn @garrett

andreasn · 2018-04-06T10:02:41Z

Are the offline VMs the ones that says n/a?
Does n/a mean "not available" or something else? If so, I think it's better to write out the whole string.

atiratree · 2018-04-06T14:05:58Z

Offline vm does not have any state by itself so there can be any empty placeholder.

Although offline vm includes state of it's running vm, like:

if it is running
possibly http readiness
conditions (addititonal info about the running vm)

atiratree · 2018-04-10T14:45:37Z

@andreasn @garrett
this is how offline vms are implemented in openshift: openshift/origin-web-console#2929

@serenamarie125 @beanh66
please can you help us with a design here in cockpit?

beanh66 · 2018-04-19T19:48:07Z

@suomiy Apologies for the delay, I've been on vacation. Let me mock something up and send it your way asap!

beanh66 · 2018-04-21T10:56:27Z

@suomiy Can you provide a screenshot of what the disks section looks like for one of the expanded list view items? Also curious where this page is in the UI? It sounds like we are updating the existing virtual machines page, correct?

beanh66 · 2018-04-23T14:24:01Z

@suomiy I have a few questions, but I took a first pass at this design:

Do we need the "Offline" column if we have another column with the more specific state?
Can you list all the possible states? In OpenShift I believe we have Running, Not Running, Starting, Failed, Unknown.
Can we just show the primary action Start or Stop and put remaining actions in the kebab? I would assume delete is not used as often so perhaps we don't need such a strong call to action.
Can you provide a screenshot of the collapsed list view state? In OpenShift we are only exposing the links in the expanded state, so trying to determine what is needed here.
Can you provide a screenshot showing what the Disks tab looks like?

atiratree · 2018-04-24T15:53:26Z

I have a few questions, but I took a first pass at this design:

thanks

It sounds like we are updating the existing virtual machines page, correct?

yes

Do we need the "Offline" column if we have another column with the more specific state?

No, we do not need that column. I put it there to just show the difference

Can you list all the possible states? In OpenShift I believe we have Running, Not Running, Starting, Failed, Unknown.

Pending, Scheduling, Scheduled, Running, Not Running, Succeeded, Failed, Unknown. It should be like that also in OpenShift

Can we just show the primary action Start or Stop and put remaining actions in the kebab? - I would assume delete is not used as often so perhaps we don't need such a strong call to action.

That is a good idea

Can you provide a screenshot of the collapsed list view state? In OpenShift we are only exposing the links in the expanded state, so trying to determine what is needed here.

It does not change in cockpit and we would like to keep it that way so it is consistent with other lists.

Can you provide a screenshot showing what the Disks tab looks like?

We would like to bundle together vm and offline vm to one list item (unlike this screenshot)
There could be:

links to detail pages of vm and offline vm at (1) (2), not sure where they should be located or how the columns should be named though
Vm and offline vm can have similar tabs with similar information. So it would be best to switch between these two views with some button/switch/slider. This button could be located at (3) or (4). But it would be shown only for offline vms.

We could also have two rows of tabs instead

Vm | Offline Vm
Overview | Disks

Also the Delete has different meaning in vm and offline vm.

offline vm's restart is the same as vm's delete + start again
offline vm's stop is the same as vm's delete

@beanh66

beanh66 · 2018-04-30T14:32:13Z

@suomiy Thank you for providing that context and additional information. We've discussed similar use cases around how to display vms and ovms inside of OpenShift. I have been holding off on updating the design until we come to some agreement on this piece in OpenShift to be sure we remain consistent across products. We have another meeting this Wednesday, so I will follow-up by the end of the week. Tagging @jniederm here as well because he has started on this effort for OpenShift. @serenamarie125 @xsgordon FYI

atiratree · 2018-05-09T10:41:33Z

@beanh66 Any updates?

beanh66 · 2018-05-14T20:12:53Z

@suomiy We just got back from summit, but will provide an update by the end of the week based on the latest work in OpenShift for vms/ovms. Just FYI there have been a number of relevant discussions on the KubeVirt and cnv mailing lists as well.

beanh66 · 2018-05-21T19:11:47Z

@suomiy We sent out a survey on 5/18 to get user feedback on two different approaches for displaying vms and ovms in OpenShift. Will keep you informed on the feedback we get and the direction that implementation is going!

beanh66 · 2018-06-07T15:04:33Z

@suomiy A few updates:
(1) OpenShift survey results have been reviewed and discussed with PM,
(2) The ovm and vm entities have been renamed to vm and vm instance respectively,
(3) OpenShift console designs for CNV have been agreed upon.

Based on the CNV designs and the new entity names, I updated the original design to the following:

1- Links to the main VM entity (previously ovm)
2- Links to the VM Instance (previously vm)
3- Primary action is displayed as a link if the instance exists and either stop or start is available
4- Display the instance state
5- Display -- for fields that do not exist. In this case there is no VM Instance that exists
6- The expanded list view is separated into two columns with Disks shown in column 2. This will eliminate the need for a Disks tab.

atiratree · 2018-06-12T15:59:27Z

@beanh66 Thanks a lot! I will start implementing it. Only one caveat. I think we will have keep the tabs there, because not everything can fit there. Currently there is Overview, Usage (WIP) and Disks. The usage especiallym, but even the disks have 5 columns.

beanh66 · 2018-06-12T17:20:29Z

@suomiy Oh got it, okay no problem on the tabs.

atiratree · 2018-06-21T15:01:51Z

updated TODO and split this PR

atiratree · 2018-07-19T14:30:06Z

I implemented VM and VM Instance detail + VMs page with @beanh66 's redesign in mind. Entities are ported to use new kubevirt v0.7.0 API and terminology

VMs

/kubernetes#/vms

links to vms are not highlighted with blue color to be consistent with cockpit. Vm detail page is shown when clicking on the row in closed state
I am not sure about the positions of all the items in Overview which should be somehow consistent with the detail (which has more information)
the buttons are not in a dropdown yet due to an issue which I cannot fix right now. Clicking on Boostrap dropdown does not work, although it works fine in angular components. I would be happy if somebody posted a suggestion what could be wrong with that.

Details

/kubernetes#/vms/default/myvm

/kubernetes#/vmis/myproject/testvm

I mocked the running vms because they seem to be broken in kubevirt v0.7.0 (VMs fail to launch with Insufficient devices.kubevirt.io/tun kubevirt/kubevirt#1316). I am not sure how I should proceed with the tests and if I should wait until this is fixed

Note: Continuation of #9466 which can be merged first and depends on the old API

@andreasn @garrett @beanh66

Please let me know if there are any issues with the design

atiratree · 2018-07-24T18:35:01Z

the buttons are not in a dropdown yet due to an issue which I cannot fix right now. Clicking on Boostrap dropdown does not work, although it works fine in angular components. I would be happy if somebody posted a suggestion what could be wrong with that.

fixed; dropdowns are working now

beanh66 · 2018-07-26T12:28:40Z

@suomiy This is looking great! Three questions,
(1) can we have the details listed in the same order for both the summary list view and the detail page? Example: in the list view you have memory, cpu, instance on the left and node, labels, pod on the right. Then in the detail you you have memory, cpu, POD, age on the left and state, node, label on the right.
(2) is there a reason the create button is shown as a link? In other products this is usually shown as a primary button, but maybe this is consistent within Cockpit.
(3) Do you show what a vm instance detail page would look like?

mareklibra

Pretty complex one for review :-)
Eager to try.

Most important review comment: To keep or not to keep Redux state across page navigation, that is the question.

mareklibra · 2018-07-26T12:57:11Z

pkg/kubernetes/scripts/virtual-machines.js

@@ -20,14 +20,25 @@
 (function() {
    "use strict";

+    /* Expose jQuery and Bootstrap to non-Angular code */
+    var jquery = require('jquery/dist/jquery.js');


Shouldn't this go somewhere higher in the hierarchy?
If this hack is needed, it should be available even without virtual machines

It is needed only for virtual machines because angular has it's own version of jQuery and angular-bootstrap-npm is used. There is no other non-angular code in kubernetes so it is sufficient to leave it only here. I would move it higher when new use case occurs.

mareklibra · 2018-07-26T13:02:51Z

pkg/kubernetes/scripts/virtual-machines/components/common/DisksTabKubevirt.jsx

-const VmDisksTabKubevirt = ({ vm, pvs }: { vm: Vm, pvs: Array<PersistenVolume> }) => {
-    const idPrefix = `${vmIdPrefx(vm)}-disks`;
+const DisksTabKubevirt = ({ vm, pvs }: { vm: Vm | Vmi, pvs: Array<PersistenVolume> }) => {
+    const idPrefix = prefixedId(kindIdPrefx(vm), 'disks');


nit: Let's enjoy the simplicity :-)

mareklibra · 2018-07-26T13:14:16Z

test/verify/check-openshift

        # The "fedoravm" VirtualMachine is created from OfflineVirtualMachine
        o.execute("oc patch offlinevirtualmachine fedoravm --type=merge -p '{\"spec\":{\"running\": true}}'")  # let's start the VM
        print("starting vm, might take a while")
        wait(lambda: "fedoravm" in o.execute("oc get vm"), msg='Start of the "fedoravm" virtual machine failed.', delay=2)
        wait(lambda: "virt-launcher-fedoravm" in o.execute("oc get pods"), delay=2)
        wait(lambda: "Running" in o.execute("oc get pods | grep virt-launcher-fedoravm"), delay=2, tries=150) # Example: virt-launcher-fedoravm-----rx59t

+    def testBrowserLocation(self, url):


use of test as prefix for function name is ambiguous in this context (it's not actual test)

thanks, I forgot I misnamed it this way

mareklibra · 2018-07-26T13:17:42Z

test/verify/check-openshift

        # The "fedoravm" VirtualMachine is created from OfflineVirtualMachine
        o.execute("oc patch offlinevirtualmachine fedoravm --type=merge -p '{\"spec\":{\"running\": true}}'")  # let's start the VM
        print("starting vm, might take a while")
        wait(lambda: "fedoravm" in o.execute("oc get vm"), msg='Start of the "fedoravm" virtual machine failed.', delay=2)
        wait(lambda: "virt-launcher-fedoravm" in o.execute("oc get pods"), delay=2)
        wait(lambda: "Running" in o.execute("oc get pods | grep virt-launcher-fedoravm"), delay=2, tries=150) # Example: virt-launcher-fedoravm-----rx59t

+    def testBrowserLocation(self, url):


Please rename, this is not actual test

mareklibra · 2018-07-26T13:19:13Z

test/verify/check-openshift

+            b = self.browser
+
+            b.wait_present(
+                "#vm-fedoravm-cpu-metric div svg")  # cannot check cpu component because it is an external dependency


no need to break the line, IMO

mareklibra · 2018-07-26T13:26:09Z

pkg/kubernetes/scripts/virtual-machines.js

@@ -42,9 +53,23 @@
        '$locationProvider',
        function($routeProvider, $locationProvider) {
            $routeProvider
-                .when('/vms', {
+                .when(paths.VMS, {


I think I wrote this somewhere already.

The drawback of this approach is the need to re-initialize redux all the time the page changes.
Works ok now but what if we will need to keep state between navigation?

Does it make sense to route /vms (or similar) URL context via angular and then leave sub-context on react (like react-router or even some super-simple custom implementation - see pkg/machines for an example)?

yes, we discussed this in #9466

Yes, but we also want the possibility to navigate to the single vm. And we want the redux store to represent only what the application needs. Plus almost all the data is cached by kube-client and the request to get the node metrics are occurring once a few seconds anyway.

Works ok now but what if we will need to keep state between navigation?

actually we are keeping the same store alive between the redirections, we are just throwing out most of the data we don't need, so this use-case is possible

atiratree · 2018-07-26T16:18:50Z

@beanh66

(1) can we have the details listed in the same order for both the summary list view and the detail page? Example: in the list view you have memory, cpu, instance on the left and node, labels, pod on the right. Then in the detail you you have memory, cpu, POD, age on the left and state, node, label on the right.

I wanted the state to be somewhere visible (possibly close to the original location in the list). There is also an "extra" node and age. Can you please suggest how both VM/VMI and VM list columns should be sorted?

When I look at it now, I think labels should be probably at the end because they can potentially take up a lot of space.

(2) is there a reason the create button is shown as a link? In other products this is usually shown as a primary button, but maybe this is consistent within Cockpit.

Yes, it is the default behaviour in Cockpit and kubernetes plugin

(3) Do you show what a vm instance detail page would look like?

Sorry I uploaded wrong content. VM should have VM label in the detail and VM Instance should have VMI label. I updated my original comment with correct image #8948 (comment)

beanh66 · 2018-07-26T19:15:59Z

@suomiy How about on the left column you have Memory, CPU, Pod, VMI and on the right column you have state, age, node, labels?

atiratree · 2018-07-31T18:58:34Z

How about on the left column you have Memory, CPU, Pod, VMI and on the right column you have state, age, node, labels?

@beanh66 Good suggestion! Final version:

VMs

VM

VM Instance

Responsive

atiratree · 2018-07-31T18:59:36Z

@mareklibra I also changed vmOverviewTab in machines

…nce details + other changes - rename OfflineVirtualMachine -> VirtualMachine - rename VirtualMachine -> VirtualMachineInstance - kubernetes#/vms shows VirtualMachines instead of VirtualMachineInstances - links in vms redirect to detail pages - visual changes in tabs and details (age,..) - refactor and add new flow types - general refactoring - kube-client: add VirtualMachine type

atiratree · 2018-08-01T16:39:32Z

fixed few bugs (mainly metrics)

atiratree · 2018-08-08T12:30:28Z

kubevirt/kubevirt#1320 needs to be fixed together with the kubevirt image #9638

martinpitt · 2018-09-17T10:33:05Z

Due to insufficient maintenance, and kubevirt UI moving to Tectonic/Quay, we are removing the kubevirt bits from Cockpit now. Details:

martinpitt changed the title ~~kubernetes: add offline vms (WIP)~~ WIP: kubernetes: add offline vms Apr 24, 2018

atiratree force-pushed the offline-vms branch from c1f1ee3 to 6bff069 Compare April 24, 2018 20:06

atiratree mentioned this pull request Jun 21, 2018

kubernetes: virtual machines - add Vm Detail page #9466

Closed

3 tasks

atiratree force-pushed the offline-vms branch 3 times, most recently from dfe2a29 to ec8d823 Compare July 19, 2018 13:54

atiratree mentioned this pull request Jul 19, 2018

openshift: Latest stable Kubevirt version respects semver comparision #9638

Closed

1 task

atiratree force-pushed the offline-vms branch 2 times, most recently from 8a8c08b to ec8d823 Compare July 19, 2018 15:30

atiratree changed the title ~~WIP: kubernetes: add offline vms~~ WIP: kubernetes: add VMs and VM/VmInstance details Jul 19, 2018

atiratree changed the title ~~WIP: kubernetes: add VMs and VM/VmInstance details~~ WIP: kubernetes: add VMs and VM/VMInstance details Jul 19, 2018

atiratree force-pushed the offline-vms branch 2 times, most recently from ec8d823 to a4b0db9 Compare July 24, 2018 15:12

atiratree force-pushed the offline-vms branch 2 times, most recently from 3974351 to 66cf394 Compare July 24, 2018 18:28

atiratree force-pushed the offline-vms branch from 66cf394 to c540971 Compare July 25, 2018 15:55

mareklibra suggested changes Jul 26, 2018

View reviewed changes

atiratree force-pushed the offline-vms branch from c540971 to b212d35 Compare July 26, 2018 15:50

atiratree force-pushed the offline-vms branch 2 times, most recently from 57400ef to 10b1f6b Compare July 31, 2018 18:50

suomiy added 4 commits August 1, 2018 13:56

support kubevirt v0.7.0

1965ff7

kubernetes: add mergeType to kube-client's patch

6f8b4c0

machines: refactor DropdownButtons and add disabled property

32b2668

machines: refactor vmOverviewTab and export Items

89d89c7

atiratree force-pushed the offline-vms branch from 10b1f6b to 3601332 Compare August 1, 2018 12:03

suomiy added 2 commits August 1, 2018 18:36

kubernetes: virtual-machines: rename non jsx files to .es6

a644348

atiratree force-pushed the offline-vms branch from 3601332 to a644348 Compare August 1, 2018 16:38

martinpitt closed this Sep 17, 2018

WIP: kubernetes: add VMs and VM/VMInstance details #8948

WIP: kubernetes: add VMs and VM/VMInstance details #8948

Conversation

atiratree commented Apr 5, 2018 • edited Loading

atiratree commented Apr 5, 2018

First version:

andreasn commented Apr 6, 2018

atiratree commented Apr 6, 2018

atiratree commented Apr 10, 2018

beanh66 commented Apr 19, 2018

beanh66 commented Apr 21, 2018

beanh66 commented Apr 23, 2018

atiratree commented Apr 24, 2018

beanh66 commented Apr 30, 2018 • edited Loading

atiratree commented May 9, 2018

beanh66 commented May 14, 2018

beanh66 commented May 21, 2018

beanh66 commented Jun 7, 2018

atiratree commented Jun 12, 2018

beanh66 commented Jun 12, 2018

atiratree commented Jun 21, 2018

atiratree commented Jul 19, 2018 • edited Loading

VMs

/kubernetes#/vms

Details

/kubernetes#/vms/default/myvm

/kubernetes#/vmis/myproject/testvm

atiratree commented Jul 24, 2018

beanh66 commented Jul 26, 2018 • edited Loading

mareklibra left a 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

atiratree commented Jul 26, 2018

beanh66 commented Jul 26, 2018

atiratree commented Jul 31, 2018

VMs

VM

VM Instance

Responsive

atiratree commented Jul 31, 2018

atiratree commented Aug 1, 2018

atiratree commented Aug 8, 2018

martinpitt commented Sep 17, 2018

atiratree commented Apr 5, 2018 •

edited

Loading

beanh66 commented Apr 30, 2018 •

edited

Loading

atiratree commented Jul 19, 2018 •

edited

Loading

beanh66 commented Jul 26, 2018 •

edited

Loading