New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Surfacing VMIs in the UI #310
Conversation
Imp ref: openshift/console#3821 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work, @yfrimanm ! Added a few comments below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is looking great Yifat. 👍
I didn't know much about VMIs before today, but after reading through this PR and the discussion in the docs I agree that this is the best approach (and shouldn't conflict with a "VM Instances" nav item if we end up needing one in the future).
Some rambling comments below. 🙂
Reference implementation of the list view, showing "orphaned" vmis created from the cli: cc:// @yfrimanm @matthewcarleton @andybraren: |
web-console/knikubevirt/VMIs/VMIs.md
Outdated
|
||
We had finalized into this option and will show 2 parallel columns in the VMs list page, one for each: VMs beside VMIs. | ||
|
||
![list of VMIs beside VMs](img/VMsListW_VMIsOp1.png) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: in the image, none running VMs do not have a "node" or "ip" should be empty or some notice.
I don't like the fact that the VM name shows up twice on every line. It violates the Dry principle. This would look worse if the VM name is very long. But I suppose we can drop (what I find as superfluous) VMI name at some point in the future. |
@dankenigsberg @matthewcarleton @andybraren can you think of what we can put instead of the VMI name ? |
@dankenigsberg @matthewcarleton @andybraren - here is a screenshot of what it will look like without using resource link ... #310 (comment) |
Did you intentionally leave out the VMI badge? I'm fine with showing the badge right next to "details". {VMI} details |
I assume the problem you refer to is the repeated "detailed" is string. I find it more reasonable than duplicated name. Did you consider how your suggestion behaves with a long vm name (e.g 200 chars)? |
@yaacov A couple of notes on this screenshot (thanks for the recording!):
|
I think it's important to note that one of the Web Console's goals is to help users learn about APIs and the relationships between resources. We know from user research that even CLI users explore the Console to learn about new CRs (like KubeVirt's), so we generally try to reflect the reality of APIs even when they aren't the prettiest. I agree that seeing the same name twice in a row is a little weird (maybe at the API level VMI names should be different when instantiated by a VM?) but I think @yfrimanm's design is the most appropriate way to show that a "Running" VM has a VMI resource with the same name underneath it. I didn't know that before seeing this PR. If we ever decide to go with Option 4 from the brainstorming doc in the future (where VMIs get their own dedicated nav item and table) then this PR's design aligns very nicely with that as well. We've done something similar to show the relationship between node/host/machine CRs in bare metal clusters. Replacing the VMI Instance name with "VMI Details" like in @yaacov's latest GIF (screenshot below) prevents users from sorting all VMI names alphabetically (assuming we show "No VM" in the VM column like the design shows, which is true) and makes it less clear that there's an actual VMI custom resource that's "Running" in each table row. I think we should reconsider this approach.
+1, resource badges are always paired with the resource's name today.
@yaacov's GIF shows this briefly. This line-wrapping behavior is consistent across all of the Web Console's tables: |
@andybraren @dankenigsberg hi,
We ended up using
That looks bad :-) it wraps on multiple lines, the same way if we have one column of many with the name, shortening the second name doesn't solve the screen real-estate or DRY problems we have. Showing using IMHO, If we have long names we will need to solve it regardless of having one or two columns repeating the name ... @andybraren do we have a design to handle very long names ?
Yes, the picture you refer to if an effort to see if we can solve the DRY problem using @dankenigsberg 's suggestions. If we use the "Instance" column to show a In current design and implementation we have "No VM" with popup.
Nice, will fix that in the implementation. |
+Andy Braren <abraren@redhat.com> Thank you! Sure, the popover text update seems
reasonable to me and I will update the design.
…On Mon, Dec 30, 2019 at 8:23 AM Andy Braren ***@***.***> wrote:
@yaacov <https://github.com/yaacov> A couple of notes on this screenshot
(thanks for the recording!):
[image: image]
<https://user-images.githubusercontent.com/9122899/71569080-76855080-2a9a-11ea-94cc-941850f6b867.png>
1.
The field label should be No VM like @yfrimanm
<https://github.com/yfrimanm>'s design, which aligns with the new No
instance label in the Instance column.
2.
@yfrimanm <https://github.com/yfrimanm> We may want to adjust the
popover text since the VMI could have been created within the Console via
YAML too (or maybe the Create VM wizard someday). Maybe something like "This
VM instance is not managed by a VM resource. Some management capabilities
may not be available.". @rmohr <https://github.com/rmohr> @jelkosz
<https://github.com/jelkosz> could we say something more useful here?
Do we want to encourage users to use VMs and not lower-level VMIs in
general? How might we persuade them in this popover?
3.
VMI Dedatails page (nit: typo) could be View VMI Details (with no
arrow icon) to align with other status popovers in the Console (IIRC, at
least in dashboards).
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#310?email_source=notifications&email_token=AKUDFDCXMXQ42CPJKZAA54TQ3GHVTA5CNFSM4J6TSMJKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHZVFJI#issuecomment-569594533>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AKUDFDBPGNRMVMBM5AOZL4LQ3GHVTANCNFSM4J6TSMJA>
.
|
@dankenigsberg @andybraren @matthewcarleton @jelkosz @rmohr @yaacov @lizsurette @mgoldboi @ohadlevy From the user's point of view, the resource kinds VMs and VMIs are both "Virtual Machines". When clicking the left nav tab "Virtual Machines", users might expect to see all "Virtual Machines", regardless of their internal implementation (VMs, VMIs...). Option 2 shows one column of "Virtual Machines" including all the VMs and also surfaces VMIs (in cases we have VMIs that aren't managed by a VM).
@dankenigsberg @mgoldboi WDYT? |
Oh, I thought that you told me that you'd show a row for every VMI (violating DRY even further).
I don't understand how you surface the VMI object in the typical case, where VMIs have a backing VM.
I can live with Option 1 (secretly hoping that we can hide the VMI name in a follow-up PR) |
No, we only show @dankenigsberg we had a discussion with @ohadlevy about option 1 in order to solve your concerns, looking at the problem from @ohadlevy perspective, made us reconsidered option 2. The things that we discussed where: 2 - The @yfrimanm and me looked at the options that address @ohadlevy 's concerns, and found that option 2 address both the DRY problem and the |
not really, he does have a VMI and does not have a VM. This design is quite honest about what is actually happening on the backend which I find to be its main strength.
I find the information that my VM is transient pretty useful. I understand that we communicate it in a quite talky way though. In case you only have VMIs and no VMs at all, that column is pretty useless I agree.
That is not entirely true. One of the reasons we started to work on this feature was to give the user the access to the VMI even if there is a VM around it (mainly for troubleshooting reasons).
The more I think of this latest design the more I like it, but it leaves us with two problems (which we can totally address in the context of this design): |
It looks like that extra lane in the
I would prefer if we just embrace how the openshift console works right now (just showing all workload types as extra tabs and link them in the details pages) and try to come up with our own usage-concepts once the openshift console workflows as such get modified. |
This is what definitely makes sense. I "think" that these links are normally at the bottom of the details pages, when looking at Also consider naming the header |
@rmohr the "owner" is listed at the bottom, in this case the link is in the opposite direction from the VM to the VMI it manages ( the VMI is not the owner of the VM it is owned by it ). p.s. |
@yaacov I am not sure I can follow. At least in the 3.11 console, which I have open right now, there is a |
I see, I thought you where referring to the "owner". An "Instances" section at the bottom will look great for VMIRS where one VMIRS manages many VMIs. |
@rmohr can you take a look at openshift/console#3841 , it has a screenshot at the description ? |
@yfrimanm @matthewcarleton WDYT about @rmohr suggestion to add an 'VM instanses' section at the bottom ? |
+1. It does make sense to me as well.
…On Thu, Jan 2, 2020 at 3:45 PM Yaacov Zamir ***@***.***> wrote:
@yfrimanm <https://github.com/yfrimanm> @matthewcarleton
<https://github.com/matthewcarleton> WDYT about @rmohr
<https://github.com/rmohr> suggestion to add an 'VM instanses' section at
the bottom ?
[image: hco operator · Details · OKD]
<https://user-images.githubusercontent.com/2181522/71669859-c99b2400-2d76-11ea-9bcc-672df6b16b8c.png>
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#310?email_source=notifications&email_token=AKUDFDET62SPUU3P4JL5BXTQ3XVV5A5CNFSM4J6TSMJKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEH6LW6Q#issuecomment-570211194>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AKUDFDAFELPHWB2MCCOBT43Q3XVV5ANCNFSM4J6TSMJA>
.
|
Just to confirm @yaacov are we only showing VMIs when they don't have an owner VM? That would represent two scenarios (correct me if I'm wrong):
Are we confident that a user would go to the VM list to discover the VMI they are looking for? If their VMI is buried deep in the list will they know to look for it? The best feature of option 1 is it's very educational. It shows the connect between VMs and VMIs. If this isn't a priority then we could look at option 2 and try and resolve some of the issues that still exist there. @yaacov would the VMI show in the VM details (like you have in the image above)? If so, is it strange to do that and follow what we are doing with for deployments/pods? There will only ever be one VMI there right? |
Yes.
Yes, for example, user is using VMIRS (and not VMs) to manage their VMIs , this is intended.
AFAIU this should never happen.
I think that the original problem @rmohr had was that he went to the "Virtual machine" tab and did not see his VMIs ( for him "Virtual machines" are VMIs ) ,@rmohr please keep me honest here.
Users that manage their VMIs using VMIRS will not have VMs at all, only VMIs, so it will be the first "Virtaul machines" they will see in the list. [ @ohadlevy suggested grouping up VMIs managed by one VMIRS and showing one line in this case like we show containers in a pos. but we do not support VMIRS at the moment ]
Yes, we had a discussion with @ohadlevy about that, my impression from that talk was that for our users "Virtual machines" are VMs and VMIs, one can do some actions that other can't, but basically they are both virtual machines, we should encourage using VMs but if someone uses VMI it's ok, @ohadlevy please keep me honest here.
Current design put it in the overview above the pod item ...
I agree, current design put it in the overview.
Yes, VM only manage one VMI ( kubevirt/kubevirt#1527 ) |
That would much nicer than listing them all. It would align with users for the most part just interacting with VMs. They won't be confused by all these VMIs in the list.
Interested in hearing from @rmohr on this one, he seemed more convinced.
Once we see VMIRS in the UI I'd say the more likely way a user would interact with their VMIs created from a VMIRS is from the VMIRS itself b/c we can list all the VMIs it's created. At this point the VM list becomes much more like the Pods list where users wouldn't be interacting with it as much. That's another discussion for another time though :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Just a few content changes :)
|
||
In order to let the user get to the VMI even when there is a VM around it, we added a link in the VM details page. | ||
|
||
![VMI's details page showing link to its VMI](img/VM_DetailsPageW_LinkToVMI.png) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a - In this image we have a link to the pod, IIRC we decided to remove this link on Jan 9 mitting, @matthewcarleton please correct me if I'm wrong.
b - In this image the link to VMI is in the same place as owner ... this is very confusing as the VM is the owner of VMI and not the other way around. IIRC we decided to replace the POD link with a VMI link (~top right area) and leave the owner link for the VM owner link (bottom left) as it is now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@yaacov ya I think you're right. I believe Fabian said that.
I'm a bit confused, this makes sense to me. If I look at the VMI I see the owner VM if I look at the VM I see the VMI. Maybe I'm not understanding correctly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a bit confused, this makes sense to me. If I look at the VMI I see the owner VM if I look at the VM I see the VMI. Maybe I'm not understanding correctly.
VM is the manager of VMI ( e.g. owner )
This is a k8s thing [1] and apply to many resources, in the UI we usually display the owner of a resource in the bottom left.
VMI is a resource that VM mange ( e.g. dependent ):
a - If their are multiple resources - we currently display them as a list at the buttom of the page, for example containers and volumes in the Pod details page.
b - If their is only one, or it's important to show in the top section, we add it to the details on top, like we currently do for Pod in VM details view.
[1] https://kubernetes.io/docs/concepts/workloads/controllers/garbage-collection/#owners-and-dependents
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and you are proposing we keep the owner for the VM? Is this where we'd possibly see VMIRS in the future? My concern is showing something that is empty all the time, which could lead to confusion.
|
||
## Actions available for the VMI | ||
|
||
![VMI's available actions](img/Op1Actions.png) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@matthewcarleton hi, do we have an example of other resource that use the info banner above the details section ?
( It's will be very helpful to have an implementation reference )
|
||
VMIs and VMs will be distinguished between different color badges. VMI badges color are darker (#002F5D) in order to provide proper contrast for low vision users. | ||
|
||
![single list view of VMs and VMIs ](img/VMsListW_VMIsOp2.png) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: The HintBlock
component used in the console ( for example in the developer view => +Add page ) does not support closing option.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, well I know there is active work around updating the hint pattern so we will see it eventually 🤞
|
||
## Actions available for the VMI | ||
|
||
![VMI's available actions](img/Op1Actions.png) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In our design we only show VMI if they have no VM, so users will not have access to a VMI details page if it has a VM that mask it.
@matthewcarleton @yfrimanm do we want to let user access the "VM-like" details page of a VMI that has a VM ( a. it will be a duplication of the VM details view, b. we do not link to it from the list view ) ?
WDYT ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@yaacov AFAIR we said we want users to interact with VMs, so IMO, we should allow that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@yaacov what happens if I go to the details page of a VM and click on the instance there? Won't I go to the VMI details page?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@yaacov what happens if I go to the details page of a VM and click on the instance there? Won't I go to the VMI details page?
in openshift/console#3949 currenlty WIP you will get to an actuall "Virtual Machine Instanse" page, that is a generic page for VMI , not the "Virtual Machine" page, with the dashboards, metrics and events.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, I'd vote for the same experience regardless of how you get to the VMI (list view as an orphan or via the VM details page.) Does that make sense?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, the vm details page is actually "vm+vmi" details page, when collecting the data to show in the page we take all the data we have from the vm + all the data we have from the vmi, and show a combined view of all the data we have. Linking from the vm details page to the vmi details page is actually linking to the same page if It make sense :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see the tricky issue here. So I think we might need to cancel that link. WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see the tricky issue here. So I think we might need to cancel that link. WDYT?
looks like it should be a link to pod ( like it was in the beginning ) :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ya I think that aligns with how we present it in the list view.
VMI only present if it exists on its own. Otherwise you interact with the VM. I like that more actually b/c then we would surface just the Pod from the VM.
@yfrimanm can you update the designs to reflect this?
The purpose of this PR is to surface the need for exposing VMIs in the UI.
Currently, they exist in the CLI, but they are less accessible in the UI, so the initial request suggested there is no logic in "hiding" them from the user.
@openshift/team-ux-leads