From b5500956d30a738b442ed0b9894a94552dfd7d09 Mon Sep 17 00:00:00 2001 From: Tony Breeds Date: Mon, 15 Jan 2018 13:36:46 +1100 Subject: [PATCH] Add --architecture support when uploading images Adding --architecture when uploading images will set a hw_architecture image meta data property and prefix all images with the arch to make them stand out in an openstack image list While tripleo doesn't yet take advantage of using the image meta data it's still helpful for manual testing to ensure that image selection implies appropriate hardware. Support is added in such a way that that image names are not altered if no --architecture is specified. Blueprint: multiarch-support Change-Id: I352759014ce4c249d8391b03317012000221f96f --- ...-architecture-option-1fca9e53bd59d353.yaml | 7 + tripleoclient/tests/test_utils.py | 43 +++- .../overcloud_image/test_overcloud_image.py | 211 ++++++++++++++++++ tripleoclient/utils.py | 26 ++- tripleoclient/v1/overcloud_image.py | 26 ++- 5 files changed, 295 insertions(+), 18 deletions(-) create mode 100644 releasenotes/notes/add-architecture-option-1fca9e53bd59d353.yaml diff --git a/releasenotes/notes/add-architecture-option-1fca9e53bd59d353.yaml b/releasenotes/notes/add-architecture-option-1fca9e53bd59d353.yaml new file mode 100644 index 000000000..1c59a6cb3 --- /dev/null +++ b/releasenotes/notes/add-architecture-option-1fca9e53bd59d353.yaml @@ -0,0 +1,7 @@ +--- +features: + - | + In order to allow overcloud and deploy images to vary based on architecture + add a ``--architecture`` option to ``openstack overcloud image upload``. + This option will add hw_architecture to the image meta-data, which will + then be used my nova to limit node selection to matching CPU architectures. diff --git a/tripleoclient/tests/test_utils.py b/tripleoclient/tests/test_utils.py index ec587197e..367292e56 100644 --- a/tripleoclient/tests/test_utils.py +++ b/tripleoclient/tests/test_utils.py @@ -776,18 +776,41 @@ class TestOvercloudNameScenarios(TestWithScenarios): dict(func=utils.overcloud_kernel, basename='overcloud-full', expected=('overcloud-full-vmlinuz', '.vmlinuz'))), + ('kernel_arch', + dict(func=utils.overcloud_kernel, + basename='overcloud-full', + arch='x86_64', + expected=('x86_64-overcloud-full-vmlinuz', '.vmlinuz'))), ('ramdisk_default', dict(func=utils.overcloud_ramdisk, basename='overcloud-full', expected=('overcloud-full-initrd', '.initrd'))), + ('ramdisk_arch', + dict(func=utils.overcloud_ramdisk, + basename='overcloud-full', + arch='x86_64', + expected=('x86_64-overcloud-full-initrd', '.initrd'))), ('image_default', dict(func=utils.overcloud_image, basename='overcloud-full', expected=('overcloud-full', '.qcow2'))), + ('image_arch', + dict(func=utils.overcloud_image, + basename='overcloud-full', + arch='x86_64', + expected=('x86_64-overcloud-full', '.qcow2'))), ] def test_overcloud_params(self): - observed = self.func(self.basename) + kwargs = dict() + for attr in ['arch']: + if hasattr(self, attr): + kwargs[attr] = getattr(self, attr) + + if kwargs: + observed = self.func(self.basename, **kwargs) + else: + observed = self.func(self.basename) self.assertEqual(self.expected, observed) @@ -797,12 +820,28 @@ class TestDeployNameScenarios(TestWithScenarios): ('kernel_default', dict(func=utils.deploy_kernel, expected=('bm-deploy-kernel', '.kernel'))), + ('kernel_arch', + dict(func=utils.deploy_kernel, + arch='x86_64', + expected=('x86_64-bm-deploy-kernel', '.kernel'))), ('ramdisk_default', dict(func=utils.deploy_ramdisk, expected=('bm-deploy-ramdisk', '.initramfs'))), + ('ramdisk_arch', + dict(func=utils.deploy_ramdisk, + arch='x86_64', + expected=('x86_64-bm-deploy-ramdisk', '.initramfs'))), ] def test_deploy_params(self): - observed = self.func() + kwargs = {} + for attr in ['arch']: + if hasattr(self, attr): + kwargs[attr] = getattr(self, attr) + + if kwargs: + observed = self.func(**kwargs) + else: + observed = self.func() self.assertEqual(self.expected, observed) diff --git a/tripleoclient/tests/v1/overcloud_image/test_overcloud_image.py b/tripleoclient/tests/v1/overcloud_image/test_overcloud_image.py index c0021808a..d533d9db7 100644 --- a/tripleoclient/tests/v1/overcloud_image/test_overcloud_image.py +++ b/tripleoclient/tests/v1/overcloud_image/test_overcloud_image.py @@ -311,7 +311,59 @@ class TestUploadOvercloudImage(TestPluginV1): ], self.app.client_manager.image.images.create.call_args_list ) + @mock.patch('os.path.isfile', autospec=True) + @mock.patch('subprocess.check_call', autospec=True) + def test_overcloud_create_images_with_arch_v1(self, mock_subprocess_call, + mock_isfile): + parsed_args = self.check_parser(self.cmd, ['--arch', 'x86_64'], []) + mock_isfile.return_value = False + + self.cmd._get_image = mock.Mock(return_value=None) + self.app.client_manager.image.version = 1.0 + + self.cmd.take_action(parsed_args) + + self.assertEqual( + 0, + self.app.client_manager.image.images.delete.call_count + ) + self.assertEqual( + 5, + self.app.client_manager.image.images.create.call_count + ) + self.assertEqual( + [mock.call(properties={'hw_architecture': 'x86_64'}, + data=b'IMGDATA', + name='x86_64-overcloud-full-vmlinuz', + disk_format='aki', + is_public=True), + mock.call(properties={'hw_architecture': 'x86_64'}, + data=b'IMGDATA', + name='x86_64-overcloud-full-initrd', + disk_format='ari', + is_public=True), + mock.call(properties={'hw_architecture': 'x86_64', + 'kernel_id': 10, 'ramdisk_id': 10}, + name='x86_64-overcloud-full', + data=b'IMGDATA', + container_format='bare', + disk_format='qcow2', + is_public=True), + mock.call(properties={'hw_architecture': 'x86_64'}, + data=b'IMGDATA', + name='x86_64-bm-deploy-kernel', + disk_format='aki', + is_public=True), + mock.call(properties={'hw_architecture': 'x86_64'}, + data=b'IMGDATA', + name='x86_64-bm-deploy-ramdisk', + disk_format='ari', + is_public=True) + ], self.app.client_manager.image.images.create.call_args_list + ) + self.assertEqual(mock_subprocess_call.call_count, 2) + # FIXME(tonyb): this is the wrong way around self.assertEqual( mock_subprocess_call.call_args_list, [ mock.call('sudo cp -f "./ironic-python-agent.kernel" ' @@ -468,6 +520,59 @@ class TestUploadOvercloudImageFull(TestPluginV1): '"/httpboot/agent.ramdisk"', shell=True) ]) + @mock.patch('os.path.isfile', autospec=True) + @mock.patch('subprocess.check_call', autospec=True) + def test_overcloud_create_images_with_arch(self, mock_subprocess_call, + mock_isfile): + parsed_args = self.check_parser(self.cmd, + ['--whole-disk', '--arch', 'x86_64'], + []) + mock_isfile.return_value = False + + self.cmd._get_image = mock.Mock(return_value=None) + + self.cmd.take_action(parsed_args) + + self.assertEqual( + 0, + self.app.client_manager.image.images.delete.call_count + ) + self.assertEqual( + 3, + self.app.client_manager.image.images.create.call_count + ) + + self.assertEqual( + [mock.call(name='x86_64-overcloud-full', + disk_format='qcow2', + container_format='bare', + visibility='public'), + mock.call(name='x86_64-bm-deploy-kernel', + disk_format='aki', + container_format='bare', + visibility='public'), + mock.call(name='x86_64-bm-deploy-ramdisk', + disk_format='ari', + container_format='bare', + visibility='public') + ], self.app.client_manager.image.images.create.call_args_list + ) + + self.assertEqual( + [mock.call(mock.ANY, hw_architecture='x86_64'), + mock.call(mock.ANY, hw_architecture='x86_64'), + mock.call(mock.ANY, hw_architecture='x86_64'), + ], self.app.client_manager.image.images.update.call_args_list + ) + self.assertEqual(mock_subprocess_call.call_count, 2) + self.assertEqual( + mock_subprocess_call.call_args_list, [ + mock.call('sudo cp -f "./ironic-python-agent.kernel" ' + '"/httpboot/agent.kernel"', shell=True), + mock.call('sudo cp -f "./ironic-python-agent.initramfs" ' + '"/httpboot/agent.ramdisk"', shell=True) + ]) + @mock.patch('os.path.isfile', autospec=True) @mock.patch('subprocess.check_call', autospec=True) def test_overcloud_create_noupdate_images(self, mock_subprocess_call, @@ -527,3 +632,109 @@ class TestUploadOvercloudImageFull(TestPluginV1): self.app.client_manager.image.images.update.call_count ) self.assertEqual(mock_subprocess_call.call_count, 2) + + +class TestUploadOvercloudImageFullMultiArch(TestPluginV1): + # NOTE(tonyb): Really only the id is important below, but the names make + # reading logfiles a little nicer + images = [ + mock.Mock(id=10, name='overcloud-full'), + mock.Mock(id=11, name='bm-deploy-kernel'), + mock.Mock(id=12, name='bm-deploy-initrd'), + mock.Mock(id=13, name='ppc64le-overcloud-full'), + mock.Mock(id=14, name='ppc64le-bm-deploy-kernel'), + mock.Mock(id=15, name='ppc64le-bm-deploy-initrd'), + ] + + def setUp(self): + super(TestUploadOvercloudImageFullMultiArch, self).setUp() + + # Get the command object to test + self.cmd = overcloud_image.UploadOvercloudImage(self.app, None) + self.app.client_manager.image = mock.Mock() + self.app.client_manager.image.version = 2.0 + # NOTE(tonyb): This is a little fragile. It works because + # GlanceV2ClientAdapter.upload_image() calls + # self.client.images.create() and self.client.images.get() once each + # call so this way we always create() and get() the same mocked "image" + self.app.client_manager.image.images.create.side_effect = self.images + self.app.client_manager.image.images.get.side_effect = self.images + self.cmd._read_image_file_pointer = mock.Mock(return_value=b'IMGDATA') + self.cmd._check_file_exists = mock.Mock(return_value=True) + + @mock.patch('os.path.isfile', autospec=True) + @mock.patch('subprocess.check_call', autospec=True) + def test_overcloud_create_images_with_arch(self, mock_subprocess_call, + mock_isfile): + mock_isfile.return_value = False + + self.cmd._get_image = mock.Mock(return_value=None) + mock.patch + + parsed_args = self.check_parser(self.cmd, + ['--whole-disk'], + []) + self.cmd.take_action(parsed_args) + parsed_args = self.check_parser(self.cmd, + ['--whole-disk', + '--http-boot', '/httpboot/ppc64le', + '--arch', 'ppc64le'], + []) + self.cmd.take_action(parsed_args) + + self.assertEqual( + 0, + self.app.client_manager.image.images.delete.call_count + ) + self.assertEqual( + 6, + self.app.client_manager.image.images.create.call_count + ) + + self.assertEqual( + [mock.call(name='overcloud-full', + disk_format='qcow2', + container_format='bare', + visibility='public'), + mock.call(name='bm-deploy-kernel', + disk_format='aki', + container_format='bare', + visibility='public'), + mock.call(name='bm-deploy-ramdisk', + disk_format='ari', + container_format='bare', + visibility='public'), + mock.call(name='ppc64le-overcloud-full', + disk_format='qcow2', + container_format='bare', + visibility='public'), + mock.call(name='ppc64le-bm-deploy-kernel', + disk_format='aki', + container_format='bare', + visibility='public'), + mock.call(name='ppc64le-bm-deploy-ramdisk', + disk_format='ari', + container_format='bare', + visibility='public') + ], self.app.client_manager.image.images.create.call_args_list + ) + + self.assertEqual( + [mock.call(13, hw_architecture='ppc64le'), + mock.call(14, hw_architecture='ppc64le'), + mock.call(15, hw_architecture='ppc64le'), + ], self.app.client_manager.image.images.update.call_args_list + ) + self.assertEqual(mock_subprocess_call.call_count, 4) + # FIXME(tonyb): this is the wrong way around + self.assertEqual( + mock_subprocess_call.call_args_list, [ + mock.call('sudo cp -f "./ironic-python-agent.kernel" ' + '"/httpboot/agent.kernel"', shell=True), + mock.call('sudo cp -f "./ironic-python-agent.initramfs" ' + '"/httpboot/agent.ramdisk"', shell=True), + mock.call('sudo cp -f "./ironic-python-agent.kernel" ' + '"/httpboot/ppc64le/agent.kernel"', shell=True), + mock.call('sudo cp -f "./ironic-python-agent.initramfs" ' + '"/httpboot/ppc64le/agent.ramdisk"', shell=True), + ]) diff --git a/tripleoclient/utils.py b/tripleoclient/utils.py index 9b33ac4f5..86f6251a2 100644 --- a/tripleoclient/utils.py +++ b/tripleoclient/utils.py @@ -1186,26 +1186,32 @@ def configure_logging(log, level, log_file): log.addHandler(fhandler) -def overcloud_kernel(basename): - return ('%s-vmlinuz' % basename, +def _name_helper(basename, arch=None): + if arch: + basename = arch + '-' + basename + return basename + + +def overcloud_kernel(basename, arch=None): + return (_name_helper('%s-vmlinuz' % basename, arch=arch), '.vmlinuz') -def overcloud_ramdisk(basename): - return ('%s-initrd' % basename, +def overcloud_ramdisk(basename, arch=None): + return (_name_helper('%s-initrd' % basename, arch=arch), '.initrd') -def overcloud_image(basename): - return (basename, +def overcloud_image(basename, arch=None): + return (_name_helper(basename, arch=arch), '.qcow2') -def deploy_kernel(): - return ('bm-deploy-kernel', +def deploy_kernel(arch=None): + return (_name_helper('bm-deploy-kernel', arch=arch), '.kernel') -def deploy_ramdisk(): - return ('bm-deploy-ramdisk', +def deploy_ramdisk(arch=None): + return (_name_helper('bm-deploy-ramdisk', arch=arch), '.initramfs') diff --git a/tripleoclient/v1/overcloud_image.py b/tripleoclient/v1/overcloud_image.py index fbf27f3c4..f8acc83ba 100644 --- a/tripleoclient/v1/overcloud_image.py +++ b/tripleoclient/v1/overcloud_image.py @@ -275,6 +275,13 @@ class UploadOvercloudImage(command.Command): help=_("When set, the overcloud-full image to be uploaded " "will be considered as a whole disk one"), ) + parser.add_argument( + "--architecture", + help=_("Architecture type for these images, " + "\'x86_64\', \'i386\' and \'ppc64le\' " + "are common options. This option should match at least " + "one \'arch\' value in instackenv.json"), + ) return parser def take_action(self, parsed_args): @@ -306,11 +313,15 @@ class UploadOvercloudImage(command.Command): overcloud_image_type) properties = {} + arch = parsed_args.architecture + if arch: + properties['hw_architecture'] = arch # vmlinuz and initrd only need to be uploaded for a partition image if not parsed_args.whole_disk: (oc_vmlinuz_name, - oc_vmlinuz_extension) = plugin_utils.overcloud_kernel(image_name) + oc_vmlinuz_extension) = plugin_utils.overcloud_kernel(image_name, + arch=arch) oc_vmlinuz_file = os.path.join(parsed_args.image_path, image_name + oc_vmlinuz_extension) @@ -327,7 +338,8 @@ class UploadOvercloudImage(command.Command): )) (oc_initrd_name, - oc_initrd_extension) = plugin_utils.overcloud_ramdisk(image_name) + oc_initrd_extension) = plugin_utils.overcloud_ramdisk(image_name, + arch=arch) oc_initrd_file = os.path.join(parsed_args.image_path, image_name + oc_initrd_extension) @@ -344,7 +356,8 @@ class UploadOvercloudImage(command.Command): )) (oc_name, - oc_extension) = plugin_utils.overcloud_image(image_name) + oc_extension) = plugin_utils.overcloud_image(image_name, + arch=arch) oc_file = os.path.join(parsed_args.image_path, image_name + oc_extension) @@ -374,7 +387,8 @@ class UploadOvercloudImage(command.Command): else: (oc_name, - oc_extension) = plugin_utils.overcloud_image(image_name) + oc_extension) = plugin_utils.overcloud_image(image_name, + arch=arch) oc_file = os.path.join(parsed_args.image_path, image_name + oc_extension) @@ -393,7 +407,7 @@ class UploadOvercloudImage(command.Command): self.log.debug("uploading bm images to glance") (deploy_kernel_name, - deploy_kernel_extension) = plugin_utils.deploy_kernel() + deploy_kernel_extension) = plugin_utils.deploy_kernel(arch=arch) deploy_kernel_file = os.path.join(parsed_args.image_path, parsed_args.ipa_name + deploy_kernel_extension) @@ -409,7 +423,7 @@ class UploadOvercloudImage(command.Command): deploy_kernel_file)) (deploy_ramdisk_name, - deploy_ramdisk_extension) = plugin_utils.deploy_ramdisk() + deploy_ramdisk_extension) = plugin_utils.deploy_ramdisk(arch=arch) deploy_ramdisk_file = os.path.join(parsed_args.image_path, parsed_args.ipa_name + deploy_ramdisk_extension)