Fix label fatboot

[marlluslustosa]

Fixing an issue originally created here vatesfr/xen-orchestra#4449.

This fix was also proposed as a form of patches here.
Now he will be able to recognize both "LABEL" and "LABEL_FATBOOT".

These two patches solve the problem. Both in the DataSourceNoCloud.py and in the ds-identify.

LP: #1841466

[smoser]

Here is a test for the new ds-identify path. It would be good to have one for the datasource change also.

diff --git a/tests/unittests/test_ds_identify.py b/tests/unittests/test_ds_identify.py
index cb57f2d04..420031c52 100644
--- a/tests/unittests/test_ds_identify.py
+++ b/tests/unittests/test_ds_identify.py
@@ -577,6 +577,10 @@ class TestDsIdentify(DsIdentifyBase):
         """NoCloud is found with uppercase filesystem label."""
         self._test_ds_found('NoCloudUpper')
 
+    def test_nocloud_upper(self):
+        """NoCloud fatboot label - LP: #184166."""
+        self._test_ds_found('NoCloud-fatboot')
+
     def test_nocloud_seed(self):
         """Nocloud seed directory."""
         self._test_ds_found('NoCloud-seed')
@@ -816,6 +820,20 @@ VALID_CFG = {
             'dev/vdb': 'pretend iso content for cidata\n',
         }
     },
+    'NoCloud-fatboot': {
+        'ds': 'NoCloud',
+        'mocks': [
+            MOCK_VIRT_IS_XEN,
+            {'name': 'blkid', 'ret': 0,
+             'out': blkid_out(
+                 BLKID_UEFI_UBUNTU +
+                 [{'DEVNAME': 'xvdb', 'TYPE': 'vfat', 'SEC_TYPE': 'msdos',
+                   'UUID': '355a-4FC2', 'LABEL_FATBOOT': 'cidata'}])},
+        ],
+        'files': {
+            'dev/vdb': 'pretend iso content for cidata\n',
+        }
+    },
     'NoCloud-seed': {
         'ds': 'NoCloud',
         'files': {

[smoser]

Thank you for your contribution.

[smoser]

This is another option. Slightly shorter... I dont have a strong feeling either way. I woudl like to request that you only do the '#' operation once by using the 'label' variable on line 272 in the assignment to 'labels' in 273. I am not sure why 269-270 didn't do that.

diff --git a/tools/ds-identify b/tools/ds-identify
index 071cdc0ca..4e5700fc6 100755
--- a/tools/ds-identify
+++ b/tools/ds-identify
@@ -266,8 +266,9 @@ read_fs_info() {
                     isodevs="${isodevs},${dev}=$label"
                 ftype=""; dev=""; label="";
                 dev=${line#DEVNAME=};;
-            LABEL=*) label="${line#LABEL=}";
-                     labels="${labels}${line#LABEL=}${delim}";;
+            LABEL=*|LABEL_FATBOOT=*)
+                label="${line#*=}";
+                labels="${labels}${label}${delim}";;
             TYPE=*) ftype=${line#TYPE=};;
             UUID=*) uuids="${uuids}${line#UUID=}$delim";;
         esac

[smoser]

find_devs_with calls some *bsd implelementation functions... and I'm pretty sure that LABEL_FATBOOT wont work for them, i'm not sure if that matters. but we shoudl be sure to not regress something there.

[marlluslustosa]

This was added with an alternative. FAT_BOOT has been included in current systems as a new form. That's why I added it to the code. LABEL will continue to function.

[smoser]

You need to at least make sure that passing 'LABEL_FATBOOT' through to the code for the 'find_devs_with_*bsd' functions (

cloud-init/cloudinit/util.py

Line 1112 in 995f8ad

def find_devs_with_freebsd(criteria=None, oformat='device',

) doesn't cause a problem. That code has never had LABEL_FATBOOT run through it before (It obviously should not, but please read and at least convince yourself that it wil not).

[marlluslustosa]

I don't know how to fix this by editing this other file.

[igalic]

@smoser, thank you for drawing attention to this

@goneri wrote most of the find_devs_with_*bsd() code, and he uses NoCloud, so perhaps he'll have some word on it

for now, the code is very naïve, and not written to be extensible to new classes of things to look for, with the justification that certain Linux specific things osten don't map under BSD, and what we had was good enough

[marlluslustosa]

fix ds-identify

[marlluslustosa]

I am not completely aware of how cloud-init works. I took these codes through this issue, then deployed locally (in a cloudinit 20.2) and run on CentOS 7, Debian 10 and Ubuntu 20.04 without any problems. This solves a problem that has been going on since 08/2019 (https://bugs.launchpad.net/cloud-init/+bug/1841466).

As I don't have time to check all the dependencies and how it could affect BSD-based systems, I end my participation here.

If anyone wants to continue, feel free to contribute to this or another PR, using my branch.

[blackboxsw]

Just confirmed signer of CLA on #499. Will merge that once CI passes there

[TheRealFalcon]

@marlluslustosa Thanks for the contribution! Since you no longer are able to continue, I'm closing this PR and continuing the work (including your commits) in a follow-on PR. Please see #513 for more details.