tests: Stop returning FakeResource in compute tests

This was still being used in places where we have our own API bindings
because SDK does not support the API. Those bindings should be returning
dicts, not FakeResource objects. Correct this, and in doing so fix the
bug this highlights in our cell-down output.

Change-Id: I6647d94fcf5ada8186edbf64c03abd3d8ae7ca56
Signed-off-by: Stephen Finucane <stephenfin@redhat.com>

openstackclient/compute/v2/server.py

@@ -3004,7 +3004,7 @@ class ListServer(command.Lister):
                 # infrastructure failure situations.
                 # For those servers with partial constructs we just skip the
                 # processing of the image and flavor information.
-                if not hasattr(s, 'image') or not hasattr(s, 'flavor'):
+                if getattr(s, 'status') == 'UNKNOWN':
                     continue
 
             if 'id' in s.image and s.image.id is not None:

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

@@ -13,7 +13,6 @@
 #   under the License.
 #
 
-import copy
 import random
 import re
 from unittest import mock
@@ -32,7 +31,6 @@ from openstack.compute.v2 import server_interface as _server_interface
 from openstack.compute.v2 import server_migration as _server_migration
 from openstack.compute.v2 import volume_attachment as _volume_attachment
 
-from openstackclient.tests.unit import fakes
 from openstackclient.tests.unit.identity.v3 import fakes as identity_fakes
 from openstackclient.tests.unit.image.v2 import fakes as image_fakes
 from openstackclient.tests.unit.network.v2 import fakes as network_fakes
@@ -79,16 +77,14 @@ class TestComputev2(
 def create_one_agent(attrs=None):
     """Create a fake agent.
 
-    :param dict attrs:
+    :param dict attrs: A dictionary with all attributes
-        A dictionary with all attributes
+    :return: A dicionarty faking the agent
-    :return:
-        A FakeResource object, with agent_id, os, and so on
     """
 
     attrs = attrs or {}
 
     # set default attributes.
-    agent_info = {
+    agent_attrs = {
         'agent_id': 'agent-id-' + uuid.uuid4().hex,
         'os': 'agent-os-' + uuid.uuid4().hex,
         'architecture': 'agent-architecture',
@@ -98,22 +94,20 @@ def create_one_agent(attrs=None):
         'hypervisor': 'hypervisor',
     }
 
-    # Overwrite default attributes.
+    assert not set(attrs) - set(agent_attrs), 'unknown keys'
-    agent_info.update(attrs)
 
-    agent = fakes.FakeResource(info=copy.deepcopy(agent_info), loaded=True)
+    # Overwrite default attributes.
-    return agent
+    agent_attrs.update(attrs)
+
+    return agent_attrs
 
 
 def create_agents(attrs=None, count=2):
     """Create multiple fake agents.
 
-    :param dict attrs:
+    :param dict attrs: A dictionary with all attributes
-        A dictionary with all attributes
+    :param int count: The number of agents to fake
-    :param int count:
+    :return: A list of dictionaries faking the agents
-        The number of agents to fake
-    :return:
-        A list of FakeResource objects faking the agents
     """
     agents = []
     for i in range(0, count):
@@ -158,10 +152,8 @@ def create_one_extension(attrs=None):
 def create_one_security_group(attrs=None):
     """Create a fake security group.
 
-    :param dict attrs:
+    :param dict attrs: A dictionary with all attributes
-        A dictionary with all attributes
+    :return: A dictionary faking the security group
-    :return:
-        A FakeResource object, with id, name, etc.
     """
     attrs = attrs or {}
 
@@ -174,6 +166,8 @@ def create_one_security_group(attrs=None):
         'rules': [],
     }
 
+    assert not set(attrs) - set(security_group_attrs), 'unknown keys'
+
     # Overwrite default attributes.
     security_group_attrs.update(attrs)
     return security_group_attrs
@@ -182,12 +176,9 @@ def create_one_security_group(attrs=None):
 def create_security_groups(attrs=None, count=2):
     """Create multiple fake security groups.
 
-    :param dict attrs:
+    :param dict attrs: A dictionary with all attributes
-        A dictionary with all attributes
+    :param int count: The number of security groups to fake
-    :param int count:
+    :return: A list of dictionaries faking the security groups
-        The number of security groups to fake
-    :return:
-        A list of FakeResource objects faking the security groups
     """
     security_groups = []
     for i in range(0, count):
@@ -199,10 +190,8 @@ def create_security_groups(attrs=None, count=2):
 def create_one_security_group_rule(attrs=None):
     """Create a fake security group rule.
 
-    :param dict attrs:
+    :param dict attrs: A dictionary with all attributes
-        A dictionary with all attributes
+    :return: A dictionary faking the security group rule
-    :return:
-        A FakeResource object, with id, etc.
     """
     attrs = attrs or {}
 
@@ -217,6 +206,8 @@ def create_one_security_group_rule(attrs=None):
         'to_port': 0,
     }
 
