From 2fae8d9cbb788cf73558d00fc7e35c7daa224df6 Mon Sep 17 00:00:00 2001 From: Tony Breeds Date: Fri, 12 Jan 2018 07:35:22 +1100 Subject: [PATCH] Add --platform support when uploading images Adding --platform when uploading images will set a tripleo_platform image meta data property and prefix all images with the platform (and arch) to make them stand out in an openstack image list. Support is added in such a way that that image names are not altered if no --platform is specified. As platform really only makes sense *within* and architecture we ensure that both options are supplied. Conflicts: lower-constraints.txt: Due to not existing in stable/queens test-requirements.txt: Due to request-mock specification difference on queens tripleoclient/utils.py: Due to the following not existing on stable/queens Ie4b7951147f5a1aec654982e21296a749fdd865c I61763f62c5d44f098ad60f9426871caef16cd6de Blueprint: multiarch-support Change-Id: Ia56d22b244cfa5dddb5d110b54a525dfd80a7aaf (cherry picked from commit 3e786fe8329914e9a3f03de72f50d9fd2d318fc6) --- .../add-platform-option-97d92380b9ff52f1.yaml | 8 ++ tripleoclient/tests/test_utils.py | 55 ++++++++- .../overcloud_image/test_overcloud_image.py | 114 ++++++++++++++++++ tripleoclient/utils.py | 31 +++-- tripleoclient/v1/overcloud_image.py | 40 ++++-- 5 files changed, 224 insertions(+), 24 deletions(-) create mode 100644 releasenotes/notes/add-platform-option-97d92380b9ff52f1.yaml diff --git a/releasenotes/notes/add-platform-option-97d92380b9ff52f1.yaml b/releasenotes/notes/add-platform-option-97d92380b9ff52f1.yaml new file mode 100644 index 000000000..6b32d61ef --- /dev/null +++ b/releasenotes/notes/add-platform-option-97d92380b9ff52f1.yaml @@ -0,0 +1,8 @@ +--- +features: + - | + In certain situations it may be desirable to provide optimised overcloud + images for deployed nodes. In order to achieve this add a ``--platform`` + option to ``openstack overcloud image upload``. This option will then be + used to select appropriate images based on the combination of architecture + and platform. diff --git a/tripleoclient/tests/test_utils.py b/tripleoclient/tests/test_utils.py index 4af0a7de9..74db5d1aa 100644 --- a/tripleoclient/tests/test_utils.py +++ b/tripleoclient/tests/test_utils.py @@ -1094,6 +1094,17 @@ class TestOvercloudNameScenarios(TestWithScenarios): basename='overcloud-full', arch='x86_64', expected=('x86_64-overcloud-full-vmlinuz', '.vmlinuz'))), + ('kernel_arch_platform', + dict(func=utils.overcloud_kernel, + basename='overcloud-full', + arch='x86_64', + platform='SNB', + expected=('SNB-x86_64-overcloud-full-vmlinuz', '.vmlinuz'))), + ('kernel_platform', + dict(func=utils.overcloud_kernel, + basename='overcloud-full', + platform='SNB', + expected=('overcloud-full-vmlinuz', '.vmlinuz'))), ('ramdisk_default', dict(func=utils.overcloud_ramdisk, basename='overcloud-full', @@ -1103,6 +1114,17 @@ class TestOvercloudNameScenarios(TestWithScenarios): basename='overcloud-full', arch='x86_64', expected=('x86_64-overcloud-full-initrd', '.initrd'))), + ('ramdisk_arch_platform', + dict(func=utils.overcloud_ramdisk, + basename='overcloud-full', + arch='x86_64', + platform='SNB', + expected=('SNB-x86_64-overcloud-full-initrd', '.initrd'))), + ('ramdisk_platform', + dict(func=utils.overcloud_ramdisk, + basename='overcloud-full', + platform='SNB', + expected=('overcloud-full-initrd', '.initrd'))), ('image_default', dict(func=utils.overcloud_image, basename='overcloud-full', @@ -1112,11 +1134,22 @@ class TestOvercloudNameScenarios(TestWithScenarios): basename='overcloud-full', arch='x86_64', expected=('x86_64-overcloud-full', '.qcow2'))), + ('image_arch_platform', + dict(func=utils.overcloud_image, + basename='overcloud-full', + arch='x86_64', + platform='SNB', + expected=('SNB-x86_64-overcloud-full', '.qcow2'))), + ('image_platform', + dict(func=utils.overcloud_image, + basename='overcloud-full', + platform='SNB', + expected=('overcloud-full', '.qcow2'))), ] def test_overcloud_params(self): kwargs = dict() - for attr in ['arch']: + for attr in ['arch', 'platform']: if hasattr(self, attr): kwargs[attr] = getattr(self, attr) @@ -1137,6 +1170,15 @@ class TestDeployNameScenarios(TestWithScenarios): dict(func=utils.deploy_kernel, arch='x86_64', expected=('x86_64-bm-deploy-kernel', '.kernel'))), + ('kernel_arch_platform', + dict(func=utils.deploy_kernel, + arch='x86_64', + platform='SNB', + expected=('SNB-x86_64-bm-deploy-kernel', '.kernel'))), + ('kernel_platform', + dict(func=utils.deploy_kernel, + platform='SNB', + expected=('bm-deploy-kernel', '.kernel'))), ('ramdisk_default', dict(func=utils.deploy_ramdisk, expected=('bm-deploy-ramdisk', '.initramfs'))), @@ -1144,11 +1186,20 @@ class TestDeployNameScenarios(TestWithScenarios): dict(func=utils.deploy_ramdisk, arch='x86_64', expected=('x86_64-bm-deploy-ramdisk', '.initramfs'))), + ('ramdisk_arch_platform', + dict(func=utils.deploy_ramdisk, + arch='x86_64', + platform='SNB', + expected=('SNB-x86_64-bm-deploy-ramdisk', '.initramfs'))), + ('ramdisk_platform', + dict(func=utils.deploy_ramdisk, + platform='SNB', + expected=('bm-deploy-ramdisk', '.initramfs'))), ] def test_deploy_params(self): kwargs = {} - for attr in ['arch']: + for attr in ['arch', 'platform']: if hasattr(self, attr): kwargs[attr] = getattr(self, attr) diff --git a/tripleoclient/tests/v1/overcloud_image/test_overcloud_image.py b/tripleoclient/tests/v1/overcloud_image/test_overcloud_image.py index 5d5f3df0c..4b6946103 100644 --- a/tripleoclient/tests/v1/overcloud_image/test_overcloud_image.py +++ b/tripleoclient/tests/v1/overcloud_image/test_overcloud_image.py @@ -209,6 +209,12 @@ class TestUploadOvercloudImage(TestPluginV1): self.cmd._copy_file.call_count ) + def test_platform_without_architecture_fail(self): + parsed_args = self.check_parser(self.cmd, ['--platform', 'SNB'], []) + self.assertRaises(exceptions.CommandError, + self.cmd.take_action, + parsed_args) + @mock.patch('os.path.isfile', autospec=True) @mock.patch('subprocess.check_call', autospec=True) def test_overcloud_create_images_v2(self, mock_subprocess_call, @@ -646,6 +652,9 @@ class TestUploadOvercloudImageFullMultiArch(TestPluginV1): 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'), + mock.Mock(id=16, name='p9-ppc64le-overcloud-full'), + mock.Mock(id=17, name='p9-ppc64le-bm-deploy-kernel'), + mock.Mock(id=18, name='p9-ppc64le-bm-deploy-initrd'), ] def setUp(self): @@ -740,3 +749,108 @@ class TestUploadOvercloudImageFullMultiArch(TestPluginV1): mock.call('sudo cp -f "./ironic-python-agent.initramfs" ' '"/httpboot/ppc64le/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_and_pltform(self, + mock_subprocess, + 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', + '--architecture', 'ppc64le'], + []) + self.cmd.take_action(parsed_args) + parsed_args = self.check_parser(self.cmd, + ['--whole-disk', + '--http-boot', '/httpboot/p9-ppc64le', + '--architecture', 'ppc64le', + '--platform', 'p9'], + []) + self.cmd.take_action(parsed_args) + + self.assertEqual( + 0, + self.app.client_manager.image.images.delete.call_count + ) + self.assertEqual( + 9, + 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'), + mock.call(name='p9-ppc64le-overcloud-full', + disk_format='qcow2', + container_format='bare', + visibility='public'), + mock.call(name='p9-ppc64le-bm-deploy-kernel', + disk_format='aki', + container_format='bare', + visibility='public'), + mock.call(name='p9-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'), + mock.call(16, hw_architecture='ppc64le', tripleo_platform='p9'), + mock.call(17, hw_architecture='ppc64le', tripleo_platform='p9'), + mock.call(18, hw_architecture='ppc64le', tripleo_platform='p9'), + ], self.app.client_manager.image.images.update.call_args_list + ) + self.assertEqual(mock_subprocess.call_count, 6) + # FIXME(tonyb): this is the wrong way around + self.assertEqual( + mock_subprocess.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), + # FIXME(tonyb): wrong + mock.call('sudo cp -f "./ironic-python-agent.kernel" ' + '"/httpboot/p9-ppc64le/agent.kernel"', shell=True), + mock.call('sudo cp -f "./ironic-python-agent.initramfs" ' + '"/httpboot/p9-ppc64le/agent.ramdisk"', shell=True), + ]) diff --git a/tripleoclient/utils.py b/tripleoclient/utils.py index 49d29c3c6..170730382 100644 --- a/tripleoclient/utils.py +++ b/tripleoclient/utils.py @@ -1270,32 +1270,39 @@ def run_command_and_log(log, cmd, cwd=None, env=None, retcode_only=True): return proc -def _name_helper(basename, arch=None): - if arch: +def _name_helper(basename, arch=None, platform=None): + # NOTE(tonyb): We don't accept a platform with an arch. This caught when + # import the nodes / process args, but lets be a little cautious here + # anyway. + if arch and platform: + basename = platform + '-' + arch + '-' + basename + elif arch: basename = arch + '-' + basename return basename -def overcloud_kernel(basename, arch=None): - return (_name_helper('%s-vmlinuz' % basename, arch=arch), +def overcloud_kernel(basename, arch=None, platform=None): + return (_name_helper('%s-vmlinuz' % basename, arch=arch, + platform=platform), '.vmlinuz') -def overcloud_ramdisk(basename, arch=None): - return (_name_helper('%s-initrd' % basename, arch=arch), +def overcloud_ramdisk(basename, arch=None, platform=None): + return (_name_helper('%s-initrd' % basename, arch=arch, + platform=platform), '.initrd') -def overcloud_image(basename, arch=None): - return (_name_helper(basename, arch=arch), +def overcloud_image(basename, arch=None, platform=None): + return (_name_helper(basename, arch=arch, platform=platform), '.qcow2') -def deploy_kernel(arch=None): - return (_name_helper('bm-deploy-kernel', arch=arch), +def deploy_kernel(arch=None, platform=None): + return (_name_helper('bm-deploy-kernel', arch=arch, platform=platform), '.kernel') -def deploy_ramdisk(arch=None): - return (_name_helper('bm-deploy-ramdisk', arch=arch), +def deploy_ramdisk(arch=None, platform=None): + return (_name_helper('bm-deploy-ramdisk', arch=arch, platform=platform), '.initramfs') diff --git a/tripleoclient/v1/overcloud_image.py b/tripleoclient/v1/overcloud_image.py index f38bc28ae..178a50da9 100644 --- a/tripleoclient/v1/overcloud_image.py +++ b/tripleoclient/v1/overcloud_image.py @@ -283,6 +283,13 @@ class UploadOvercloudImage(command.Command): "are common options. This option should match at least " "one \'arch\' value in instackenv.json"), ) + parser.add_argument( + "--platform", + help=_("Platform type for these images. Platform is a " + "sub-category of architecture. For example you may have " + "generic images for x86_64 but offer images specific to " + "SandyBridge (SNB)."), + ) return parser def take_action(self, parsed_args): @@ -290,6 +297,11 @@ class UploadOvercloudImage(command.Command): glance_client_adaptor = self._get_glance_client_adaptor() self.updated = False + if parsed_args.platform and not parsed_args.architecture: + raise exceptions.CommandError('You supplied a platform (%s) ' + 'without specifying the ' + 'architecture') + self.log.debug("checking if image files exist") if parsed_args.whole_disk: @@ -318,12 +330,15 @@ class UploadOvercloudImage(command.Command): arch = parsed_args.architecture if arch: properties['hw_architecture'] = arch + platform = parsed_args.platform + if platform: + properties['tripleo_platform'] = platform # 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, - arch=arch) + oc_vmlinuz_extension) = plugin_utils.overcloud_kernel( + image_name, arch=arch, platform=platform) oc_vmlinuz_file = os.path.join(parsed_args.image_path, image_name + oc_vmlinuz_extension) @@ -340,8 +355,8 @@ class UploadOvercloudImage(command.Command): )) (oc_initrd_name, - oc_initrd_extension) = plugin_utils.overcloud_ramdisk(image_name, - arch=arch) + oc_initrd_extension) = plugin_utils.overcloud_ramdisk( + image_name, arch=arch, platform=platform) oc_initrd_file = os.path.join(parsed_args.image_path, image_name + oc_initrd_extension) @@ -358,8 +373,8 @@ class UploadOvercloudImage(command.Command): )) (oc_name, - oc_extension) = plugin_utils.overcloud_image(image_name, - arch=arch) + oc_extension) = plugin_utils.overcloud_image( + image_name, arch=arch, platform=platform) oc_file = os.path.join(parsed_args.image_path, image_name + oc_extension) @@ -389,8 +404,8 @@ class UploadOvercloudImage(command.Command): else: (oc_name, - oc_extension) = plugin_utils.overcloud_image(image_name, - arch=arch) + oc_extension) = plugin_utils.overcloud_image( + image_name, arch=arch, platform=platform) oc_file = os.path.join(parsed_args.image_path, image_name + oc_extension) @@ -409,7 +424,8 @@ class UploadOvercloudImage(command.Command): self.log.debug("uploading bm images to glance") (deploy_kernel_name, - deploy_kernel_extension) = plugin_utils.deploy_kernel(arch=arch) + deploy_kernel_extension) = plugin_utils.deploy_kernel( + arch=arch, platform=platform) deploy_kernel_file = os.path.join(parsed_args.image_path, parsed_args.ipa_name + deploy_kernel_extension) @@ -425,7 +441,8 @@ class UploadOvercloudImage(command.Command): deploy_kernel_file)) (deploy_ramdisk_name, - deploy_ramdisk_extension) = plugin_utils.deploy_ramdisk(arch=arch) + deploy_ramdisk_extension) = plugin_utils.deploy_ramdisk( + arch=arch, platform=platform) deploy_ramdisk_file = os.path.join(parsed_args.image_path, parsed_args.ipa_name + deploy_ramdisk_extension) @@ -441,6 +458,9 @@ class UploadOvercloudImage(command.Command): self.log.debug("copy agent images to HTTP BOOT dir") + # TODO(tonyb) Decide how to handle platform specific httpboot + # files/names + self._file_create_or_update( os.path.join(parsed_args.image_path, '%s.kernel' % parsed_args.ipa_name),