From 8daade000cc242507429a762b0df1e167801144b Mon Sep 17 00:00:00 2001 From: Lingxian Kong Date: Sun, 23 Aug 2020 08:32:59 +1200 Subject: [PATCH] Support to check if subnet is associated with router Change-Id: I8041fbfdb01a7a1efa721c623ab3f43efd2cc0f0 --- ...toria-check-subnet-router-association.yaml | 7 ++++++ trove/common/cfg.py | 9 +++++++- trove/common/neutron.py | 23 ++++++++++++++++++- trove/taskmanager/models.py | 14 +++++------ .../unittests/taskmanager/test_models.py | 11 ++++++++- 5 files changed, 54 insertions(+), 10 deletions(-) create mode 100644 releasenotes/notes/victoria-check-subnet-router-association.yaml diff --git a/releasenotes/notes/victoria-check-subnet-router-association.yaml b/releasenotes/notes/victoria-check-subnet-router-association.yaml new file mode 100644 index 0000000000..a41bc8855d --- /dev/null +++ b/releasenotes/notes/victoria-check-subnet-router-association.yaml @@ -0,0 +1,7 @@ +--- +features: + - Added a config option ``enable_access_check`` (default True) to decide if + Trove should check the subnet of the user port is associated with a Neutron + router. This check is needed for creating public-facing instances and the + instance initialization. This check could be skipped When using Neutron + provider network. diff --git a/trove/common/cfg.py b/trove/common/cfg.py index b173d9d8b2..0f0d6d5450 100644 --- a/trove/common/cfg.py +++ b/trove/common/cfg.py @@ -1396,7 +1396,14 @@ network_opts = [ help='ID of the Neutron public network to create floating IP for the ' 'public trove instance. If not given, Trove will try to query ' 'all the public networks and use the first one in the list.' - ) + ), + cfg.BoolOpt( + 'enable_access_check', default=True, + help='Check if the user provided network is associated with router. ' + 'This is needed for the instance initialization. The check is ' + 'also necessary when creating public facing instance. A scenario ' + 'to set this option False is when using Neutron provider ' + 'network.') ] service_credentials_group = cfg.OptGroup( diff --git a/trove/common/neutron.py b/trove/common/neutron.py index 70df7da225..25690ad0b3 100644 --- a/trove/common/neutron.py +++ b/trove/common/neutron.py @@ -54,8 +54,21 @@ def reset_management_networks(): MGMT_NETWORKS = None +def check_subnet_router(client, subnet_id): + """Check if the subnet is associated with a router.""" + router_ports = client.list_ports( + device_owner="network:router_interface", + fixed_ips=f"subnet_id={subnet_id}")["ports"] + if not router_ports: + raise exception.TroveError(f"Subnet {subnet_id} is not " + f"associated with router.") + + def create_port(client, name, description, network_id, security_groups, - is_public=False, subnet_id=None, ip=None): + is_public=False, subnet_id=None, ip=None, is_mgmt=False): + enable_access_check = (not is_mgmt and + (CONF.network.enable_access_check or is_public)) + port_body = { "port": { "name": name, @@ -66,6 +79,9 @@ def create_port(client, name, description, network_id, security_groups, } if subnet_id: + if enable_access_check: + check_subnet_router(client, subnet_id) + fixed_ips = { "fixed_ips": [{"subnet_id": subnet_id}] } @@ -76,6 +92,11 @@ def create_port(client, name, description, network_id, security_groups, port = client.create_port(body=port_body) port_id = port['port']['id'] + if not subnet_id and enable_access_check: + # Check if the subnet has been associated with a router. + subnet_id = port['port']['fixed_ips'][0]['subnet_id'] + check_subnet_router(client, subnet_id) + if is_public: make_port_public(client, port_id) diff --git a/trove/taskmanager/models.py b/trove/taskmanager/models.py index e18e5bc71e..c7168a1a32 100755 --- a/trove/taskmanager/models.py +++ b/trove/taskmanager/models.py @@ -471,15 +471,15 @@ class FreshInstanceTasks(FreshInstance, NotifyMixin, ConfigurationMixin): security_groups, is_public=is_public, subnet_id=network_info.get('subnet_id'), - ip=network_info.get('ip_address') + ip=network_info.get('ip_address'), + is_mgmt=is_mgmt ) - except Exception: - error = ("Failed to create %s port for instance %s" - % (type, self.id)) - LOG.exception(error) + except Exception as e: self.update_db( task_status=inst_models.InstanceTasks.BUILDING_ERROR_PORT ) + error = (f"Failed to create {type} port for instance {self.id}: " + f"{str(e)}") raise TroveError(message=error) return port_id @@ -518,7 +518,7 @@ class FreshInstanceTasks(FreshInstance, NotifyMixin, ConfigurationMixin): port_id = self._create_port( {'network_id': CONF.management_networks[-1]}, port_sgs, - is_mgmt=True + is_mgmt=True, ) LOG.info("Management port %s created for instance: %s", port_id, self.id) @@ -533,7 +533,7 @@ class FreshInstanceTasks(FreshInstance, NotifyMixin, ConfigurationMixin): network_info, port_sgs, is_mgmt=False, - is_public=access.get('is_public', False) + is_public=access.get('is_public', False), ) LOG.info("User port %s created for instance %s", port_id, self.id) diff --git a/trove/tests/unittests/taskmanager/test_models.py b/trove/tests/unittests/taskmanager/test_models.py index 3c09876656..a1c1d37a5e 100644 --- a/trove/tests/unittests/taskmanager/test_models.py +++ b/trove/tests/unittests/taskmanager/test_models.py @@ -417,12 +417,21 @@ class FreshInstanceTasksTest(BaseFreshInstanceTasksTest): } mock_client.create_port.side_effect = [ {'port': {'id': 'fake-mgmt-port-id'}}, - {'port': {'id': 'fake-user-port-id'}} + { + 'port': { + 'id': 'fake-user-port-id', + 'fixed_ips': [{'subnet_id': 'fake-subnet-id'}] + } + } ] mock_client.list_networks.return_value = { 'networks': [{'id': 'fake-public-net-id'}] } + mock_client.list_ports.return_value = { + 'ports': [{'id': 'fake-port-id'}] + } mock_neutron_client.return_value = mock_client + mock_flavor = {'id': 8, 'ram': 768, 'name': 'bigger_flavor'} config_content = {'config_contents': 'some junk'} mock_single_instance_template.return_value.config_contents = (