Use correct VMDK sizes when building OVA

[zjs]

The builder OVF used to construct the OVA has hard-coded disk sizes. Correct these to match the final size of the VMDKs to avoid warnings during the OVA deployment process.

VIC Appliance Checklist:

• Up to date with master branch
• Added tests
• Considered impact to upgrade
• Tests passing
• Updated documentation
• Impact assessment checklist

If this is a feature or change to existing functionality, consider areas of impact with the Impact
Assessment Checklist

Fixes #1166

[hickeng]

Please use the same separator characters as above and a space between the filter and the replacement:
-e "/<File.*vic-disk1.vmdk.*/ s~ovf:size=\"[^\"]*\"~ovf:size=\"$(stat --printf="%s" vic-disk1.vmdk)\"~"

Add an array of the expected disks and loop over it. Multiple sed passes are a fine cost to pay for increased clarity.

[ghost]

All of this logic could all probably go in https://github.com/zjs/vic-product/blob/22cc2ea882424c0b3cf0b80b132d239e8ca73201/installer/build/bootable/build-disks.sh#L208 where disk names are already listed.

[zjs]

I don't like listing the disk names in three places (the OVF, build-main.sh, and build-disks.sh), like my current change does.

However, one thing that I like about the current design is that build-disks.sh and build-main.sh seem to have fairly separate purposes: one creates the VMDKs, while the other bundles them into an OVA.

I'm not sure how important that distinction is in practice, but I worry that moving this logic into build-disks.sh blurs the line too much.

Perhaps build-main.sh should update the disk sizes, but not have a hardcoded value for DISKS. Instead, it could just base the array on the contents of the directory. This is a little fragile (what happens if there's something unexpected in the directory?), but may be better than explicitly listing the names in multiple places. (On the other hand, explicitly referring to them my name makes it easy for someone to grep for them by name and find this logic.)

[ghost]

Fair concerns about blurring the separation of logic between build-main and build-disks.

My original intent was to have all bull attributes (disk size, names, etc) parsed and read from ether a json file or the ovf template in build-main, and passed to the other builder scripts.

Something similar could be done here - just pass the newly-defined disk array you’ve added in build-main as an argument in calls to build-disk. That preserves the separation of logic while removing one repeated definition of the disks array.

Furthermore, it serves as a good stepping stone to eventually reading those values from the ovf config in build-main and passing them to any desired subsequent build step.

[zjs]

I did basically what you describe, except to avoid separating the disk names from the other information about them, I moved all three arrays into build-main. I think this is consistent with the idea of eventually reading the values from somewhere (JSON or OVF).

[hickeng]

From a purely pedantic perspective, these should be tested for equality with each other as well as !0

[hickeng]

This cannot be compressed?

[zjs]

Filed #1837 to track.