Fix delete_ips on delete_server and add tests

This codepath was basically completely untested and essentially
completely broken. Fix the break, but also add tests for it.

Also add tests for config_drive while we're in there.

Change-Id: I9f44474000213bab9e08266a376e41fecc4fbc1f

releasenotes/notes/fix-delete-ips-1d4eebf7bc4d4733.yaml

@@ -0,0 +1,6 @@
+---
+issues:
+  - Fixed the logic in delete_ips and added regression
+    tests to cover it. The old logic was incorrectly looking
+    for floating ips using port syntax. It was also not
+    swallowing errors when it should.

shade/openstackcloud.py

@@ -5895,11 +5895,18 @@ class OpenStackCloud(
         # addresses dict? If not, skip this.
         server_floats = meta.find_nova_interfaces(
             server['addresses'], ext_tag='floating')
-        if not server_floats:
+        for fip in server_floats:
-            return
+            try:
-        ips = self.search_floating_ips(filters={
+                ip = self.get_floating_ip(id=None, filters={
-            'device_id': server['id']})
+                    'floating_ip_address': fip['addr']})
-        for ip in ips:
+            except OpenStackCloudURINotFound:
+                # We're deleting. If it doesn't exist - awesome
+                # NOTE(mordred) If the cloud is a nova FIP cloud but
+                #               floating_ip_source is set to neutron, this
+                #               can lead to a FIP leak.
+                continue
+            if not ip:
+                continue
             deleted = self.delete_floating_ip(
                 ip['id'], retry=delete_ip_retry)
             if not deleted:

shade/tests/functional/test_compute.py

@@ -71,6 +71,23 @@ class TestCompute(base.BaseFunctionalTestCase):
             self.user_cloud.delete_server(self.server_name, wait=True))
         self.assertIsNone(self.user_cloud.get_server(self.server_name))
 
+    def test_create_and_delete_server_auto_ip_delete_ips(self):
+        self.addCleanup(self._cleanup_servers_and_volumes, self.server_name)
+        server = self.user_cloud.create_server(
+            name=self.server_name,
+            image=self.image,
+            flavor=self.flavor,
+            auto_ip=True,
+            wait=True)
+        self.assertEqual(self.server_name, server['name'])
+        self.assertEqual(self.image.id, server['image']['id'])
+        self.assertEqual(self.flavor.id, server['flavor']['id'])
+        self.assertIsNotNone(server['adminPass'])
+        self.assertTrue(
+            self.user_cloud.delete_server(
+                self.server_name, wait=True, delete_ips=True))
+        self.assertIsNone(self.user_cloud.get_server(self.server_name))
+
     def test_attach_detach_volume(self):
         server_name = self.getUniqueString()
         self.addCleanup(self._cleanup_servers_and_volumes, server_name)

shade/tests/unit/test_delete_server.py

