From d49d038b1ba8e7cb983d7864e2ddbd98153c82dd Mon Sep 17 00:00:00 2001 From: Mike Shriver Date: Thu, 30 May 2019 08:28:00 -0400 Subject: [PATCH] Add all_ips method to VM entities abstractmethod and implementations for subclasses of VM OVIRT and vmware in particular are a real pain in the ass when it comes to addresses With no reasonable way to determine what's a local network IP and what's a public network IP Provide a property that just returns all the available addresses, allowing the caller to process them Changed GCE's properties, as `ip` was returning an internal IP, and `ip_external` should have been `ip` --- wrapanapi/entities/vm.py | 10 +++++++++- wrapanapi/systems/ec2.py | 8 ++++++++ wrapanapi/systems/google.py | 24 +++++++++++++++++++----- wrapanapi/systems/msazure.py | 12 ++++++++++++ wrapanapi/systems/openstack.py | 9 +++++++++ wrapanapi/systems/rhevm.py | 26 ++++++++++++++++---------- wrapanapi/systems/scvmm.py | 5 +++++ wrapanapi/systems/virtualcenter.py | 7 +++++++ 8 files changed, 85 insertions(+), 16 deletions(-) diff --git a/wrapanapi/entities/vm.py b/wrapanapi/entities/vm.py index 058865c6..e54ca641 100644 --- a/wrapanapi/entities/vm.py +++ b/wrapanapi/entities/vm.py @@ -156,11 +156,19 @@ def is_stopping(self): @abstractproperty def ip(self): """ - Returns IP address of the VM/instance + Returns: (string) externally reachable IP address of the VM/instance (when possible) Should refresh if necessary to get most up-to-date info """ + @abstractproperty + def all_ips(self): + """ + Returns: (list) All ip addresses available on the VM + + For consistency on platforms that return link-local and restricted network addresses + """ + @abstractproperty def creation_time(self): """ diff --git a/wrapanapi/systems/ec2.py b/wrapanapi/systems/ec2.py index 59eb4391..c67d65c1 100644 --- a/wrapanapi/systems/ec2.py +++ b/wrapanapi/systems/ec2.py @@ -101,6 +101,14 @@ def ip(self): self.refresh() return self.raw.public_ip_address + @property + def all_ips(self): + """ Wrapping self.ip to meet abstractproperty requirement + + Returns: (list) the addresses assigned to the machine + """ + return [self.ip] + @property def type(self): return self.raw.instance_type diff --git a/wrapanapi/systems/google.py b/wrapanapi/systems/google.py index a98e6dd9..4f8f9c91 100644 --- a/wrapanapi/systems/google.py +++ b/wrapanapi/systems/google.py @@ -101,15 +101,29 @@ def _get_state(self): return self._api_state_to_vmstate(self.raw['status']) @property - def ip(self): + def ip_internal(self): self.refresh() - return self.raw.get('networkInterfaces')[0].get('networkIP') + try: + return self.raw.get('networkInterfaces')[0].get('networkIP') + except IndexError: + return None @property - def ip_external(self): + def ip(self): self.refresh() - access_configs = self.raw.get('networkInterfaces')[0].get('accessConfigs')[0] - return access_configs.get('natIP') + try: + access_configs = self.raw.get('networkInterfaces', [{}])[0].get('accessConfigs', [])[0] + return access_configs.get('natIP') + except IndexError: + return None + + @property + def all_ips(self): + """ Wrapping self.ip and self.ip_internal to meet abstractproperty requirement + + Returns: (list) the addresses assigned to the machine + """ + return [self.ip, self.ip_internal] @property def type(self): diff --git a/wrapanapi/systems/msazure.py b/wrapanapi/systems/msazure.py index f54c2699..73ef6982 100644 --- a/wrapanapi/systems/msazure.py +++ b/wrapanapi/systems/msazure.py @@ -173,6 +173,18 @@ def ip(self): return public_ip.ip_address + @property + def all_ips(self): + """ Wrapping self.ip to meet abstractproperty requirement + + TODO: Actually fetch the various addresses on non-primary interfaces + + non-public addresses are not necessary for testing at this time, so not implementing + + Returns: (list) the public IPv4 address in a list + """ + return [self.ip] + @property def type(self): return self.raw.hardware_profile.vm_size diff --git a/wrapanapi/systems/openstack.py b/wrapanapi/systems/openstack.py index cc4501a2..c3029164 100644 --- a/wrapanapi/systems/openstack.py +++ b/wrapanapi/systems/openstack.py @@ -164,6 +164,15 @@ def ip(self): if nic['OS-EXT-IPS:type'] == 'floating': return str(nic['addr']) + @property + def all_ips(self): + """ Get all the IPs on the machine + + Returns: (list) the addresses assigned to the machine + """ + # raw.networks is dict of network: [ip, ip] key:value pairs + return [ip for nets in self.raw.networks.values() for ip in nets] + @property def flavor(self): if not self._flavor: diff --git a/wrapanapi/systems/rhevm.py b/wrapanapi/systems/rhevm.py index 79ead665..37a76b7d 100644 --- a/wrapanapi/systems/rhevm.py +++ b/wrapanapi/systems/rhevm.py @@ -61,7 +61,7 @@ def uuid(self): @property def creation_time(self): """ - Returns creation time of VM/instance + Returns creation time of VM """ self.refresh() return self.raw.creation_time.astimezone(pytz.UTC) @@ -252,16 +252,22 @@ def _get_state(self): @property def ip(self): """ - Returns IPv4 or global IPv6 address of the VM/instance + Returns IPv4 or global IPv6 address of the VM + + If there are multiple IP's on the VM, just returns the first non-link-local """ - link_local_prefix = 'fe80::' + potentials = [] for ip in self.all_ips: - if link_local_prefix not in ip[:len(link_local_prefix)]: - return ip - return None + if not ip.startswith('fe80::'): + potentials.append(ip) + return potentials[0] if potentials else None @property def all_ips(self): + """ Return all of the IPs + + Returns: (list) the addresses assigned to the machine + """ ips = [] rep_dev_service = self.api.reported_devices_service() for dev in rep_dev_service.list(): @@ -271,7 +277,7 @@ def all_ips(self): def start(self): """ - Starts the VM/instance. Blocks until task completes. + Starts the VM. Blocks until task completes. Returns: True if vm action has been initiated properly """ @@ -287,7 +293,7 @@ def start(self): def stop(self): """ - Stops the VM/instance. Blocks until task completes. + Stops the VM. Blocks until task completes. Returns: True if vm action has been initiated properly """ @@ -303,7 +309,7 @@ def stop(self): def restart(self): """ - Restarts the VM/instance. Blocks until task completes. + Restarts the VM. Blocks until task completes. Returns: True if vm action has been initiated properly """ @@ -312,7 +318,7 @@ def restart(self): def suspend(self): """ - Suspends the VM/instance. Blocks until task completes. + Suspends the VM. Blocks until task completes. Returns: True if vm action has been initiated properly """ diff --git a/wrapanapi/systems/scvmm.py b/wrapanapi/systems/scvmm.py index 3ae642a2..20833804 100644 --- a/wrapanapi/systems/scvmm.py +++ b/wrapanapi/systems/scvmm.py @@ -144,6 +144,11 @@ def ip(self): ip = data.translate(None, '{}') return ip if ip else None + @property + def all_ips(self): + """ wrap self.ip to meet abstractproperty """ + return [self.ip] + @property def creation_time(self): self.refresh() diff --git a/wrapanapi/systems/virtualcenter.py b/wrapanapi/systems/virtualcenter.py index adac253a..52995faf 100644 --- a/wrapanapi/systems/virtualcenter.py +++ b/wrapanapi/systems/virtualcenter.py @@ -640,6 +640,13 @@ def ip(self): # TypeError: ip address wasn't a string return None + @property + def all_ips(self): + """vmware is limited, can't get more than one IP for a vm summary + wrapper for API consistency + """ + return [self.ip] + @property def creation_time(self): """Detect the vm_creation_time either via uptime if non-zero, or by last boot time