From 75e15dce944bcc9f534cf47f56e8ed8eb8dc7ebf Mon Sep 17 00:00:00 2001 From: OpenStack Release Bot Date: Fri, 23 Sep 2022 08:41:50 +0000 Subject: [PATCH 01/11] Update .gitreview for stable/zed Change-Id: I90bb4608e46c45259542993f04253026cedf4f0b --- .gitreview | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitreview b/.gitreview index 9df0f11ec..5babe14a0 100644 --- a/.gitreview +++ b/.gitreview @@ -2,3 +2,4 @@ host=review.opendev.org port=29418 project=openstack/ironic-python-agent.git +defaultbranch=stable/zed From ae2fe9443da7168b51cbd03e9c747c86473990e8 Mon Sep 17 00:00:00 2001 From: OpenStack Release Bot Date: Fri, 23 Sep 2022 08:41:51 +0000 Subject: [PATCH 02/11] Update TOX_CONSTRAINTS_FILE for stable/zed Update the URL to the upper-constraints file to point to the redirect rule on releases.openstack.org so that anyone working on this branch will switch to the correct upper-constraints list automatically when the requirements repository branches. Until the requirements repository has as stable/zed branch, tests will continue to use the upper-constraints list on master. Change-Id: I67f8b3e937f81ab9271b89f1f0bb10cd762bfa74 --- tox.ini | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tox.ini b/tox.ini index 8605785dc..431a1bbd1 100644 --- a/tox.ini +++ b/tox.ini @@ -15,7 +15,7 @@ setenv = LANGUAGE=en_US LC_ALL=en_US.utf-8 deps = - -c{env:TOX_CONSTRAINTS_FILE:https://releases.openstack.org/constraints/upper/master} + -c{env:TOX_CONSTRAINTS_FILE:https://releases.openstack.org/constraints/upper/zed} -r{toxinidir}/requirements.txt -r{toxinidir}/test-requirements.txt commands = stestr run {posargs} @@ -71,7 +71,7 @@ setenv = PYTHONHASHSEED=0 sitepackages = False # NOTE(dtantsur): documentation building process requires importing IPA deps = - -c{env:TOX_CONSTRAINTS_FILE:https://releases.openstack.org/constraints/upper/master} + -c{env:TOX_CONSTRAINTS_FILE:https://releases.openstack.org/constraints/upper/zed} -r{toxinidir}/requirements.txt -r{toxinidir}/doc/requirements.txt commands = @@ -89,7 +89,7 @@ commands = [testenv:releasenotes] usedevelop = False deps = - -c{env:TOX_CONSTRAINTS_FILE:https://releases.openstack.org/constraints/upper/master} + -c{env:TOX_CONSTRAINTS_FILE:https://releases.openstack.org/constraints/upper/zed} -r{toxinidir}/doc/requirements.txt commands = sphinx-build -a -E -W -d releasenotes/build/doctrees -b html releasenotes/source releasenotes/build/html @@ -128,7 +128,7 @@ commands = [testenv:bandit] usedevelop = False deps = - -c{env:TOX_CONSTRAINTS_FILE:https://releases.openstack.org/constraints/upper/master} + -c{env:TOX_CONSTRAINTS_FILE:https://releases.openstack.org/constraints/upper/zed} -r{toxinidir}/test-requirements.txt commands = bandit -r ironic_python_agent -x tests -n5 -ll -c tools/bandit.yml From 72dd56920bb50160b388e417f80c774fa37f5a7a Mon Sep 17 00:00:00 2001 From: Jay Faulkner Date: Thu, 15 Dec 2022 09:55:34 -0800 Subject: [PATCH 03/11] Fixes for tox 4.0 - passenv needed better structure - usedevelop no longer allowed alongside skipsdist Change-Id: I22dae3b1e4cca2dc328c21e889acf7fe98a80e0b --- tox.ini | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/tox.ini b/tox.ini index 431a1bbd1..3a4ad1f61 100644 --- a/tox.ini +++ b/tox.ini @@ -1,11 +1,9 @@ [tox] minversion = 3.18.0 -skipsdist = True envlist = py3,functional,pep8 ignore_basepython_conflict=true [testenv] -usedevelop = True basepython = python3 setenv = VIRTUAL_ENV={envdir} @@ -19,7 +17,13 @@ deps = -r{toxinidir}/requirements.txt -r{toxinidir}/test-requirements.txt commands = stestr run {posargs} -passenv = http_proxy HTTP_PROXY https_proxy HTTPS_PROXY no_proxy NO_PROXY +passenv = + http_proxy + HTTP_PROXY + https_proxy + HTTPS_PROXY + no_proxy + NO_PROXY [testenv:functional] # Define virtualenv directory, port to use for functional testing, and number @@ -33,8 +37,7 @@ setenv = commands = stestr run {posargs} [testenv:pep8] -usedevelop = False -deps= +deps = hacking>=4.1.0,<5.0.0 # Apache-2.0 bashate>=0.5.1 # Apache-2.0 flake8-import-order>=0.17.1 # LGPLv3 @@ -87,7 +90,6 @@ commands = make -C doc/build/pdf [testenv:releasenotes] -usedevelop = False deps = -c{env:TOX_CONSTRAINTS_FILE:https://releases.openstack.org/constraints/upper/zed} -r{toxinidir}/doc/requirements.txt @@ -126,7 +128,6 @@ commands = oslo-config-generator --config-file=tools/config/ipa-config-generator.conf [testenv:bandit] -usedevelop = False deps = -c{env:TOX_CONSTRAINTS_FILE:https://releases.openstack.org/constraints/upper/zed} -r{toxinidir}/test-requirements.txt From fa7e33b0b48480f0836f9ded8561cd35bf40ebab Mon Sep 17 00:00:00 2001 From: Jay Faulkner Date: Wed, 25 Jan 2023 13:18:01 -0800 Subject: [PATCH 04/11] Make reno ignore bugfix eol tags Reno was assuming all tags ending in -eol represented an old, EOL'd stable branch. That's not true for Ironic projects which have bugfix branches. Update the regexp to exclude those branches. Co-Authored-By: Adam McArthur Change-Id: I265969ab40a98a02962c2fc8460b6519ab576f99 (cherry picked from commit f8fc7e52f36de5c3515e60f5d2e6347a538a80d8) --- releasenotes/config.yaml | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 releasenotes/config.yaml diff --git a/releasenotes/config.yaml b/releasenotes/config.yaml new file mode 100644 index 000000000..3230b6810 --- /dev/null +++ b/releasenotes/config.yaml @@ -0,0 +1,2 @@ +--- +closed_branch_tag_re: 'r"(?!^bugfix.*-eol$).+-eol"' From 09c3c412193187c8b71f8fd3326b012d8f0953bf Mon Sep 17 00:00:00 2001 From: Julia Kreger Date: Fri, 7 Apr 2023 07:32:44 -0700 Subject: [PATCH 05/11] Fix UTF-16 result handling for efibootmgr The tl;dr is that UEFI NVRAM is in encoded in UTF-16, and when we run the efibootmgr command, we can get unicode characters back. Except we previously were forcing everything to be treated as UTF-8 due to the way oslo.concurrency's processutils module works. This could be observed with UTF character 0x00FF which raises up a nice exception when we try to decode it. Anyhow! while fixing handling of this, we discovered we could get basically the cruft out of the NVRAM, by getting what was most likey a truncated string out of our own test VMs. As such, we need to also permit decoding to be tollerant of failures. This could be binary data or as simple as flipped bits which get interpretted invalid characters. As such, we have introduced such data into one of our tests involving UEFI record de-duplication. Closes-Bug: 2015602 Change-Id: I006535bf124379ed65443c7b283bc99ecc95568b (cherry picked from commit 76accfb880474445a5dcb07825889123b3dd0237) --- ironic_python_agent/efi_utils.py | 15 +++- .../tests/unit/extensions/test_image.py | 45 +++++----- .../tests/unit/test_efi_utils.py | 85 ++++++++++--------- ...r-character-encoding-19e531ba694824c1.yaml | 9 ++ 4 files changed, 91 insertions(+), 63 deletions(-) create mode 100644 releasenotes/notes/fixes-efibootmgr-character-encoding-19e531ba694824c1.yaml diff --git a/ironic_python_agent/efi_utils.py b/ironic_python_agent/efi_utils.py index 48e643d3f..6fc8dff19 100644 --- a/ironic_python_agent/efi_utils.py +++ b/ironic_python_agent/efi_utils.py @@ -275,8 +275,15 @@ def get_boot_records(): :return: an iterator yielding pairs (boot number, boot record). """ - efi_output = utils.execute('efibootmgr', '-v') - for line in efi_output[0].split('\n'): + # Invokes binary=True so we get a bytestream back. + efi_output = utils.execute('efibootmgr', '-v', binary=True) + # Bytes must be decoded before regex can be run and + # matching to work as intended. + # Also ignore errors on decoding, as we can basically get + # garbage out of the nvram record, this way we don't fail + # hard on unrelated records. + cmd_output = efi_output[0].decode('utf-16', errors='ignore') + for line in cmd_output.split('\n'): match = _ENTRY_LABEL.match(line) if match is not None: yield (match[1], match[2]) @@ -293,7 +300,7 @@ def add_boot_record(device, efi_partition, loader, label): # https://linux.die.net/man/8/efibootmgr utils.execute('efibootmgr', '-v', '-c', '-d', device, '-p', str(efi_partition), '-w', '-L', label, - '-l', loader) + '-l', loader, binary=True) def remove_boot_record(boot_num): @@ -301,7 +308,7 @@ def remove_boot_record(boot_num): :param boot_num: the number of the boot record """ - utils.execute('efibootmgr', '-b', boot_num, '-B') + utils.execute('efibootmgr', '-b', boot_num, '-B', binary=True) def _run_efibootmgr(valid_efi_bootloaders, device, efi_partition, diff --git a/ironic_python_agent/tests/unit/extensions/test_image.py b/ironic_python_agent/tests/unit/extensions/test_image.py index e488b74f7..32f4addb5 100644 --- a/ironic_python_agent/tests/unit/extensions/test_image.py +++ b/ironic_python_agent/tests/unit/extensions/test_image.py @@ -32,6 +32,9 @@ from ironic_python_agent.tests.unit.samples import hardware_samples as hws +EFI_RESULT = ''.encode('utf-16') + + @mock.patch.object(hardware, 'dispatch_to_managers', autospec=True) @mock.patch.object(ilib_utils, 'execute', autospec=True) @mock.patch.object(tempfile, 'mkdtemp', lambda *_: '/tmp/fake-dir') @@ -231,7 +234,7 @@ def test__uefi_bootloader_given_partition( mock_execute.side_effect = iter([('', ''), ('', ''), ('', ''), ('', ''), - ('', ''), ('', ''), + (EFI_RESULT, ''), (EFI_RESULT, ''), ('', ''), ('', '')]) expected = [mock.call('efibootmgr', '--version'), @@ -240,11 +243,11 @@ def test__uefi_bootloader_given_partition( mock.call('udevadm', 'settle'), mock.call('mount', self.fake_efi_system_part, self.fake_dir + '/boot/efi'), - mock.call('efibootmgr', '-v'), + mock.call('efibootmgr', '-v', binary=True), mock.call('efibootmgr', '-v', '-c', '-d', self.fake_dev, '-p', '1', '-w', '-L', 'ironic1', '-l', - '\\EFI\\BOOT\\BOOTX64.EFI'), + '\\EFI\\BOOT\\BOOTX64.EFI', binary=True), mock.call('umount', self.fake_dir + '/boot/efi', attempts=3, delay_on_retry=True), mock.call('sync')] @@ -279,7 +282,7 @@ def test__uefi_bootloader_find_partition( mock_efi_bl.return_value = ['EFI/BOOT/BOOTX64.EFI'] mock_execute.side_effect = iter([('', ''), ('', ''), ('', ''), ('', ''), - ('', ''), ('', ''), + (EFI_RESULT, ''), (EFI_RESULT, ''), ('', ''), ('', '')]) expected = [mock.call('efibootmgr', '--version'), @@ -288,11 +291,11 @@ def test__uefi_bootloader_find_partition( mock.call('udevadm', 'settle'), mock.call('mount', self.fake_efi_system_part, self.fake_dir + '/boot/efi'), - mock.call('efibootmgr', '-v'), + mock.call('efibootmgr', '-v', binary=True), mock.call('efibootmgr', '-v', '-c', '-d', self.fake_dev, '-p', '1', '-w', '-L', 'ironic1', '-l', - '\\EFI\\BOOT\\BOOTX64.EFI'), + '\\EFI\\BOOT\\BOOTX64.EFI', binary=True), mock.call('umount', self.fake_dir + '/boot/efi', attempts=3, delay_on_retry=True), mock.call('sync')] @@ -333,10 +336,11 @@ def test__uefi_bootloader_with_entry_removal( Boot0001 ironic2 HD(1,GPT,4f3c6294-bf9b-4208-9808-111111111112)File(\EFI\Boot\BOOTX64.EFI) Boot0002 VENDMAGIC FvFile(9f3c6294-bf9b-4208-9808-be45dfc34b51) """ # noqa This is a giant literal string for testing. + stdout_msg = stdout_msg.encode('utf-16') mock_execute.side_effect = iter([('', ''), ('', ''), ('', ''), ('', ''), - (stdout_msg, ''), ('', ''), - ('', ''), ('', ''), + (stdout_msg, ''), (EFI_RESULT, ''), + (EFI_RESULT, ''), (EFI_RESULT, ''), ('', ''), ('', '')]) expected = [mock.call('efibootmgr', '--version'), @@ -345,12 +349,12 @@ def test__uefi_bootloader_with_entry_removal( mock.call('udevadm', 'settle'), mock.call('mount', self.fake_efi_system_part, self.fake_dir + '/boot/efi'), - mock.call('efibootmgr', '-v'), - mock.call('efibootmgr', '-b', '0000', '-B'), + mock.call('efibootmgr', '-v', binary=True), + mock.call('efibootmgr', '-b', '0000', '-B', binary=True), mock.call('efibootmgr', '-v', '-c', '-d', self.fake_dev, '-p', '1', '-w', '-L', 'ironic1', '-l', - '\\EFI\\BOOT\\BOOTX64.EFI'), + '\\EFI\\BOOT\\BOOTX64.EFI', binary=True), mock.call('umount', self.fake_dir + '/boot/efi', attempts=3, delay_on_retry=True), mock.call('sync')] @@ -396,6 +400,7 @@ def test__uefi_bootloader_with_entry_removal_lenovo( Boot0003* Network VenHw(1fad3248-0000-7950-2166-a1e506fdb83a,05000000)..GO..NO............U.E.F.I.:. . . .S.L.O.T.2. .(.2.F./.0./.0.). .P.X.E. .I.P.4. . .Q.L.o.g.i.c. .Q.L.4.1.2.6.2. .P.C.I.e. .2.5.G.b. .2.-.P.o.r.t. .S.F.P.2.8. .E.t.h.e.r.n.e.t. .A.d.a.p.t.e.r. .-. .P.X.E........A....................%.4..Z...............................................................Gd-.;.A..MQ..L.P.X.E. .I.P.4. .Q.L.o.g.i.c. .Q.L.4.1.2.6.2. .P.C.I.e. .2.5.G.b. .2.-.P.o.r.t. .S.F.P.2.8. .E.t.h.e.r.n.e.t. .A.d.a.p.t.e.r. .-. .P.X.E.......BO..NO............U.E.F.I.:. . . .S.L.O.T.1. .(.3.0./.0./.0.). .P.X.E. .I.P.4. . .Q.L.o.g.i.c. .Q.L.4.1.2.6.2. .P.C.I.e. .2.5.G.b. .2.-.P.o.r.t. .S.F.P.2.8. .E.t.h.e.r.n.e.t. .A.d.a.p.t.e.r. .-. Boot0004* ironic1 HD(1,GPT,55db8d03-c8f6-4a5b-9155-790dddc348fa,0x800,0x64000)/File(\EFI\boot\shimx64.efi) """ # noqa This is a giant literal string for testing. + stdout_msg = stdout_msg.encode('utf-16') mock_execute.side_effect = iter([('', ''), ('', ''), ('', ''), ('', ''), (stdout_msg, ''), ('', ''), @@ -407,13 +412,13 @@ def test__uefi_bootloader_with_entry_removal_lenovo( mock.call('udevadm', 'settle'), mock.call('mount', self.fake_efi_system_part, self.fake_dir + '/boot/efi'), - mock.call('efibootmgr', '-v'), - mock.call('efibootmgr', '-b', '0000', '-B'), - mock.call('efibootmgr', '-b', '0004', '-B'), + mock.call('efibootmgr', '-v', binary=True), + mock.call('efibootmgr', '-b', '0000', '-B', binary=True), + mock.call('efibootmgr', '-b', '0004', '-B', binary=True), mock.call('efibootmgr', '-v', '-c', '-d', self.fake_dev, '-p', '1', '-w', '-L', 'ironic1', '-l', - '\\EFI\\BOOT\\BOOTX64.EFI'), + '\\EFI\\BOOT\\BOOTX64.EFI', binary=True), mock.call('umount', self.fake_dir + '/boot/efi', attempts=3, delay_on_retry=True), mock.call('sync')] @@ -450,8 +455,8 @@ def test__add_multi_bootloaders( mock_execute.side_effect = iter([('', ''), ('', ''), ('', ''), ('', ''), - ('', ''), ('', ''), - ('', ''), ('', ''), + (EFI_RESULT, ''), (EFI_RESULT, ''), + (EFI_RESULT, ''), ('', ''), ('', '')]) expected = [mock.call('efibootmgr', '--version'), @@ -460,15 +465,15 @@ def test__add_multi_bootloaders( mock.call('udevadm', 'settle'), mock.call('mount', self.fake_efi_system_part, self.fake_dir + '/boot/efi'), - mock.call('efibootmgr', '-v'), + mock.call('efibootmgr', '-v', binary=True), mock.call('efibootmgr', '-v', '-c', '-d', self.fake_dev, '-p', '1', '-w', '-L', 'ironic1', '-l', - '\\EFI\\BOOT\\BOOTX64.EFI'), + '\\EFI\\BOOT\\BOOTX64.EFI', binary=True), mock.call('efibootmgr', '-v', '-c', '-d', self.fake_dev, '-p', '1', '-w', '-L', 'ironic2', '-l', - '\\WINDOWS\\system32\\winload.efi'), + '\\WINDOWS\\system32\\winload.efi', binary=True), mock.call('umount', self.fake_dir + '/boot/efi', attempts=3, delay_on_retry=True), mock.call('sync')] diff --git a/ironic_python_agent/tests/unit/test_efi_utils.py b/ironic_python_agent/tests/unit/test_efi_utils.py index 64de61bce..09e897966 100644 --- a/ironic_python_agent/tests/unit/test_efi_utils.py +++ b/ironic_python_agent/tests/unit/test_efi_utils.py @@ -28,6 +28,9 @@ from ironic_python_agent import utils +EFI_RESULT = ''.encode('utf-16') + + @mock.patch.object(os, 'walk', autospec=True) @mock.patch.object(os, 'access', autospec=False) class TestGetEfiBootloaders(base.IronicAgentTest): @@ -115,16 +118,16 @@ def test__run_efibootmgr_no_bootloaders(self, mock_execute): mock_execute.assert_has_calls(expected) def test__run_efibootmgr(self, mock_execute): - mock_execute.return_value = ('', '') + mock_execute.return_value = (''.encode('utf-16'), '') result = efi_utils._run_efibootmgr(['EFI/BOOT/BOOTX64.EFI'], self.fake_dev, self.fake_efi_system_part, self.fake_dir) - expected = [mock.call('efibootmgr', '-v'), + expected = [mock.call('efibootmgr', '-v', binary=True), mock.call('efibootmgr', '-v', '-c', '-d', self.fake_dev, '-p', self.fake_efi_system_part, '-w', '-L', 'ironic1', '-l', - '\\EFI\\BOOT\\BOOTX64.EFI')] + '\\EFI\\BOOT\\BOOTX64.EFI', binary=True)] self.assertIsNone(result) mock_execute.assert_has_calls(expected) @@ -176,19 +179,18 @@ def test_ok(self, mkdir_mock, mock_efi_bl, mock_is_md_device, mock_is_md_device.return_value = False mock_efi_bl.return_value = ['EFI/BOOT/BOOTX64.EFI'] - - mock_execute.side_effect = iter([('', ''), ('', ''), - ('', ''), ('', ''), + mock_execute.side_effect = iter([('', ''), (EFI_RESULT, ''), + (EFI_RESULT, ''), ('', ''), ('', ''), ('', ''), ('', '')]) expected = [mock.call('mount', self.fake_efi_system_part, self.fake_dir + '/boot/efi'), - mock.call('efibootmgr', '-v'), + mock.call('efibootmgr', '-v', binary=True), mock.call('efibootmgr', '-v', '-c', '-d', self.fake_dev, '-p', '1', '-w', '-L', 'ironic1', '-l', - '\\EFI\\BOOT\\BOOTX64.EFI'), + '\\EFI\\BOOT\\BOOTX64.EFI', binary=True), mock.call('umount', self.fake_dir + '/boot/efi', attempts=3, delay_on_retry=True), mock.call('sync')] @@ -219,30 +221,34 @@ def test_found_csv(self, mkdir_mock, mock_efi_bl, mock_is_md_device, # at the start of the file, where as Red Hat *does* csv_file_data = u'shimx64.efi,Vendor String,,Grub2MadeUSDoThis\n' # This test also handles deleting a pre-existing matching vendor - # string in advance. + # string in advance. This string also includes a UTF16 character + # *on* purpose, to force proper decoding to be tested and garbage + # characters which can be found in OVMF test VM NVRAM records. dupe_entry = """ BootCurrent: 0001 Timeout: 0 seconds BootOrder: 0000,00001 -Boot0000* Vendor String HD(1,GPT,4f3c6294-bf9b-4208-9808-be45dfc34b5c)File(\EFI\Boot\BOOTX64.EFI) -Boot0001 Vendor String HD(2,GPT,4f3c6294-bf9b-4208-9808-be45dfc34b5c)File(\EFI\Boot\BOOTX64.EFI) -Boot0002: VENDMAGIC FvFile(9f3c6294-bf9b-4208-9808-be45dfc34b51) +Boot0000 UTF16ΓΏ HD(1,GPT,4f3c6294-bf9b-4208-9808-be45dfc34b5c)File(\EFI\Boot\BOOTX64.EFI) +Boot0001* Vendor String HD(1,GPT,4f3c6294-bf9b-4208-9808-be45dfc34b5c)File(\EFI\Boot\BOOTX64.EFI) +Boot0002 Vendor String HD(2,GPT,4f3c6294-bf9b-4208-9808-be45dfc34b5c)File(\EFI\Boot\BOOTX64.EFI) +Boot0003: VENDMAGIC FvFile(9f3c6294-bf9b-4208-9808-be45dfc34b51)N.....YM....R,Y. """ # noqa This is a giant literal string for testing. - - mock_execute.side_effect = iter([('', ''), (dupe_entry, ''), + dupe_entry = dupe_entry.encode('utf-16') + mock_execute.side_effect = iter([('', ''), + (dupe_entry, ''), ('', ''), ('', ''), ('', ''), ('', ''), ('', '')]) expected = [mock.call('mount', self.fake_efi_system_part, self.fake_dir + '/boot/efi'), - mock.call('efibootmgr', '-v'), - mock.call('efibootmgr', '-b', '0000', '-B'), - mock.call('efibootmgr', '-b', '0001', '-B'), + mock.call('efibootmgr', '-v', binary=True), + mock.call('efibootmgr', '-b', '0001', '-B', binary=True), + mock.call('efibootmgr', '-b', '0002', '-B', binary=True), mock.call('efibootmgr', '-v', '-c', '-d', self.fake_dev, '-p', '1', '-w', '-L', 'Vendor String', '-l', - '\\EFI\\vendor\\shimx64.efi'), + '\\EFI\\vendor\\shimx64.efi', binary=True), mock.call('umount', self.fake_dir + '/boot/efi', attempts=3, delay_on_retry=True), mock.call('sync')] @@ -266,19 +272,18 @@ def test_nvme_device(self, mkdir_mock, mock_efi_bl, mock_is_md_device, mock_is_md_device.return_value = False mock_efi_bl.return_value = ['EFI/BOOT/BOOTX64.EFI'] - - mock_execute.side_effect = iter([('', ''), ('', ''), - ('', ''), ('', ''), + mock_execute.side_effect = iter([('', ''), (EFI_RESULT, ''), + (EFI_RESULT, ''), ('', ''), ('', ''), ('', ''), ('', '')]) expected = [mock.call('mount', '/dev/fakenvme0p1', self.fake_dir + '/boot/efi'), - mock.call('efibootmgr', '-v'), + mock.call('efibootmgr', '-v', binary=True), mock.call('efibootmgr', '-v', '-c', '-d', '/dev/fakenvme0', '-p', '1', '-w', '-L', 'ironic1', '-l', - '\\EFI\\BOOT\\BOOTX64.EFI'), + '\\EFI\\BOOT\\BOOTX64.EFI', binary=True), mock.call('umount', self.fake_dir + '/boot/efi', attempts=3, delay_on_retry=True), mock.call('sync')] @@ -301,19 +306,18 @@ def test_wholedisk(self, mkdir_mock, mock_efi_bl, mock_is_md_device, mock_is_md_device.return_value = False mock_efi_bl.return_value = ['EFI/BOOT/BOOTX64.EFI'] - - mock_execute.side_effect = iter([('', ''), ('', ''), - ('', ''), ('', ''), + mock_execute.side_effect = iter([('', ''), (EFI_RESULT, ''), + (EFI_RESULT, ''), ('', ''), ('', ''), ('', ''), ('', '')]) expected = [mock.call('mount', self.fake_efi_system_part, self.fake_dir + '/boot/efi'), - mock.call('efibootmgr', '-v'), + mock.call('efibootmgr', '-v', binary=True), mock.call('efibootmgr', '-v', '-c', '-d', self.fake_dev, '-p', '1', '-w', '-L', 'ironic1', '-l', - '\\EFI\\BOOT\\BOOTX64.EFI'), + '\\EFI\\BOOT\\BOOTX64.EFI', binary=True), mock.call('umount', self.fake_dir + '/boot/efi', attempts=3, delay_on_retry=True), mock.call('sync')] @@ -347,11 +351,14 @@ def test_software_raid(self, mkdir_mock, mock_efi_bl, mock_is_md_device, mock_get_component_devices.return_value = ['/dev/sda3', '/dev/sdb3'] mock_efi_bl.return_value = ['EFI/BOOT/BOOTX64.EFI'] - - mock_execute.side_effect = iter([('', ''), ('', ''), - ('', ''), ('', ''), - ('', ''), ('', ''), - ('', ''), ('', ''), + mock_execute.side_effect = iter([('', ''), + ('', ''), + ('', ''), + (EFI_RESULT, ''), + (EFI_RESULT, ''), + (EFI_RESULT, ''), + (EFI_RESULT, ''), + ('', ''), ('', '')]) expected = [mock.call('mount', self.fake_efi_system_part, @@ -360,14 +367,14 @@ def test_software_raid(self, mkdir_mock, mock_efi_bl, mock_is_md_device, attempts=3, delay_on_retry=True), mock.call('mount', self.fake_efi_system_part, self.fake_dir + '/boot/efi'), - mock.call('efibootmgr', '-v'), + mock.call('efibootmgr', '-v', binary=True), mock.call('efibootmgr', '-v', '-c', '-d', '/dev/sda3', '-p', '3', '-w', '-L', 'ironic1 (RAID, part0)', - '-l', '\\EFI\\BOOT\\BOOTX64.EFI'), - mock.call('efibootmgr', '-v'), + '-l', '\\EFI\\BOOT\\BOOTX64.EFI', binary=True), + mock.call('efibootmgr', '-v', binary=True), mock.call('efibootmgr', '-v', '-c', '-d', '/dev/sdb3', '-p', '3', '-w', '-L', 'ironic1 (RAID, part1)', - '-l', '\\EFI\\BOOT\\BOOTX64.EFI'), + '-l', '\\EFI\\BOOT\\BOOTX64.EFI', binary=True), mock.call('umount', self.fake_dir + '/boot/efi', attempts=3, delay_on_retry=True), mock.call('sync')] @@ -425,7 +432,7 @@ def test_failure_after_mount(self, mkdir_mock, mock_efi_bl, expected = [mock.call('mount', self.fake_efi_system_part, self.fake_dir + '/boot/efi'), - mock.call('efibootmgr', '-v'), + mock.call('efibootmgr', '-v', binary=True), mock.call('umount', self.fake_dir + '/boot/efi', attempts=3, delay_on_retry=True), mock.call('sync')] @@ -462,7 +469,7 @@ def test_failure_after_failure(self, mkdir_mock, mock_efi_bl, expected = [mock.call('mount', self.fake_efi_system_part, self.fake_dir + '/boot/efi'), - mock.call('efibootmgr', '-v'), + mock.call('efibootmgr', '-v', binary=True), mock.call('umount', self.fake_dir + '/boot/efi', attempts=3, delay_on_retry=True)] diff --git a/releasenotes/notes/fixes-efibootmgr-character-encoding-19e531ba694824c1.yaml b/releasenotes/notes/fixes-efibootmgr-character-encoding-19e531ba694824c1.yaml new file mode 100644 index 000000000..b4c3270cf --- /dev/null +++ b/releasenotes/notes/fixes-efibootmgr-character-encoding-19e531ba694824c1.yaml @@ -0,0 +1,9 @@ +--- +fixes: + - | + Fixes UEFI NVRAM record handling with efibootmgr so we can accept and + handle UTF-16 encoded data which is to be expected in UEFI NVRAM as + the records are UTF-16 encoded. + - | + Fixes handling of UEFI NVRAM records to allow for unexpected characters + in the response, so it is non-fatal to Ironic. From 06214d7ce25fcb7dd6f66328024620804d2b1148 Mon Sep 17 00:00:00 2001 From: Julia Kreger Date: Mon, 14 Aug 2023 14:03:32 -0700 Subject: [PATCH 06/11] Handle the node being locked If the node is locked, a lookup cannot be performed when an agent token needs to be generated, which tends to error like this: ironic_python_agent.ironic_api_client [-] Failed looking up node with addresses '00:6f:bb:34:b3:4d,00:6f:bb:34:b3:4b' at https://172.22.0.2:6385. Error 409: Node c25e451b-d2fb-4168-b690-f15bc8365520 is locked by host 172.22.0.2, please retry after the current operation is completed.. Check if inspection has completed. Problem is, if we keep pounding on the door, we can actually worsen the situation, and previously we would just just let tenacity retry. We will now hold for 30 seconds before proceeding, so we have hopefully allowed the operation to complete. Also fixes the error logging to help human's sanity. Change-Id: I97d3e27e2adb731794a7746737d3788c6e7977a0 (cherry picked from commit 5ed520df895997e571732c305b3f1ac2f954209a) --- ironic_python_agent/ironic_api_client.py | 27 +++++++++++++++++-- .../tests/unit/test_ironic_api_client.py | 27 +++++++++++++++++++ ...e-conflict-is-locked-2ea21dd45abfe4f7.yaml | 13 +++++++++ 3 files changed, 65 insertions(+), 2 deletions(-) create mode 100644 releasenotes/notes/understand-node-conflict-is-locked-2ea21dd45abfe4f7.yaml diff --git a/ironic_python_agent/ironic_api_client.py b/ironic_python_agent/ironic_api_client.py index a2e7e1d00..8ea7029e7 100644 --- a/ironic_python_agent/ironic_api_client.py +++ b/ironic_python_agent/ironic_api_client.py @@ -13,6 +13,7 @@ # limitations under the License. import json +import time from oslo_config import cfg from oslo_log import log @@ -45,6 +46,7 @@ class APIClient(object): heartbeat_api = '/%s/heartbeat/{uuid}' % api_version _ironic_api_version = None agent_token = None + lookup_lock_pause = 0 def __init__(self, api_url): self.api_url = api_url.rstrip('/') @@ -176,7 +178,7 @@ def heartbeat(self, uuid, advertise_address, advertise_protocol='http', raise errors.HeartbeatError(error) def lookup_node(self, hardware_info, timeout, starting_interval, - node_uuid=None, max_interval=30): + node_uuid=None, max_interval=60): retry = tenacity.retry( retry=tenacity.retry_if_result(lambda r: r is False), stop=tenacity.stop_after_delay(timeout), @@ -244,10 +246,31 @@ def _do_lookup(self, hardware_info, node_uuid): LOG.error(exc_msg) raise errors.LookupNodeError(msg) + if response.status_code == requests.codes.CONFLICT: + if self.lookup_lock_pause == 0: + self.lookup_lock_pause = 5 + elif self.lookup_lock_pause == 5: + self.lookup_lock_pause = 10 + elif self.lookup_lock_pause == 10: + # If we're reaching this point, we've got a long held + # persistent lock, which means things can go very sideways + # or the ironic deployment is downright grumpy. Either way, + # we need to slow things down. + self.lookup_lock_pause = 30 + LOG.warning( + 'Ironic has responded with a conflict, signaling the ' + 'node is locked. We will wait %(time)s seconds before trying ' + 'again. %(err)s', + {'time': self.lookup_lock_pause, + 'error': self._error_from_response(response)} + ) + time.sleep(self.lookup_lock_pause) + return False + if response.status_code != requests.codes.OK: LOG.warning( 'Failed looking up node with addresses %r at %s. ' - '%s. Check if inspection has completed.', + 'Check if inspection has completed? %s', params['addresses'], self.api_url, self._error_from_response(response) ) diff --git a/ironic_python_agent/tests/unit/test_ironic_api_client.py b/ironic_python_agent/tests/unit/test_ironic_api_client.py index 8c05652ce..6c101acc7 100644 --- a/ironic_python_agent/tests/unit/test_ironic_api_client.py +++ b/ironic_python_agent/tests/unit/test_ironic_api_client.py @@ -13,6 +13,7 @@ # limitations under the License. import json +import time from unittest import mock from oslo_config import cfg @@ -461,6 +462,32 @@ def test_do_lookup_transient_exceptions(self, mock_log): mock_log.error.assert_has_calls([]) self.assertEqual(1, mock_log.warning.call_count) + @mock.patch.object(time, 'sleep', autospec=True) + @mock.patch.object(ironic_api_client, 'LOG', autospec=True) + def test_do_lookup_node_locked(self, mock_log, mock_sleep): + response = FakeResponse(status_code=409, content={}) + self.api_client.session.request = mock.Mock() + self.api_client.session.request.return_value = response + self.assertEqual(0, self.api_client.lookup_lock_pause) + error = self.api_client._do_lookup(self.hardware_info, + node_uuid=None) + self.assertFalse(error) + mock_log.error.assert_has_calls([]) + self.assertEqual(1, mock_log.warning.call_count) + self.assertEqual(1, mock_sleep.call_count) + self.assertEqual(5, self.api_client.lookup_lock_pause) + error = self.api_client._do_lookup(self.hardware_info, + node_uuid=None) + self.assertEqual(10, self.api_client.lookup_lock_pause) + error = self.api_client._do_lookup(self.hardware_info, + node_uuid=None) + self.assertFalse(error) + self.assertEqual(30, self.api_client.lookup_lock_pause) + error = self.api_client._do_lookup(self.hardware_info, + node_uuid=None) + self.assertFalse(error) + self.assertEqual(30, self.api_client.lookup_lock_pause) + @mock.patch.object(ironic_api_client, 'LOG', autospec=True) def test_do_lookup_unknown_exception(self, mock_log): self.api_client.session.request = mock.Mock() diff --git a/releasenotes/notes/understand-node-conflict-is-locked-2ea21dd45abfe4f7.yaml b/releasenotes/notes/understand-node-conflict-is-locked-2ea21dd45abfe4f7.yaml new file mode 100644 index 000000000..c0e77964e --- /dev/null +++ b/releasenotes/notes/understand-node-conflict-is-locked-2ea21dd45abfe4f7.yaml @@ -0,0 +1,13 @@ +--- +fixes: + - | + Fixes, or at least lessens the case where a running Ironic agent can stack + up numerous lookup requests against an Ironic deployment when a node is + locked. In particular, this is beause the lookup also drives generation of + the agent token, which requires the conductor to allocate a worker, and + generate the token, and return the result to the API client. + Ironic's retry logic will now wait up to ``60`` seconds, and if an HTTP + Conflict (409) message is received, the agent will automatically pause + lookup operations for thirty seconds as opposed continue to attempt + lookups which could create more work for the Ironic deployment + needlessly. From 5a78bf286f11c4de095b936a14ca2e69896f6a78 Mon Sep 17 00:00:00 2001 From: Julia Kreger Date: Tue, 18 Jul 2023 11:01:22 -0700 Subject: [PATCH 07/11] preserve/handle config drives on 4k block devices When an underlying block device (or driver) only supports 4KB IO, this can cause some issues with aspects like using an ISO9660 filesystem which can only support a maximum of 2KB IO. The agent will now attempt to mount the filesystem *before* deleting the supplied file, and should that fail it will mount the configuration drive file from the ramdisk utilizing a loopback, and then extract the contents of the ramdisk into a newly created VFAT filesystem which supports 4KB block IO. Closes-Bug: #2028002 Change-Id: I336acb8e8eb5a02dde2f5e24c258e23797d200ee (cherry picked from commit b6c263a5dc34c10b50a480e3fdcf22784f2aa77c) --- ironic_python_agent/config.py | 5 + ironic_python_agent/partition_utils.py | 98 +++++++- .../tests/unit/test_partition_utils.py | 229 +++++++++++++++++- ...k-size-config-drives-4470828dd06d2600.yaml | 12 + 4 files changed, 340 insertions(+), 4 deletions(-) create mode 100644 releasenotes/notes/4k-block-size-config-drives-4470828dd06d2600.yaml diff --git a/ironic_python_agent/config.py b/ironic_python_agent/config.py index 9251a3e37..d81e67c43 100644 --- a/ironic_python_agent/config.py +++ b/ironic_python_agent/config.py @@ -326,6 +326,11 @@ 'cleaning from inadvertently destroying a running ' 'cluster which may be visible over a storage fabric ' 'such as FibreChannel.'), + cfg.BoolOpt('config_drive_rebuild', + default=False, + help='If the agent should rebuild the configuration drive ' + 'using a local filesystem, instead of letting Ironic ' + 'determine if this action is necessary.'), ] CONF.register_cli_opts(cli_opts) diff --git a/ironic_python_agent/partition_utils.py b/ironic_python_agent/partition_utils.py index cb8225fbc..ab933e03f 100644 --- a/ironic_python_agent/partition_utils.py +++ b/ironic_python_agent/partition_utils.py @@ -465,12 +465,27 @@ def create_config_drive_partition(node_uuid, device, configdrive): {'part': config_drive_part, 'node': node_uuid}) utils.execute('test', '-e', config_drive_part, attempts=15, delay_on_retry=True) - - disk_utils.dd(confdrive_file, config_drive_part) + if not CONF.config_drive_rebuild: + disk_utils.dd(confdrive_file, config_drive_part) + if not _does_config_drive_work(config_drive_part): + # If we have reached this point, we might have an + # invalid configuration drive, OR the block device + # layer doesn't support 2K block Logical IO (iso9660) + _try_build_fat32_config_drive(config_drive_part, + confdrive_file) + else: + LOG.info('Extracting configuration drive to write copy to disk.') + _try_build_fat32_config_drive(config_drive_part, confdrive_file) LOG.info("Configdrive for node %(node)s successfully " "copied onto partition %(part)s", {'node': node_uuid, 'part': config_drive_part}) + except exception.InstanceDeployFailure: + # Since we no longer have a final action on the decorator, we need + # to catch the failure, and still perform the cleanup. + if confdrive_file: + utils.unlink_without_raise(confdrive_file) + raise except (processutils.UnknownArgumentError, processutils.ProcessExecutionError, OSError) as e: msg = ('Failed to create config drive on disk %(disk)s ' @@ -478,13 +493,90 @@ def create_config_drive_partition(node_uuid, device, configdrive): {'disk': device, 'node': node_uuid, 'error': e}) LOG.error(msg) raise exception.InstanceDeployFailure(msg) - finally: # If the configdrive was requested make sure we delete the file # after copying the content to the partition + + finally: if confdrive_file: utils.unlink_without_raise(confdrive_file) +def _does_config_drive_work(config_drive_part): + """Attempts to mount the config drive to validate it works. + + :param config_drive_part: The partition to which the configuration drive + was written. + :returns: True if we were able to mount the configuration drive partition. + """ + temp_folder = tempfile.mkdtemp() + try: + # Why: If the filesystem is ISO9660 or vfat, and the logical sector + # size which is supported is *not* something which supports 512 bytes, + # i.e. a 4k Block size, then ISO9660 just will not work. Vfat also + # will not work because the logical size needs to match the logical + # size which is usable. If the underlying driver cannot use that size, + # then the filesystem will not work and cannot be updated because + # structurally it is incompaible with the block device driver. + utils.execute('mount', '-o', 'ro', '-t', 'auto', config_drive_part, + temp_folder) + utils.execute('umount', temp_folder) + except (processutils.ProcessExecutionError, OSError) as e: + LOG.error('Encountered issue attempting to validate the ' + 'supplied configuration drive. Error: %s', e) + return False + finally: + utils.unlink_without_raise(temp_folder) + + return True + + +def _try_build_fat32_config_drive(partition, confdrive_file): + conf_drive_temp = tempfile.mkdtemp() + try: + utils.execute('mount', '-o', 'loop,ro', '-t', 'auto', + confdrive_file, conf_drive_temp) + except (processutils.ProcessExecutionError, OSError) as e: + # Config drive is invalid, at least to our point of view. + # Bailing. + LOG.warning('We were unable to examine the configuration drive, ' + 'bypassing. Error: %s', e) + return + + new_drive_temp = tempfile.mkdtemp() + try: + # While creating a config drive file from scratch or on + # a loopback will likely result in a 512 byte sector size, + # the underlying fat filesystem utilities *automatically* + # check the device block sector sizing. This *will* break + # above 4k blocks, or at least might. Officially, 4k is the + # *maximum* in the FAT standard. See: + # https://github.com/dosfstools/dosfstools/blame/c483196dd46eab22abba756cef511d36f5f42070/src/mkfs.fat.c#L1987 + utils.mkfs(fs='vfat', path=partition, label='CONFIG-2') + utils.execute('mount', '-t', 'auto', partition, new_drive_temp) + # copytree, using copy2, copies everything in the source folder + # into the destination folder, so we should be good, and metadata + # is attempted to be preserved. + shutil.copytree(conf_drive_temp, new_drive_temp, dirs_exist_ok=True) + except (processutils.ProcessExecutionError, OSError) as e: + # We failed to make the filesystem :( + # This is a fairly hard error as we could not use the + # config drive, nor could we recover the state. + LOG.error('We were unable to make a new filesystem for the ' + 'configuration drive. Error: %s', e) + msg = ('A failure occured while attempting to format, copy, and ' + 're-create the configuration drive in a structure which ' + 'is compatible with the underlying hardware and Operating ' + 'System. Due to the nature of configuration drive, it could ' + 'have been incorrectly formatted. Operator investigation is ' + 'required. Error: {}'.format(str(e))) + raise exception.InstanceDeployFailure(msg) + finally: + utils.execute('umount', conf_drive_temp) + utils.execute('umount', new_drive_temp) + utils.unlink_without_raise(new_drive_temp) + utils.unlink_without_raise(conf_drive_temp) + + def _is_disk_larger_than_max_size(device, node_uuid): """Check if total disk size exceeds 2TB msdos limit diff --git a/ironic_python_agent/tests/unit/test_partition_utils.py b/ironic_python_agent/tests/unit/test_partition_utils.py index 4923642b7..b9893f1bd 100644 --- a/ironic_python_agent/tests/unit/test_partition_utils.py +++ b/ironic_python_agent/tests/unit/test_partition_utils.py @@ -20,6 +20,7 @@ from ironic_lib import exception from ironic_lib import utils from oslo_concurrency import processutils +from oslo_config import cfg import requests from ironic_python_agent import errors @@ -28,6 +29,9 @@ from ironic_python_agent.tests.unit import base +CONF = cfg.CONF + + @mock.patch.object(shutil, 'copyfileobj', autospec=True) @mock.patch.object(requests, 'get', autospec=True) class GetConfigdriveTestCase(base.IronicAgentTest): @@ -649,7 +653,10 @@ def test_create_partition_exists(self, mock_get_configdrive, self.config_part_label, self.node_uuid) self.assertFalse(mock_list_partitions.called) - self.assertFalse(mock_execute.called) + mock_execute.assert_has_calls([ + mock.call('mount', '-o', 'ro', '-t', 'auto', + '/dev/fake-part1', mock.ANY), + mock.call('umount', mock.ANY)]) self.assertFalse(mock_table_type.called) mock_dd.assert_called_with(configdrive_file, configdrive_part) mock_unlink.assert_called_with(configdrive_file) @@ -706,6 +713,135 @@ def test_create_partition_gpt(self, mock_get_configdrive, mock_dd.assert_called_with(configdrive_file, expected_part) mock_unlink.assert_called_with(configdrive_file) + @mock.patch('oslo_utils.uuidutils.generate_uuid', lambda: 'fake-uuid') + @mock.patch.object(partition_utils, '_try_build_fat32_config_drive', + autospec=True) + @mock.patch.object(partition_utils, '_does_config_drive_work', + autospec=True) + @mock.patch.object(utils, 'execute', autospec=True) + @mock.patch.object(utils, 'unlink_without_raise', + autospec=True) + @mock.patch.object(disk_utils, 'dd', + autospec=True) + @mock.patch.object(disk_utils, 'fix_gpt_partition', + autospec=True) + @mock.patch.object(disk_utils, 'get_partition_table_type', + autospec=True) + @mock.patch.object(partition_utils, 'get_partition', + autospec=True) + @mock.patch.object(partition_utils, 'get_labelled_partition', + autospec=True) + @mock.patch.object(partition_utils, 'get_configdrive', + autospec=True) + def test_create_partition_gpt_with_fallback( + self, mock_get_configdrive, + mock_get_labelled_partition, + mock_get_partition_by_uuid, + mock_table_type, + mock_fix_gpt_partition, + mock_dd, mock_unlink, mock_execute, + mock_config_drive_work, + mock_rebuild_config_drive): + config_url = 'http://1.2.3.4/cd' + configdrive_file = '/tmp/xyz' + configdrive_mb = 10 + + mock_get_configdrive.return_value = (configdrive_mb, configdrive_file) + mock_get_labelled_partition.return_value = None + + mock_table_type.return_value = 'gpt' + expected_part = '/dev/fake4' + mock_get_partition_by_uuid.return_value = expected_part + mock_config_drive_work.return_value = False + + partition_utils.create_config_drive_partition(self.node_uuid, self.dev, + config_url) + mock_execute.assert_has_calls([ + mock.call('sgdisk', '-n', '0:-64MB:0', '-u', '0:fake-uuid', + self.dev, run_as_root=True), + mock.call('sync'), + mock.call('udevadm', 'settle'), + mock.call('partprobe', self.dev, attempts=10, run_as_root=True), + mock.call('sgdisk', '-v', self.dev, run_as_root=True), + + mock.call('udevadm', 'settle'), + mock.call('test', '-e', expected_part, attempts=15, + delay_on_retry=True) + ]) + + mock_table_type.assert_called_with(self.dev) + mock_fix_gpt_partition.assert_called_with(self.dev, self.node_uuid) + mock_dd.assert_called_with(configdrive_file, expected_part) + mock_unlink.assert_called_with(configdrive_file) + mock_config_drive_work.assert_called_once_with(expected_part) + mock_rebuild_config_drive.assert_called_once_with(expected_part, + configdrive_file) + + @mock.patch('oslo_utils.uuidutils.generate_uuid', lambda: 'fake-uuid') + @mock.patch.object(partition_utils, '_try_build_fat32_config_drive', + autospec=True) + @mock.patch.object(partition_utils, '_does_config_drive_work', + autospec=True) + @mock.patch.object(utils, 'execute', autospec=True) + @mock.patch.object(utils, 'unlink_without_raise', + autospec=True) + @mock.patch.object(disk_utils, 'dd', + autospec=True) + @mock.patch.object(disk_utils, 'fix_gpt_partition', + autospec=True) + @mock.patch.object(disk_utils, 'get_partition_table_type', + autospec=True) + @mock.patch.object(partition_utils, 'get_partition', + autospec=True) + @mock.patch.object(partition_utils, 'get_labelled_partition', + autospec=True) + @mock.patch.object(partition_utils, 'get_configdrive', + autospec=True) + def test_create_partition_gpt_use_vfat( + self, mock_get_configdrive, + mock_get_labelled_partition, + mock_get_partition_by_uuid, + mock_table_type, + mock_fix_gpt_partition, + mock_dd, mock_unlink, mock_execute, + mock_config_drive_work, + mock_rebuild_config_drive): + config_url = 'http://1.2.3.4/cd' + configdrive_file = '/tmp/xyz' + configdrive_mb = 10 + + CONF.set_override('config_drive_rebuild', True) + mock_get_configdrive.return_value = (configdrive_mb, configdrive_file) + mock_get_labelled_partition.return_value = None + + mock_table_type.return_value = 'gpt' + expected_part = '/dev/fake4' + mock_get_partition_by_uuid.return_value = expected_part + mock_config_drive_work.return_value = True + + partition_utils.create_config_drive_partition(self.node_uuid, self.dev, + config_url) + mock_execute.assert_has_calls([ + mock.call('sgdisk', '-n', '0:-64MB:0', '-u', '0:fake-uuid', + self.dev, run_as_root=True), + mock.call('sync'), + mock.call('udevadm', 'settle'), + mock.call('partprobe', self.dev, attempts=10, run_as_root=True), + mock.call('sgdisk', '-v', self.dev, run_as_root=True), + + mock.call('udevadm', 'settle'), + mock.call('test', '-e', expected_part, attempts=15, + delay_on_retry=True) + ]) + + mock_table_type.assert_called_with(self.dev) + mock_fix_gpt_partition.assert_called_with(self.dev, self.node_uuid) + mock_dd.assert_not_called() + mock_unlink.assert_called_with(configdrive_file) + mock_config_drive_work.assert_not_called() + mock_rebuild_config_drive.assert_called_once_with(expected_part, + configdrive_file) + @mock.patch.object(disk_utils, 'count_mbr_partitions', autospec=True) @mock.patch.object(utils, 'execute', autospec=True) @mock.patch.object(partition_utils.LOG, 'warning', autospec=True) @@ -1288,3 +1424,94 @@ def test_label(self, mock_is_md_device, mock_execute): mock.call('lsblk', '-PbioKNAME,UUID,PARTUUID,TYPE,LABEL', self.fake_dev)] mock_execute.assert_has_calls(expected) + + +@mock.patch.object(utils, 'execute', autospec=True) +class TestConfigDriveTestRecovery(base.IronicAgentTest): + + fake_dev = '/dev/fake' + configdrive_file = '/tmp/config-drive' + + def test__does_config_drive_work(self, mock_execute): + self.assertTrue(partition_utils._does_config_drive_work(self.fake_dev)) + mock_execute.assert_has_calls([ + mock.call('mount', '-o', 'ro', '-t', 'auto', self.fake_dev, + mock.ANY), + mock.call('umount', mock.ANY)]) + + def test__does_config_drive_failed(self, mock_execute): + mock_execute.side_effect = processutils.ProcessExecutionError('boom') + self.assertFalse( + partition_utils._does_config_drive_work(self.fake_dev) + ) + mock_execute.assert_has_calls([ + mock.call('mount', '-o', 'ro', '-t', 'auto', self.fake_dev, + mock.ANY)]) + + @mock.patch.object(shutil, 'copytree', autospec=True) + @mock.patch.object(utils, 'mkfs', autospec=True) + def test__try_build_fat32_config_drive(self, + mock_mkfs, + mock_copy, + mock_execute): + partition_utils._try_build_fat32_config_drive(self.fake_dev, + self.configdrive_file) + mock_execute.assert_has_calls([ + mock.call('mount', '-o', 'loop,ro', '-t', 'auto', + self.configdrive_file, mock.ANY), + mock.call('mount', '-t', 'auto', self.fake_dev, mock.ANY), + mock.call('umount', mock.ANY), + mock.call('umount', mock.ANY), + ]) + mock_mkfs.assert_called_once_with(fs='vfat', path=self.fake_dev, + label='CONFIG-2') + # Validate we called copy as we expect, both source and destination + # are temporary folders. + mock_copy.assert_called_once_with(mock.ANY, mock.ANY, + dirs_exist_ok=True) + + @mock.patch.object(shutil, 'copytree', autospec=True) + @mock.patch.object(utils, 'mkfs', autospec=True) + def test__try_build_fat32_config_drive_graceful_fail( + self, + mock_mkfs, + mock_copy, + mock_execute): + mock_execute.side_effect = processutils.ProcessExecutionError('boom') + self.assertIsNone( + partition_utils._try_build_fat32_config_drive( + self.fake_dev, + self.configdrive_file) + ) + mock_execute.assert_called_once_with( + 'mount', '-o', 'loop,ro', '-t', 'auto', + self.configdrive_file, mock.ANY) + mock_mkfs.assert_not_called() + # Validate we called copy as we expect, both source and destination + # are temporary folders. + mock_copy.assert_not_called() + + @mock.patch.object(shutil, 'copytree', autospec=True) + @mock.patch.object(utils, 'mkfs', autospec=True) + def test__try_build_fat32_config_drive_fails_once_invalid( + self, + mock_mkfs, + mock_copy, + mock_execute): + mock_mkfs.side_effect = processutils.ProcessExecutionError('boom') + self.assertRaisesRegex( + exception.InstanceDeployFailure, + 'A failure occured while attempting to format.*', + partition_utils._try_build_fat32_config_drive, + self.fake_dev, + self.configdrive_file) + mock_execute.assert_has_calls([ + mock.call('mount', '-o', 'loop,ro', '-t', 'auto', + self.configdrive_file, mock.ANY), + mock.call('umount', mock.ANY), + mock.call('umount', mock.ANY), + ]) + + mock_mkfs.assert_called_once_with(fs='vfat', path=self.fake_dev, + label='CONFIG-2') + mock_copy.assert_not_called() diff --git a/releasenotes/notes/4k-block-size-config-drives-4470828dd06d2600.yaml b/releasenotes/notes/4k-block-size-config-drives-4470828dd06d2600.yaml new file mode 100644 index 000000000..7e65348c6 --- /dev/null +++ b/releasenotes/notes/4k-block-size-config-drives-4470828dd06d2600.yaml @@ -0,0 +1,12 @@ +--- +fixes: + - | + Fixes a failure case where a deployed instance may be unable to access + the configuration drive post-deployment. This can occur when block + devices only support 4KB IO interactions. When 4KB block IO sizes + are in use, the ISO9660 filesystem driver in Linux cannot be used + as it is modeled around a 2KB block. We now attempt to verify, and + rebuild the configuration drive on a FAT filesystem when we cannot + mount the supplied configuration drive. Operators can force the agent + to write configuration drives using the FAT filesystem using the + ``[DEFAULT]config_drive_rebuild`` option. From 256dfe467c9da5b2ce78ea998e4008d9f5fd5300 Mon Sep 17 00:00:00 2001 From: Arne Wiebalck Date: Wed, 16 Aug 2023 15:51:59 +0200 Subject: [PATCH 08/11] Conditional creation of RAIDed ESP for UEFI Software RAID Rebuilding an instance on a RAIDed ESPs will fail due to sgdisk running against an non-clean disk and bailing out. Check if there is a RAIDed ESP already and skip creation if it exists. Change-Id: I13617ae77515a9d34bc4bb3caf9fae73d5e4e578 (cherry picked from commit 286d66709a1b2f2068360db641079c67197b21b6) --- ironic_python_agent/raid_utils.py | 112 +++++++++++------- .../tests/unit/samples/hardware_samples.py | 30 +++++ .../tests/unit/test_raid_utils.py | 26 +++- .../rebuild_on_esp_raid-33f359bdf5ccaa09.yaml | 5 + 4 files changed, 126 insertions(+), 47 deletions(-) create mode 100644 releasenotes/notes/rebuild_on_esp_raid-33f359bdf5ccaa09.yaml diff --git a/ironic_python_agent/raid_utils.py b/ironic_python_agent/raid_utils.py index 84c6941fd..27b0bb5b2 100644 --- a/ironic_python_agent/raid_utils.py +++ b/ironic_python_agent/raid_utils.py @@ -12,6 +12,7 @@ import copy import re +import shlex from ironic_lib import disk_utils from ironic_lib import utils as il_utils @@ -342,50 +343,58 @@ def prepare_boot_partitions_for_softraid(device, holders, efi_part, if efi_part: efi_part = '{}p{}'.format(device, efi_part['number']) - LOG.info("Creating EFI partitions on software RAID holder disks") - # We know that we kept this space when configuring raid,see - # hardware.GenericHardwareManager.create_configuration. - # We could also directly get the EFI partition size. - partsize_mib = ESP_SIZE_MIB - partlabel_prefix = 'uefi-holder-' - efi_partitions = [] - for number, holder in enumerate(holders): - # NOTE: see utils.get_partition_table_type_from_specs - # for uefi we know that we have setup a gpt partition table, - # sgdisk can be used to edit table, more user friendly - # for alignment and relative offsets - partlabel = '{}{}'.format(partlabel_prefix, number) - out, _u = utils.execute('sgdisk', '-F', holder) - start_sector = '{}s'.format(out.splitlines()[-1].strip()) - out, _u = utils.execute( - 'sgdisk', '-n', '0:{}:+{}MiB'.format(start_sector, - partsize_mib), - '-t', '0:ef00', '-c', '0:{}'.format(partlabel), holder) - - # Refresh part table - utils.execute("partprobe") - utils.execute("blkid") - - target_part, _u = utils.execute( - "blkid", "-l", "-t", "PARTLABEL={}".format(partlabel), holder) - - target_part = target_part.splitlines()[-1].split(':', 1)[0] - efi_partitions.append(target_part) - - LOG.debug("EFI partition %s created on holder disk %s", - target_part, holder) - - # RAID the ESPs, metadata=1.0 is mandatory to be able to boot - md_device = get_next_free_raid_device() - LOG.debug("Creating md device %(md_device)s for the ESPs " - "on %(efi_partitions)s", - {'md_device': md_device, 'efi_partitions': efi_partitions}) - utils.execute('mdadm', '--create', md_device, '--force', - '--run', '--metadata=1.0', '--level', '1', - '--name', 'esp', '--raid-devices', len(efi_partitions), - *efi_partitions) - - disk_utils.trigger_device_rescan(md_device) + # check if we have a RAIDed ESP already + md_device = find_esp_raid() + if md_device: + LOG.info("Found RAIDed ESP %s, skip creation", md_device) + else: + LOG.info("Creating EFI partitions on software RAID holder disks") + # We know that we kept this space when configuring raid,see + # hardware.GenericHardwareManager.create_configuration. + # We could also directly get the EFI partition size. + partsize_mib = ESP_SIZE_MIB + partlabel_prefix = 'uefi-holder-' + efi_partitions = [] + for number, holder in enumerate(holders): + # NOTE: see utils.get_partition_table_type_from_specs + # for uefi we know that we have setup a gpt partition table, + # sgdisk can be used to edit table, more user friendly + # for alignment and relative offsets + partlabel = '{}{}'.format(partlabel_prefix, number) + out, _u = utils.execute('sgdisk', '-F', holder) + start_sector = '{}s'.format(out.splitlines()[-1].strip()) + out, _u = utils.execute( + 'sgdisk', '-n', '0:{}:+{}MiB'.format(start_sector, + partsize_mib), + '-t', '0:ef00', '-c', '0:{}'.format(partlabel), holder) + + # Refresh part table + utils.execute("partprobe") + utils.execute("blkid") + + target_part, _u = utils.execute( + "blkid", "-l", "-t", "PARTLABEL={}".format(partlabel), + holder) + + target_part = target_part.splitlines()[-1].split(':', 1)[0] + efi_partitions.append(target_part) + + LOG.debug("EFI partition %s created on holder disk %s", + target_part, holder) + + # RAID the ESPs, metadata=1.0 is mandatory to be able to boot + md_device = get_next_free_raid_device() + LOG.debug("Creating md device %(md_device)s for the ESPs " + "on %(efi_partitions)s", + {'md_device': md_device, + 'efi_partitions': efi_partitions}) + utils.execute('mdadm', '--create', md_device, '--force', + '--run', '--metadata=1.0', '--level', '1', + '--name', 'esp', '--raid-devices', + len(efi_partitions), + *efi_partitions) + + disk_utils.trigger_device_rescan(md_device) if efi_part: # Blockdev copy the source ESP and erase it @@ -420,3 +429,18 @@ def prepare_boot_partitions_for_softraid(device, holders, efi_part, # disk, as in virtual disk, where to load the data from. # Since there is a structural difference, this means it will # fail. + + +def find_esp_raid(): + """Find the ESP md device in case of a rebuild.""" + + # find devices of type 'RAID1' and fstype 'VFAT' + lsblk = utils.execute('lsblk', '-PbioNAME,TYPE,FSTYPE') + report = lsblk[0] + for line in report.split('\n'): + dev = {} + vals = shlex.split(line) + for key, val in (v.split('=', 1) for v in vals): + dev[key] = val.strip() + if dev.get('TYPE') == 'raid1' and dev.get('FSTYPE') == 'vfat': + return '/dev/' + dev.get('NAME') diff --git a/ironic_python_agent/tests/unit/samples/hardware_samples.py b/ironic_python_agent/tests/unit/samples/hardware_samples.py index c9e597a94..b8b80585d 100644 --- a/ironic_python_agent/tests/unit/samples/hardware_samples.py +++ b/ironic_python_agent/tests/unit/samples/hardware_samples.py @@ -1667,3 +1667,33 @@ ' `-+- policy=\'service-time 0\' prio=1 status=active\n' ' `- 0:0:0:0 device s 8:0 active ready running\n' ) + +LSBLK_OUPUT = (""" +NAME="sda" TYPE="disk" FSTYPE="" +NAME="sdb" TYPE="disk" FSTYPE="" +""") + +LSBLK_OUPUT_ESP_RAID = (""" +NAME="sda" TYPE="disk" FSTYPE="" +NAME="sda1" TYPE="part" FSTYPE="linux_raid_member" +NAME="md127" TYPE="raid1" FSTYPE="" +NAME="md127p1" TYPE="md" FSTYPE="xfs" +NAME="md127p2" TYPE="md" FSTYPE="iso9660" +NAME="md127p14" TYPE="md" FSTYPE="" +NAME="md127p15" TYPE="md" FSTYPE="" +NAME="sda2" TYPE="part" FSTYPE="linux_raid_member" +NAME="md126" TYPE="raid0" FSTYPE="" +NAME="sda3" TYPE="part" FSTYPE="linux_raid_member" +NAME="md125" TYPE="raid1" FSTYPE="vfat" +NAME="sdb" TYPE="disk" FSTYPE="" +NAME="sdb1" TYPE="part" FSTYPE="linux_raid_member" +NAME="md127" TYPE="raid1" FSTYPE="" +NAME="md127p1" TYPE="md" FSTYPE="xfs" +NAME="md127p2" TYPE="md" FSTYPE="iso9660" +NAME="md127p14" TYPE="md" FSTYPE="" +NAME="md127p15" TYPE="md" FSTYPE="" +NAME="sdb2" TYPE="part" FSTYPE="linux_raid_member" +NAME="md126" TYPE="raid0" FSTYPE="" +NAME="sdb3" TYPE="part" FSTYPE="linux_raid_member" +NAME="md125" TYPE="raid1" FSTYPE="vfat" +""") diff --git a/ironic_python_agent/tests/unit/test_raid_utils.py b/ironic_python_agent/tests/unit/test_raid_utils.py index a82027c8a..f68796056 100644 --- a/ironic_python_agent/tests/unit/test_raid_utils.py +++ b/ironic_python_agent/tests/unit/test_raid_utils.py @@ -153,6 +153,7 @@ def test_get_volume_name_of_raid_device_invalid(self, mock_execute): volume_name = raid_utils.get_volume_name_of_raid_device('/dev/md0') self.assertIsNone(volume_name) + @mock.patch.object(raid_utils, 'find_esp_raid', autospec=True) @mock.patch.object(disk_utils, 'trigger_device_rescan', autospec=True) @mock.patch.object(raid_utils, 'get_next_free_raid_device', autospec=True, return_value='/dev/md42') @@ -161,7 +162,7 @@ def test_get_volume_name_of_raid_device_invalid(self, mock_execute): @mock.patch.object(disk_utils, 'find_efi_partition', autospec=True) def test_prepare_boot_partitions_for_softraid_uefi_gpt( self, mock_efi_part, mock_execute, mock_dispatch, - mock_free_raid_device, mock_rescan): + mock_free_raid_device, mock_rescan, mock_find_esp): mock_efi_part.return_value = {'number': '12'} mock_execute.side_effect = [ ('451', None), # sgdisk -F @@ -178,6 +179,7 @@ def test_prepare_boot_partitions_for_softraid_uefi_gpt( (None, None), # cp (None, None), # wipefs ] + mock_find_esp.return_value = None efi_part = raid_utils.prepare_boot_partitions_for_softraid( '/dev/md0', ['/dev/sda', '/dev/sdb'], None, @@ -209,6 +211,7 @@ def test_prepare_boot_partitions_for_softraid_uefi_gpt( self.assertEqual(efi_part, '/dev/md42') mock_rescan.assert_called_once_with('/dev/md42') + @mock.patch.object(raid_utils, 'find_esp_raid', autospec=True) @mock.patch.object(disk_utils, 'trigger_device_rescan', autospec=True) @mock.patch.object(raid_utils, 'get_next_free_raid_device', autospec=True, return_value='/dev/md42') @@ -218,7 +221,7 @@ def test_prepare_boot_partitions_for_softraid_uefi_gpt( @mock.patch.object(ilib_utils, 'mkfs', autospec=True) def test_prepare_boot_partitions_for_softraid_uefi_gpt_esp_not_found( self, mock_mkfs, mock_efi_part, mock_execute, mock_dispatch, - mock_free_raid_device, mock_rescan): + mock_free_raid_device, mock_rescan, mock_find_esp): mock_efi_part.return_value = None mock_execute.side_effect = [ ('451', None), # sgdisk -F @@ -233,6 +236,7 @@ def test_prepare_boot_partitions_for_softraid_uefi_gpt_esp_not_found( ('/dev/sdb14: whatever', None), # blkid (None, None), # mdadm ] + mock_find_esp.return_value = None efi_part = raid_utils.prepare_boot_partitions_for_softraid( '/dev/md0', ['/dev/sda', '/dev/sdb'], None, @@ -262,6 +266,7 @@ def test_prepare_boot_partitions_for_softraid_uefi_gpt_esp_not_found( self.assertEqual(efi_part, '/dev/md42') mock_rescan.assert_called_once_with('/dev/md42') + @mock.patch.object(raid_utils, 'find_esp_raid', autospec=True) @mock.patch.object(disk_utils, 'trigger_device_rescan', autospec=True) @mock.patch.object(raid_utils, 'get_next_free_raid_device', autospec=True, return_value='/dev/md42') @@ -269,7 +274,7 @@ def test_prepare_boot_partitions_for_softraid_uefi_gpt_esp_not_found( @mock.patch.object(ilib_utils, 'execute', autospec=True) def test_prepare_boot_partitions_for_softraid_uefi_gpt_efi_provided( self, mock_execute, mock_dispatch, mock_free_raid_device, - mock_rescan): + mock_rescan, mock_find_esp): mock_execute.side_effect = [ ('451', None), # sgdisk -F (None, None), # sgdisk create part @@ -285,6 +290,7 @@ def test_prepare_boot_partitions_for_softraid_uefi_gpt_efi_provided( (None, None), # cp (None, None), # wipefs ] + mock_find_esp.return_value = None efi_part = raid_utils.prepare_boot_partitions_for_softraid( '/dev/md0', ['/dev/sda', '/dev/sdb'], '/dev/md0p15', @@ -389,3 +395,17 @@ def test_no_device(self, mock_dispatch): ] self.assertRaises(errors.SoftwareRAIDError, raid_utils.get_next_free_raid_device) + + +@mock.patch.object(utils, 'execute', autospec=True) +class TestFindESPRAID(base.IronicAgentTest): + + def test_no_esp_raid(self, mock_execute): + mock_execute.side_effect = [(hws.LSBLK_OUPUT, '')] + result = raid_utils.find_esp_raid() + self.assertIsNone(result) + + def test_esp_raid(self, mock_execute): + mock_execute.side_effect = [(hws.LSBLK_OUPUT_ESP_RAID, '')] + result = raid_utils.find_esp_raid() + self.assertEqual('/dev/md125', result) diff --git a/releasenotes/notes/rebuild_on_esp_raid-33f359bdf5ccaa09.yaml b/releasenotes/notes/rebuild_on_esp_raid-33f359bdf5ccaa09.yaml new file mode 100644 index 000000000..09e8bf4d1 --- /dev/null +++ b/releasenotes/notes/rebuild_on_esp_raid-33f359bdf5ccaa09.yaml @@ -0,0 +1,5 @@ +--- +fixes: + - | + Fixes an issue with rebuilding instances on Software RAID with + RAIDed ESP partitions. From f2712779aa7fa0a142d7b518db9350b64ca1f203 Mon Sep 17 00:00:00 2001 From: OpenStack Release Bot Date: Mon, 6 May 2024 06:22:57 +0000 Subject: [PATCH 09/11] Update .gitreview for unmaintained/zed Change-Id: I75ffadbcb790c47eab8ac74fa8f96c7b189cf6b6 --- .gitreview | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitreview b/.gitreview index 5babe14a0..0ee69f4b9 100644 --- a/.gitreview +++ b/.gitreview @@ -2,4 +2,4 @@ host=review.opendev.org port=29418 project=openstack/ironic-python-agent.git -defaultbranch=stable/zed +defaultbranch=unmaintained/zed From a1b9170520696cf5d14972a31781e4770588cb41 Mon Sep 17 00:00:00 2001 From: Jay Faulkner Date: Tue, 20 Aug 2024 15:09:33 -0700 Subject: [PATCH 10/11] Warn about CVE-2024-44082 Unmaintained Ironic-Python-Agent branches will not be patched against CVE-2024-44082. This patch updates the release notes and readme instructing deployers how to mitigate their risk using the provided Ironic conductor patches. Related-Bug: 2071740 Change-Id: Ie4aeef4af01ead5c18b359a22ab488de0c35248a --- README.rst | 5 +++++ ...44082-image-security-warning-37ac1ac7647a806a.yaml | 11 +++++++++++ 2 files changed, 16 insertions(+) create mode 100644 releasenotes/notes/cve-2024-44082-image-security-warning-37ac1ac7647a806a.yaml diff --git a/README.rst b/README.rst index 4494ed5a9..42e65dcd8 100644 --- a/README.rst +++ b/README.rst @@ -11,6 +11,11 @@ Team and repository tags Overview ======== +*WARNING:* The Ironic-Python-Agent version in this branch is vulnerable to +CVE-2024-44082. Do not run this in production unless using a patched +conductor with ``[conductor]/conductor_always_validate_images`` set to +``True``. + An agent for controlling and deploying Ironic controlled baremetal nodes. The ironic-python-agent works with the agent driver in Ironic to provision diff --git a/releasenotes/notes/cve-2024-44082-image-security-warning-37ac1ac7647a806a.yaml b/releasenotes/notes/cve-2024-44082-image-security-warning-37ac1ac7647a806a.yaml new file mode 100644 index 000000000..92509277a --- /dev/null +++ b/releasenotes/notes/cve-2024-44082-image-security-warning-37ac1ac7647a806a.yaml @@ -0,0 +1,11 @@ +--- +security: + - | + Ironic-Python-Agent versions prior to the 2023.1 release are vulnerable to + CVE-2024-44082, tracked in + `bug 2071740 _`. Deployers of + Ironic versions Zed or older must apply CVE-2024-44082 fixes to their + Ironic environment and leave (default for all releases Zed and older) + ``[conductor]/conductor_always_validates_images`` set to ``True``. This + ensures the conductor will security check the image because + Ironic-Python-Agent will not. From 8740cd5738548853b410137baf6e5b07fd53e868 Mon Sep 17 00:00:00 2001 From: Steve Baker Date: Fri, 18 Oct 2024 11:51:25 +1300 Subject: [PATCH 11/11] Fix CI issues This is a combination of 2 commits, squashed together in order to make all jobs pass. This is the 1st commit message: Remove non RE2 job config This causees a linting failure, and the equivalent of this rule was removed later in I64909aa932635b729cc85717dc241ae31798b558 Original-Change-Id: Id8e1a0901090f062ff36101f07acafe01a98a67b (cherry picked from commit b39d9d67a64440bfbc41e571736cc6016f8f7cf8) (cherry picked from commit a488be6e21b036e13ea567ed334707ad09901d88) This is the commit message #2: ci: Pin setuptools to a range that still ships Recent setuptools releases (82.0.0 and later) have removed the ``pkg_resources`` module. Our docs environment still depends on ``pkg_resources`` transitively via ``openstackdocstheme/pbr``, causing import error when run under newer environments. Modeled off of CID's ironic commit. NOTE(elod.illes): ironic-python-agent-builder's zed branch moved to EOL so it has to be used from zed-eol tag for the jobs. Furthermore, sushy-tools is cycle independent project. in Zed release 0.21.0 version was officially used. metalsmith jobs used centos8 which is not available anymore in zuul, hence this patch removes them from the gate. Additionally, ipa-tempest-bios-ipmi-direct-src job and ipa-tempest-uefi-redfish-vmedia-src job are intermittently failing, which makes the gate unstable and the error seems to be environment related. Setting them as non-voting to do not waste CI resource. Signed-off-by: Dr. Jens Harbott Change-Id: Ie69c2398ff2d4f807cca40df2cca0f45298a133f Signed-off-by: Clif Houck (cherry picked from commit 0474f3006029a240801710616b9e7dbd81819266) Signed-off-by: Elod Illes --- tox.ini | 4 ++++ zuul.d/ironic-python-agent-jobs.yaml | 16 +++++++++++----- zuul.d/project.yaml | 15 +++++++-------- 3 files changed, 22 insertions(+), 13 deletions(-) diff --git a/tox.ini b/tox.ini index 3a4ad1f61..b76573f9a 100644 --- a/tox.ini +++ b/tox.ini @@ -16,6 +16,7 @@ deps = -c{env:TOX_CONSTRAINTS_FILE:https://releases.openstack.org/constraints/upper/zed} -r{toxinidir}/requirements.txt -r{toxinidir}/test-requirements.txt + setuptools>=64.0.0,<82.0.0 # MIT commands = stestr run {posargs} passenv = http_proxy @@ -77,6 +78,7 @@ deps = -c{env:TOX_CONSTRAINTS_FILE:https://releases.openstack.org/constraints/upper/zed} -r{toxinidir}/requirements.txt -r{toxinidir}/doc/requirements.txt + setuptools>=64.0.0,<82.0.0 # MIT commands = sphinx-build -b html doc/source doc/build/html @@ -93,6 +95,7 @@ commands = deps = -c{env:TOX_CONSTRAINTS_FILE:https://releases.openstack.org/constraints/upper/zed} -r{toxinidir}/doc/requirements.txt + setuptools>=64.0.0,<82.0.0 # MIT commands = sphinx-build -a -E -W -d releasenotes/build/doctrees -b html releasenotes/source releasenotes/build/html [flake8] @@ -131,6 +134,7 @@ commands = deps = -c{env:TOX_CONSTRAINTS_FILE:https://releases.openstack.org/constraints/upper/zed} -r{toxinidir}/test-requirements.txt + setuptools>=64.0.0,<82.0.0 # MIT commands = bandit -r ironic_python_agent -x tests -n5 -ll -c tools/bandit.yml [testenv:examples] diff --git a/zuul.d/ironic-python-agent-jobs.yaml b/zuul.d/ironic-python-agent-jobs.yaml index 3e06c8c06..3f5c6068a 100644 --- a/zuul.d/ironic-python-agent-jobs.yaml +++ b/zuul.d/ironic-python-agent-jobs.yaml @@ -15,6 +15,8 @@ - ^tox.ini$ required-projects: - openstack/ironic-lib + - name: openstack/ironic-python-agent-builder + override-checkout: zed-eol vars: # The default is 1GB, we need a little more to prevent OOMs killing the jobs configure_swap_size: 8192 @@ -47,7 +49,8 @@ name: ipa-tempest-uefi-redfish-vmedia-src parent: ironic-ipa-base required-projects: - - opendev.org/openstack/sushy-tools + - name: opendev.org/openstack/sushy-tools + override-checkout: 0.21.0 vars: devstack_services: s-account: True @@ -105,7 +108,6 @@ - ^ironic_python_agent/tests/.*$ - ^releasenotes/.*$ - ^setup.cfg$ - - ^tools/(?!bandit.yml).*$ - ^tox.ini$ - job: @@ -121,7 +123,6 @@ - ^ironic_python_agent/tests/.*$ - ^releasenotes/.*$ - ^setup.cfg$ - - ^tools/(?!bandit.yml).*$ - ^tox.ini$ # used by ironic-python-agent-builder @@ -129,6 +130,9 @@ name: ironic-standalone-ipa-src parent: ironic-standalone description: Test ironic standalone with IPA from source + required-projects: + - name: openstack/ironic-python-agent-builder + override-checkout: zed-eol vars: devstack_localrc: IRONIC_BUILD_DEPLOY_RAMDISK: True @@ -141,7 +145,8 @@ parent: metalsmith-integration-glance-centos8-uefi required-projects: - openstack/ironic-python-agent - - openstack/ironic-python-agent-builder + - name: openstack/ironic-python-agent-builder + override-checkout: zed-eol - openstack/ironic-lib vars: devstack_localrc: @@ -154,7 +159,8 @@ parent: metalsmith-integration-glance-centos8-legacy required-projects: - openstack/ironic-python-agent - - openstack/ironic-python-agent-builder + - name: openstack/ironic-python-agent-builder + override-checkout: zed-eol - openstack/ironic-lib vars: devstack_localrc: diff --git a/zuul.d/project.yaml b/zuul.d/project.yaml index a4f9ee05f..f17f28ddb 100644 --- a/zuul.d/project.yaml +++ b/zuul.d/project.yaml @@ -12,10 +12,13 @@ - ipa-tox-examples # NOTE(iurygregory) Only run this two jobs since we are testing # wholedisk + partition on tempest - - ipa-tempest-bios-ipmi-direct-src - - ipa-tempest-uefi-redfish-vmedia-src - - metalsmith-integration-ipa-src-uefi - - metalsmith-integration-ipa-src-legacy + # NOTE(elod.illes) The below two jobs are intermittently failing, + # which makes the gate unstable and the error seems to be environment + # related. Setting them as non-voting to do not waste CI resource. + - ipa-tempest-bios-ipmi-direct-src: + voting: false + - ipa-tempest-uefi-redfish-vmedia-src: + voting: false - ironic-standalone-ipa-src # NOTE(dtantsur): non-voting because IPA source code is very unlikely # to break them. They rather serve as a canary for broken POST jobs. @@ -32,10 +35,6 @@ jobs: - openstack-tox-functional - ipa-tox-examples - - ipa-tempest-bios-ipmi-direct-src - - ipa-tempest-uefi-redfish-vmedia-src - - metalsmith-integration-ipa-src-uefi - - metalsmith-integration-ipa-src-legacy - ironic-standalone-ipa-src post: jobs: