cloud-init-dev team mailing list archive

[Scott Moser <ssmoser2+ubuntu@xxxxxxxxx>]

Scott Moser has proposed merging ~smoser/cloud-init:fix-ds-identify-vmware-ovf into cloud-init:master.

Commit message:
OVF: improve ds-identify to support finding OVF iso transport.

Previously the OVF transport would not be identified except for when
config files set 'ovf_vmware_guest_customization'.

The change here is to add support to ds-identify for storing the
filesystem types found (FS_TYPES) as well as the devices on which an
iso9660 filesystem is found (ISO9660_DEVS)

Then the OVF check will check that the iso9660 filesystem has
ovf-env.xml on it.  The least wonderful part of this is that the
check is done by 'grep' for case insensitive ovf-env.xml.

LP: #1731868


Requested reviews:
  cloud-init commiters (cloud-init-dev)
Related bugs:
  Bug #1731868 in cloud-init: "cloud-id: enable ESXi 6.5.0"
  https://bugs.launchpad.net/cloud-init/+bug/1731868

For more details, see:
https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/334880
-- 
Your team cloud-init commiters is requested to review the proposed merge of ~smoser/cloud-init:fix-ds-identify-vmware-ovf into cloud-init:master.

diff --git a/tests/unittests/test_ds_identify.py b/tests/unittests/test_ds_identify.py
index 7a920d4..74bf29e 100644
--- a/tests/unittests/test_ds_identify.py
+++ b/tests/unittests/test_ds_identify.py
@@ -55,6 +55,7 @@ P_SEED_DIR = "var/lib/cloud/seed"
 P_DSID_CFG = "etc/cloud/ds-identify.cfg"
 
 MOCK_VIRT_IS_KVM = {'name': 'detect_virt', 'RET': 'kvm', 'ret': 0}
+MOCK_VIRT_IS_VMWARE = {'name': 'detect_virt', 'RET': 'vmware', 'ret': 0}
 MOCK_UNAME_IS_PPC64 = {'name': 'uname', 'out': UNAME_PPC64EL, 'ret': 0}
 
 
@@ -296,6 +297,19 @@ class TestDsIdentify(CiTestCase):
             data, RC_FOUND, dslist=['OpenStack', 'None'])
         self.assertIn("check for 'OpenStack' returned maybe", err)
 
+    def test_default_ovf_is_found(self):
+        """OVF is identified found when ovf/ovf-env.xml seed file exists."""
+        self._test_ds_found('OVF-seed')
+
+    def test_default_ovf_with_detect_virt_none_not_found(self):
+        """OVF identifies not found when detect_virt returns "none"."""
+        self._check_via_dict(
+            {'ds': 'OVF'}, rc=RC_NOT_FOUND, policy_dmi="disabled")
+
+    def test_ovf_on_vmware_iso_found(self):
+        """OVF is identified when iso9660 on cdrom path and virt of vmware."""
+        self._test_ds_found('OVF')
+
 
 def blkid_out(disks=None):
     """Convert a list of disk dictionaries into blkid content."""
@@ -383,6 +397,26 @@ VALID_CFG = {
         'policy_dmi': POLICY_FOUND_ONLY,
         'policy_no_dmi': POLICY_FOUND_ONLY,
     },
