Handle create_router when network and subnet exist

Subnets were not assigned if they already existed and were not created,
but the router creation code for the configure-resources depends on the
subnets variable for iteration. Initialize the subnets from the query
for the subnets for the case of existing subnets.

Additionally, the octavia charm uses a service user which can see other
subnets that are tagged 'charm-octavia'. Narrow the scope of the subnet
listing to the network being used by the charm.

Closes-Bug: #1866126
Change-Id: Id88a68a398a50532e11daff33fb774caf65a0c0a
Co-authored-by: Aurelien Lourot <aurelien.lourot@canonical.com>

src/lib/charm/openstack/api_crud.py

@@ -617,12 +617,16 @@ def get_mgmt_network(identity_service, create=True):
             raise APIUnavailable('neutron', 'networks', e)
 
     try:
-        resp = nc.list_subnets(tags='charm-octavia')
+        # The service user can see other subnets that are tagged with
+        # 'charm-octavia' but are not part of this service's lb-mgmt-net. To
+        # avoid that, ensure that the subnets are filtered by the network the
+        # charm cares about.
+        resp = nc.list_subnets(network_id=network['id'], tags='charm-octavia')
     except NEUTRON_TEMP_EXCS as e:
         raise APIUnavailable('neutron', 'subnets', e)
 
-    n_resp = len(resp.get('subnets', []))
-    subnets = None
+    subnets = resp.get('subnets', [])
+    n_resp = len(subnets)
     if n_resp < 1 and create:
         # make rfc4193 Unique Local IPv6 Unicast Addresses from network UUID
         rfc4193_addr = 'fc00'

unit_tests/test_lib_charm_openstack_api_crud.py

@@ -374,7 +374,8 @@ class TestAPICrud(test_utils.PatchHelper):
         nc.create_network.assert_called_once_with({
             'network': {'name': octavia.OCTAVIA_MGMT_NET}})
 
-        nc.list_subnets.assert_called_once_with(tags=resource_tag)
+        nc.list_subnets.assert_called_once_with(network_id=network_uuid,
+                                                tags=resource_tag)
         nc.list_routers.assert_called_once_with(tags=resource_tag)
         nc.create_router.assert_called_once_with(
             {'router': {'name': 'lb-mgmt', 'distributed': False}})
@@ -414,7 +415,8 @@ class TestAPICrud(test_utils.PatchHelper):
         self.init_neutron_client.assert_called_once_with(
             self.session_from_identity_service())
         nc.list_networks.assert_called_once_with(tags=resource_tag)
-        nc.list_subnets.assert_called_once_with(tags=resource_tag)
+        nc.list_subnets.assert_called_once_with(network_id=network_uuid,
+                                                tags=resource_tag)
         nc.list_routers.assert_called_once_with(tags=resource_tag)
         nc.list_security_groups.assert_has_calls([
             mock.call(tags=resource_tag),
@@ -426,3 +428,51 @@ class TestAPICrud(test_utils.PatchHelper):
             {'id': network_uuid},
             {'id': self.secgrp_uuid},),
         )
+
+    def test_get_mgmt_network_exists_create_router(self):
+        resource_tag = 'charm-octavia'
+        self.patch_object(api_crud, 'session_from_identity_service')
+        self.patch_object(api_crud, 'init_neutron_client')
+        identity_service = mock.MagicMock()
+        nc = mock.MagicMock()
+        self.init_neutron_client.return_value = nc
+        network_uuid = '83f1a860-9aed-4c0b-8b72-47195580a0c1'
+        nc.list_networks.return_value = {'networks': [{'id': network_uuid}]}
+        nc.list_subnets.return_value = {
+            'subnets': [{'id': 'fake-subnet-uuid'}]}
+        # network and subnet exists, but router doesn't
+        nc.list_routers.return_value = {'routers': []}
+        nc.create_router.return_value = {
+            'router': {'id': 'fake-router-uuid'}}
+        nc.list_security_groups.side_effect = [
+            {'security_groups': [{'id': self.secgrp_uuid}]},
+            {'security_groups': [{'id': self.health_secgrp_uuid}]},
+        ]
+
+        self.patch_object(api_crud.neutronclient.common, 'exceptions',
+                          name='neutron_exceptions')
+        self.neutron_exceptions.Conflict = FakeNeutronConflictException
+        nc.create_security_group_rule.side_effect = \
+            FakeNeutronConflictException
+        result = api_crud.get_mgmt_network(identity_service)
+        self.init_neutron_client.assert_called_once_with(
+            self.session_from_identity_service())
+        nc.list_networks.assert_called_once_with(tags=resource_tag)
+        self.assertFalse(nc.create_networks.called)
+        nc.list_subnets.assert_called_once_with(network_id=network_uuid,
+                                                tags=resource_tag)
+        self.assertFalse(nc.create_subnet.called)
+        nc.list_routers.assert_called_once_with(tags=resource_tag)
+        self.assertTrue(nc.create_router.called)
+        nc.add_interface_router.assert_called_once_with('fake-router-uuid', {
+            'subnet_id': 'fake-subnet-uuid'})
+        nc.list_security_groups.assert_has_calls([
+            mock.call(tags=resource_tag),
+            mock.call(tags=resource_tag + '-health'),
+        ])
+        nc.create_security_group_rule.assert_has_calls(
+            self.security_group_rule_calls)
+        self.assertEqual(result, (
+            {'id': network_uuid},
+            {'id': self.secgrp_uuid},),
+        )