Address review feedback for ipxe boot file fix

Address feedback in review Id545d6cf93227cf1fc2ff0c05dbdceb8fb6aa5b9
as all requested changes were basically limited to tests.

Partial-bug: #1559691
Change-Id: Ic414581c145d0e4b94163c9bd2dd4ae160196b28
This commit is contained in:
Julia Kreger 2017-07-25 03:59:44 +00:00 committed by Dmitry Tantsur
parent b05a6a64b1
commit fb5def4ebc
3 changed files with 30 additions and 35 deletions

View File

@ -451,7 +451,8 @@ class AgentDeploy(AgentDeployMixin, base.DeployInterface):
task.driver.storage.attach_volumes(task)
if not task.driver.storage.should_write_image(task):
# We have nothing else to do as this is handled in the
# backend storage system.
# backend storage system, and we can return to the caller
# as we do not need to boot the agent to deploy.
return
if node.provision_state == states.ACTIVE:
# Call is due to conductor takeover

View File

@ -532,16 +532,15 @@ class TestPXEUtils(db_base.DbTestCase):
rmtree_mock.assert_called_once_with(
os.path.join(CONF.pxe.tftp_root, self.node.uuid))
@mock.patch.object(os.path, 'isfile', autospec=True)
@mock.patch.object(os.path, 'isfile', lambda path: False)
@mock.patch('ironic.common.utils.file_has_content', autospec=True)
@mock.patch('ironic.common.utils.write_to_file', autospec=True)
@mock.patch('ironic.common.utils.render_template', autospec=True)
def test_create_ipxe_boot_script(self, render_mock, write_mock,
cmp_mock, isfile_mock):
isfile_mock.return_value = False
file_has_content_mock):
render_mock.return_value = 'foo'
self.assertFalse(cmp_mock.called)
pxe_utils.create_ipxe_boot_script()
self.assertFalse(file_has_content_mock.called)
write_mock.assert_called_once_with(
os.path.join(CONF.deploy.http_root,
os.path.basename(CONF.pxe.ipxe_boot_script)),
@ -550,17 +549,19 @@ class TestPXEUtils(db_base.DbTestCase):
CONF.pxe.ipxe_boot_script,
{'ipxe_for_mac_uri': 'pxelinux.cfg/'})
@mock.patch.object(os.path, 'isfile', autospec=True)
@mock.patch.object(os.path, 'isfile', lambda path: True)
@mock.patch('ironic.common.utils.file_has_content', autospec=True)
@mock.patch('ironic.common.utils.write_to_file', autospec=True)
@mock.patch('ironic.common.utils.render_template', autospec=True)
def test_create_ipxe_boot_script_copy_file_different(
self, render_mock, write_mock, cmp_mock, isfile_mock):
isfile_mock.return_value = True
cmp_mock.return_value = False
self, render_mock, write_mock, file_has_content_mock):
file_has_content_mock.return_value = False
render_mock.return_value = 'foo'
self.assertFalse(cmp_mock.called)
pxe_utils.create_ipxe_boot_script()
file_has_content_mock.assert_called_once_with(
os.path.join(CONF.deploy.http_root,
os.path.basename(CONF.pxe.ipxe_boot_script)),
'foo')
write_mock.assert_called_once_with(
os.path.join(CONF.deploy.http_root,
os.path.basename(CONF.pxe.ipxe_boot_script)),
@ -569,15 +570,14 @@ class TestPXEUtils(db_base.DbTestCase):
CONF.pxe.ipxe_boot_script,
{'ipxe_for_mac_uri': 'pxelinux.cfg/'})
@mock.patch.object(os.path, 'isfile', autospec=True)
@mock.patch.object(os.path, 'isfile', lambda path: True)
@mock.patch('ironic.common.utils.file_has_content', autospec=True)
@mock.patch('ironic.common.utils.write_to_file', autospec=True)
@mock.patch('ironic.common.utils.render_template', autospec=True)
def test_create_ipxe_boot_script_already_exists(self, render_mock,
write_mock, cmp_mock,
isfile_mock):
isfile_mock.return_value = True
cmp_mock.return_value = True
write_mock,
file_has_content_mock):
file_has_content_mock.return_value = True
pxe_utils.create_ipxe_boot_script()
self.assertFalse(write_mock.called)

View File

