compute: Add missing options for 'server rebuild'

This accepts a large number of options that we weren't exposing. Add the
following options: '--name', '--preserve-ephemeral', '--user-data',
'--no-user-data', '--trusted-image-cert' and '--no-trusted-image-certs'.
In addition, rename the '--key-unset' parameter to '--no-key-name', to
mimic e.g. '--no-property' on other commands.

Change-Id: I61c46e18bef1f086b62a015ebdc56be91071b826
Signed-off-by: Stephen Finucane <sfinucan@redhat.com>

openstackclient/compute/v2/server.py

@@ -2445,10 +2445,15 @@ class RebuildServer(command.ShowOne):
                 'Defaults to the currently used one.'
             ),
         )
+        parser.add_argument(
+            '--name',
+            metavar='<name>',
+            help=_('Set the new name of the rebuilt server'),
+        )
         parser.add_argument(
             '--password',
             metavar='<password>',
-            help=_('Set a password on the rebuilt server'),
+            help=_('Set the password on the rebuilt server'),
         )
         parser.add_argument(
             '--property',
@@ -2467,6 +2472,24 @@ class RebuildServer(command.ShowOne):
                 '(supported by --os-compute-api-version 2.19 or above)'
             ),
         )
+        preserve_ephemeral_group = parser.add_mutually_exclusive_group()
+        preserve_ephemeral_group.add_argument(
+            '--preserve-ephemeral',
+            action='store_true',
+            default=None,
+            help=_(
+                'Preserve the default ephemeral storage partition on rebuild.'
+            ),
+        )
+        preserve_ephemeral_group.add_argument(
+            '--no-preserve-ephemeral',
+            action='store_false',
+            dest='preserve_ephemeral',
+            help=_(
+                'Do not preserve the default ephemeral storage partition on '
+                'rebuild.'
+            ),
+        )
         key_group = parser.add_mutually_exclusive_group()
         key_group.add_argument(
             '--key-name',
@@ -2478,15 +2501,69 @@ class RebuildServer(command.ShowOne):
             ),
         )
         key_group.add_argument(
-            '--key-unset',
+            '--no-key-name',
             action='store_true',
-            default=False,
+            dest='no_key_name',
             help=_(
                 'Unset the key name of key pair on the rebuilt server. '
                 'Cannot be specified with the --key-name option. '
                 '(supported by --os-compute-api-version 2.54 or above)'
             ),
         )
