From fc85cb9230f85f2b5d434be724ba629518bb8cb3 Mon Sep 17 00:00:00 2001 From: Dmitry Tantsur Date: Tue, 11 Sep 2018 10:32:48 +0200 Subject: [PATCH] Support partition HTTP images in CLI Also removed some dead code from sources. Change-Id: I570eda45285771068711ef90d22550632411e98f Story: #2002048 Task: #26208 --- metalsmith/_cmd.py | 17 +++-- metalsmith/sources.py | 72 +++++++++++++++++-- metalsmith/test/test_cmd.py | 15 ++++ playbooks/integration/cirros-image.yaml | 27 +++++++ playbooks/integration/exercise.yaml | 2 - playbooks/integration/run.yaml | 12 ++-- roles/metalsmith_deployment/README.rst | 20 ++++++ roles/metalsmith_deployment/defaults/main.yml | 2 + roles/metalsmith_deployment/tasks/main.yml | 16 +++-- 9 files changed, 163 insertions(+), 20 deletions(-) diff --git a/metalsmith/_cmd.py b/metalsmith/_cmd.py index 41582ad..38b56b4 100644 --- a/metalsmith/_cmd.py +++ b/metalsmith/_cmd.py @@ -58,14 +58,21 @@ def _do_deploy(api, args, formatter): raise RuntimeError("%s cannot be used as a hostname" % args.hostname) if _is_http(args.image): + kwargs = {} if not args.image_checksum: raise RuntimeError("HTTP(s) images require --image-checksum") elif _is_http(args.image_checksum): - source = sources.HttpWholeDiskImage( - args.image, checksum_url=args.image_checksum) + kwargs['checksum_url'] = args.image_checksum else: - source = sources.HttpWholeDiskImage( - args.image, checksum=args.image_checksum) + kwargs['checksum'] = args.image_checksum + + if args.image_kernel or args.image_ramdisk: + source = sources.HttpPartitionImage(args.image, + args.image_kernel, + args.image_ramdisk, + **kwargs) + else: + source = sources.HttpWholeDiskImage(args.image, **kwargs) else: source = args.image @@ -145,6 +152,8 @@ def _parse_args(args, config): required=True) deploy.add_argument('--image-checksum', help='image MD5 checksum or URL with checksums') + deploy.add_argument('--image-kernel', help='URL of the image\'s kernel') + deploy.add_argument('--image-ramdisk', help='URL of the image\'s ramdisk') deploy.add_argument('--network', help='network to use (name or UUID)', dest='nics', action=NICAction) deploy.add_argument('--port', help='port to attach (name or UUID)', diff --git a/metalsmith/sources.py b/metalsmith/sources.py index 44c361f..f5c72d6 100644 --- a/metalsmith/sources.py +++ b/metalsmith/sources.py @@ -90,8 +90,7 @@ class HttpWholeDiskImage(_Source): specifically, by **ironic-conductor** processes). """ - def __init__(self, url, checksum=None, checksum_url=None, - kernel_url=None, ramdisk_url=None): + def __init__(self, url, checksum=None, checksum_url=None): """Create an HTTP source. :param url: URL of the image. @@ -108,8 +107,6 @@ class HttpWholeDiskImage(_Source): self.url = url self.checksum = checksum self.checksum_url = checksum_url - self.kernel_url = kernel_url - self.ramdisk_url = ramdisk_url def _validate(self, connection): # TODO(dtantsur): should we validate image URLs here? Ironic will do it @@ -178,3 +175,70 @@ class HttpPartitionImage(HttpWholeDiskImage): updates['/instance_info/kernel'] = self.kernel_url updates['/instance_info/ramdisk'] = self.ramdisk_url return updates + + +class FileWholeDiskImage(_Source): + """A whole-disk image from a local file location. + + .. warning:: + The location must be local to the **ironic-conductor** process handling + the node, not to metalsmith itself! Since there is no easy way to + determine which conductor handles a node, the same file must be + available at the same location to all conductors in the same group. + """ + + def __init__(self, location, checksum): + """Create a local file source. + + :param location: Location of the image, optionally starting with + ``file://``. + :param checksum: MD5 checksum of the image. + """ + if not location.startswith('file://'): + location = 'file://' + location + self.location = location + self.checksum = checksum + + def _node_updates(self, connection): + LOG.debug('Image: %(image)s, checksum %(checksum)s', + {'image': self.location, 'checksum': self.checksum}) + return { + '/instance_info/image_source': self.location, + '/instance_info/image_checksum': self.checksum, + } + + +class FilePartitionImage(_Source): + """A partition image from a local file location. + + .. warning:: + The location must be local to the **ironic-conductor** process handling + the node, not to metalsmith itself! Since there is no easy way to + determine which conductor handles a node, the same file must be + available at the same location to all conductors in the same group. + """ + + def __init__(self, location, kernel_location, ramdisk_location, checksum): + """Create a local file source. + + :param location: Location of the image, optionally starting with + ``file://``. + :param kernel_location: Location of the kernel of the image, + optionally starting with ``file://``. + :param ramdisk_location: Location of the ramdisk of the image, + optionally starting with ``file://``. + :param checksum: MD5 checksum of the image. + """ + super(FilePartitionImage, self).__init__(location, checksum) + if not kernel_location.startswith('file://'): + kernel_location = 'file://' + kernel_location + if not ramdisk_location.startswith('file://'): + ramdisk_location = 'file://' + ramdisk_location + self.kernel_location = kernel_location + self.ramdisk_location = ramdisk_location + + def _node_updates(self, connection): + updates = super(FilePartitionImage, self)._node_updates(connection) + updates['/instance_info/kernel'] = self.kernel_location + updates['/instance_info/ramdisk'] = self.ramdisk_location + return updates diff --git a/metalsmith/test/test_cmd.py b/metalsmith/test/test_cmd.py index d22f755..f844018 100644 --- a/metalsmith/test/test_cmd.py +++ b/metalsmith/test/test_cmd.py @@ -378,6 +378,21 @@ class TestDeploy(testtools.TestCase): self.assertFalse(mock_pr.return_value.reserve_node.called) self.assertFalse(mock_pr.return_value.provision_node.called) + def test_args_http_partition_image(self, mock_pr): + args = ['deploy', '--image', 'https://example.com/image.img', + '--image-kernel', 'https://example.com/kernel', + '--image-ramdisk', 'https://example.com/ramdisk', + '--image-checksum', '95e750180c7921ea0d545c7165db66b8', + '--network', 'mynet', '--resource-class', 'compute'] + self._check(mock_pr, args, {}, {'image': mock.ANY}) + + source = mock_pr.return_value.provision_node.call_args[1]['image'] + self.assertIsInstance(source, sources.HttpPartitionImage) + self.assertEqual('https://example.com/image.img', source.url) + self.assertEqual('https://example.com/kernel', source.kernel_url) + self.assertEqual('https://example.com/ramdisk', source.ramdisk_url) + self.assertEqual('95e750180c7921ea0d545c7165db66b8', source.checksum) + def test_args_custom_wait(self, mock_pr): args = ['deploy', '--network', 'mynet', '--image', 'myimg', '--wait', '3600', '--resource-class', 'compute'] diff --git a/playbooks/integration/cirros-image.yaml b/playbooks/integration/cirros-image.yaml index 8f64b12..e5d6b08 100644 --- a/playbooks/integration/cirros-image.yaml +++ b/playbooks/integration/cirros-image.yaml @@ -24,6 +24,29 @@ register: baremetal_endpoint_result failed_when: baremetal_endpoint_result.stdout == "" + - name: Copy partition images directory + command: > + cp -r /opt/stack/devstack/files/images/{{ cirros_uec_image_result.stdout }} + /opt/stack/data/ironic/httpboot/metalsmith + args: + creates: /opt/stack/data/ironic/httpboot/metalsmith + become: yes + + - name: Create MD5 checksums file for partition images + shell: md5sum cirros-* > CHECKSUMS + args: + chdir: /opt/stack/data/ironic/httpboot/metalsmith + become: yes + + - name: Change ownership of image files + file: + path: /opt/stack/data/ironic/httpboot/metalsmith + state: directory + owner: "{{ ansible_user }}" + recurse: yes + mode: a+r + become: yes + - name: Calculate MD5 checksum for HTTP disk image shell: | md5sum /opt/stack/devstack/files/{{ cirros_disk_image_result.stdout }}.img \ @@ -33,6 +56,10 @@ - name: Set facts for HTTP image set_fact: + metalsmith_partition_image: "{{ baremetal_endpoint_result.stdout}}/metalsmith/{{ cirros_uec_image_result.stdout | replace('-uec', '-blank') }}.img" + metalsmith_partition_kernel_image: "{{ baremetal_endpoint_result.stdout}}/metalsmith/{{ cirros_uec_image_result.stdout | replace('-uec', '-vmlinuz') }}" + metalsmith_partition_ramdisk_image: "{{ baremetal_endpoint_result.stdout}}/metalsmith/{{ cirros_uec_image_result.stdout | replace('-uec', '-initrd') }}" + metalsmith_partition_checksum: "{{ baremetal_endpoint_result.stdout}}/metalsmith/CHECKSUMS" metalsmith_whole_disk_image: "{{ baremetal_endpoint_result.stdout}}/{{ cirros_disk_image_result.stdout }}.img" metalsmith_whole_disk_checksum: "{{ cirros_disk_image_checksum_result.stdout }}" diff --git a/playbooks/integration/exercise.yaml b/playbooks/integration/exercise.yaml index 855c19a..8d9ddae 100644 --- a/playbooks/integration/exercise.yaml +++ b/playbooks/integration/exercise.yaml @@ -22,8 +22,6 @@ metalsmith_resource_class: baremetal metalsmith_instances: - hostname: test - image: "{{ image }}" - image_checksum: "{{ image_checksum | default('') }}" nics: - "{{ nic }}" ssh_public_keys: diff --git a/playbooks/integration/run.yaml b/playbooks/integration/run.yaml index 5d64ad1..21d7196 100644 --- a/playbooks/integration/run.yaml +++ b/playbooks/integration/run.yaml @@ -11,15 +11,15 @@ - name: Test a whole-disk image include: exercise.yaml vars: - image: "{{ metalsmith_whole_disk_image }}" - image_checksum: "{{ metalsmith_whole_disk_checksum | default('') }}" + metalsmith_image: "{{ metalsmith_whole_disk_image }}" + metalsmith_image_checksum: "{{ metalsmith_whole_disk_checksum | default('') }}" # NOTE(dtantsur): cannot specify swap with whole disk images metalsmith_swap_size: - name: Test a partition image include: exercise.yaml vars: - image: "{{ metalsmith_partition_image }}" - image_checksum: "{{ metalsmith_partition_checksum | default('') }}" - # FIXME(dtantsur): cover partition images - when: not (metalsmith_use_http | default(false)) + metalsmith_image: "{{ metalsmith_partition_image }}" + metalsmith_image_checksum: "{{ metalsmith_partition_checksum | default('') }}" + metalsmith_image_kernel: "{{ metalsmith_partition_kernel_image | default('') }}" + metalsmith_image_ramdisk: "{{ metalsmith_partition_ramdisk_image | default('') }}" diff --git a/roles/metalsmith_deployment/README.rst b/roles/metalsmith_deployment/README.rst index 9080bb7..dd588d2 100644 --- a/roles/metalsmith_deployment/README.rst +++ b/roles/metalsmith_deployment/README.rst @@ -25,6 +25,10 @@ The following optional variables provide the defaults for Instance_ attributes: the default for ``image``. ``metalsmith_image_checksum`` the default for ``image_checksum``. +``metalsmith_image_kernel`` + the default for ``image_kernel``. +``metalsmith_image_ramdisk`` + the default for ``image_ramdisk``. ``metalsmith_netboot`` the default for ``netboot`` ``metalsmith_nics`` @@ -62,6 +66,12 @@ Each instances has the following attributes: UUID, name or HTTP(s) URL of the image to use for deployment. Mandatory. ``image_checksum`` (defaults to ``metalsmith_image_checksum``) MD5 checksum or checksum file URL for an HTTP(s) image. +``image_kernel`` (defaults to ``metalsmith_image_kernel``) + URL of the kernel image if and only if the ``image`` is a URL of + a partition image. +``image_ramdisk`` (defaults to ``metalsmith_image_ramdisk``) + URL of the ramdisk image if and only if the ``image`` is a URL of + a partition image. ``netboot`` whether to boot the deployed instance from network (PXE, iPXE, etc). The default is to use local boot (requires a bootloader on the image). @@ -152,3 +162,13 @@ Example nics: - network: ctlplane - port: 1899af15-149d-47dc-b0dc-a68614eeb5c4 + - hostname: custom-partition-image + resource_class: custom + image: https://example.com/images/custom-1.0.root.img + image_kernel: https://example.com/images/custom-1.0.vmlinuz + image_ramdisk: https://example.com/images/custom-1.0.initrd + image_checksum: https://example.com/images/MD5SUMS + - hostname: custom-whole-disk-image + resource_class: custom + image: https://example.com/images/custom-1.0.qcow2 + image_checksum: https://example.com/images/MD5SUMS diff --git a/roles/metalsmith_deployment/defaults/main.yml b/roles/metalsmith_deployment/defaults/main.yml index 8d17d7e..a770b15 100644 --- a/roles/metalsmith_deployment/defaults/main.yml +++ b/roles/metalsmith_deployment/defaults/main.yml @@ -4,6 +4,8 @@ metalsmith_capabilities: {} metalsmith_conductor_group: metalsmith_extra_args: metalsmith_image_checksum: +metalsmith_image_kernel: +metalsmith_image_ramdisk: metalsmith_netboot: false metalsmith_nics: [] metalsmith_resource_class: diff --git a/roles/metalsmith_deployment/tasks/main.yml b/roles/metalsmith_deployment/tasks/main.yml index 4a19915..240f81a 100644 --- a/roles/metalsmith_deployment/tasks/main.yml +++ b/roles/metalsmith_deployment/tasks/main.yml @@ -24,6 +24,15 @@ --ssh-public-key {{ ssh_key }} {% endfor %} --image {{ image }} + {% if image_checksum %} + --image-checksum {{ image_checksum }} + {% endif %} + {% if image_kernel %} + --image-kernel {{ image_kernel }} + {% endif %} + {% if image_ramdisk %} + --image-ramdisk {{ image_ramdisk }} + {% endif %} --hostname {{ instance.hostname }} {% if netboot %} --netboot @@ -40,9 +49,6 @@ {% for node in candidates %} --candidate {{ node }} {% endfor %} - {% if image_checksum %} - --image-checksum {{ image_checksum }} - {% endif %} when: state == 'present' vars: candidates: "{{ instance.candidates | default(metalsmith_candidates) }}" @@ -50,7 +56,9 @@ conductor_group: "{{ instance.conductor_group | default(metalsmith_conductor_group) }}" extra_args: "{{ instance.extra_args | default(metalsmith_extra_args) }}" image: "{{ instance.image | default(metalsmith_image) }}" - image: "{{ instance.image_checksum | default(metalsmith_image_checksum) }}" + image_checksum: "{{ instance.image_checksum | default(metalsmith_image_checksum) }}" + image_kernel: "{{ instance.image_kernel | default(metalsmith_image_kernel) }}" + image_ramdisk: "{{ instance.image_ramdisk | default(metalsmith_image_ramdisk) }}" netboot: "{{ instance.netboot | default(metalsmith_netboot) }}" nics: "{{ instance.nics | default(metalsmith_nics) }}" resource_class: "{{ instance.resource_class | default(metalsmith_resource_class) }}"