@ -31,6 +31,7 @@ from ironic.common import exception
from ironic.common.glance_service import base_image_service
from ironic.common import pxe_utils
from ironic.common import states
from ironic.common import utils as common_utils
from ironic.conductor import task_manager
from ironic.conductor import utils as manager_utils
from ironic.drivers.modules import agent_base_vendor
@ -880,18 +881,16 @@ class PXEBootTestCase(db_base.DbTestCase):
self.node.save()
self._test_prepare_ramdisk(uefi=True)
@mock.patch.object(os.path, 'isfile', autospec=True)
@mock.patch('ironic.common.utils.file_has_content', autospec=True)
@mock.patch.object(os.path, 'isfile', lambda path: True)
@mock.patch.object(common_utils, 'file_has_content', lambda *args: False)
@mock.patch('ironic.common.utils.write_to_file', autospec=True)
@mock.patch('ironic.common.utils.render_template', autospec=True)
def test_prepare_ramdisk_ipxe_with_copy_file_different(
self, render_mock, write_mock, cmp_mock, isfile_mock):
self, render_mock, write_mock):
self.node.provision_state = states.DEPLOYING
self.node.save()
self.config(group='pxe', ipxe_enabled=True)
self.config(group='deploy', http_url='http://myserver')
isfile_mock.return_value = True
cmp_mock.return_value = False
render_mock.return_value = 'foo'
self._test_prepare_ramdisk()
write_mock.assert_called_once_with(
@ -903,20 +902,19 @@ class PXEBootTestCase(db_base.DbTestCase):
CONF.pxe.ipxe_boot_script,
{'ipxe_for_mac_uri': 'pxelinux.cfg/'})
@mock.patch.object(os.path, 'isfile', autospec=True)
@mock.patch.object(os.path, 'isfile', lambda path: False)
@mock.patch('ironic.common.utils.file_has_content', autospec=True)
@mock.patch('ironic.common.utils.write_to_file', autospec=True)
@mock.patch('ironic.common.utils.render_template', autospec=True)
def test_prepare_ramdisk_ipxe_with_copy_no_file(
self, render_mock, write_mock, cmp_mock, isfile_mock):
self, render_mock, write_mock, file_has_content_mock):
self.node.provision_state = states.DEPLOYING
self.node.save()
self.config(group='pxe', ipxe_enabled=True)
self.config(group='deploy', http_url='http://myserver')
isfile_mock.return_value = False
render_mock.return_value = 'foo'
self._test_prepare_ramdisk()
self.assertFalse(cmp_mock.called)
self.assertFalse(file_has_content_mock.called)
write_mock.assert_called_once_with(
os.path.join(
CONF.deploy.http_root,
@ -926,30 +924,27 @@ class PXEBootTestCase(db_base.DbTestCase):
CONF.pxe.ipxe_boot_script,
{'ipxe_for_mac_uri': 'pxelinux.cfg/'})
@mock.patch.object(os.path, 'isfile', autospec=True)
@mock.patch('ironic.common.utils.file_has_content', autospec=True)
@mock.patch.object(os.path, 'isfile', lambda path: True)
@mock.patch.object(common_utils, 'file_has_content', lambda *args: True)
@mock.patch('ironic.common.utils.write_to_file', autospec=True)
@mock.patch('ironic.common.utils.render_template', autospec=True)
def test_prepare_ramdisk_ipxe_without_copy(
self, render_mock, write_mock, cmp_mock, isfile_mock):
self, render_mock, write_mock):
self.node.provision_state = states.DEPLOYING
self.node.save()
self.config(group='pxe', ipxe_enabled=True)
self.config(group='deploy', http_url='http://myserver')
isfile_mock.return_value = True
cmp_mock.return_value = True
self._test_prepare_ramdisk()
self.assertFalse(write_mock.called)
@mock.patch.object(common_utils, 'render_template', lambda *args: 'foo')
@mock.patch('ironic.common.utils.write_to_file', autospec=True)
@mock.patch('ironic.common.utils.render_template', autospec=True)
def test_prepare_ramdisk_ipxe_swift(self, render_mock, write_mock):
def test_prepare_ramdisk_ipxe_swift(self, write_mock):
self.node.provision_state = states.DEPLOYING
self.node.save()
self.config(group='pxe', ipxe_enabled=True)
self.config(group='pxe', ipxe_use_swift=True)
self.config(group='deploy', http_url='http://myserver')
render_mock.return_value = 'foo'
self._test_prepare_ramdisk(ipxe_use_swift=True)
write_mock.assert_called_once_with(
os.path.join(
@ -957,16 +952,15 @@ class PXEBootTestCase(db_base.DbTestCase):
os.path.basename(CONF.pxe.ipxe_boot_script)),
'foo')
@mock.patch.object(common_utils, 'render_template', lambda *args: 'foo')
@mock.patch('ironic.common.utils.write_to_file', autospec=True)
@mock.patch('ironic.common.utils.render_template', autospec=True)
def test_prepare_ramdisk_ipxe_swift_whole_disk_image(
self, render_mock, write_mock):
self, write_mock):
self.node.provision_state = states.DEPLOYING
self.node.save()
self.config(group='pxe', ipxe_enabled=True)
self.config(group='pxe', ipxe_use_swift=True)
self.config(group='deploy', http_url='http://myserver')
render_mock.return_value = 'foo'
self._test_prepare_ramdisk(ipxe_use_swift=True, whole_disk_image=True)
write_mock.assert_called_once_with(
os.path.join(