+    assert not set(attrs) - set(security_group_rule_attrs), 'unknown keys'
+
     # Overwrite default attributes.
     security_group_rule_attrs.update(attrs)
 
@@ -226,12 +217,9 @@ def create_one_security_group_rule(attrs=None):
 def create_security_group_rules(attrs=None, count=2):
     """Create multiple fake security group rules.
 
-    :param dict attrs:
+    :param dict attrs: A dictionary with all attributes
-        A dictionary with all attributes
+    :param int count: The number of security group rules to fake
-    :param int count:
+    :return: A list of dictionaries faking the security group rules
-        The number of security group rules to fake
-    :return:
-        A list of FakeResource objects faking the security group rules
     """
     security_group_rules = []
     for i in range(0, count):
@@ -379,10 +367,8 @@ def create_flavors(attrs=None, count=2):
 def create_one_flavor_access(attrs=None):
     """Create a fake flavor access.
 
-    :param dict attrs:
+    :param dict attrs: A dictionary with all attributes
-        A dictionary with all attributes
+    :return: A dictionary faking the flavor access
-    :return:
-        A FakeResource object, with flavor_id, tenat_id
     """
     attrs = attrs or {}
 
@@ -392,14 +378,12 @@ def create_one_flavor_access(attrs=None):
         'tenant_id': 'tenant-id-' + uuid.uuid4().hex,
     }
 
+    assert not set(attrs) - set(flavor_access_info), 'unknown keys'
+
     # Overwrite default attributes.
     flavor_access_info.update(attrs)
 
-    flavor_access = fakes.FakeResource(
+    return flavor_access_info
-        info=copy.deepcopy(flavor_access_info), loaded=True
-    )
-
-    return flavor_access
 
 
 def create_one_availability_zone(attrs=None):
@@ -454,12 +438,10 @@ def create_availability_zones(attrs=None, count=2):
 
 
 def create_one_floating_ip(attrs=None):
-    """Create a fake floating ip.
+    """Create a fake floating IP.
 
-    :param dict attrs:
+    :param dict attrs: A dictionary with all attributes
-        A dictionary with all attributes
+    :return: A dictionary faking the floating IP
-    :return:
-        A FakeResource object, with id, ip, and so on
     """
     attrs = attrs or {}
 
@@ -472,6 +454,8 @@ def create_one_floating_ip(attrs=None):
         'pool': 'public',
     }
 
+    assert not set(attrs) - set(floating_ip_attrs), 'unknown keys'
+
     # Overwrite default attributes.
     floating_ip_attrs.update(attrs)
 
@@ -479,14 +463,11 @@ def create_one_floating_ip(attrs=None):
 
 
 def create_floating_ips(attrs=None, count=2):
-    """Create multiple fake floating ips.
+    """Create multiple fake floating IPs.
 
-    :param dict attrs:
+    :param dict attrs: A dictionary with all attributes
-        A dictionary with all attributes
+    :param int count: The number of floating IPs to fake
-    :param int count:
+    :return: A list of dictionaries faking the floating IPs
-        The number of floating ips to fake
-    :return:
-        A list of FakeResource objects faking the floating ips
     """
     floating_ips = []
     for i in range(0, count):
@@ -494,32 +475,11 @@ def create_floating_ips(attrs=None, count=2):
     return floating_ips
 
 
-def get_floating_ips(floating_ips=None, count=2):
-    """Get an iterable MagicMock object with a list of faked floating ips.
-
-    If floating_ips list is provided, then initialize the Mock object
-    with the list. Otherwise create one.
-
-    :param List floating_ips:
-        A list of FakeResource objects faking floating ips
-    :param int count:
-        The number of floating ips to fake
-    :return:
-        An iterable Mock object with side_effect set to a list of faked
-        floating ips
-    """
-    if floating_ips is None:
-        floating_ips = create_floating_ips(count)
-    return mock.Mock(side_effect=floating_ips)
-
-
 def create_one_floating_ip_pool(attrs=None):
-    """Create a fake floating ip pool.
+    """Create a fake floating IP pool.
 
