diff --git a/shade/_utils.py b/shade/_utils.py index f8fa5a58c..9d8782e44 100644 --- a/shade/_utils.py +++ b/shade/_utils.py @@ -141,6 +141,36 @@ def _get_entity(func, name_or_id, filters): return entities[0] +def normalize_servers(servers, cloud_name, region_name): + # Here instead of _utils because we need access to region and cloud + # name from the cloud object + ret = [] + for server in servers: + ret.append(normalize_server(server, cloud_name, region_name)) + return ret + + +def normalize_server(server, cloud_name, region_name): + server.pop('links', None) + server['flavor'].pop('links', None) + # OpenStack can return image as a string when you've booted + # from volume + if str(server['image']) != server['image']: + server['image'].pop('links', None) + + server['region'] = region_name + server['cloud'] = cloud_name + + az = server.get('OS-EXT-AZ:availability_zone', None) + if az: + server['az'] = az + + # Ensure volumes is always in the server dict, even if empty + server['volumes'] = [] + + return server + + def normalize_keystone_services(services): """Normalize the structure of keystone services diff --git a/shade/inventory.py b/shade/inventory.py index 64a9cce06..98065971c 100644 --- a/shade/inventory.py +++ b/shade/inventory.py @@ -61,9 +61,7 @@ class OpenStackInventory(object): if expand: server_vars = cloud.get_openstack_vars(server) else: - # expand_server_vars gets renamed in a follow on - # patch which should make this a bit clearer. - server_vars = meta.expand_server_vars(cloud, server) + server_vars = meta.add_server_interfaces(cloud, server) hostvars.append(server_vars) return hostvars diff --git a/shade/meta.py b/shade/meta.py index ac10bc4cf..c2d492bf2 100644 --- a/shade/meta.py +++ b/shade/meta.py @@ -224,57 +224,47 @@ def get_groups_from_server(cloud, server, server_vars): def expand_server_vars(cloud, server): - """Add and clean up the server dict with information that is essential. + """Backwards compatibility function.""" + return add_server_interfaces(cloud, server) - This function should not make additional calls to the cloud with one - exception - which is getting external/internal IPs, which have to do - some calls to neutron in some cases to find out which IP is routable. This - is essential info as you can't otherwise connect to the server without - it. + +def add_server_interfaces(cloud, server): + """Add network interface information to server. + + Query the cloud as necessary to add information to the server record + about the network information needed to interface with the server. + + Ensures that public_v4, public_v6, private_v4, private_v6, interface_ip, + accessIPv4 and accessIPv6 are always set. """ - server_vars = server - server_vars.pop('links', None) - server_vars['flavor'].pop('links', None) - # OpenStack can return image as a string when you've booted from volume - if str(server['image']) != server['image']: - server_vars['image'].pop('links', None) - # First, add an IP address. Set it to '' rather than None if it does # not exist to remain consistent with the pre-existing missing values - server_vars['public_v4'] = get_server_external_ipv4(cloud, server) or '' - server_vars['public_v6'] = get_server_external_ipv6(server) or '' - server_vars['private_v4'] = get_server_private_ip(server, cloud) or '' + server['public_v4'] = get_server_external_ipv4(cloud, server) or '' + server['public_v6'] = get_server_external_ipv6(server) or '' + server['private_v4'] = get_server_private_ip(server, cloud) or '' interface_ip = None - if cloud.private and server_vars['private_v4']: - interface_ip = server_vars['private_v4'] + if cloud.private and server['private_v4']: + interface_ip = server['private_v4'] else: - if (server_vars['public_v6'] and cloud._local_ipv6 + if (server['public_v6'] and cloud._local_ipv6 and not cloud.force_ipv4): - interface_ip = server_vars['public_v6'] + interface_ip = server['public_v6'] else: - interface_ip = server_vars['public_v4'] + interface_ip = server['public_v4'] if interface_ip: - server_vars['interface_ip'] = interface_ip + server['interface_ip'] = interface_ip # Some clouds do not set these, but they're a regular part of the Nova # server record. Since we know them, go ahead and set them. In the case # where they were set previous, we use the values, so this will not break # clouds that provide the information - if cloud.private and server_vars['private_v4']: - server_vars['accessIPv4'] = server_vars['private_v4'] + if cloud.private and server['private_v4']: + server['accessIPv4'] = server['private_v4'] else: - server_vars['accessIPv4'] = server_vars['public_v4'] - server_vars['accessIPv6'] = server_vars['public_v6'] + server['accessIPv4'] = server['public_v4'] + server['accessIPv6'] = server['public_v6'] - server_vars['region'] = cloud.region_name - server_vars['cloud'] = cloud.name - - az = server_vars.get('OS-EXT-AZ:availability_zone', None) - if az: - server_vars['az'] = az - # Ensure volumes is always in the server dict, even if empty - server_vars['volumes'] = [] - return server_vars + return server def expand_server_security_groups(cloud, server): @@ -293,7 +283,7 @@ def get_hostvars_from_server(cloud, server, mounts=None): expand_server_vars if caching is not set up. If caching is set up, the extra cost should be minimal. """ - server_vars = expand_server_vars(cloud, server) + server_vars = add_server_interfaces(cloud, server) flavor_id = server['flavor']['id'] flavor_name = cloud.get_flavor_name(flavor_id) diff --git a/shade/openstackcloud.py b/shade/openstackcloud.py index aaee1b180..01c004e95 100644 --- a/shade/openstackcloud.py +++ b/shade/openstackcloud.py @@ -1130,7 +1130,9 @@ class OpenStackCloud(object): "Error fetching server list on {cloud}:{region}:".format( cloud=self.name, region=self.region_name)): - servers = self.manager.submitTask(_tasks.ServerList()) + servers = _utils.normalize_servers( + self.manager.submitTask(_tasks.ServerList()), + cloud_name=self.name, region_name=self.region_name) if detailed: return [ @@ -1485,7 +1487,9 @@ class OpenStackCloud(object): return _utils._get_entity(searchfunc, name_or_id, filters) def get_server_by_id(self, id): - return self.manager.submitTask(_tasks.ServerGet(server=id)) + return _utils.normalize_server( + self.manager.submitTask(_tasks.ServerGet(server=id)), + cloud_name=self.name, region_name=self.region_name) def get_image(self, name_or_id, filters=None): """Get an image by name or ID. diff --git a/shade/tests/fakes.py b/shade/tests/fakes.py index d20d19e50..c3bf28e31 100644 --- a/shade/tests/fakes.py +++ b/shade/tests/fakes.py @@ -72,7 +72,11 @@ class FakeServer(object): self.name = name self.status = status self.addresses = addresses + if not flavor: + flavor = {} self.flavor = flavor + if not image: + image = {} self.image = image self.accessIPv4 = accessIPv4 self.accessIPv6 = accessIPv6 diff --git a/shade/tests/unit/test_create_server.py b/shade/tests/unit/test_create_server.py index 8abf6cc4d..42ed3c1ea 100644 --- a/shade/tests/unit/test_create_server.py +++ b/shade/tests/unit/test_create_server.py @@ -21,6 +21,7 @@ Tests for the `create_server` command. from mock import patch, Mock import os_client_config +from shade import _utils from shade import meta from shade import OpenStackCloud from shade.exc import (OpenStackCloudException, OpenStackCloudTimeout) @@ -132,10 +133,14 @@ class TestCreateServer(base.TestCase): "servers.get.return_value": fake_server } OpenStackCloud.nova_client = Mock(**config) - self.assertEqual(meta.obj_to_dict(fake_server), - self.client.create_server( - name='server-name', image='image=id', - flavor='flavor-id')) + self.assertEqual( + _utils.normalize_server( + meta.obj_to_dict(fake_server), + cloud_name=self.client.name, + region_name=self.client.region_name), + self.client.create_server( + name='server-name', image='image=id', + flavor='flavor-id')) def test_create_server_wait(self): """ diff --git a/shade/tests/unit/test_inventory.py b/shade/tests/unit/test_inventory.py index 9c0a6a441..88a2d12e2 100644 --- a/shade/tests/unit/test_inventory.py +++ b/shade/tests/unit/test_inventory.py @@ -16,7 +16,10 @@ import mock import os_client_config +from shade import _utils from shade import inventory +from shade import meta +from shade.tests import fakes from shade.tests.unit import base @@ -59,14 +62,17 @@ class TestInventory(base.TestCase): self.assertEqual([server], ret) @mock.patch("os_client_config.config.OpenStackConfig") - @mock.patch("shade.meta.expand_server_vars") + @mock.patch("shade.meta.add_server_interfaces") @mock.patch("shade.OpenStackCloud") - def test_list_hosts_no_detail(self, mock_cloud, mock_expand, mock_config): + def test_list_hosts_no_detail(self, mock_cloud, mock_add, mock_config): mock_config.return_value.get_all_clouds.return_value = [{}] inv = inventory.OpenStackInventory() - server = dict(id='server_id', name='server_name') + server = _utils.normalize_server( + meta.obj_to_dict(fakes.FakeServer( + '1234', 'test', 'ACTIVE', addresses={})), + region_name='', cloud_name='') self.assertIsInstance(inv.clouds, list) self.assertEqual(1, len(inv.clouds)) inv.clouds[0].list_servers.return_value = [server] @@ -75,7 +81,7 @@ class TestInventory(base.TestCase): inv.clouds[0].list_servers.assert_called_once_with() self.assertFalse(inv.clouds[0].get_openstack_vars.called) - mock_expand.assert_called_once_with(inv.clouds[0], server) + mock_add.assert_called_once_with(inv.clouds[0], server) @mock.patch("os_client_config.config.OpenStackConfig") @mock.patch("shade.OpenStackCloud") diff --git a/shade/tests/unit/test_meta.py b/shade/tests/unit/test_meta.py index dece29784..77688adfa 100644 --- a/shade/tests/unit/test_meta.py +++ b/shade/tests/unit/test_meta.py @@ -19,6 +19,7 @@ import warlock from neutronclient.common import exceptions as neutron_exceptions import shade +from shade import _utils from shade import meta from shade.tests import fakes @@ -359,14 +360,17 @@ class TestMeta(testtools.TestCase): mock_get_server_external_ipv6.return_value = PUBLIC_V6 hostvars = meta.get_hostvars_from_server( - FakeCloud(), meta.obj_to_dict(FakeServer())) + FakeCloud(), _utils.normalize_server( + meta.obj_to_dict(FakeServer()), + cloud_name='CLOUD_NAME', + region_name='REGION_NAME')) self.assertNotIn('links', hostvars) self.assertEqual(PRIVATE_V4, hostvars['private_v4']) self.assertEqual(PUBLIC_V4, hostvars['public_v4']) self.assertEqual(PUBLIC_V6, hostvars['public_v6']) self.assertEqual(PUBLIC_V6, hostvars['interface_ip']) - self.assertEquals(FakeCloud.region_name, hostvars['region']) - self.assertEquals(FakeCloud.name, hostvars['cloud']) + self.assertEquals('REGION_NAME', hostvars['region']) + self.assertEquals('CLOUD_NAME', hostvars['cloud']) self.assertEquals("test-image-name", hostvars['image']['name']) self.assertEquals(FakeServer.image['id'], hostvars['image']['id']) self.assertNotIn('links', hostvars['image']) @@ -411,14 +415,13 @@ class TestMeta(testtools.TestCase): FakeCloud(), meta.obj_to_dict(server)) self.assertEquals('fake-image-id', hostvars['image']['id']) - @mock.patch.object(shade.meta, 'get_server_external_ipv4') - def test_az(self, mock_get_server_external_ipv4): - mock_get_server_external_ipv4.return_value = PUBLIC_V4 - + def test_az(self): server = FakeServer() server.__dict__['OS-EXT-AZ:availability_zone'] = 'az1' - hostvars = meta.get_hostvars_from_server( - FakeCloud(), meta.obj_to_dict(server)) + + hostvars = _utils.normalize_server( + meta.obj_to_dict(server), + cloud_name='', region_name='') self.assertEquals('az1', hostvars['az']) def test_has_volume(self): diff --git a/shade/tests/unit/test_rebuild_server.py b/shade/tests/unit/test_rebuild_server.py index 436824e0d..a76ce188f 100644 --- a/shade/tests/unit/test_rebuild_server.py +++ b/shade/tests/unit/test_rebuild_server.py @@ -21,6 +21,7 @@ Tests for the `rebuild_server` command. from mock import patch, Mock import os_client_config +from shade import _utils from shade import meta from shade import OpenStackCloud from shade.exc import (OpenStackCloudException, OpenStackCloudTimeout) @@ -108,5 +109,9 @@ class TestRebuildServer(base.TestCase): "servers.get.return_value": active_server } OpenStackCloud.nova_client = Mock(**config) - self.assertEqual(meta.obj_to_dict(active_server), - self.client.rebuild_server("a", "b", wait=True)) + self.client.name = 'cloud-name' + self.assertEqual( + _utils.normalize_server( + meta.obj_to_dict(active_server), + cloud_name='cloud-name', region_name=''), + self.client.rebuild_server("a", "b", wait=True)) diff --git a/shade/tests/unit/test_shade.py b/shade/tests/unit/test_shade.py index 402cefb24..83349308f 100644 --- a/shade/tests/unit/test_shade.py +++ b/shade/tests/unit/test_shade.py @@ -25,6 +25,7 @@ import shade from shade import _utils from shade import exc from shade import meta +from shade.tests import fakes from shade.tests.unit import base @@ -499,7 +500,9 @@ class TestShade(base.TestCase): to the ServerList task.''' mock_serverlist.return_value = [ munch.Munch({'name': 'testserver', - 'id': '1'}) + 'id': '1', + 'flavor': {}, + 'image': ''}) ] r = self.cloud.list_servers() @@ -514,7 +517,10 @@ class TestShade(base.TestCase): '''This test verifies that when list_servers is called with `detailed=True` that it calls `get_hostvars_from_server` for each server in the list.''' - mock_serverlist.return_value = ['server1', 'server2'] + mock_serverlist.return_value = [ + fakes.FakeServer('server1', '', 'ACTIVE'), + fakes.FakeServer('server2', '', 'ACTIVE'), + ] mock_get_hostvars_from_server.side_effect = [ {'name': 'server1', 'id': '1'}, {'name': 'server2', 'id': '2'},