From 4dad7b2e693f02da2fe9f1dc64af51b3c4f891dc Mon Sep 17 00:00:00 2001 From: Monty Taylor Date: Mon, 17 Oct 2016 11:19:04 -0500 Subject: [PATCH] Document and be more explicit in normalization Put extra keys in both the root resource and in a properties dict. Ensure data types are correct. Make sure int, float and bool values are returned as int and bool. Change disabled in flavor to is_disabled for consistency with other bools we've added. There has been no release with the addition of disabled, so changing it now is still safe. Add locations and direct_url to images. They're optional in glance, but that's evil. Let image schema attribute fall through to extra properties. Add zone to current_location. Add readable mappings for power_state, task_state, vm_state, launched_at and terminated_at for Servers. Also add a non-camel-cased host_id. This is a big patch, but it's mostly just reorganizing and adding docs. Looking at the changes to the tests and seeing that the only change is adding zone and properties into a couple of fixtures is a good place to start. Change-Id: If5674c049c8dd85ca0b3483b7c2dc82b9e139bd6 --- doc/source/index.rst | 1 + doc/source/model.rst | 203 +++++++++++++ .../notes/data-model-cf50d86982646370.yaml | 8 + shade/_normalize.py | 287 ++++++++++++++---- shade/openstackcloud.py | 3 +- shade/tests/unit/test__utils.py | 6 + shade/tests/unit/test_meta.py | 3 +- 7 files changed, 445 insertions(+), 66 deletions(-) create mode 100644 doc/source/model.rst create mode 100644 releasenotes/notes/data-model-cf50d86982646370.yaml diff --git a/doc/source/index.rst b/doc/source/index.rst index 2aa8bd35e..ad8969530 100644 --- a/doc/source/index.rst +++ b/doc/source/index.rst @@ -13,6 +13,7 @@ Contents: installation usage + model contributing coding future diff --git a/doc/source/model.rst b/doc/source/model.rst new file mode 100644 index 000000000..3b50641df --- /dev/null +++ b/doc/source/model.rst @@ -0,0 +1,203 @@ +========== +Data Model +========== + +shade has a very strict policy on not breaking backwards compatability ever. +However, with the data structures returned from OpenStack, there are places +where the resource structures from OpenStack are returned to the user somewhat +directly, leaving a shade user open to changes/differences in result content. + +To combat that, shade 'normalizes' the return structure from OpenStack in many +places, and the results of that normalization are listed below. Where shade +performs normalization, a user can count on any fields declared in the docs +as being completely safe to use - they are as much a part of shade's API +contract as any other Python method. + +Some OpenStack objects allow for arbitrary attributes at +the root of the object. shade will pass those through so as not to break anyone +who may be counting on them, but as they are arbitrary shade can make no +guarantees as to their existence. As part of normalization, shade will put any +attribute from an OpenStack resource that is not in its data model contract +into an attribute called 'properties'. The contents of properties are +defined to be an arbitrary collection of key value pairs with no promises as +to any particular key ever existing. + +Location +-------- + +A Location defines where a resource lives. It includes a cloud name and a +region name, an availability zone as well as information about the project +that owns the resource. + +The project information may contain a project id, or a combination of one or +more of a project name with a domain name or id. If a project id is present, +it should be considered correct. + +Some resources do not carry ownership information with them. For those, the +project information will be filled in from the project the user currently +has a token for. + +Some resources do not have information about availability zones, or may exist +region wide. Those resources will have None as their availability zone. + +If all of the project information is None, then + +.. code-block:: python + + Location = dict( + cloud=str(), + region=str(), + zone=str() or None, + project=dict( + id=str() or None, + name=str() or None, + domain_id=str() or None, + domain_name=str() or None)) + + +Flavor +------ + +A flavor for a Nova Server. + +.. code-block:: python + + Flavor = dict( + location=Location(), + id=str(), + name=str(), + is_public=bool(), + is_disabled=bool(), + ram=int(), + vcpus=int(), + disk=int(), + ephemeral=int(), + swap=int(), + rxtx_factor=float(), + extra_specs=dict(), + properties=dict()) + +Image +----- + +A Glance Image. + +.. code-block:: python + + Image = dict( + location=Location(), + id=str(), + name=str(), + min_ram=int(), + min_disk=int(), + size=int(), + virtual_size=int(), + container_format=str(), + disk_format=str(), + checksum=str(), + created_at=str(), + updated_at=str(), + owner=str(), + is_public=bool(), + is_protected=bool(), + status=str(), + locations=list(), + direct_url=str() or None, + tags=list(), + properties=dict()) + +Security Group +-------------- + +A Security Group from either Nova or Neutron + +.. code-block:: python + + SecurityGroup = dict( + location=Location(), + id=str(), + name=str(), + description=str(), + security_group_rules=list(), + properties=dict()) + +Security Group Rule +------------------- + +A Security Group Rule from either Nova or Neutron + +.. code-block:: python + + SecurityGroupRule = dict( + location=Location(), + id=str(), + direction=str(), # oneof('ingress', 'egress') + ethertype=str(), + port_range_min=int() or None, + port_range_max=int() or None, + protocol=str() or None, + remote_ip_prefix=str() or None, + security_group_id=str() or None, + remote_group_id=str() or None + properties=dict()) + +Server +------ + +A Server from Nova + +.. code-block:: python + + Server = dict( + location=Location(), + id=str(), + name=str(), + image=dict() or str(), + flavor=dict(), + volumes=list(), + interface_ip=str(), + has_config_drive=bool(), + accessIPv4=str(), + accessIPv6=str(), + addresses=dict(), + created=str(), + key_name=str(), + metadata=dict(), + networks=dict(), + private_v4=str(), + progress=int(), + public_v4=str(), + public_v6=str(), + security_groups=list(), + status=str(), + updated=str(), + user_id=str(), + host_id=str() or None, + power_state=str() or None, + task_state=str() or None, + vm_state=str() or None, + launched_at=str() or None, + terminated_at=str() or None, + task_state=str() or None, + properties=dict()) + +Floating IP +----------- + +A Floating IP from Neutron or Nova + + +.. code-block:: python + + FloatingIP = dict( + location=Location(), + id=str(), + attached=bool(), + fixed_ip_address=str() or None, + floating_ip_address=str() or None, + floating_network_id=str() or None, + network=str(), + port_id=str() or None, + router_id=str(), + status=str(), + properties=dict()) diff --git a/releasenotes/notes/data-model-cf50d86982646370.yaml b/releasenotes/notes/data-model-cf50d86982646370.yaml new file mode 100644 index 000000000..66a814aae --- /dev/null +++ b/releasenotes/notes/data-model-cf50d86982646370.yaml @@ -0,0 +1,8 @@ +--- +features: + - Explicit data model contracts are now defined for + Flavors, Images, Security Groups, Security Group Rules, + and Servers. + - Resources with data model contracts are now being returned with + 'location' attribute. The location carries cloud name, region + name and information about the project that owns the resource. diff --git a/shade/_normalize.py b/shade/_normalize.py index fbcc3ef30..24dbbdfaf 100644 --- a/shade/_normalize.py +++ b/shade/_normalize.py @@ -12,12 +12,16 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. +import ast + import munch +import six _IMAGE_FIELDS = ( 'checksum', 'container_format', 'created_at', + 'direct_url', 'disk_format', 'file', 'id', @@ -25,8 +29,6 @@ _IMAGE_FIELDS = ( 'min_ram', 'name', 'owner', - 'protected', - 'schema', 'size', 'status', 'tags', @@ -34,6 +36,41 @@ _IMAGE_FIELDS = ( 'virtual_size', ) +_SERVER_FIELDS = ( + 'accessIPv4', + 'accessIPv6', + 'addresses', + 'adminPass', + 'created', + 'key_name', + 'metadata', + 'networks', + 'private_v4', + 'public_v4', + 'public_v6', + 'security_groups', + 'status', + 'updated', + 'user_id', +) + + +def _to_bool(value): + if isinstance(value, six.string_types): + # ast.literal_eval becomes VERY unhappy on empty strings + if not value: + return False + return ast.literal_eval(value.lower().capitalize()) + return bool(value) + + +def _pop_int(resource, key): + return int(resource.pop(key, 0) or 0) + + +def _pop_float(resource, key): + return float(resource.pop(key, 0) or 0) + class Normalizer(object): '''Mix-in class to provide the normalization functions. @@ -51,25 +88,47 @@ class Normalizer(object): def _normalize_flavor(self, flavor): """ Normalize a flavor object """ + new_flavor = munch.Munch() + + # Copy incoming group because of shared dicts in unittests + flavor = flavor.copy() + + # Discard noise flavor.pop('links', None) flavor.pop('NAME_ATTR', None) flavor.pop('HUMAN_ID', None) flavor.pop('human_id', None) - if 'extra_specs' not in flavor: - flavor['extra_specs'] = {} - ephemeral = flavor.pop('OS-FLV-EXT-DATA:ephemeral', 0) - is_public = flavor.pop('os-flavor-access:is_public', True) - disabled = flavor.pop('OS-FLV-DISABLED:disabled', False) - # Make sure both the extension version and a sane version are present - flavor['OS-FLV-DISABLED:disabled'] = disabled - flavor['disabled'] = disabled - flavor['OS-FLV-EXT-DATA:ephemeral'] = ephemeral - flavor['ephemeral'] = ephemeral - flavor['os-flavor-access:is_public'] = is_public - flavor['is_public'] = is_public - flavor['location'] = self.current_location - return flavor + ephemeral = int(flavor.pop('OS-FLV-EXT-DATA:ephemeral', 0)) + ephemeral = flavor.pop('ephemeral', ephemeral) + is_public = _to_bool(flavor.pop('os-flavor-access:is_public', True)) + is_public = _to_bool(flavor.pop('is_public', True)) + is_disabled = _to_bool(flavor.pop('OS-FLV-DISABLED:disabled', False)) + extra_specs = flavor.pop('extra_specs', {}) + + new_flavor['location'] = self.current_location + new_flavor['id'] = flavor.pop('id') + new_flavor['name'] = flavor.pop('name') + new_flavor['is_public'] = is_public + new_flavor['is_disabled'] = is_disabled + new_flavor['ram'] = _pop_int(flavor, 'ram') + new_flavor['vcpus'] = _pop_int(flavor, 'vcpus') + new_flavor['disk'] = _pop_int(flavor, 'disk') + new_flavor['ephemeral'] = ephemeral + new_flavor['swap'] = _pop_int(flavor, 'swap') + new_flavor['rxtx_factor'] = _pop_float(flavor, 'rxtx_factor') + + new_flavor['properties'] = flavor.copy() + new_flavor['extra_specs'] = extra_specs + + # Backwards compat with nova - passthrough values + for (k, v) in new_flavor['properties'].items(): + new_flavor.setdefault(k, v) + new_flavor['OS-FLV-DISABLED:disabled'] = is_disabled + new_flavor['OS-FLV-EXT-DATA:ephemeral'] = ephemeral + new_flavor['os-flavor-access:is_public'] = is_public + + return new_flavor def _normalize_images(self, images): ret = [] @@ -80,8 +139,11 @@ class Normalizer(object): def _normalize_image(self, image): new_image = munch.Munch( location=self._get_current_location(project_id=image.get('owner'))) + properties = image.pop('properties', {}) visibility = image.pop('visibility', None) + protected = _to_bool(image.pop('protected', False)) + if visibility: is_public = (visibility == 'public') else: @@ -90,12 +152,21 @@ class Normalizer(object): for field in _IMAGE_FIELDS: new_image[field] = image.pop(field, None) + for field in ('min_ram', 'min_disk', 'size', 'virtual_size'): + new_image[field] = _pop_int(new_image, field) + new_image['is_protected'] = protected + new_image['locations'] = image.pop('locations', []) + for key, val in image.items(): - properties[key] = val - new_image[key] = val + properties.setdefault(key, val) new_image['properties'] = properties new_image['visibility'] = visibility new_image['is_public'] = is_public + + # Backwards compat with glance + for key, val in properties.items(): + new_image[key] = val + new_image['protected'] = protected return new_image def _normalize_secgroups(self, groups): @@ -116,16 +187,29 @@ class Normalizer(object): def _normalize_secgroup(self, group): - rules = group.pop('security_group_rules', None) - if not rules and 'rules' in group: - rules = group.pop('rules') - group['security_group_rules'] = self._normalize_secgroup_rules(rules) - project_id = group.get('project_id', group.get('tenant_id', '')) - group['location'] = self._get_current_location(project_id=project_id) - # neutron sets these. we don't care about it, but let's be the same - group['tenant_id'] = project_id - group['project_id'] = project_id - return munch.Munch(group) + ret = munch.Munch() + # Copy incoming group because of shared dicts in unittests + group = group.copy() + + rules = self._normalize_secgroup_rules( + group.pop('security_group_rules', group.pop('rules', []))) + project_id = group.pop('tenant_id', '') + project_id = group.pop('project_id', project_id) + + ret['location'] = self._get_current_location(project_id=project_id) + ret['id'] = group.pop('id') + ret['name'] = group.pop('name') + ret['security_group_rules'] = rules + ret['description'] = group.pop('description') + ret['properties'] = group + + # Backwards compat with Neutron + ret['tenant_id'] = project_id + ret['project_id'] = project_id + for key, val in ret['properties'].items(): + ret.setdefault(key, val) + + return ret def _normalize_secgroup_rules(self, rules): """Normalize the structure of nova security group rules @@ -144,30 +228,42 @@ class Normalizer(object): def _normalize_secgroup_rule(self, rule): ret = munch.Munch() - ret['id'] = rule['id'] - ret['direction'] = rule.get('direction', 'ingress') - ret['ethertype'] = rule.get('ethertype', 'IPv4') + # Copy incoming rule because of shared dicts in unittests + rule = rule.copy() + + ret['id'] = rule.pop('id') + ret['direction'] = rule.pop('direction', 'ingress') + ret['ethertype'] = rule.pop('ethertype', 'IPv4') port_range_min = rule.get( - 'port_range_min', rule.get('from_port', None)) + 'port_range_min', rule.pop('from_port', None)) if port_range_min == -1: port_range_min = None + if port_range_min is not None: + port_range_min = int(port_range_min) ret['port_range_min'] = port_range_min - port_range_max = rule.get( - 'port_range_max', rule.get('to_port', None)) + port_range_max = rule.pop( + 'port_range_max', rule.pop('to_port', None)) if port_range_max == -1: port_range_max = None + if port_range_min is not None: + port_range_min = int(port_range_min) ret['port_range_max'] = port_range_max - ret['protocol'] = rule.get('protocol', rule.get('ip_protocol')) - ret['remote_ip_prefix'] = rule.get( - 'remote_ip_prefix', rule.get('ip_range', {}).get('cidr', None)) - ret['security_group_id'] = rule.get( - 'security_group_id', rule.get('parent_group_id')) - ret['remote_group_id'] = rule.get('remote_group_id') - project_id = rule.get('project_id', rule.get('tenant_id', '')) + ret['protocol'] = rule.pop('protocol', rule.pop('ip_protocol', None)) + ret['remote_ip_prefix'] = rule.pop( + 'remote_ip_prefix', rule.pop('ip_range', {}).get('cidr', None)) + ret['security_group_id'] = rule.pop( + 'security_group_id', rule.pop('parent_group_id', None)) + ret['remote_group_id'] = rule.pop('remote_group_id', None) + project_id = rule.pop('tenant_id', '') + project_id = rule.pop('project_id', project_id) ret['location'] = self._get_current_location(project_id=project_id) - # neutron sets these. we don't care about it, but let's be the same + ret['properties'] = rule + + # Backwards compat with Neutron ret['tenant_id'] = project_id ret['project_id'] = project_id + for key, val in ret['properties'].items(): + ret.setdefault(key, val) return ret def _normalize_servers(self, servers): @@ -179,26 +275,73 @@ class Normalizer(object): return ret def _normalize_server(self, server): + ret = munch.Munch() + # Copy incoming server because of shared dicts in unittests + server = server.copy() + server.pop('links', None) + server.pop('NAME_ATTR', None) + server.pop('HUMAN_ID', None) + server.pop('human_id', None) + + ret['id'] = server.pop('id') + ret['name'] = server.pop('name') + server['flavor'].pop('links', None) + ret['flavor'] = server.pop('flavor') + # OpenStack can return image as a string when you've booted # from volume if str(server['image']) != server['image']: server['image'].pop('links', None) + ret['image'] = server.pop('image') - server['region'] = self.region_name - server['cloud'] = self.name - server['location'] = self._get_current_location( - project_id=server.get('tenant_id')) + project_id = server.pop('tenant_id', '') + project_id = server.pop('project_id', project_id) az = server.get('OS-EXT-AZ:availability_zone', None) - if az: - server['az'] = az + ret['location'] = self._get_current_location( + project_id=project_id, zone=az) # Ensure volumes is always in the server dict, even if empty - server['volumes'] = [] + ret['volumes'] = [] - return server + config_drive = server.pop('config_drive', False) + ret['has_config_drive'] = _to_bool(config_drive) + + host_id = server.pop('hostId', None) + ret['host_id'] = host_id + + ret['progress'] = _pop_int(server, 'progress') + + # Leave these in so that the general properties handling works + ret['disk_config'] = server.get('OS-DCF:diskConfig') + for key in ( + 'OS-EXT-STS:power_state', + 'OS-EXT-STS:task_state', + 'OS-EXT-STS:vm_state', + 'OS-SRV-USG:launched_at', + 'OS-SRV-USG:terminated_at'): + short_key = key.split(':')[1] + ret[short_key] = server.get(key) + + for field in _SERVER_FIELDS: + ret[field] = server.pop(field, None) + ret['interface_ip'] = '' + + ret['properties'] = server.copy() + for key, val in ret['properties'].items(): + ret.setdefault(key, val) + + # Backwards compat + ret['hostId'] = host_id + ret['config_drive'] = config_drive + ret['project_id'] = project_id + ret['tenant_id'] = project_id + ret['region'] = self.region_name + ret['cloud'] = self.name + ret['az'] = az + return ret def _normalize_floating_ips(self, ips): """Normalize the structure of floating IPs @@ -234,31 +377,47 @@ class Normalizer(object): ] def _normalize_floating_ip(self, ip): - fixed_ip_address = ip.get('fixed_ip_address', ip.get('fixed_ip')) - floating_ip_address = ip.get('floating_ip_address', ip.get('ip')) - network_id = ip.get( - 'floating_network_id', ip.get('network', ip.get('pool'))) - project_id = ip.get('project_id', ip.get('tenant_id', '')) + ret = munch.Munch() + + # Copy incoming floating ip because of shared dicts in unittests + ip = ip.copy() + + fixed_ip_address = ip.pop('fixed_ip_address', ip.pop('fixed_ip', None)) + floating_ip_address = ip.pop('floating_ip_address', ip.pop('ip', None)) + network_id = ip.pop( + 'floating_network_id', ip.pop('network', ip.pop('pool', None))) + project_id = ip.pop('tenant_id', '') + project_id = ip.pop('project_id', project_id) + + instance_id = ip.pop('instance_id', None) + router_id = ip.pop('router_id', None) + id = ip.pop('id') + port_id = ip.pop('port_id', None) + if self._use_neutron_floating(): - attached = (ip.get('port_id') is not None and ip['port_id'] != '') - status = ip.get('status', 'UNKNOWN') + attached = bool(port_id) + status = ip.pop('status', 'UNKNOWN') else: - instance_id = ip.get('instance_id') - attached = instance_id is not None and instance_id != '' + attached = bool(instance_id) # In neutron's terms, Nova floating IPs are always ACTIVE status = 'ACTIVE' - return munch.Munch( + ret = munch.Munch( attached=attached, fixed_ip_address=fixed_ip_address, floating_ip_address=floating_ip_address, floating_network_id=network_id, - id=ip['id'], + id=id, location=self._get_current_location(project_id=project_id), network=network_id, - port_id=ip.get('port_id'), + port_id=port_id, project_id=project_id, - router_id=ip.get('router_id'), + router_id=router_id, status=status, - tenant_id=project_id + tenant_id=project_id, + properties=ip.copy(), ) + for key, val in ret['properties'].items(): + ret.setdefault(key, val) + + return ret diff --git a/shade/openstackcloud.py b/shade/openstackcloud.py index a454ac596..ce8e6c1a6 100644 --- a/shade/openstackcloud.py +++ b/shade/openstackcloud.py @@ -448,10 +448,11 @@ class OpenStackCloud(_normalize.Normalizer): """Return a ``munch.Munch`` explaining the current cloud location.""" return self._get_current_location() - def _get_current_location(self, project_id=None): + def _get_current_location(self, project_id=None, zone=None): return munch.Munch( cloud=self.name, region_name=self.region_name, + zone=zone, project=self._get_project_info(project_id), ) diff --git a/shade/tests/unit/test__utils.py b/shade/tests/unit/test__utils.py index 02bfffd0c..704e66e1b 100644 --- a/shade/tests/unit/test__utils.py +++ b/shade/tests/unit/test__utils.py @@ -97,8 +97,10 @@ class TestUtils(base.TestCase): description='A Nova security group', tenant_id='', project_id='', + properties={}, location=dict( region_name='RegionOne', + zone=None, project=dict( domain_name=None, id=mock.ANY, @@ -109,11 +111,13 @@ class TestUtils(base.TestCase): dict(id='123', direction='ingress', ethertype='IPv4', port_range_min=80, port_range_max=81, protocol='tcp', remote_ip_prefix='0.0.0.0/0', security_group_id='xyz123', + properties={}, tenant_id='', project_id='', remote_group_id=None, location=dict( region_name='RegionOne', + zone=None, project=dict( domain_name=None, id=mock.ANY, @@ -151,8 +155,10 @@ class TestUtils(base.TestCase): port_range_min=80, port_range_max=81, protocol='tcp', remote_ip_prefix='0.0.0.0/0', security_group_id='xyz123', tenant_id='', project_id='', remote_group_id=None, + properties={}, location=dict( region_name='RegionOne', + zone=None, project=dict( domain_name=None, id=mock.ANY, diff --git a/shade/tests/unit/test_meta.py b/shade/tests/unit/test_meta.py index 035169c75..7570dbe1e 100644 --- a/shade/tests/unit/test_meta.py +++ b/shade/tests/unit/test_meta.py @@ -885,7 +885,8 @@ class TestMeta(base.TestCase): 'domain_id': None, 'domain_name': None }, - 'region_name': u'RegionOne'}, + 'region_name': u'RegionOne', + 'zone': None}, self.cloud.current_location) def test_current_project(self):