+        # TODO(stephenfin): Remove this in a future major version bump
+        key_group.add_argument(
+            '--key-unset',
+            action='store_true',
+            dest='no_key_name',
+            help=argparse.SUPPRESS,
+        )
+        user_data_group = parser.add_mutually_exclusive_group()
+        user_data_group.add_argument(
+            '--user-data',
+            metavar='<user-data>',
+            help=_(
+                'Add a new user data file to the rebuilt server. '
+                'Cannot be specified with the --no-user-data option. '
+                '(supported by --os-compute-api-version 2.57 or above)'
+            ),
+        )
+        user_data_group.add_argument(
+            '--no-user-data',
+            action='store_true',
+            default=False,
+            help=_(
+                'Remove existing user data when rebuilding server. '
+                'Cannot be specified with the --user-data option. '
+                '(supported by --os-compute-api-version 2.57 or above)'
+            ),
+        )
+        trusted_certs_group = parser.add_mutually_exclusive_group()
+        trusted_certs_group.add_argument(
+            '--trusted-image-cert',
+            metavar='<trusted-cert-id>',
+            action='append',
+            dest='trusted_image_certs',
+            help=_(
+                'Trusted image certificate IDs used to validate certificates '
+                'during the image signature verification process. '
+                'Defaults to env[OS_TRUSTED_IMAGE_CERTIFICATE_IDS]. '
+                'May be specified multiple times to pass multiple trusted '
+                'image certificate IDs. '
+                'Cannot be specified with the --no-trusted-certs option. '
+                '(supported by --os-compute-api-version 2.63 or above)'
+            ),
+        )
+        trusted_certs_group.add_argument(
+            '--no-trusted-image-certs',
+            action='store_true',
+            default=False,
+            help=_(
+                'Remove any existing trusted image certificates from the '
+                'server. '
+                'Cannot be specified with the --trusted-certs option. '
+                '(supported by --os-compute-api-version 2.63 or above)'
+            ),
+        )
         parser.add_argument(
             '--wait',
             action='store_true',
@@ -2517,11 +2594,17 @@ class RebuildServer(command.ShowOne):
 
         kwargs = {}
 
+        if parsed_args.name is not None:
+            kwargs['name'] = parsed_args.name
+
+        if parsed_args.preserve_ephemeral is not None:
+            kwargs['preserve_ephemeral'] = parsed_args.preserve_ephemeral
+
         if parsed_args.property:
             kwargs['meta'] = parsed_args.property
 
         if parsed_args.description:
-            if server.api_version < api_versions.APIVersion("2.19"):
+            if compute_client.api_version < api_versions.APIVersion('2.19'):
                 msg = _(
                     '--os-compute-api-version 2.19 or greater is required to '
                     'support the --description option'
@@ -2539,7 +2622,7 @@ class RebuildServer(command.ShowOne):
                 raise exceptions.CommandError(msg)
 
             kwargs['key_name'] = parsed_args.key_name
-        elif parsed_args.key_unset:
+        elif parsed_args.no_key_name:
             if compute_client.api_version < api_versions.APIVersion('2.54'):
                 msg = _(
                     '--os-compute-api-version 2.54 or greater is required to '
@@ -2549,7 +2632,61 @@ class RebuildServer(command.ShowOne):
 
             kwargs['key_name'] = None
 
-        server = server.rebuild(image, parsed_args.password, **kwargs)
+        userdata = None
+        if parsed_args.user_data:
+            if compute_client.api_version < api_versions.APIVersion('2.54'):
+                msg = _(
+                    '--os-compute-api-version 2.54 or greater is required to '
+                    'support the --user-data option'
+                )
+                raise exceptions.CommandError(msg)
+
+            try:
+                userdata = io.open(parsed_args.user_data)
+            except IOError as e:
+                msg = _("Can't open '%(data)s': %(exception)s")
+                raise exceptions.CommandError(
+                    msg % {'data': parsed_args.user_data, 'exception': e}
+                )
+
+            kwargs['userdata'] = userdata
+        elif parsed_args.no_user_data:
+            if compute_client.api_version < api_versions.APIVersion('2.54'):
+                msg = _(
+                    '--os-compute-api-version 2.54 or greater is required to '
+                    'support the --no-user-data option'
+                )
+                raise exceptions.CommandError(msg)
+
+            kwargs['userdata'] = None
+
+        # TODO(stephenfin): Handle OS_TRUSTED_IMAGE_CERTIFICATE_IDS
+        if parsed_args.trusted_image_certs:
+            if compute_client.api_version < api_versions.APIVersion('2.63'):
+                msg = _(
+                    '--os-compute-api-version 2.63 or greater is required to '
+                    'support the --trusted-certs option'
+                )
+                raise exceptions.CommandError(msg)
+
+            certs = parsed_args.trusted_image_certs
+            kwargs['trusted_image_certificates'] = certs
+        elif parsed_args.no_trusted_image_certs:
+            if compute_client.api_version < api_versions.APIVersion('2.63'):
+                msg = _(
+                    '--os-compute-api-version 2.63 or greater is required to '
+                    'support the --no-trusted-certs option'
+                )
+                raise exceptions.CommandError(msg)
+
+            kwargs['trusted_image_certificates'] = None
+
+        try:
+            server = server.rebuild(image, parsed_args.password, **kwargs)
+        finally:
+            if userdata and hasattr(userdata, 'close'):
+                userdata.close()
+
         if parsed_args.wait:
             if utils.wait_for_status(
                 compute_client.servers.get,

openstackclient/tests/unit/compute/v2/test_server.py

@@ -4765,7 +4765,64 @@ class TestServerRebuild(TestServer):
         self.get_image_mock.assert_called_with(self.image.id)
         self.server.rebuild.assert_called_with(self.image, None)
 
-    def test_rebuild_with_current_image_and_password(self):
+    def test_rebuild_with_name(self):
+        name = 'test-server-xxx'
+        arglist = [
+            self.server.id,
+            '--name', name,
+        ]
+        verifylist = [
+            ('server', self.server.id),
+            ('name', name),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        # Get the command object to test
+        self.cmd.take_action(parsed_args)
+
+        self.servers_mock.get.assert_called_with(self.server.id)
+        self.get_image_mock.assert_called_with(self.image.id)
+        self.server.rebuild.assert_called_with(self.image, None, name=name)
+
+    def test_rebuild_with_preserve_ephemeral(self):
+        arglist = [
+            self.server.id,
+            '--preserve-ephemeral',
+        ]
+        verifylist = [
+            ('server', self.server.id),
+            ('preserve_ephemeral', True),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        # Get the command object to test
+        self.cmd.take_action(parsed_args)
+
+        self.servers_mock.get.assert_called_with(self.server.id)
+        self.get_image_mock.assert_called_with(self.image.id)
+        self.server.rebuild.assert_called_with(
+            self.image, None, preserve_ephemeral=True)
+
+    def test_rebuild_with_no_preserve_ephemeral(self):
+        arglist = [
+            self.server.id,
+            '--no-preserve-ephemeral',
+        ]
+        verifylist = [
+            ('server', self.server.id),
+            ('preserve_ephemeral', False),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        # Get the command object to test
+        self.cmd.take_action(parsed_args)
+
+        self.servers_mock.get.assert_called_with(self.server.id)
+        self.get_image_mock.assert_called_with(self.image.id)
+        self.server.rebuild.assert_called_with(
+            self.image, None, preserve_ephemeral=False)
+
+    def test_rebuild_with_password(self):
         password = 'password-xxx'
         arglist = [
             self.server.id,
@@ -4784,10 +4841,9 @@ class TestServerRebuild(TestServer):
         self.get_image_mock.assert_called_with(self.image.id)
         self.server.rebuild.assert_called_with(self.image, password)
 
-    def test_rebuild_with_description_api_older(self):
-
-        # Description is not supported for nova api version below 2.19
-        self.server.api_version = 2.18
+    def test_rebuild_with_description(self):
+        self.app.client_manager.compute.api_version = \
+            api_versions.APIVersion('2.19')
 
         description = 'description1'
         arglist = [
@@ -4800,39 +4856,33 @@ class TestServerRebuild(TestServer):
         ]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
 
-        with mock.patch.object(api_versions,
-                               'APIVersion',
-                               return_value=2.19):
-            self.assertRaises(exceptions.CommandError, self.cmd.take_action,
-                              parsed_args)
-
-    def test_rebuild_with_description_api_newer(self):
-
-        # Description is supported for nova api version 2.19 or above
-        self.server.api_version = 2.19
-
-        description = 'description1'
-        arglist = [
-            self.server.id,
-            '--description', description
-        ]
-        verifylist = [
-            ('server', self.server.id),
-            ('description', description)
-        ]
-        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
-
-        with mock.patch.object(api_versions,
-                               'APIVersion',
-                               return_value=2.19):
-            # Get the command object to test
-            self.cmd.take_action(parsed_args)
+        self.cmd.take_action(parsed_args)
 
         self.servers_mock.get.assert_called_with(self.server.id)
         self.get_image_mock.assert_called_with(self.image.id)
         self.server.rebuild.assert_called_with(self.image, None,
                                                description=description)
 
+    def test_rebuild_with_description_pre_v219(self):
+        self.app.client_manager.compute.api_version = \
+            api_versions.APIVersion('2.18')
+
+        description = 'description1'
+        arglist = [
+            self.server.id,
+            '--description', description
+        ]
+        verifylist = [
+            ('server', self.server.id),
+            ('description', description)
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        self.assertRaises(
+            exceptions.CommandError,
+            self.cmd.take_action,
+            parsed_args)
+
     @mock.patch.object(common_utils, 'wait_for_status', return_value=True)
     def test_rebuild_with_wait_ok(self, mock_wait_for_status):
         arglist = [
@@ -4907,6 +4957,9 @@ class TestServerRebuild(TestServer):
             self.image, None, meta=expected_property)
 
     def test_rebuild_with_keypair_name(self):
+        self.app.client_manager.compute.api_version = \
+            api_versions.APIVersion('2.54')
+
         self.server.key_name = 'mykey'
         arglist = [
             self.server.id,
@@ -4918,23 +4971,17 @@ class TestServerRebuild(TestServer):
         ]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
 
-        self.app.client_manager.compute.api_version = 2.54
-        with mock.patch.object(api_versions,
-                               'APIVersion',
-                               return_value=2.54):
-            self.cmd.take_action(parsed_args)
-            args = (
-                self.image,
-                None,
-            )
-            kwargs = dict(
-                key_name=self.server.key_name,
-            )
-            self.servers_mock.get.assert_called_with(self.server.id)
-            self.get_image_mock.assert_called_with(self.image.id)
-            self.server.rebuild.assert_called_with(*args, **kwargs)
+        self.cmd.take_action(parsed_args)
+
+        self.servers_mock.get.assert_called_with(self.server.id)
+        self.get_image_mock.assert_called_with(self.image.id)
+        self.server.rebuild.assert_called_with(
+            self.image, None, key_name=self.server.key_name)
+
+    def test_rebuild_with_keypair_name_pre_v254(self):
+        self.app.client_manager.compute.api_version = \
+            api_versions.APIVersion('2.53')
 
-    def test_rebuild_with_keypair_name_older_version(self):
         self.server.key_name = 'mykey'
         arglist = [
             self.server.id,
@@ -4946,55 +4993,230 @@ class TestServerRebuild(TestServer):
         ]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
 
-        self.app.client_manager.compute.api_version = 2.53
-        with mock.patch.object(api_versions,
-                               'APIVersion',
-                               return_value=2.54):
-            self.assertRaises(exceptions.CommandError,
-                              self.cmd.take_action,
-                              parsed_args)
+        self.assertRaises(
+            exceptions.CommandError,
+            self.cmd.take_action,
+            parsed_args)
+
+    def test_rebuild_with_no_keypair_name(self):
+        self.app.client_manager.compute.api_version = \
+            api_versions.APIVersion('2.54')
 
-    def test_rebuild_with_keypair_unset(self):
         self.server.key_name = 'mykey'
         arglist = [
             self.server.id,
-            '--key-unset',
+            '--no-key-name',
         ]
         verifylist = [
             ('server', self.server.id),
         ]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
 
-        self.app.client_manager.compute.api_version = 2.54
-        with mock.patch.object(api_versions,
-                               'APIVersion',
-                               return_value=2.54):
-            self.cmd.take_action(parsed_args)
-            args = (
-                self.image,
-                None,
-            )
-            kwargs = dict(
-                key_name=None,
-            )
-            self.servers_mock.get.assert_called_with(self.server.id)
-            self.get_image_mock.assert_called_with(self.image.id)
-            self.server.rebuild.assert_called_with(*args, **kwargs)
+        self.cmd.take_action(parsed_args)
+        self.servers_mock.get.assert_called_with(self.server.id)
+        self.get_image_mock.assert_called_with(self.image.id)
+        self.server.rebuild.assert_called_with(
+            self.image, None, key_name=None)
 
-    def test_rebuild_with_key_name_and_unset(self):
+    def test_rebuild_with_keypair_name_and_unset(self):
         self.server.key_name = 'mykey'
         arglist = [
             self.server.id,
             '--key-name', self.server.key_name,
-            '--key-unset',
+            '--no-key-name',
         ]
         verifylist = [
             ('server', self.server.id),
             ('key_name', self.server.key_name)
         ]
-        self.assertRaises(utils.ParserException,
-                          self.check_parser,
-                          self.cmd, arglist, verifylist)
+        self.assertRaises(
+            utils.ParserException,
+            self.check_parser,
+            self.cmd, arglist, verifylist)
+
+    @mock.patch('openstackclient.compute.v2.server.io.open')
+    def test_rebuild_with_user_data(self, mock_open):
+        self.app.client_manager.compute.api_version = \
+            api_versions.APIVersion('2.57')
+
+        mock_file = mock.Mock(name='File')
+        mock_open.return_value = mock_file
+        mock_open.read.return_value = '#!/bin/sh'
+
+        arglist = [
+            self.server.id,
+            '--user-data', 'userdata.sh',
+        ]
+        verifylist = [
+            ('server', self.server.id),
+            ('user_data', 'userdata.sh'),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        self.cmd.take_action(parsed_args)
+
+        # Ensure the userdata file is opened
+        mock_open.assert_called_with('userdata.sh')
+
+        # Ensure the userdata file is closed
+        mock_file.close.assert_called_with()
+
+        self.servers_mock.get.assert_called_with(self.server.id)
+        self.get_image_mock.assert_called_with(self.image.id)
+        self.server.rebuild.assert_called_with(
+            self.image, None,
+            userdata=mock_file,)
+
+    def test_rebuild_with_user_data_pre_257(self):
+        self.app.client_manager.compute.api_version = \
+            api_versions.APIVersion('2.56')
+
+        arglist = [
+            self.server.id,
+            '--user-data', 'userdata.sh',
+        ]
+        verifylist = [
+            ('server', self.server.id),
+            ('user_data', 'userdata.sh'),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        self.assertRaises(
+            exceptions.CommandError,
+            self.cmd.take_action,
+            parsed_args)
+
+    def test_rebuild_with_no_user_data(self):
+        self.app.client_manager.compute.api_version = \
+            api_versions.APIVersion('2.54')
+
+        self.server.key_name = 'mykey'
+        arglist = [
+            self.server.id,
+            '--no-user-data',
+        ]
+        verifylist = [
+            ('server', self.server.id),
+            ('no_user_data', True),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        self.cmd.take_action(parsed_args)
+        self.servers_mock.get.assert_called_with(self.server.id)
+        self.get_image_mock.assert_called_with(self.image.id)
+        self.server.rebuild.assert_called_with(
+            self.image, None, userdata=None)
+
+    def test_rebuild_with_no_user_data_pre_254(self):
+        self.app.client_manager.compute.api_version = \
+            api_versions.APIVersion('2.53')
+
+        arglist = [
+            self.server.id,
+            '--no-user-data',
+        ]
+        verifylist = [
+            ('server', self.server.id),
+            ('no_user_data', True),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        self.assertRaises(
+            exceptions.CommandError,
+            self.cmd.take_action,
+            parsed_args)
+
+    def test_rebuild_with_user_data_and_unset(self):
+        arglist = [
+            self.server.id,
+            '--user-data', 'userdata.sh',
+            '--no-user-data',
+        ]
+        self.assertRaises(
+            utils.ParserException,
+            self.check_parser,
+            self.cmd, arglist, None)
+
+    def test_rebuild_with_trusted_image_cert(self):
+        self.app.client_manager.compute.api_version = \
+            api_versions.APIVersion('2.63')
+
+        arglist = [
+            self.server.id,
+            '--trusted-image-cert', 'foo',
+            '--trusted-image-cert', 'bar',
+        ]
+        verifylist = [
+            ('server', self.server.id),
+            ('trusted_image_certs', ['foo', 'bar']),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        self.cmd.take_action(parsed_args)
+
+        self.servers_mock.get.assert_called_with(self.server.id)
+        self.get_image_mock.assert_called_with(self.image.id)
+        self.server.rebuild.assert_called_with(
+            self.image, None, trusted_image_certificates=['foo', 'bar'])
+
+    def test_rebuild_with_trusted_image_cert_pre_v263(self):
+        self.app.client_manager.compute.api_version = \
+            api_versions.APIVersion('2.62')
+
+        arglist = [
+            self.server.id,
+            '--trusted-image-cert', 'foo',
+            '--trusted-image-cert', 'bar',
+        ]
+        verifylist = [
+            ('server', self.server.id),
+            ('trusted_image_certs', ['foo', 'bar']),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        self.assertRaises(
+            exceptions.CommandError,
+            self.cmd.take_action,
+            parsed_args)
+
+    def test_rebuild_with_no_trusted_image_cert(self):
+        self.app.client_manager.compute.api_version = \
+            api_versions.APIVersion('2.63')
+
+        arglist = [
+            self.server.id,
+            '--no-trusted-image-certs',
+        ]
+        verifylist = [
+            ('server', self.server.id),
+            ('no_trusted_image_certs', True),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        self.cmd.take_action(parsed_args)
+        self.servers_mock.get.assert_called_with(self.server.id)
+        self.get_image_mock.assert_called_with(self.image.id)
+        self.server.rebuild.assert_called_with(
+            self.image, None, trusted_image_certificates=None)
+
+    def test_rebuild_with_no_trusted_image_cert_pre_257(self):
+        self.app.client_manager.compute.api_version = \
+            api_versions.APIVersion('2.62')
+
+        arglist = [
+            self.server.id,
+            '--no-trusted-image-certs',
+        ]
+        verifylist = [
+            ('server', self.server.id),
+            ('no_trusted_image_certs', True),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        self.assertRaises(
+            exceptions.CommandError,
+            self.cmd.take_action,
+            parsed_args)
 
 
 class TestEvacuateServer(TestServer):

releasenotes/notes/add-missing-server-rebuild-opts-5c75e838d8f0487d.yaml

@@ -0,0 +1,13 @@
+---
+features:
+  - |
+    Add a number of additional options to the ``server rebuild`` command:
+
+    - ``--name``
+    - ``--preserve-ephemeral``, ``--no-preserve-ephemeral``
+    - ``--user-data``, ``--no-user-data``
+    - ``--trusted-image-cert``, ``--no-trusted-image-certs``
+upgrade:
+  - |
+    The ``--key-unset`` option of the ``server rebuild`` command has been
+    replaced by ``--no-key-name``. An alias is provided.