Description of problem: When cloning a data volume, on CNV4.8, if the default storageclass is defined as different to that of the source pvc, it is important for the user to specify the storageClass in the target dv manifest, to avoid the target dv defaulting to the default storage class. The user needs to be made aware of this in the documentation Version-Release number of selected component (if applicable): CNV4.8.4 How reproducible: 100% Steps to Reproduce: Checked this with the following code: ---------------------------------------------- oc version Client Version: 4.10.0-202110050529.p0.git.8857282.assembly.stream-8857282 Server Version: 4.8.17 Kubernetes Version: v1.21.1+6438632 OpenShift Virtualization 4.8.3 Performed the following scenario: --------------------------------------------- 1. Created a source-dv on storage class ocs-storagecluster-ceph-rbd and name space golden-ns 2. Created a target-dv without specifying the storage class and on name space golden-ns The target dv defaulted to being created on the default storage class 'local-block' and on the default name space but the pvc was not created The same happened when defining the default storage class as 'nfs' oc dv describe indicates that the "DataVolume.storage spec is missing accessMode and cannot get access mode from StorageProfile local-block" When trying again and specifying the storage class in the target dv, the datavolume is cloned successfully. We need to document the need to specify the storage class in the target dv to prevent the user from facing this? E.g. ---------- apiVersion: cdi.kubevirt.io/v1beta1 kind: DataVolume metadata: name: <cloner-datavolume> spec: source: pvc: namespace: "<source-namespace>" name: "<my-favorite-vm-disk>" storage: resources: requests: storage: <2Gi> storageClassName: "<required-storageclass-name>" Additional information ----------------------------------------- oc get dv -A NAMESPACE NAME PHASE PROGRESS RESTARTS AGE default dv-target 58s golden-ns dv-source Succeeded 100.0% 7h48m oc get pvc -A NAMESPACE NAME STATUS VOLUME CAPACITY ACCESS MODES STORAGECLASS AGE golden-ns dv-source Bound pvc-28a278dc-8893-41b9-a5a2-4ab2c096f11f 25Gi RWO ocs-storagecluster-ceph-rbd 7h48m openshift-storage db-noobaa-db-pg-0 Bound pvc-027d78e0-da78-4fc1-9aaa-b5ec6b5fa066 50Gi RWO ocs-storagecluster-ceph-rbd 4d6h openshift-storage ocs-deviceset-0-data-0hf4hn Bound local-pv-9f1d607c 70Gi RWO local-block 4d6h openshift-storage ocs-deviceset-1-data-0wvvg8 Bound local-pv-e354c10f 70Gi RWO local-block 4d6h openshift-storage ocs-deviceset-2-data-0zbstp Bound local-pv-87aab96 70Gi RWO local-block 4d6h oc describe dv dv-target Name: dv-target Namespace: default Labels: <none> Annotations: cdi.kubevirt.io/storage.clone.token: eyJhbGciOiJQUzI1NiIsImtpZCI6IiJ9.eyJleHAiOjE2MzY5MTIzNDcsImlhdCI6MTYzNjkxMjA0NywiaXNzIjoiY2RpLWFwaXNlcnZlciIsIm5hbWUiOiJkdi1zb3VyY2UiLCJuY... API Version: cdi.kubevirt.io/v1beta1 Kind: DataVolume Metadata: Creation Timestamp: 2021-11-14T17:47:27Z Generation: 1 Managed Fields: API Version: cdi.kubevirt.io/v1beta1 Fields Type: FieldsV1 fieldsV1: f:spec: .: f:source: .: f:pvc: .: f:name: f:namespace: f:storage: .: f:resources: .: f:requests: .: f:storage: Manager: kubectl-create Operation: Update Time: 2021-11-14T17:47:27Z Resource Version: 6279388 UID: 1f94766c-456b-45df-a50f-2b9364af30b5 Spec: Source: Pvc: Name: dv-source Namespace: golden-ns Storage: Resources: Requests: Storage: 25Gi Events: Type Reason Age From Message ---- ------ ---- ---- ------- Warning ErrClaimNotValid 60s (x15 over 2m21s) datavolume-controller DataVolume.storage spec is missing accessMode and cannot get access mode from StorageProfile local-block Yamls for source and target dv's: ---------------------------------------------- apiVersion: cdi.kubevirt.io/v1beta1 kind: DataVolume metadata: name: dv-source namespace: golden-ns spec: source: http: url: "http://cnv-qe-server.rhevdev.lab.eng.rdu2.redhat.com/files/cnv-tests/rhel-images/rhel-84.qcow2" pvc: accessModes: - ReadWriteOnce storageClassName: ocs-storagecluster-ceph-rbd resources: requests: storage: 25Gi apiVersion: cdi.kubevirt.io/v1beta1 kind: DataVolume metadata: name: dv-target spec: source: pvc: namespace: golden-ns name: dv-source storage: resources: requests: storage: 25Gi
After considering this issue a bit more I think we should add some documentation (and possibly a new UI feature eventually). We could add a note in the documentation that explains how to create a VM from a template: You may use any available storage class for your VM disks but the VM may be cloned more efficiently if the boot disk uses the same storage class as the boot source PVC. Matt, would it make sense to alter the wizard so that it offers two options when selecting to clone existing PVC: 1) Use the same storage class as the existing PVC (default), or 2) Use the default storage class. Any storage class could still be selected using the advanced settings. Thoughts?
what do you think about the suggestions for this issue?
Hi @yadu That's fine with me. I can add the note to the documentation that Adam suggests. Thanks, Bob
So the default storage class is configurable on the cluster and is used during cloning unless the user specifies otherwise. This may not result in the most efficient cloning method. What we want here is just to make the user aware of this. We suggest the same storage class to match that of the existing pvc which is the "most efficient" (don't mention "default" here) , and offer them the option to select the "Current cluster default storage class" in the second box. Users may forget or not be aware of this and end up facing unintended repercussions. Any other storage class could still be selected using the advanced settings.
Please review the UX suggestion here: https://docs.google.com/document/d/1yIpXL962R_UUcExRHsSfWUUVHXx1Il1pGctyWf5RYQA/edit?usp=sharing Thanks.
My suggestions for the UX test is: The storage class used by the existing PVC, recommended for optimal cloning performance. The cluster storage class, which may not provide optimal cloning performance. You can configure another storage class using Advanced Settings. Question for @alitke : Are any doc changes needed for this bug? I may have misread this as a doc bug.
@bgaydos Looks good. Would make one change to 2) The cluster storage class, which may not provide optimal cloning performance -> The default cluster storage class, which may not provide optimal cloning performance.
Another thing: You can configure another storage class using Advanced Settings -> You can select a different storage class using Advanced Settings.
No problem. If the user can actually select a class from a list for instance, then 'select' would be more appropriate. 'Configure' can be used in a general sense. I will let you make that call. 'Configure' is most often used with YAMLs (config files), anyway. Bob
@Bob Gaydos Ok got it, if we have a list to select from then use "select a different". If not use "Configure" Looks good to me
@kgoldbla - Please verify, then I will close per our last Jira convo. Thanks Bob
Looks good. Moving to Verify.
Kevin, what version you verified the bug? could you please provide the doc link?
@yadu@redhat.com https://docs.google.com/document/d/1yIpXL962R_UUcExRHsSfWUUVHXx1Il1pGctyWf5RYQA/edit#heading=h.vskeehwg6jz3
Hi Yifat, Do we still want this for UI? Currently, the UI is consistent with CLI, the default SC(storageclass) is used while creating the VM. This issue is about adding a warning message if the selected SC on UI is different from the SC of the template's boot source.
Hi Guohua, TMO we should add the warning message if the selected SC on UI is different from the SC of the template's boot source. Thanks.
Hi @apinnick Can we use something like this for the warning message we want to add here: "The selected SC on UI is different from the SC of the template's boot source" And then for the description below: "Different SC may cause slow import"? Thanks! cc @mschatzm
@apinnick The same question applies for the text in the Add disk modal: Can we say "Disk SC is different from default SC" and for the description below add "Different SC may cause slow import"? Thanks!
@yfrimanm I suggest this wording for the warning: "Selecting a StorageClass that is different from the boot source StorageClass might cause poor performance." For the Add disk modal: "Selecting a StorageClass that is different from the default StorageClass might cause poor performance." If you need to have a warning message+description, you could break it up like this: Message: "Selected StorageClass is different from the [boot source/default] StorageClass". Description: "Selecting a different StorageClass might cause poor performance."
Thank you @apinnick cc @mschatzm
Hi @apinnick About the disk title and description - looking good :) There isn't a place for SC selection about the quick VM creation flow, so saying selected feels a bit off to me; the SC can only be changed from the customization wizard. It feels to me like it should be something like this for quick creation: Message: "Boot source StorageClass is different from the default StorageClass." Description: "A different StorageClass might cause poor performance. StorageClass can be changed from customization VirtualMachine." They don't have the option to change it from the quick flow. I am adding a screenshot of this screen. quick-flow. What do you think? :)
(In reply to Matan Schatzman from comment #22) > Hi @apinnick > > About the disk title and description - looking good :) > > There isn't a place for SC selection about the quick VM creation flow, so > saying selected feels a bit off to me; the SC can only be changed from the > customization wizard. I did not realize that this was for VM quick create. That does make a difference. > It feels to me like it should be something like this for quick creation: > > Message: "Boot source StorageClass is different from the default > StorageClass." > Description: "A different StorageClass might cause poor performance. LGTM > StorageClass can be changed from customization VirtualMachine." This sentence sounds odd because it is not clear that "Customize VirtualMachine" is a button. Maybe change to "Click Customize VirtualMachine to change the StorageClass." Does that work for you?
(In reply to Avital Pinnick from comment #24) > (In reply to Matan Schatzman from comment #22) > > Hi @apinnick > > > > About the disk title and description - looking good :) > > > > There isn't a place for SC selection about the quick VM creation flow, so > > saying selected feels a bit off to me; the SC can only be changed from the > > customization wizard. > > I did not realize that this was for VM quick create. That does make a > difference. > > > It feels to me like it should be something like this for quick creation: > > > > Message: "Boot source StorageClass is different from the default > > StorageClass." > > Description: "A different StorageClass might cause poor performance. > > LGTM > > > StorageClass can be changed from customization VirtualMachine." > > This sentence sounds odd because it is not clear that "Customize > VirtualMachine" is a button. Maybe change to "Click Customize VirtualMachine > to change the StorageClass." Does that work for you? Thank @apinnick Actually I forgot to mention that we will replace the SC in quick create to default one automatically same as the CLI will do. so maybe a description that sounds something like this? A different StorageClass might cause poor performance. We will change to default StorageClass. to change to boot source StorageClass Click Customize VirtualMachine to change the StorageClass.
Matan, For Quick Create, what is not clear to me is whether the user can select a StorageClass. Are you forcing the default StorageClass while the user is using Quick Create? Or are you advising the user that if they select a non-default StorageClass, the performance will be poor? It sounds like the user will automatically have the default StorageClass if they use Quick Create, and that they have to click Customize VirtualMachine to change the StorageClass. Do you have a cluster that I can log in to? Or a video from a sprint demo?
Matan shared a video clip with me showing the "Customize VirtualMachine" flow. The user is on Virtualization > Catalog page. If the user clicks a template with a boot source that does not use the default StorageClass, the following message (warning, important or whatever), appears on the template card: "The boot source StorageClass is different from the default StorageClass. Using a different StorageClass might cause poor performance. To change the StorageClass, click Customize VirtualMachine, click Review and create VirtualMachine, click the Disk tab, select Edit from the Options menu beside the boot disk, and then select the default StorageClass." It's a very long message. I'm not sure the user will be able to remember all that. Is there a way to make the "Edit StorageClass" process easier? If you have no choice and you have to shorten this procedure, here's a possibility: "To change the StorageClass, click Customize VirtualMachine > Review and create VirtualMachine, and then edit the boot disk." It is not ideal but it might be enough to get a user to the right place. In the Edit/Add disk modal, you can have this warning appear if user selects non-default SC: "The selected StorageClass is different from the default StorageClass. Using a different StorageClass might cause poor performance."
Update. After talking to Matan, I realized that the default (Quick create) changes SC to default, while the user has to use "Customize" to change SC to non-default. I'll update my suggestions.
Would it make sense to put the warning in a banner on the Catalog page instead of the template card? "Quick creating a VirtualMachine creates a VirtualMachine boot disk with the default StorageClass. To select a different StorageClass, click Customize VirtualMachine > Review and create VirtualMachine, and then edit the boot disk. Using a different StorageClass might cause performance problems."
Created attachment 1920644 [details] short notification in the quick create VM path Hi @apinnick can we simplify it to something like in this mockup?
(In reply to Yifat Menchik from comment #30) > Hi @apinnick Thinking it over I think we should keep it only in the documentation and not surface it in the UI as it cultures the UI with an edge case that is not necessarily relevant for most use cases.
verified on v4.12.0-1652, now it shows a message "Selecting a different StorageClass might cause poor performance."
Since the problem described in this bug report should be resolved in a recent advisory, it has been closed with a resolution of ERRATA. For information on the advisory (Important: OpenShift Virtualization 4.12.0 Images security update), and where to find the updated files, follow the link below. If the solution does not work for you, open a new bug report. https://access.redhat.com/errata/RHSA-2023:0408