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:
parent
b05a6a64b1
commit
fb5def4ebc
@ -451,7 +451,8 @@ class AgentDeploy(AgentDeployMixin, base.DeployInterface):
|
|||||||
task.driver.storage.attach_volumes(task)
|
task.driver.storage.attach_volumes(task)
|
||||||
if not task.driver.storage.should_write_image(task):
|
if not task.driver.storage.should_write_image(task):
|
||||||
# We have nothing else to do as this is handled in the
|
# 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
|
return
|
||||||
if node.provision_state == states.ACTIVE:
|
if node.provision_state == states.ACTIVE:
|
||||||
# Call is due to conductor takeover
|
# Call is due to conductor takeover
|
||||||
|
@ -532,16 +532,15 @@ class TestPXEUtils(db_base.DbTestCase):
|
|||||||
rmtree_mock.assert_called_once_with(
|
rmtree_mock.assert_called_once_with(
|
||||||
os.path.join(CONF.pxe.tftp_root, self.node.uuid))
|
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.file_has_content', autospec=True)
|
||||||
@mock.patch('ironic.common.utils.write_to_file', autospec=True)
|
@mock.patch('ironic.common.utils.write_to_file', autospec=True)
|
||||||
@mock.patch('ironic.common.utils.render_template', autospec=True)
|
@mock.patch('ironic.common.utils.render_template', autospec=True)
|
||||||
def test_create_ipxe_boot_script(self, render_mock, write_mock,
|
def test_create_ipxe_boot_script(self, render_mock, write_mock,
|
||||||
cmp_mock, isfile_mock):
|
file_has_content_mock):
|
||||||
isfile_mock.return_value = False
|
|
||||||
render_mock.return_value = 'foo'
|
render_mock.return_value = 'foo'
|
||||||
self.assertFalse(cmp_mock.called)
|
|
||||||
pxe_utils.create_ipxe_boot_script()
|
pxe_utils.create_ipxe_boot_script()
|
||||||
|
self.assertFalse(file_has_content_mock.called)
|
||||||
write_mock.assert_called_once_with(
|
write_mock.assert_called_once_with(
|
||||||
os.path.join(CONF.deploy.http_root,
|
os.path.join(CONF.deploy.http_root,
|
||||||
os.path.basename(CONF.pxe.ipxe_boot_script)),
|
os.path.basename(CONF.pxe.ipxe_boot_script)),
|
||||||
@ -550,17 +549,19 @@ class TestPXEUtils(db_base.DbTestCase):
|
|||||||
CONF.pxe.ipxe_boot_script,
|
CONF.pxe.ipxe_boot_script,
|
||||||
{'ipxe_for_mac_uri': 'pxelinux.cfg/'})
|
{'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.file_has_content', autospec=True)
|
||||||
@mock.patch('ironic.common.utils.write_to_file', autospec=True)
|
@mock.patch('ironic.common.utils.write_to_file', autospec=True)
|
||||||
@mock.patch('ironic.common.utils.render_template', autospec=True)
|
@mock.patch('ironic.common.utils.render_template', autospec=True)
|
||||||
def test_create_ipxe_boot_script_copy_file_different(
|
def test_create_ipxe_boot_script_copy_file_different(
|
||||||
self, render_mock, write_mock, cmp_mock, isfile_mock):
|
self, render_mock, write_mock, file_has_content_mock):
|
||||||
isfile_mock.return_value = True
|
file_has_content_mock.return_value = False
|
||||||
cmp_mock.return_value = False
|
|
||||||
render_mock.return_value = 'foo'
|
render_mock.return_value = 'foo'
|
||||||
self.assertFalse(cmp_mock.called)
|
|
||||||
pxe_utils.create_ipxe_boot_script()
|
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(
|
write_mock.assert_called_once_with(
|
||||||
os.path.join(CONF.deploy.http_root,
|
os.path.join(CONF.deploy.http_root,
|
||||||
os.path.basename(CONF.pxe.ipxe_boot_script)),
|
os.path.basename(CONF.pxe.ipxe_boot_script)),
|
||||||
@ -569,15 +570,14 @@ class TestPXEUtils(db_base.DbTestCase):
|
|||||||
CONF.pxe.ipxe_boot_script,
|
CONF.pxe.ipxe_boot_script,
|
||||||
{'ipxe_for_mac_uri': 'pxelinux.cfg/'})
|
{'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.file_has_content', autospec=True)
|
||||||
@mock.patch('ironic.common.utils.write_to_file', autospec=True)
|
@mock.patch('ironic.common.utils.write_to_file', autospec=True)
|
||||||
@mock.patch('ironic.common.utils.render_template', autospec=True)
|
@mock.patch('ironic.common.utils.render_template', autospec=True)
|
||||||
def test_create_ipxe_boot_script_already_exists(self, render_mock,
|
def test_create_ipxe_boot_script_already_exists(self, render_mock,
|
||||||
write_mock, cmp_mock,
|
write_mock,
|
||||||
isfile_mock):
|
file_has_content_mock):
|
||||||
isfile_mock.return_value = True
|
file_has_content_mock.return_value = True
|
||||||
cmp_mock.return_value = True
|
|
||||||
pxe_utils.create_ipxe_boot_script()
|
pxe_utils.create_ipxe_boot_script()
|
||||||
self.assertFalse(write_mock.called)
|
self.assertFalse(write_mock.called)
|
||||||
|
|
||||||
|
@ -31,6 +31,7 @@ from ironic.common import exception
|
|||||||
from ironic.common.glance_service import base_image_service
|
from ironic.common.glance_service import base_image_service
|
||||||
from ironic.common import pxe_utils
|
from ironic.common import pxe_utils
|
||||||
from ironic.common import states
|
from ironic.common import states
|
||||||
|
from ironic.common import utils as common_utils
|
||||||
from ironic.conductor import task_manager
|
from ironic.conductor import task_manager
|
||||||
from ironic.conductor import utils as manager_utils
|
from ironic.conductor import utils as manager_utils
|
||||||
from ironic.drivers.modules import agent_base_vendor
|
from ironic.drivers.modules import agent_base_vendor
|
||||||
@ -880,18 +881,16 @@ class PXEBootTestCase(db_base.DbTestCase):
|
|||||||
self.node.save()
|
self.node.save()
|
||||||
self._test_prepare_ramdisk(uefi=True)
|
self._test_prepare_ramdisk(uefi=True)
|
||||||
|
|
||||||
@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.object(common_utils, 'file_has_content', lambda *args: False)
|
||||||
@mock.patch('ironic.common.utils.write_to_file', autospec=True)
|
@mock.patch('ironic.common.utils.write_to_file', autospec=True)
|
||||||
@mock.patch('ironic.common.utils.render_template', autospec=True)
|
@mock.patch('ironic.common.utils.render_template', autospec=True)
|
||||||
def test_prepare_ramdisk_ipxe_with_copy_file_different(
|
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.provision_state = states.DEPLOYING
|
||||||
self.node.save()
|
self.node.save()
|
||||||
self.config(group='pxe', ipxe_enabled=True)
|
self.config(group='pxe', ipxe_enabled=True)
|
||||||
self.config(group='deploy', http_url='http://myserver')
|
self.config(group='deploy', http_url='http://myserver')
|
||||||
isfile_mock.return_value = True
|
|
||||||
cmp_mock.return_value = False
|
|
||||||
render_mock.return_value = 'foo'
|
render_mock.return_value = 'foo'
|
||||||
self._test_prepare_ramdisk()
|
self._test_prepare_ramdisk()
|
||||||
write_mock.assert_called_once_with(
|
write_mock.assert_called_once_with(
|
||||||
@ -903,20 +902,19 @@ class PXEBootTestCase(db_base.DbTestCase):
|
|||||||
CONF.pxe.ipxe_boot_script,
|
CONF.pxe.ipxe_boot_script,
|
||||||
{'ipxe_for_mac_uri': 'pxelinux.cfg/'})
|
{'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.file_has_content', autospec=True)
|
||||||
@mock.patch('ironic.common.utils.write_to_file', autospec=True)
|
@mock.patch('ironic.common.utils.write_to_file', autospec=True)
|
||||||
@mock.patch('ironic.common.utils.render_template', autospec=True)
|
@mock.patch('ironic.common.utils.render_template', autospec=True)
|
||||||
def test_prepare_ramdisk_ipxe_with_copy_no_file(
|
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.provision_state = states.DEPLOYING
|
||||||
self.node.save()
|
self.node.save()
|
||||||
self.config(group='pxe', ipxe_enabled=True)
|
self.config(group='pxe', ipxe_enabled=True)
|
||||||
self.config(group='deploy', http_url='http://myserver')
|
self.config(group='deploy', http_url='http://myserver')
|
||||||
isfile_mock.return_value = False
|
|
||||||
render_mock.return_value = 'foo'
|
render_mock.return_value = 'foo'
|
||||||
self._test_prepare_ramdisk()
|
self._test_prepare_ramdisk()
|
||||||
self.assertFalse(cmp_mock.called)
|
self.assertFalse(file_has_content_mock.called)
|
||||||
write_mock.assert_called_once_with(
|
write_mock.assert_called_once_with(
|
||||||
os.path.join(
|
os.path.join(
|
||||||
CONF.deploy.http_root,
|
CONF.deploy.http_root,
|
||||||
@ -926,30 +924,27 @@ class PXEBootTestCase(db_base.DbTestCase):
|
|||||||
CONF.pxe.ipxe_boot_script,
|
CONF.pxe.ipxe_boot_script,
|
||||||
{'ipxe_for_mac_uri': 'pxelinux.cfg/'})
|
{'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.object(common_utils, 'file_has_content', lambda *args: True)
|
||||||
@mock.patch('ironic.common.utils.write_to_file', autospec=True)
|
@mock.patch('ironic.common.utils.write_to_file', autospec=True)
|
||||||
@mock.patch('ironic.common.utils.render_template', autospec=True)
|
@mock.patch('ironic.common.utils.render_template', autospec=True)
|
||||||
def test_prepare_ramdisk_ipxe_without_copy(
|
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.provision_state = states.DEPLOYING
|
||||||
self.node.save()
|
self.node.save()
|
||||||
self.config(group='pxe', ipxe_enabled=True)
|
self.config(group='pxe', ipxe_enabled=True)
|
||||||
self.config(group='deploy', http_url='http://myserver')
|
self.config(group='deploy', http_url='http://myserver')
|
||||||
isfile_mock.return_value = True
|
|
||||||
cmp_mock.return_value = True
|
|
||||||
self._test_prepare_ramdisk()
|
self._test_prepare_ramdisk()
|
||||||
self.assertFalse(write_mock.called)
|
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.write_to_file', autospec=True)
|
||||||
@mock.patch('ironic.common.utils.render_template', autospec=True)
|
def test_prepare_ramdisk_ipxe_swift(self, write_mock):
|
||||||
def test_prepare_ramdisk_ipxe_swift(self, render_mock, write_mock):
|
|
||||||
self.node.provision_state = states.DEPLOYING
|
self.node.provision_state = states.DEPLOYING
|
||||||
self.node.save()
|
self.node.save()
|
||||||
self.config(group='pxe', ipxe_enabled=True)
|
self.config(group='pxe', ipxe_enabled=True)
|
||||||
self.config(group='pxe', ipxe_use_swift=True)
|
self.config(group='pxe', ipxe_use_swift=True)
|
||||||
self.config(group='deploy', http_url='http://myserver')
|
self.config(group='deploy', http_url='http://myserver')
|
||||||
render_mock.return_value = 'foo'
|
|
||||||
self._test_prepare_ramdisk(ipxe_use_swift=True)
|
self._test_prepare_ramdisk(ipxe_use_swift=True)
|
||||||
write_mock.assert_called_once_with(
|
write_mock.assert_called_once_with(
|
||||||
os.path.join(
|
os.path.join(
|
||||||
@ -957,16 +952,15 @@ class PXEBootTestCase(db_base.DbTestCase):
|
|||||||
os.path.basename(CONF.pxe.ipxe_boot_script)),
|
os.path.basename(CONF.pxe.ipxe_boot_script)),
|
||||||
'foo')
|
'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.write_to_file', autospec=True)
|
||||||
@mock.patch('ironic.common.utils.render_template', autospec=True)
|
|
||||||
def test_prepare_ramdisk_ipxe_swift_whole_disk_image(
|
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.provision_state = states.DEPLOYING
|
||||||
self.node.save()
|
self.node.save()
|
||||||
self.config(group='pxe', ipxe_enabled=True)
|
self.config(group='pxe', ipxe_enabled=True)
|
||||||
self.config(group='pxe', ipxe_use_swift=True)
|
self.config(group='pxe', ipxe_use_swift=True)
|
||||||
self.config(group='deploy', http_url='http://myserver')
|
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)
|
self._test_prepare_ramdisk(ipxe_use_swift=True, whole_disk_image=True)
|
||||||
write_mock.assert_called_once_with(
|
write_mock.assert_called_once_with(
|
||||||
os.path.join(
|
os.path.join(
|
||||||
|
Loading…
Reference in New Issue
Block a user