From 5c47a76bb7fb16aae756e2281c81999759fb3c8c Mon Sep 17 00:00:00 2001 From: Ivan Zinoviev Date: Tue, 18 Jul 2017 12:19:53 +0300 Subject: [PATCH] Allow specification of only neutron net without subnet in addition to existing approach where we are allowed to choose only network with specific subnet. It is useful when we have lots of small subnets and it is possible to exceed amount of available IP addresses in it. In this case, we will allow user to specify only network and delegate subnet choice to server. Change-Id: I23c21fe65552c0def527c98a5fa6a4eae56a6a7a --- muranodashboard/common/net.py | 28 +++-- muranodashboard/dynamic_ui/fields.py | 3 - muranodashboard/tests/unit/common/test_net.py | 103 +++++++++--------- .../tests/unit/dynamic_ui/test_fields.py | 3 +- 4 files changed, 66 insertions(+), 71 deletions(-) diff --git a/muranodashboard/common/net.py b/muranodashboard/common/net.py index fe9d9aca6..1a4716134 100644 --- a/muranodashboard/common/net.py +++ b/muranodashboard/common/net.py @@ -56,8 +56,7 @@ def get_project_assigned_network(request): return [] -def get_available_networks(request, include_subnets=True, - filter=None, murano_networks=None): +def get_available_networks(request, filter=None, murano_networks=None): if murano_networks: env_names = [e.name for e in env_api.environments_list(request)] @@ -94,21 +93,20 @@ def get_available_networks(request, include_subnets=True, else: netname = _("Network of '%s'") % env - if include_subnets: - for subnet in net.subnets: - if not netname: - full_name = ( - "%(net)s: %(cidr)s %(subnet)s" % - dict(net=net.name_or_id, - cidr=subnet.cidr, - subnet=subnet.name_or_id)) + for subnet in net.subnets: + if not netname: + full_name = ( + "%(net)s: %(cidr)s %(subnet)s" % + dict(net=net.name_or_id, + cidr=subnet.cidr, + subnet=subnet.name_or_id)) - network_choices.append( - ((net.id, subnet.id), netname or full_name)) + network_choices.append( + ((net.id, subnet.id), netname or full_name)) - else: - netname = netname or net.name_or_id - network_choices.append(((net.id, None), netname)) + netname = _("%s: random subnet") % ( + netname or net.name_or_id) + network_choices.append(((net.id, None), netname)) return network_choices diff --git a/muranodashboard/dynamic_ui/fields.py b/muranodashboard/dynamic_ui/fields.py index 2e6fc6dd1..03157b9f8 100644 --- a/muranodashboard/dynamic_ui/fields.py +++ b/muranodashboard/dynamic_ui/fields.py @@ -490,7 +490,6 @@ class ImageChoiceField(ChoiceField): class NetworkChoiceField(ChoiceField): def __init__(self, - include_subnets=True, filter=None, murano_networks=None, allow_auto=True, @@ -501,7 +500,6 @@ class NetworkChoiceField(ChoiceField): if murano_networks.lower() not in ["exclude", "translate"]: raise ValueError(_("Invalid value of 'murano_nets' option")) self.murano_networks = murano_networks - self.include_subnets = include_subnets self.allow_auto = allow_auto super(NetworkChoiceField, self).__init__(*args, **kwargs) @@ -514,7 +512,6 @@ class NetworkChoiceField(ChoiceField): rendered """ network_choices = net.get_available_networks(request, - self.include_subnets, self.filter, self.murano_networks) if self.allow_auto: diff --git a/muranodashboard/tests/unit/common/test_net.py b/muranodashboard/tests/unit/common/test_net.py index a3dc9ca8b..5115c968a 100644 --- a/muranodashboard/tests/unit/common/test_net.py +++ b/muranodashboard/tests/unit/common/test_net.py @@ -51,12 +51,12 @@ class TestNet(testtools.TestCase): test_filter = '^foo\-[\w]+' result = net.get_available_networks(self.mock_request, - include_subnets=True, filter=test_filter, murano_networks='include') expected_result = [ - (('foo-network-id', 'foo-subnet-id'), "Network of 'foo'") + (('foo-network-id', 'foo-subnet-id'), "Network of 'foo'"), + (('foo-network-id', None), "Network of 'foo': random subnet"), ] self.assertEqual(expected_result, result) @@ -84,14 +84,15 @@ class TestNet(testtools.TestCase): test_filter = '^[\w]+\-[\w]+' result = net.get_available_networks(self.mock_request, - include_subnets=True, filter=test_filter, murano_networks='include') expected_result = [ (('foo-network-id', 'foo-subnet-id'), "Network of 'foo'"), + (('foo-network-id', None), "Network of 'foo': random subnet"), (('bar-network-id', 'bar-subnet-id'), - 'bar-network: 255.0.0.0 bar-subnet') + 'bar-network: 255.0.0.0 bar-subnet'), + (('bar-network-id', None), "bar-network: random subnet"), ] self.assertEqual(expected_result, result) @@ -101,60 +102,60 @@ class TestNet(testtools.TestCase): self.mock_request) @mock.patch.object(net, 'neutron', autospec=True) - def test_get_available_networks_with_exclude(self, mock_neutron): - foo_mock_network = mock.Mock(router__external=False, - id='foo-network-id', - subnets=[mock.Mock(id='foo-subnet-id')]) - foo_mock_network.configure_mock(name='foo-network') - bar_mock_subnet = mock.Mock( - id='bar-subnet-id', name_or_id='bar-subnet', cidr='255.0.0.0') - bar_mock_network = mock.Mock(router__external=False, - id='bar-network-id', - name_or_id='bar-network', - subnets=[bar_mock_subnet]) - bar_mock_network.configure_mock(name='bar-network') + def test_get_available_networks(self, mock_neutron): + foo_subnets = [ + type('%s-subnet' % k, (object, ), + {'id': '%s-subnet-id' % k, 'cidr': '255.0.0.0', + 'name_or_id': '%s-name-or-id' % k}) + for k in ('fake1', 'fake2')] + bar_subnets = [ + type('fake3-subnet', (object, ), + {'id': 'fake3-subnet-id', 'cidr': '255.255.0.0', + 'name_or_id': 'fake3-name-or-id'})] + foo_network = type('FooNetwork', (object, ), { + 'router__external': False, + 'id': 'foo-network-id', + 'subnets': foo_subnets, + 'name': 'foo-network-name', + 'name_or_id': 'foo-network-name-or-id', + }) + bar_network = type('BarNetwork', (object, ), { + 'router__external': False, + 'id': 'bar-network-id', + 'subnets': bar_subnets, + 'name': 'bar-network-name', + 'name_or_id': 'bar-network-name-or-id', + }) mock_neutron.network_list_for_tenant.return_value = [ - foo_mock_network, bar_mock_network + foo_network, bar_network, ] - result = net.get_available_networks(self.mock_request, - include_subnets=True, - filter=None, - murano_networks='exclude') + result = net.get_available_networks( + self.mock_request, filter=None, murano_networks='exclude') expected_result = [ - (('bar-network-id', 'bar-subnet-id'), - 'bar-network: 255.0.0.0 bar-subnet') + ((foo_network.id, foo_subnets[0].id), + '%s: %s %s' % ( + foo_network.name_or_id, foo_subnets[0].cidr, + foo_subnets[0].name_or_id)), + ((foo_network.id, foo_subnets[1].id), + '%s: %s %s' % ( + foo_network.name_or_id, foo_subnets[1].cidr, + foo_subnets[1].name_or_id)), + ((foo_network.id, None), + '%s: random subnet' % foo_network.name_or_id), + ((bar_network.id, bar_subnets[0].id), + '%s: %s %s' % ( + bar_network.name_or_id, bar_subnets[0].cidr, + bar_subnets[0].name_or_id)), + ((bar_network.id, None), + '%s: random subnet' % bar_network.name_or_id), ] - self.assertEqual(expected_result, result) - mock_neutron.network_list_for_tenant.assert_called_once_with( - self.mock_request, tenant_id='foo_tenant_id') - self.mock_env_api.environments_list.assert_called_once_with( - self.mock_request) - - @mock.patch.object(net, 'neutron', autospec=True) - def test_get_available_networks_without_include_subnet(self, mock_neutron): - mock_netwok = mock.Mock(router__external=False, - id='foo-network-id', - subnets=[mock.Mock(id='foo-subnet-id')]) - mock_netwok.configure_mock(name='foo-network') - mock_neutron.network_list_for_tenant.return_value = [ - mock_netwok - ] - - result = net.get_available_networks(self.mock_request, - include_subnets=False, - filter=None, - murano_networks='include') - - # Subnet specified in mock_network should be None with include_subnets - # set to False. - expected_result = [ - (('foo-network-id', None), "Network of 'foo'") - ] - - self.assertEqual(expected_result, result) + self.assertIsInstance(result, list) + self.assertEqual(len(expected_result), len(result)) + for choice in expected_result: + self.assertIn(choice, result) mock_neutron.network_list_for_tenant.assert_called_once_with( self.mock_request, tenant_id='foo_tenant_id') self.mock_env_api.environments_list.assert_called_once_with( diff --git a/muranodashboard/tests/unit/dynamic_ui/test_fields.py b/muranodashboard/tests/unit/dynamic_ui/test_fields.py index ed4db45eb..487378636 100644 --- a/muranodashboard/tests/unit/dynamic_ui/test_fields.py +++ b/muranodashboard/tests/unit/dynamic_ui/test_fields.py @@ -648,7 +648,6 @@ class TestNetworkChoiceField(testtools.TestCase): def setUp(self): super(TestNetworkChoiceField, self).setUp() self.network_choice_field = fields.NetworkChoiceField( - include_subnets=True, filter=None, murano_networks='exclude', allow_auto=True) @@ -667,7 +666,7 @@ class TestNetworkChoiceField(testtools.TestCase): self.network_choice_field.update(self.request) self.assertEqual(expected_choices, self.network_choice_field.choices) mock_net.get_available_networks.assert_called_once_with( - self.request['request'], True, None, 'exclude') + self.request['request'], None, 'exclude') def test_to_python(self): self.assertEqual({'foo': 'bar'},