Take image virtual size into account when creating Volumes

[tserong]

This is a followup to #1104

When creating new volumes on the Volume screen in Harvester, the default size is empty, i.e. it's not pre-filled with anything at all. If the source is set to "VM image" we can now take the image virtual size and use that as the default size for the volume. Unlike when creating VMs (which always set the volume to a minimum of 10GiB regardless of virtual size, for consistency with earlier harvester versions), on this screen we just use the virtual size as-is. If the user wants to make it bigger, they can.

Related issue: harvester/harvester#4905

[tserong]

Wait, I've just discovered this isn't quite right -- it's now effectively impossible to manually edit the size once an image is chosen (I think because everything calls update()). I'll have to rework this somehow :-/

[tserong]

OK, that seems to have done the trick... If there's a better way to structure anything here, please LMK.

[a110605]

Sorry, not sure if I misunderstand this PR purpose, we want to pre-fill the Size based on user selected image virtual size or actual size.

Does this PR need specific backend change or master-head is enough ?

It's not work as expected when test locally.

bug.mov

[tserong]

Sorry, not sure if I misunderstand this PR purpose, we want to pre-fill the Size based on user selected image virtual size or actual size.

Yep, it's meant to pre-fill the Size based on whichever is greater of the image virtual size or actual size.

Does this PR need specific backend change or master-head is enough ?

It should work fine provided your backend harvester cluster is v1.4 or master -- looking at the version in the video, it seems you're on v1.3.1, which doesn't support virtual size... But even in that case I would have expected it to at least fill the field with the physical size of the image -- const imageSize = Math.max(imageResource?.status?.size, imageResource?.status?.virtualSize) should pick up whichever is available, right? -- so I'm not sure what's going on here :-/

[votdev]

This patch contains some more improvements. You can make use of the already fetched images (see the fetch() method). Don't know why they are fetched because the current code does not make use of it. Maybe @torchiaf should have a final look on it if you want to apply this patch.

diff --git a/pkg/harvester/edit/harvesterhci.io.volume.vue b/pkg/harvester/edit/harvesterhci.io.volume.vue
--- a/pkg/harvester/edit/harvesterhci.io.volume.vue	(revision 3ee5fbdbd0b0d589ebbf7002ceab1ff488846b55)
+++ b/pkg/harvester/edit/harvesterhci.io.volume.vue	(date 1724838170157)
@@ -56,6 +56,7 @@
     const hash = await allHash(_hash);
 
     this.snapshots = hash.snapshots;
+    this.images = hash.images;
 
     const defaultStorage = this.$store.getters[`harvester/all`](STORAGE_CLASS).find( O => O.isDefault);
 
@@ -77,6 +78,7 @@
       storage,
       imageId,
       snapshots: [],
+      images: [],
     };
   },
 
@@ -108,10 +110,8 @@
     },
 
     imageOption() {
-      const choices = this.$store.getters['harvester/all'](HCI.IMAGE);
-
       return sortBy(
-        choices
+        this.images
           .filter(obj => obj.isReady)
           .map((obj) => {
             return {
@@ -251,8 +251,7 @@
     },
     updateImage() {
       if (this.isVMImage && this.imageId) {
-        const images = this.$store.getters['harvester/all'](HCI.IMAGE);
-        const imageResource = images?.find(image => this.imageId === image.id);
+        const imageResource = this.images?.find(image => this.imageId === image.id);
         const imageSize = Math.max(imageResource?.status?.size, imageResource?.status?.virtualSize);
 
         if (imageSize) {

[a110605]

Sorry, not sure if I misunderstand this PR purpose, we want to pre-fill the Size based on user selected image virtual size

Test with master-head deployment. It's good. 👍

Just notice the size only accepted integer.... I turn to use formatSi to get more precise size number to decimal.

  this.storage = formatSi(imageSize, {
            increment: 1024, addSuffix: true, maxExponent: 3, minExponent: 3, suffix: 'i'
          });

But got webhook error 😓

[tserong]

It might be best to just stay with an integer value - I could be wrong, but I don't think we allow decimals anywhere in any size fields, it's all just whole GiB.

[tserong]

Thanks @votdev, I've applied that patch and it seems to work fine. I'm happy to wait for @torchiaf to review as well.

[a110605]

Remember to backport v1.4 after merge.

[torchiaf]

LGTM

[tserong]

@Mergifyio backport release-harvester-v1.4

[mergify]

backport release-harvester-v1.4

✅ Backports have been created

• #1144 Take image virtual size into account when creating Volumes (backport #1113) has been created for branch release-harvester-v1.4