@@ -16,6 +16,7 @@ test_delete_server
 
 Tests for the `delete_server` command.
 """
+import uuid
 
 from shade import exc as shade_exc
 from shade.tests import fakes
@@ -140,3 +141,125 @@ class TestDeleteServer(base.RequestsMockTestCase):
         self.assertTrue(self.cloud.delete_server('porky', wait=False))
 
         self.assert_calls()
+
+    def test_delete_server_delete_ips(self):
+        """
+        Test that deleting server and fips works
+        """
+        server = fakes.make_fake_server('1234', 'porky', 'ACTIVE')
+        fip_id = uuid.uuid4().hex
+
+        self.register_uris([
+            dict(method='GET',
+                 uri=self.get_mock_url(
+                     'compute', 'public', append=['servers', 'detail']),
+                 json={'servers': [server]}),
+            dict(method='GET',
+                 uri=self.get_mock_url(
+                     'network', 'public', append=['v2.0', 'floatingips.json'],
+                     qs_elements=['floating_ip_address=172.24.5.5']),
+                 complete_qs=True,
+                 json={'floatingips': [{
+                     'router_id': 'd23abc8d-2991-4a55-ba98-2aaea84cc72f',
+                     'tenant_id': '4969c491a3c74ee4af974e6d800c62de',
+                     'floating_network_id': '376da547-b977-4cfe-9cba7',
+                     'fixed_ip_address': '10.0.0.4',
+                     'floating_ip_address': '172.24.5.5',
+                     'port_id': 'ce705c24-c1ef-408a-bda3-7bbd946164ac',
+                     'id': fip_id,
+                     'status': 'ACTIVE'}]}),
+            dict(method='DELETE',
+                 uri=self.get_mock_url(
+                     'network', 'public',
+                     append=['v2.0', 'floatingips',
+                             '{fip_id}.json'.format(fip_id=fip_id)])),
+            dict(method='GET',
+                 uri=self.get_mock_url(
+                     'network', 'public', append=['v2.0', 'floatingips.json']),
+                 complete_qs=True,
+                 json={'floatingips': []}),
+            dict(method='DELETE',
+                 uri=self.get_mock_url(
+                     'compute', 'public', append=['servers', '1234'])),
+            dict(method='GET',
+                 uri=self.get_mock_url(
+                     'compute', 'public', append=['servers', 'detail']),
+                 json={'servers': []}),
+        ])
+        self.assertTrue(self.cloud.delete_server(
+            'porky', wait=True, delete_ips=True))
+
+        self.assert_calls()
+
+    def test_delete_server_delete_ips_bad_neutron(self):
+        """
+        Test that deleting server with a borked neutron doesn't bork
+        """
+        server = fakes.make_fake_server('1234', 'porky', 'ACTIVE')
+
+        self.register_uris([
+            dict(method='GET',
+                 uri=self.get_mock_url(
+                     'compute', 'public', append=['servers', 'detail']),
+                 json={'servers': [server]}),
+            dict(method='GET',
+                 uri=self.get_mock_url(
+                     'network', 'public', append=['v2.0', 'floatingips.json'],
+                     qs_elements=['floating_ip_address=172.24.5.5']),
+                 complete_qs=True,
+                 status_code=404),
+            dict(method='DELETE',
+                 uri=self.get_mock_url(
+                     'compute', 'public', append=['servers', '1234'])),
+            dict(method='GET',
+                 uri=self.get_mock_url(
+                     'compute', 'public', append=['servers', 'detail']),
+                 json={'servers': []}),
+        ])
+        self.assertTrue(self.cloud.delete_server(
+            'porky', wait=True, delete_ips=True))
+
+        self.assert_calls()
+
+    def test_delete_server_delete_fips_nova(self):
+        """
+        Test that deleting server with a borked neutron doesn't bork
+        """
+        self.cloud._floating_ip_source = 'nova'
+        server = fakes.make_fake_server('1234', 'porky', 'ACTIVE')
+
+        self.register_uris([
+            dict(method='GET',
+                 uri=self.get_mock_url(
+                     'compute', 'public', append=['servers', 'detail']),
+                 json={'servers': [server]}),
+            dict(method='GET',
+                 uri=self.get_mock_url(
+                     'compute', 'public', append=['os-floating-ips']),
+                 json={'floating_ips': [
+                     {
+                         'fixed_ip': None,
+                         'id': 1,
+                         'instance_id': None,
+                         'ip': '172.24.5.5',
+                         'pool': 'nova'
+                     }]}),
+            dict(method='DELETE',
+                 uri=self.get_mock_url(
+                     'compute', 'public', append=['os-floating-ips', '1'])),
+            dict(method='GET',
+                 uri=self.get_mock_url(
+                     'compute', 'public', append=['os-floating-ips']),
+                 json={'floating_ips': []}),
+            dict(method='DELETE',
+                 uri=self.get_mock_url(
+                     'compute', 'public', append=['servers', '1234'])),
+            dict(method='GET',
+                 uri=self.get_mock_url(
+                     'compute', 'public', append=['servers', 'detail']),
+                 json={'servers': []}),
+        ])
+        self.assertTrue(self.cloud.delete_server(
+            'porky', wait=True, delete_ips=True))
+
+        self.assert_calls()