From 80ee69aa30d36e941547a9b67668adfaf57355fd Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Wed, 1 Sep 2021 12:18:43 +0100 Subject: trivial: Remove use of kwargs Make use of a "sentinel" object to allow us to remove the use of kwargs and provide a more helpful docstring. With any luck, Python will support these objects natively in a future release [1]. [1] https://www.python.org/dev/peps/pep-0661/ Change-Id: I411c0393754c8fe8a6698f0d278b73f12209ace8 Signed-off-by: Stephen Finucane --- novaclient/v2/servers.py | 411 +++++++++++++++++++++++++++++++---------------- novaclient/v2/shell.py | 13 +- 2 files changed, 278 insertions(+), 146 deletions(-) diff --git a/novaclient/v2/servers.py b/novaclient/v2/servers.py index 8a9301f6..e564e818 100644 --- a/novaclient/v2/servers.py +++ b/novaclient/v2/servers.py @@ -29,6 +29,7 @@ from novaclient import crypto from novaclient import exceptions from novaclient.i18n import _ +_SENTINEL = object() REBOOT_SOFT, REBOOT_HARD = 'SOFT', 'HARD' @@ -404,34 +405,90 @@ class Server(base.Resource): """ return self.manager.reboot(self, reboot_type) - def rebuild(self, image, password=None, preserve_ephemeral=False, - **kwargs): + # NOTE(stephenfin): It would be nice to make everything bar image a + # kwarg-only argument but there are backwards-compatbility concerns + def rebuild( + self, + image, + password=None, + preserve_ephemeral=False, + *, + disk_config=None, + name=None, + meta=None, + files=None, + description=_SENTINEL, + key_name=_SENTINEL, + userdata=_SENTINEL, + trusted_image_certificates=_SENTINEL, + hostname=_SENTINEL, + ): """ Rebuild -- shut down and then re-image -- this server. - :param image: the :class:`Image` (or its ID) to re-image with. - :param password: string to set as the admin password on the rebuilt - server. + :param image: The :class:`Image` (or its ID) to re-image with. + :param password: String to set as password on the rebuilt server. :param preserve_ephemeral: If True, request that any ephemeral device be preserved when rebuilding the instance. Defaults to False. + :param disk_config: Partitioning mode to use on the rebuilt server. + Valid values are 'AUTO' or 'MANUAL' + :param name: Something to name the server. + :param meta: A dict of arbitrary key/value metadata to store for this + server. Both keys and values must be <=255 characters. + :param files: A dict of files to overwrite on the server upon boot. + Keys are file names (i.e. ``/etc/passwd``) and values are the file + contents (either as a string or as a file-like object). A maximum + of five entries is allowed, and each file must be 10k or less. + (deprecated starting with microversion 2.57) + :param description: Optional description of the server. If None is + specified, the existing description will be unset. + (starting from microversion 2.19) + :param key_name: Optional key pair name for rebuild operation. If None + is specified, the existing key will be unset. + (starting from microversion 2.54) + :param userdata: Optional user data to pass to be exposed by the + metadata server; this can be a file type object as well or a + string. If None is specified, the existing user_data is unset. + (starting from microversion 2.57) + :param trusted_image_certificates: A list of trusted certificate IDs + or None to unset/reset the servers trusted image certificates + (starting from microversion 2.63) + :param hostname: Optional hostname to configure for the instance. If + None is specified, the existing hostname will be unset. + (starting from microversion 2.90) + :returns: :class:`Server` """ - return self.manager.rebuild(self, image, password=password, - preserve_ephemeral=preserve_ephemeral, - **kwargs) + return self.manager.rebuild( + self, + image, + password=password, + disk_config=disk_config, + preserve_ephemeral=preserve_ephemeral, + name=name, + meta=meta, + files=files, + description=description, + key_name=key_name, + userdata=userdata, + trusted_image_certificates=trusted_image_certificates, + hostname=hostname, + ) - def resize(self, flavor, **kwargs): + def resize(self, flavor, *, disk_config=None): """ Resize the server's resources. - :param flavor: the :class:`Flavor` (or its ID) to resize to. + :param flavor: The :class:`Flavor` (or its ID) to resize to. + :param disk_config: Partitioning mode to use on the rebuilt server. + Valid values are 'AUTO' or 'MANUAL'. :returns: An instance of novaclient.base.TupleWithMeta Until a resize event is confirmed with :meth:`confirm_resize`, the old server will be kept around and you'll be able to roll back to the old flavor quickly with :meth:`revert_resize`. All resizes are - automatically confirmed after 24 hours. + automatically confirmed after 24 hours by default. """ - return self.manager.resize(self, flavor, **kwargs) + return self.manager.resize(self, flavor, disk_config=disk_config) def create_image(self, image_name, metadata=None): """ @@ -743,16 +800,38 @@ class ServerManager(base.BootingManagerWithFind): return base64.b64encode(userdata).decode('utf-8') - def _boot(self, response_key, name, image, flavor, - meta=None, files=None, userdata=None, - reservation_id=False, return_raw=False, min_count=None, - max_count=None, security_groups=None, key_name=None, - availability_zone=None, block_device_mapping=None, - block_device_mapping_v2=None, nics=None, scheduler_hints=None, - config_drive=None, admin_pass=None, disk_config=None, - access_ip_v4=None, access_ip_v6=None, description=None, - tags=None, trusted_image_certificates=None, - host=None, hypervisor_hostname=None, hostname=None, **kwargs): + def _boot( + self, + response_key, + name, + image, + flavor, + meta=None, + files=None, + userdata=None, + reservation_id=False, + return_raw=False, + min_count=None, + max_count=None, + security_groups=None, + key_name=None, + availability_zone=None, + block_device_mapping=None, + block_device_mapping_v2=None, + nics=None, + scheduler_hints=None, + config_drive=None, + admin_pass=None, + disk_config=None, + access_ip_v4=None, + access_ip_v6=None, + description=None, + tags=None, + trusted_image_certificates=None, + host=None, + hypervisor_hostname=None, + hostname=None, + ): """ Create (boot) a new server. """ @@ -884,8 +963,9 @@ class ServerManager(base.BootingManagerWithFind): if hostname: body['server']['hostname'] = hostname - return self._create('/servers', body, response_key, - return_raw=return_raw, **kwargs) + return self._create( + '/servers', body, response_key, return_raw=return_raw, + ) def get(self, server): """ @@ -1390,17 +1470,39 @@ class ServerManager(base.BootingManagerWithFind): raise ValueError('nics must be a list or a tuple, not %s' % type(nics)) - def create(self, name, image, flavor, meta=None, files=None, - reservation_id=False, min_count=None, - max_count=None, security_groups=None, userdata=None, - key_name=None, availability_zone=None, - block_device_mapping=None, block_device_mapping_v2=None, - nics=None, scheduler_hints=None, - config_drive=None, disk_config=None, admin_pass=None, - access_ip_v4=None, access_ip_v6=None, - trusted_image_certificates=None, - host=None, hypervisor_hostname=None, hostname=None, - **kwargs): + # NOTE(stephenfin): It would be nice to make everything bar name, image and + # flavor a kwarg-only argument but there are backwards-compatbility + # concerns + def create( + self, + name, + image, + flavor, + meta=None, + files=None, + reservation_id=False, + min_count=None, + max_count=None, + security_groups=None, + userdata=None, + key_name=None, + availability_zone=None, + block_device_mapping=None, + block_device_mapping_v2=None, + nics=None, + scheduler_hints=None, + config_drive=None, + disk_config=None, + admin_pass=None, + access_ip_v4=None, + access_ip_v6=None, + description=None, + tags=None, + trusted_image_certificates=None, + host=None, + hypervisor_hostname=None, + hostname=None, + ): """ Create (boot) a new server. @@ -1412,65 +1514,59 @@ class ServerManager(base.BootingManagerWithFind): :param image: The :class:`Image` to boot with. :param flavor: The :class:`Flavor` to boot onto. :param meta: A dict of arbitrary key/value metadata to store for this - server. Both keys and values must be <=255 characters. + server. Both keys and values must be <=255 characters. :param files: A dict of files to overwrite on the server upon boot. - Keys are file names (i.e. ``/etc/passwd``) and values - are the file contents (either as a string or as a - file-like object). A maximum of five entries is allowed, - and each file must be 10k or less. - (deprecated starting with microversion 2.57) - :param reservation_id: return a reservation_id for the set of - servers being requested, boolean. - :param min_count: (optional extension) The minimum number of - servers to launch. - :param max_count: (optional extension) The maximum number of - servers to launch. + Keys are file names (i.e. ``/etc/passwd``) and values + are the file contents (either as a string or as a + file-like object). A maximum of five entries is allowed, + and each file must be 10k or less. + (deprecated starting with microversion 2.57) + :param reservation_id: Return a reservation_id for the set of + servers being requested, boolean. + :param min_count: The minimum number of servers to launch. + :param max_count: The maximum number of servers to launch. :param security_groups: A list of security group names - :param userdata: user data to pass to be exposed by the metadata - server this can be a file type object as well or a - string. - :param key_name: (optional extension) name of previously created - keypair to inject into the instance. + :param userdata: User data to pass to be exposed by the metadata + server this can be a file type object as well or a string. + :param key_name: Name of previously created keypair to inject into the + instance. :param availability_zone: Name of the availability zone for instance - placement. - :param block_device_mapping: (optional extension) A dict of block - device mappings for this server. - :param block_device_mapping_v2: (optional extension) A list of block - device mappings (dicts) for this server. - :param nics: An ordered list of nics (dicts) to be added to this - server, with information about connected networks, - fixed IPs, port etc. - Beginning in microversion 2.37 this field is required and - also supports a single string value of 'auto' or 'none'. - The 'auto' value means the Compute service will - automatically allocate a network for the project if one - is not available. This is the same behavior as not - passing anything for nics before microversion 2.37. The - 'none' value tells the Compute service to not allocate - any networking for the server. - :param scheduler_hints: (optional extension) arbitrary key-value pairs - specified by the client to help boot an instance - :param config_drive: (optional extension) a boolean value to enable - config drive - :param disk_config: (optional extension) control how the disk is - partitioned when the server is created. possible - values are 'AUTO' or 'MANUAL'. - :param admin_pass: (optional extension) add a user supplied admin - password. - :param access_ip_v4: (optional extension) add alternative access ip v4 - :param access_ip_v6: (optional extension) add alternative access ip v6 - :param description: optional description of the server (allowed since - microversion 2.19) - :param tags: A list of arbitrary strings to be added to the - server as tags (allowed since microversion 2.52) + placement. + :param block_device_mapping: A dict of block device mappings for this + server. + :param block_device_mapping_v2: A list of block device mappings (dicts) + for this server. + :param nics: An ordered list of nics (dicts) to be added to this + server, with information about connected networks, fixed IPs, port + etc. Beginning in microversion 2.37 this field is required and also + supports a single string value of 'auto' or 'none'. The 'auto' + value means the Compute service will automatically allocate a + network for the project if one is not available. This is the same + behavior as not passing anything for nics before microversion 2.37. + The 'none' value tells the Compute service to not allocate any + networking for the server. + :param scheduler_hints: Arbitrary key-value pairs specified by the + client to help boot an instance. + :param config_drive: A boolean value to enable config drive. + :param disk_config: Control how the disk is partitioned when the server + is created. Possible values are 'AUTO' or 'MANUAL'. + :param admin_pass: Add a user supplied admin password. + :param access_ip_v4: Add alternative access IP (v4) + :param access_ip_v6: Add alternative access IP (v6) + :param description: Optional description of the server + (allowed since microversion 2.19) + :param tags: A list of arbitrary strings to be added to the server as + tags + (allowed since microversion 2.52) :param trusted_image_certificates: A list of trusted certificate IDs - (allowed since microversion 2.63) - :param host: requested host to create servers - (allowed since microversion 2.74) - :param hypervisor_hostname: requested hypervisor hostname to create - servers (allowed since microversion 2.74) - :param hostname: requested hostname of server (allowed since - microversion 2.90) + (allowed since microversion 2.63) + :param host: Requested host to create servers + (allowed since microversion 2.74) + :param hypervisor_hostname: Requested hypervisor hostname to create + servers + (allowed since microversion 2.74) + :param hostname: Requested hostname of server + (allowed since microversion 2.90) """ if not min_count: min_count = 1 @@ -1481,8 +1577,7 @@ class ServerManager(base.BootingManagerWithFind): boot_args = [name, image, flavor] - descr_microversion = api_versions.APIVersion("2.19") - if "description" in kwargs and self.api_version < descr_microversion: + if description and self.api_version < api_versions.APIVersion("2.19"): raise exceptions.UnsupportedAttribute("description", "2.19") self._validate_create_nics(nics) @@ -1503,8 +1598,7 @@ class ServerManager(base.BootingManagerWithFind): "unsupported before microversion " "2.32") - boot_tags_microversion = api_versions.APIVersion("2.52") - if "tags" in kwargs and self.api_version < boot_tags_microversion: + if tags and self.api_version < api_versions.APIVersion("2.52"): raise exceptions.UnsupportedAttribute("tags", "2.52") personality_files_deprecation = api_versions.APIVersion('2.57') @@ -1546,9 +1640,10 @@ class ServerManager(base.BootingManagerWithFind): scheduler_hints=scheduler_hints, config_drive=config_drive, disk_config=disk_config, admin_pass=admin_pass, access_ip_v4=access_ip_v4, access_ip_v6=access_ip_v6, + description=description, tags=tags, trusted_image_certificates=trusted_image_certificates, host=host, hypervisor_hostname=hypervisor_hostname, - hostname=hostname, **kwargs) + hostname=hostname) if block_device_mapping: boot_kwargs['block_device_mapping'] = block_device_mapping @@ -1665,10 +1760,25 @@ class ServerManager(base.BootingManagerWithFind): """ return self._action('reboot', server, {'type': reboot_type}) - # TODO(stephenfin): Expand out kwargs - def rebuild(self, server, image, password=None, disk_config=None, - preserve_ephemeral=False, name=None, meta=None, files=None, - **kwargs): + # NOTE(stephenfin): It would be nice to make everything bar server and + # image a kwarg-only argument but there are backwards-compatbility concerns + def rebuild( + self, + server, + image, + password=None, + disk_config=None, + preserve_ephemeral=False, + name=None, + meta=None, + files=None, + *, + description=_SENTINEL, + key_name=_SENTINEL, + userdata=_SENTINEL, + trusted_image_certificates=_SENTINEL, + hostname=_SENTINEL, + ): """ Rebuild -- shut down and then re-image -- a server. @@ -1705,63 +1815,80 @@ class ServerManager(base.BootingManagerWithFind): (starting from microversion 2.90) :returns: :class:`Server` """ - descr_microversion = api_versions.APIVersion("2.19") - if "description" in kwargs and self.api_version < descr_microversion: - raise exceptions.UnsupportedAttribute("description", "2.19") + # Microversion 2.19 adds the optional 'description' parameter + if ( + description is not _SENTINEL and + self.api_version < api_versions.APIVersion('2.19') + ): + raise exceptions.UnsupportedAttribute('description', '2.19') - # Starting from microversion 2.54, - # the optional 'key_name' parameter has been added. - if ('key_name' in kwargs and - self.api_version < api_versions.APIVersion('2.54')): + # Microversion 2.54 adds the optional 'key_name' parameter + if ( + key_name is not _SENTINEL and + self.api_version < api_versions.APIVersion('2.54') + ): raise exceptions.UnsupportedAttribute('key_name', '2.54') # Microversion 2.57 deprecates personality files and adds support # for user_data. - files_and_userdata = api_versions.APIVersion('2.57') - if files and self.api_version >= files_and_userdata: + if files and self.api_version >= api_versions.APIVersion('2.57'): raise exceptions.UnsupportedAttribute('files', '2.0', '2.56') - if 'userdata' in kwargs and self.api_version < files_and_userdata: + + if ( + userdata is not _SENTINEL and + self.api_version < api_versions.APIVersion('2.57') + ): raise exceptions.UnsupportedAttribute('userdata', '2.57') - trusted_certs_microversion = api_versions.APIVersion("2.63") - # trusted_image_certificates is intentionally *not* a named kwarg - # so that trusted_image_certificates=None is not confused with an - # intentional unset/reset request. - if ("trusted_image_certificates" in kwargs and - self.api_version < trusted_certs_microversion): - raise exceptions.UnsupportedAttribute("trusted_image_certificates", - "2.63") + # Microversion 2.63 adds trusted image certificate support + if ( + trusted_image_certificates is not _SENTINEL and + self.api_version < api_versions.APIVersion('2.63') + ): + raise exceptions.UnsupportedAttribute( + 'trusted_image_certificates', '2.63') + # Microversion 2.90 adds the optional 'hostname' parameter if ( - 'hostname' in kwargs and - self.api_version < api_versions.APIVersion("2.90") + hostname is not _SENTINEL and + self.api_version < api_versions.APIVersion('2.90') ): raise exceptions.UnsupportedAttribute('hostname', '2.90') body = {'imageRef': base.getid(image)} + if password is not None: body['adminPass'] = password + if disk_config is not None: body['OS-DCF:diskConfig'] = disk_config + if preserve_ephemeral is not False: body['preserve_ephemeral'] = True + if name is not None: body['name'] = name - if "description" in kwargs: - body["description"] = kwargs["description"] - if 'key_name' in kwargs: - body['key_name'] = kwargs['key_name'] - if "trusted_image_certificates" in kwargs: - body["trusted_image_certificates"] = kwargs[ - "trusted_image_certificates"] - if "hostname" in kwargs: - body["hostname"] = kwargs["hostname"] + + if description is not _SENTINEL: + body["description"] = description + + if key_name is not _SENTINEL: + body['key_name'] = key_name + + if trusted_image_certificates is not _SENTINEL: + body["trusted_image_certificates"] = trusted_image_certificates + + if hostname is not _SENTINEL: + body["hostname"] = hostname + if meta: body['metadata'] = meta + if files: personality = body['personality'] = [] - for filepath, file_or_string in sorted(files.items(), - key=lambda x: x[0]): + for filepath, file_or_string in sorted( + files.items(), key=lambda x: x[0], + ): if hasattr(file_or_string, 'read'): data = file_or_string.read() else: @@ -1772,15 +1899,17 @@ class ServerManager(base.BootingManagerWithFind): 'path': filepath, 'contents': cont, }) - if 'userdata' in kwargs: + + if userdata is not _SENTINEL: # If userdata is specified but None, it means unset the existing # user_data on the instance. - userdata = kwargs['userdata'] + userdata = userdata body['user_data'] = (userdata if userdata is None else self.transform_userdata(userdata)) - resp, body = self._action_return_resp_and_body('rebuild', server, - body, **kwargs) + resp, body = self._action_return_resp_and_body( + 'rebuild', server, body, + ) return Server(self, body['server'], resp=resp) @api_versions.wraps("2.0", "2.55") @@ -1809,26 +1938,28 @@ class ServerManager(base.BootingManagerWithFind): return self._action('migrate', server, info) - def resize(self, server, flavor, disk_config=None, **kwargs): + # NOTE(stephenfin): It would be nice to make disk_config a kwarg-only + # argument but there are backwards-compatbility concerns + def resize(self, server, flavor, disk_config=None): """ Resize a server's resources. :param server: The :class:`Server` (or its ID) to share onto. - :param flavor: the :class:`Flavor` (or its ID) to resize to. - :param disk_config: partitioning mode to use on the rebuilt server. - Valid values are 'AUTO' or 'MANUAL' + :param flavor: The :class:`Flavor` (or its ID) to resize to. + :param disk_config: Partitioning mode to use on the rebuilt server. + Valid values are 'AUTO' or 'MANUAL'. :returns: An instance of novaclient.base.TupleWithMeta Until a resize event is confirmed with :meth:`confirm_resize`, the old server will be kept around and you'll be able to roll back to the old flavor quickly with :meth:`revert_resize`. All resizes are - automatically confirmed after 24 hours. + automatically confirmed after 24 hours by default. """ info = {'flavorRef': base.getid(flavor)} if disk_config is not None: info['OS-DCF:diskConfig'] = disk_config - return self._action('resize', server, info=info, **kwargs) + return self._action('resize', server, info=info) def confirm_resize(self, server): """ diff --git a/novaclient/v2/shell.py b/novaclient/v2/shell.py index 58823fc7..0ca9e794 100644 --- a/novaclient/v2/shell.py +++ b/novaclient/v2/shell.py @@ -2057,14 +2057,15 @@ def do_rebuild(cs, args): server = _find_server(cs, args.server) image = _find_image(cs, args.image) - if args.rebuild_password is not False: - _password = args.rebuild_password - else: - _password = None - kwargs = {'preserve_ephemeral': args.preserve_ephemeral, 'name': args.name, 'meta': _meta_parsing(args.meta)} + + if args.rebuild_password is not False: + kwargs['password'] = args.rebuild_password + else: + kwargs['password'] = None + if 'description' in args: kwargs['description'] = args.description @@ -2145,7 +2146,7 @@ def do_rebuild(cs, args): if 'hostname' in args and args.hostname is not None: kwargs['hostname'] = args.hostname - server = server.rebuild(image, _password, **kwargs) + server = server.rebuild(image, **kwargs) _print_server(cs, args, server) if args.poll: -- cgit v1.2.1