From 9b17eaf1d4f5fb3c8108236c52259ad95ff585a0 Mon Sep 17 00:00:00 2001 From: Luqman Aden Date: Thu, 9 Feb 2023 14:42:29 -0800 Subject: [PATCH] Accept drives with vFAT label 'cidata' as a configdrive. Match vFAT label using a given label instead of hardcoding 'config-2'. The NoCloud and ConfigDrive metadata services both attempt to find vFAT formatted drives but the label used was hardcoded to 'config-2' which only works with ConfigDrive. BaseConfigDriveService which both inherit from already has a drive_label property set appropriately for both so let's use that for the vFAT finding logic as well. Change-Id: I8004a8565338b0615450bb28cecc86901be94766 --- .../services/osconfigdrive/windows.py | 2 +- .../services/osconfigdrive/test_windows.py | 4 ++-- .../tests/utils/windows/test_vfat.py | 23 ++++++++++++++++--- cloudbaseinit/utils/windows/vfat.py | 6 ++--- 4 files changed, 26 insertions(+), 9 deletions(-) diff --git a/cloudbaseinit/metadata/services/osconfigdrive/windows.py b/cloudbaseinit/metadata/services/osconfigdrive/windows.py index a9163832..3fa212dd 100644 --- a/cloudbaseinit/metadata/services/osconfigdrive/windows.py +++ b/cloudbaseinit/metadata/services/osconfigdrive/windows.py @@ -165,7 +165,7 @@ class WindowsConfigDriveManager(base.BaseConfigDriveManager): def _get_config_drive_from_vfat(self, drive_label, metadata_file): for drive_path in self._osutils.get_physical_disks(): - if vfat.is_vfat_drive(self._osutils, drive_path): + if vfat.is_vfat_drive(self._osutils, drive_path, drive_label): LOG.info('Config Drive found on disk %r', drive_path) vfat.copy_from_vfat_drive(self._osutils, drive_path, self.target_path) diff --git a/cloudbaseinit/tests/metadata/services/osconfigdrive/test_windows.py b/cloudbaseinit/tests/metadata/services/osconfigdrive/test_windows.py index efc6a186..b58f1f3f 100644 --- a/cloudbaseinit/tests/metadata/services/osconfigdrive/test_windows.py +++ b/cloudbaseinit/tests/metadata/services/osconfigdrive/test_windows.py @@ -329,8 +329,8 @@ class TestWindowsConfigDriveManager(unittest.TestCase): self.osutils.get_physical_disks.assert_called_once_with() expected_is_vfat_calls = [ - mock.call(self.osutils, mock.sentinel.drive1), - mock.call(self.osutils, mock.sentinel.drive2), + mock.call(self.osutils, mock.sentinel.drive1, self._fake_label), + mock.call(self.osutils, mock.sentinel.drive2, self._fake_label), ] self.assertEqual(expected_is_vfat_calls, mock_is_vfat_drive.mock_calls) mock_copy_from_vfat_drive.assert_called_once_with( diff --git a/cloudbaseinit/tests/utils/windows/test_vfat.py b/cloudbaseinit/tests/utils/windows/test_vfat.py index 53219717..f46dca9c 100644 --- a/cloudbaseinit/tests/utils/windows/test_vfat.py +++ b/cloudbaseinit/tests/utils/windows/test_vfat.py @@ -31,7 +31,8 @@ class TestVfat(unittest.TestCase): def _test_is_vfat_drive(self, execute_process_value, expected_logging, - expected_response): + expected_response, + drive_label='config-2'): mock_osutils = mock.Mock() mock_osutils.execute_process.return_value = execute_process_value @@ -41,7 +42,8 @@ class TestVfat(unittest.TestCase): with testutils.ConfPatcher('mtools_path', 'mtools_path'): response = vfat.is_vfat_drive(mock_osutils, - mock.sentinel.drive) + mock.sentinel.drive, + drive_label) mdir = os.path.join(CONF.mtools_path, "mlabel.exe") mock_osutils.execute_process.assert_called_once_with( @@ -105,6 +107,20 @@ class TestVfat(unittest.TestCase): expected_logging=expected_logging, expected_response=expected_response) + def test_is_vfat_drive_works_alternate_drive_label(self): + mock_out = b"Volume label is CIDATA \r\n" + expected_logging = [ + "Obtained label information for drive %r: %r" + % (mock.sentinel.drive, mock_out) + ] + execute_process_value = (mock_out, None, 0) + expected_response = True + + self._test_is_vfat_drive(execute_process_value=execute_process_value, + expected_logging=expected_logging, + expected_response=expected_response, + drive_label='cidata') + def test_is_vfat_drive_with_wrong_label(self): mock_out = b"Not volu label \r\n" expected_logging = [ @@ -143,7 +159,8 @@ class TestVfat(unittest.TestCase): def test_is_vfat_drive_mtools_not_given(self): with self.assertRaises(exception.CloudbaseInitException) as cm: vfat.is_vfat_drive(mock.sentinel.osutils, - mock.sentinel.target_path) + mock.sentinel.target_path, + mock.sentinel.drive_label) expected_message = ('"mtools_path" needs to be provided in order ' 'to access VFAT drives') self.assertEqual(expected_message, str(cm.exception.args[0])) diff --git a/cloudbaseinit/utils/windows/vfat.py b/cloudbaseinit/utils/windows/vfat.py index 9705d965..edd3cd4f 100644 --- a/cloudbaseinit/utils/windows/vfat.py +++ b/cloudbaseinit/utils/windows/vfat.py @@ -22,7 +22,6 @@ from cloudbaseinit import exception CONF = cloudbaseinit_conf.CONF -CONFIG_DRIVE_LABELS = ['config-2', 'CONFIG-2'] LOG = oslo_logging.getLogger(__name__) VOLUME_LABEL_REGEX = re.compile("Volume label is (.*?)$") @@ -34,7 +33,7 @@ def _check_mtools_path(): 'to access VFAT drives') -def is_vfat_drive(osutils, drive_path): +def is_vfat_drive(osutils, drive_path, drive_label): """Check if the given drive contains a VFAT filesystem.""" _check_mtools_path() mlabel = os.path.join(CONF.mtools_path, "mlabel.exe") @@ -50,7 +49,8 @@ def is_vfat_drive(osutils, drive_path): LOG.debug("Obtained label information for drive %r: %r", drive_path, out) out = out.decode().strip() match = VOLUME_LABEL_REGEX.search(out) - return match.group(1) in CONFIG_DRIVE_LABELS if match else False + drive_labels = [drive_label.lower(), drive_label.upper()] + return match.group(1) in drive_labels if match else False def copy_from_vfat_drive(osutils, drive_path, target_path):