-    :param dict attrs:
+    :param dict attrs: A dictionary with all attributes
-        A dictionary with all attributes
+    :return: A dictionary faking the floating IP pool
-    :return:
-        A FakeResource object, with name, etc
     """
     if attrs is None:
         attrs = {}
@@ -529,6 +489,8 @@ def create_one_floating_ip_pool(attrs=None):
         'name': 'floating-ip-pool-name-' + uuid.uuid4().hex,
     }
 
+    assert not set(attrs) - set(floating_ip_pool_attrs), 'unknown keys'
+
     # Overwrite default attributes.
     floating_ip_pool_attrs.update(attrs)
 
@@ -536,14 +498,11 @@ def create_one_floating_ip_pool(attrs=None):
 
 
 def create_floating_ip_pools(attrs=None, count=2):
-    """Create multiple fake floating ip pools.
+    """Create multiple fake floating IP pools.
 
-    :param dict attrs:
+    :param dict attrs: A dictionary with all attributes
-        A dictionary with all attributes
+    :param int count: The number of floating IP pools to fake
-    :param int count:
+    :return: A list of dictionaries faking the floating IP pools
-        The number of floating ip pools to fake
-    :return:
-        A list of FakeResource objects faking the floating ip pools
     """
     floating_ip_pools = []
     for i in range(0, count):
@@ -554,10 +513,8 @@ def create_floating_ip_pools(attrs=None, count=2):
 def create_one_network(attrs=None):
     """Create a fake network.
 
-    :param dict attrs:
+    :param dict attrs: A dictionary with all attributes
-        A dictionary with all attributes
+    :return: A dictionary faking the network
-    :return:
-        A FakeResource object, with id, label, cidr and so on
     """
     attrs = attrs or {}
 
@@ -597,6 +554,8 @@ def create_one_network(attrs=None):
         'vpn_public_port': None,
     }
 
+    assert not set(attrs) - set(network_attrs), 'unknown keys'
+
     # Overwrite default attributes.
     network_attrs.update(attrs)
 
@@ -606,12 +565,9 @@ def create_one_network(attrs=None):
 def create_networks(attrs=None, count=2):
     """Create multiple fake networks.
 
-    :param dict attrs:
+    :param dict attrs: A dictionary with all attributes
-        A dictionary with all attributes
+    :param int count: The number of networks to fake
-    :param int count:
+    :return: A list of dictionaries faking the networks
-        The number of networks to fake
-    :return:
-        A list of FakeResource objects faking the networks
     """
     networks = []
     for i in range(0, count):

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

@@ -1052,7 +1052,7 @@ class TestFlavorShow(TestFlavor):
         data_with_project = (
             private_flavor.is_disabled,
             private_flavor.ephemeral,
-            [self.flavor_access.tenant_id],
+            [self.flavor_access['tenant_id']],
             private_flavor.description,
             private_flavor.disk,
             private_flavor.id,

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

@@ -21,6 +21,7 @@ from unittest import mock
 import uuid
 
 import iso8601
+from openstack.compute.v2 import server as _server
 from openstack.compute.v2 import server_group as _server_group
 from openstack import exceptions as sdk_exceptions
 from openstack.test import fakes as sdk_fakes
@@ -5487,9 +5488,7 @@ class TestServerListV273(_TestServerList):
             ],
             "networks": {},
         }
-        fake_server = compute_fakes.fakes.FakeResource(
+        fake_server = _server.Server(**server_dict)
-            info=server_dict,
-        )
         self.servers.append(fake_server)
         columns, data = self.cmd.take_action(parsed_args)
         # get the first three servers out since our interest is in the partial
@@ -5500,9 +5499,9 @@ class TestServerListV273(_TestServerList):
         partial_server = next(data)
         expected_row = (
             'server-id-95a56bfc4xxxxxx28d7e418bfd97813a',
-            '',
+            None,
             'UNKNOWN',
-            server.AddressesColumn(''),
+            server.AddressesColumn(None),
             '',
             '',
         )

openstackclient/tests/unit/network/v2/test_security_group_rule_compute.py

@@ -383,7 +383,6 @@ class TestListSecurityGroupRuleCompute(compute_fakes.TestComputev2):
     _security_group_rule_tcp = compute_fakes.create_one_security_group_rule(
         {
             'ip_protocol': 'tcp',
-            'ethertype': 'IPv4',
             'from_port': 80,
             'to_port': 80,
             'group': {'name': _security_group['name']},
@@ -392,7 +391,6 @@ class TestListSecurityGroupRuleCompute(compute_fakes.TestComputev2):
     _security_group_rule_icmp = compute_fakes.create_one_security_group_rule(
         {
             'ip_protocol': 'icmp',
-            'ethertype': 'IPv4',
             'from_port': -1,
             'to_port': -1,
             'ip_range': {'cidr': '10.0.2.0/24'},
@@ -426,7 +424,7 @@ class TestListSecurityGroupRuleCompute(compute_fakes.TestComputev2):
         expected_rule_with_group = (
             rule['id'],
             rule['ip_protocol'],
-            rule['ethertype'],
+            '',  # ethertype is a neutron-only thing
             rule['ip_range'],
             rule['port_range'],
             rule['remote_security_group'],