+    'OVF-seed': {
+        'ds': 'OVF',
+        'files': {
+            os.path.join(P_SEED_DIR, 'ovf', 'ovf-env.xml'): 'present\n',
+        }
+    },
+    'OVF': {
+        'ds': 'OVF',
+        'mocks': [
+            {'name': 'blkid', 'ret': 0,
+             'out': blkid_out(
+                 [{'DEVNAME': 'vda1', 'TYPE': 'vfat', 'PARTUUID': uuid4()},
+                  {'DEVNAME': 'sr0', 'TYPE': 'iso9660', 'LABEL': ''}])
+             },
+            MOCK_VIRT_IS_VMWARE,
+        ],
+        'files': {
+            'dev/sr0': 'pretend ovf iso with file ovf-env.xml\n',
+        }
+    },
     'ConfigDrive': {
         'ds': 'ConfigDrive',
         'mocks': [
diff --git a/tools/ds-identify b/tools/ds-identify
index ee5e05a..d1d2c6a 100755
--- a/tools/ds-identify
+++ b/tools/ds-identify
@@ -83,6 +83,7 @@ _DI_LOGGED=""
 # set DI_MAIN='noop' in environment to source this file with no main called.
 DI_MAIN=${DI_MAIN:-main}
 
+DI_BLKID_OUTPUT=""
 DI_DEFAULT_POLICY="search,found=all,maybe=all,notfound=${DI_DISABLED}"
 DI_DEFAULT_POLICY_NO_DMI="search,found=all,maybe=all,notfound=${DI_ENABLED}"
 DI_DMI_CHASSIS_ASSET_TAG=""
@@ -91,6 +92,8 @@ DI_DMI_SYS_VENDOR=""
 DI_DMI_PRODUCT_SERIAL=""
 DI_DMI_PRODUCT_UUID=""
 DI_FS_LABELS=""
+DI_FS_TYPES=""
+DI_ISO9660_DEVS=""
 DI_KERNEL_CMDLINE=""
 DI_VIRT=""
 DI_PID_1_PRODUCT_NAME=""
@@ -181,32 +184,47 @@ block_dev_with_label() {
     return 0
 }
 
-read_fs_labels() {
-    cached "${DI_FS_LABELS}" && return 0
+read_fs_info() {
+    cached "${DI_BLKID_OUTPUT}" && return 0
     # do not rely on links in /dev/disk which might not be present yet.
     # note that older blkid versions do not report DEVNAME in 'export' output.
-    local out="" ret=0 oifs="$IFS" line="" delim=","
-    local labels=""
     if is_container; then
         # blkid will in a container, or at least currently in lxd
         # not provide useful information.
         DI_FS_LABELS="$UNAVAILABLE:container"
-    else
-        out=$(blkid -c /dev/null -o export) || {
-            ret=$?
-            error "failed running [$ret]: blkid -c /dev/null -o export"
-            return $ret
-        }
-        IFS="$CR"
-        set -- $out
-        IFS="$oifs"
-        for line in "$@"; do
-            case "${line}" in
-                LABEL=*) labels="${labels}${line#LABEL=}${delim}";;
-            esac
-        done
-        DI_FS_LABELS="${labels%${delim}}"
+        DI_FS_TYPES="$UNAVAILABLE:container"
+        DI_ISO9660_DEVS="$UNAVAILBLE:container"
+        return
     fi
+    local oifs="$IFS" line="" delim=","
+    local ret=0 out="" labels="" fstypes="" dev="" ftype="" isodevs=""
+    out=$(blkid -c /dev/null -o export) || {
+        ret=$?
+        error "failed running [$ret]: blkid -c /dev/null -o export"
+        DI_FS_LABELS="$UNAVAILABLE:error"
+        DI_FS_TYPES="$UNAVAILABLE:error"
+        DI_ISO9660_DEVS="$UNAVAILBLE:error"
+        return $ret
+    }
+    IFS="$CR"
+    set -- $out
+    IFS="$oifs"
+    for line in "$@"; do
+        case "${line}" in
+            DEVNAME=*) dev=${line#DEVNAME=};;
+            LABEL=*) labels="${labels}${line#LABEL=}${delim}";;
+            TYPE=*)
+                ftype=${line#TYPE=}
+                fstypes="${fstypes}$ftype${delim}"
+                if [ "$ftype" = "iso9660" ]; then
+                    isodevs="${isodevs} ${dev}";
+                fi
+                ;;
+        esac
+    done
+    DI_FS_LABELS="${labels%${delim}}"
+    DI_FS_TYPES="${fstypes%${delim}}"
+    DI_ISO9660_DEVS="${isodevs# }"
 }
 
 cached() {
@@ -214,10 +232,6 @@ cached() {
 }
 
 
-has_cdrom() {
-    [ -e "${PATH_ROOT}/dev/cdrom" ]
-}
-
 detect_virt() {
     local virt="${UNAVAILABLE}" r="" out=""
     if [ -d /run/systemd ]; then
@@ -645,19 +659,37 @@ ovf_vmware_guest_customization() {
 }
 
 dscheck_OVF() {
-    local p=""
     check_seed_dir ovf ovf-env.xml && return "${DS_FOUND}"
 
+    [ "${DI_VIRT}" = "none" ] && return ${DS_NOT_FOUND}
+
+    local isodevs="${DI_ISO9660_DEVS}"
+    case "$isodevs" in
+        ""|$UNAVAILABLE:*) return ${DS_NOT_FOUND};;
+    esac
+
+    # OVF transport does not require any specific label.
+    # but we support case insensitive ovf-transport to enable.
+    has_fs_with_label "[Oo][Vv][Ff]*" && return $DS_FOUND
+
+    # look for ovf-env.xml on the filesystem via 'grep'.
+    # not the best, but alternative is mount, ls, unmount.
+    local d=""
+    for d in $isodevs; do
+        case "$d" in
+            /dev/sr[0-9]|/dev/hd[a-z]) :;;
+            *) debug 1 "skipping iso dev $d"
+               continue;;
+        esac
+        grep --quiet --ignore-case ovf-env.xml "${PATH_ROOT}$d" &&
+            return $DS_FOUND
+    done
+
     if ovf_vmware_guest_customization; then
         return ${DS_FOUND}
     fi
 
-    has_cdrom || return ${DS_NOT_FOUND}
-
-    # FIXME: currently just return maybe if there is a cdrom
-    # ovf iso9660 transport does not specify an fs label.
-    # better would be to check if
-    return ${DS_MAYBE}
+    return ${DS_NOT_FOUND}
 }
 
 dscheck_Azure() {
@@ -930,7 +962,7 @@ collect_info() {
     read_dmi_product_name
     read_dmi_product_serial
     read_dmi_product_uuid
-    read_fs_labels
+    read_fs_info
 }
 
 print_info() {
@@ -942,7 +974,7 @@ _print_info() {
     local n="" v="" vars=""
     vars="DMI_PRODUCT_NAME DMI_SYS_VENDOR DMI_PRODUCT_SERIAL"
     vars="$vars DMI_PRODUCT_UUID PID_1_PRODUCT_NAME DMI_CHASSIS_ASSET_TAG"
-    vars="$vars FS_LABELS KERNEL_CMDLINE VIRT"
+    vars="$vars FS_LABELS FS_TYPES ISO9660_DEVS KERNEL_CMDLINE VIRT"
     vars="$vars UNAME_KERNEL_NAME UNAME_KERNEL_RELEASE UNAME_KERNEL_VERSION"
     vars="$vars UNAME_MACHINE UNAME_NODENAME UNAME_OPERATING_SYSTEM"
     vars="$vars DSNAME DSLIST"