openstack/python-openstackclient
Code Issues Proposed changes

Fix Nova-net netowrk commands

Browse Source 
In cleaning up functional tests for nova-net, I discovered some
problems in network create:
* --subnet option is required in network create command
* Switch API to use /os-networks rather than /os-tenant-networks as this
  is what we were actually using via novaclient
* Fix functional tests for nova-net
* Normalize some private function names in network/v2/network.py

Change-Id: I426b864406756d58d140575a3a45ee9aee67ce84
This commit is contained in:
Dean Troyer 2017-04-27 10:26:07 -05:00
parent 7b609ebd55
commit 589a65c3fe
7 changed files with 300 additions and 144 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
doc/source/backwards-incompatible.rst
View File

@ -30,7 +30,13 @@ Backwards Incompatible Changes
Release 3.10
------------
1. The positional argument ``<snapshot-name>`` of the ``volume snapshot create``
1. The ``network create`` command now requires the ``--subnet`` option when used
with Nova-network clouds.
* As of: 3.10
* Commit: https://review.openstack.org/460679
2. The positional argument ``<snapshot-name>`` of the ``volume snapshot create``
command is no longer optional.
Previously when the ``--volume`` option was

2
doc/source/command-objects/network.rst
View File

@ -71,6 +71,8 @@ Create new network
Set network description
*Network version 2 only*
.. option:: --availability-zone-hint <availability-zone>
Availability Zone in which to create this network

23
openstackclient/api/compute_v2.py
View File

@ -202,24 +202,25 @@ class APIv2(api.BaseAPI):
):
"""Create a new network
https://developer.openstack.org/api-ref/compute/#create-project-network
https://developer.openstack.org/api-ref/compute/#create-network
:param string name:
Network label
Network label (required)
:param integer subnet:
Subnet for IPv4 fixed addresses in CIDR notation
Subnet for IPv4 fixed addresses in CIDR notation (required)
:param integer share_subnet:
Shared subnet between projects, True or False
:returns: A dict of the network attributes
"""
url = "/os-tenant-networks"
url = "/os-networks"
params = {
'label': name,
'cidr': subnet,
'share_address': share_subnet,
}
if share_subnet is not None:
params['share_address'] = share_subnet
return self.create(
url,
@ -232,13 +233,13 @@ class APIv2(api.BaseAPI):
):
"""Delete a network
https://developer.openstack.org/api-ref/compute/#delete-project-network
https://developer.openstack.org/api-ref/compute/#delete-network
:param string network:
Network name or ID
"""
url = "/os-tenant-networks"
url = "/os-networks"
network = self.find(
url,
@ -256,14 +257,14 @@ class APIv2(api.BaseAPI):
):
"""Return a network given name or ID
https://developer.openstack.org/api-ref/compute/#show-project-network-details
https://developer.openstack.org/api-ref/compute/#show-network-details
:param string network:
Network name or ID
:returns: A dict of the network attributes
"""
url = "/os-tenant-networks"
url = "/os-networks"
return self.find(
url,
@ -276,13 +277,13 @@ class APIv2(api.BaseAPI):
):
"""Get networks
https://developer.openstack.org/api-ref/compute/#list-project-networks
https://developer.openstack.org/api-ref/compute/#list-networks
:returns:
list of networks
"""
url = "/os-tenant-networks"
url = "/os-networks"
return self.list(url)["networks"]

45
openstackclient/network/v2/network.py
View File

@ -42,7 +42,7 @@ _formatters = {
}
def _get_network_columns(item):
def _get_columns_network(item):
column_map = {
'subnet_ids': 'subnets',
'is_admin_state_up': 'admin_state_up',
@ -59,14 +59,14 @@ def _get_network_columns(item):
return sdk_utils.get_osc_show_columns_for_sdk_resource(item, column_map)
def _get_columns(item):
def _get_columns_compute(item):
column_map = {
'tenant_id': 'project_id',
}
return sdk_utils.get_osc_show_columns_for_sdk_resource(item, column_map)
def _get_attrs(client_manager, parsed_args):
def _get_attrs_network(client_manager, parsed_args):
attrs = {}
if parsed_args.name is not None:
attrs['name'] = str(parsed_args.name)
@ -135,6 +135,19 @@ def _get_attrs(client_manager, parsed_args):
return attrs
def _get_attrs_compute(client_manager, parsed_args):
attrs = {}
if parsed_args.name is not None:
attrs['name'] = str(parsed_args.name)
if parsed_args.share:
attrs['share_subnet'] = True
if parsed_args.no_share:
attrs['share_subnet'] = False
if parsed_args.subnet is not None:
attrs['subnet'] = parsed_args.subnet
return attrs
def _add_additional_network_options(parser):
# Add additional network options
@ -168,19 +181,6 @@ def _add_additional_network_options(parser):
help=_("Do not make the network VLAN transparent"))
def _get_attrs_compute(client_manager, parsed_args):
attrs = {}
if parsed_args.name is not None:
attrs['name'] = str(parsed_args.name)
if parsed_args.share:
attrs['share_subnet'] = True
if parsed_args.no_share:
attrs['share_subnet'] = False
if parsed_args.subnet is not None:
attrs['subnet'] = parsed_args.subnet
return attrs
# TODO(sindhu): Use the SDK resource mapped attribute names once the
# OSC minimum requirements include SDK 1.0.
class CreateNetwork(common.NetworkAndComputeShowOne):
@ -289,21 +289,22 @@ class CreateNetwork(common.NetworkAndComputeShowOne):
parser.add_argument(
'--subnet',
metavar='<subnet>',
required=True,
help=_("IPv4 subnet for fixed IPs (in CIDR notation)")
)
return parser
def take_action_network(self, client, parsed_args):
attrs = _get_attrs(self.app.client_manager, parsed_args)
attrs = _get_attrs_network(self.app.client_manager, parsed_args)
obj = client.create_network(**attrs)
display_columns, columns = _get_network_columns(obj)
display_columns, columns = _get_columns_network(obj)
data = utils.get_item_properties(obj, columns, formatters=_formatters)
return (display_columns, data)
def take_action_compute(self, client, parsed_args):
attrs = _get_attrs_compute(self.app.client_manager, parsed_args)
obj = client.api.network_create(**attrs)
display_columns, columns = _get_columns(obj)
display_columns, columns = _get_columns_compute(obj)
data = utils.get_dict_properties(obj, columns)
return (display_columns, data)
@ -660,7 +661,7 @@ class SetNetwork(command.Command):
client = self.app.client_manager.network
obj = client.find_network(parsed_args.network, ignore_missing=False)
attrs = _get_attrs(self.app.client_manager, parsed_args)
attrs = _get_attrs_network(self.app.client_manager, parsed_args)
client.update_network(obj, **attrs)
@ -677,12 +678,12 @@ class ShowNetwork(common.NetworkAndComputeShowOne):
def take_action_network(self, client, parsed_args):
obj = client.find_network(parsed_args.network, ignore_missing=False)
display_columns, columns = _get_network_columns(obj)
display_columns, columns = _get_columns_network(obj)
data = utils.get_item_properties(obj, columns, formatters=_formatters)
return (display_columns, data)
def take_action_compute(self, client, parsed_args):
obj = client.api.network_find(parsed_args.network)
display_columns, columns = _get_columns(obj)
display_columns, columns = _get_columns_compute(obj)
data = utils.get_dict_properties(obj, columns)
return (display_columns, data)

318
openstackclient/tests/functional/network/v2/test_network.py
View File

@ -19,8 +19,65 @@ from openstackclient.tests.functional import base
class NetworkTests(base.TestCase):
"""Functional tests for network"""
def test_network_create(self):
"""Test create options, delete"""
@classmethod
def setUpClass(cls):
cls.haz_network = base.is_service_enabled('network')
cls.PROJECT_NAME =\
cls.get_openstack_configuration_value('auth.project_name')
def test_network_create_compute(self):
"""Test Nova-net create options, delete"""
if self.haz_network:
self.skipTest("Skip Nova-net test")
# Network create with minimum options
name1 = uuid.uuid4().hex
cmd_output = json.loads(self.openstack(
'network create -f json ' +
'--subnet 1.2.3.4/28 ' +
name1
))
self.addCleanup(self.openstack, 'network delete ' + name1)
self.assertIsNotNone(cmd_output["id"])
self.assertEqual(
name1,
cmd_output["label"],
)
self.assertEqual(
'1.2.3.0/28',
cmd_output["cidr"],
)
# Network create with more options
name2 = uuid.uuid4().hex
cmd_output = json.loads(self.openstack(
'network create -f json ' +
'--subnet 1.2.4.4/28 ' +
'--share ' +
name2
))
self.addCleanup(self.openstack, 'network delete ' + name2)
self.assertIsNotNone(cmd_output["id"])
self.assertEqual(
name2,
cmd_output["label"],
)
self.assertEqual(
'1.2.4.0/28',
cmd_output["cidr"],
)
self.assertEqual(
True,
cmd_output["share_address"],
)
def test_network_create_network(self):
"""Test Neutron create options, delete"""
if not self.haz_network:
self.skipTest("No Network service present")
# Get project IDs
cmd_output = json.loads(self.openstack('token issue -f json '))
auth_project_id = cmd_output['project_id']
@ -43,7 +100,7 @@ class NetworkTests(base.TestCase):
self.assertNotEqual(admin_project_id, demo_project_id)
self.assertEqual(admin_project_id, auth_project_id)
# network create with no options
# Network create with no options
name1 = uuid.uuid4().hex
cmd_output = json.loads(self.openstack(
'network create -f json ' +
@ -74,6 +131,7 @@ class NetworkTests(base.TestCase):
cmd_output["router:external"],
)
# Network create with options
name2 = uuid.uuid4().hex
cmd_output = json.loads(self.openstack(
'network create -f json ' +
@ -91,8 +149,40 @@ class NetworkTests(base.TestCase):
cmd_output["description"],
)
def test_network_delete(self):
def test_network_delete_compute(self):
"""Test create, delete multiple"""
if self.haz_network:
self.skipTest("Skip Nova-net test")
name1 = uuid.uuid4().hex
cmd_output = json.loads(self.openstack(
'network create -f json ' +
'--subnet 9.8.7.6/28 ' +
name1
))
self.assertIsNotNone(cmd_output["id"])
self.assertEqual(
name1,
cmd_output["label"],
)
name2 = uuid.uuid4().hex
cmd_output = json.loads(self.openstack(
'network create -f json ' +
'--subnet 8.7.6.5/28 ' +
name2
))
self.assertIsNotNone(cmd_output["id"])
self.assertEqual(
name2,
cmd_output["label"],
)
def test_network_delete_network(self):
"""Test create, delete multiple"""
if not self.haz_network:
self.skipTest("No Network service present")
name1 = uuid.uuid4().hex
cmd_output = json.loads(self.openstack(
'network create -f json ' +
@ -123,122 +213,157 @@ class NetworkTests(base.TestCase):
def test_network_list(self):
"""Test create defaults, list filters, delete"""
name1 = uuid.uuid4().hex
if self.haz_network:
network_options = '--description aaaa --no-default '
else:
network_options = '--subnet 3.4.5.6/28 '
cmd_output = json.loads(self.openstack(
'network create -f json ' +
'--description aaaa ' +
'--no-default ' +
network_options +
name1
))
self.addCleanup(self.openstack, 'network delete ' + name1)
self.assertIsNotNone(cmd_output["id"])
self.assertEqual(
'aaaa',
cmd_output["description"],
)
# Check the default values
self.assertEqual(
'UP',
cmd_output["admin_state_up"],
)
self.assertEqual(
False,
cmd_output["shared"],
)
self.assertEqual(
'Internal',
cmd_output["router:external"],
)
self.assertEqual(
False,
cmd_output["is_default"],
)
self.assertEqual(
True,
cmd_output["port_security_enabled"],
)
if self.haz_network:
self.assertEqual(
'aaaa',
cmd_output["description"],
)
# Check the default values
self.assertEqual(
'UP',
cmd_output["admin_state_up"],
)
self.assertEqual(
False,
cmd_output["shared"],
)
self.assertEqual(
'Internal',
cmd_output["router:external"],
)
self.assertEqual(
False,
cmd_output["is_default"],
)
self.assertEqual(
True,
cmd_output["port_security_enabled"],
)
else:
self.assertEqual(
'3.4.5.0/28',
cmd_output["cidr"],
)
name2 = uuid.uuid4().hex
if self.haz_network:
network_options = '--description bbbb --disable '
else:
network_options = '--subnet 4.5.6.7/28 '
cmd_output = json.loads(self.openstack(
'network create -f json ' +
'--description bbbb ' +
'--disable ' +
'--share ' +
network_options +
name2
))
self.addCleanup(self.openstack, 'network delete ' + name2)
self.assertIsNotNone(cmd_output["id"])
self.assertEqual(
'bbbb',
cmd_output["description"],
)
self.assertEqual(
'DOWN',
cmd_output["admin_state_up"],
)
self.assertEqual(
True,
cmd_output["shared"],
)
self.assertEqual(
False,
cmd_output["is_default"],
)
self.assertEqual(
True,
cmd_output["port_security_enabled"],
)
if self.haz_network:
self.assertEqual(
'bbbb',
cmd_output["description"],
)
self.assertEqual(
'DOWN',
cmd_output["admin_state_up"],
)
self.assertEqual(
True,
cmd_output["shared"],
)
self.assertEqual(
False,
cmd_output["is_default"],
)
self.assertEqual(
True,
cmd_output["port_security_enabled"],
)
else:
self.assertEqual(
'4.5.6.0/28',
cmd_output["cidr"],
)
self.assertEqual(
True,
cmd_output["share_address"],
)
# Test list
cmd_output = json.loads(self.openstack(
"network list -f json "
))
col_name = [x["Name"] for x in cmd_output]
self.assertIn(name1, col_name)
self.assertIn(name2, col_name)
# Test list --long
cmd_output = json.loads(self.openstack(
"network list -f json " +
"--long"
))
col_name = [x["Name"] for x in cmd_output]
self.assertIn(name1, col_name)
self.assertIn(name2, col_name)
if self.haz_network:
cmd_output = json.loads(self.openstack(
"network list -f json " +
"--long"
))
col_name = [x["Name"] for x in cmd_output]
self.assertIn(name1, col_name)
self.assertIn(name2, col_name)
# Test list --long --enable
cmd_output = json.loads(self.openstack(
"network list -f json " +
"--enable " +
"--long"
))
col_name = [x["Name"] for x in cmd_output]
self.assertIn(name1, col_name)
self.assertNotIn(name2, col_name)
if self.haz_network:
cmd_output = json.loads(self.openstack(
"network list -f json " +
"--enable " +
"--long"
))
col_name = [x["Name"] for x in cmd_output]
self.assertIn(name1, col_name)
self.assertNotIn(name2, col_name)
# Test list --long --disable
cmd_output = json.loads(self.openstack(
"network list -f json " +
"--disable " +
"--long"
))
col_name = [x["Name"] for x in cmd_output]
self.assertNotIn(name1, col_name)
self.assertIn(name2, col_name)
if self.haz_network:
cmd_output = json.loads(self.openstack(
"network list -f json " +
"--disable " +
"--long"
))
col_name = [x["Name"] for x in cmd_output]
self.assertNotIn(name1, col_name)
self.assertIn(name2, col_name)
# Test list --long --share
cmd_output = json.loads(self.openstack(
"network list -f json " +
"--share " +
"--long"
))
col_name = [x["Name"] for x in cmd_output]
self.assertNotIn(name1, col_name)
self.assertIn(name2, col_name)
# Test list --share
if self.haz_network:
cmd_output = json.loads(self.openstack(
"network list -f json " +
"--share "
))
col_name = [x["Name"] for x in cmd_output]
self.assertNotIn(name1, col_name)
self.assertIn(name2, col_name)
# Test list --long --no-share
cmd_output = json.loads(self.openstack(
"network list -f json " +
"--no-share " +
"--long"
))
col_name = [x["Name"] for x in cmd_output]
self.assertIn(name1, col_name)
self.assertNotIn(name2, col_name)
# Test list --no-share
if self.haz_network:
cmd_output = json.loads(self.openstack(
"network list -f json " +
"--no-share "
))
col_name = [x["Name"] for x in cmd_output]
self.assertIn(name1, col_name)
self.assertNotIn(name2, col_name)
def test_network_dhcp_agent(self):
if self.haz_network:
self.skipTest("No Network service present")
name1 = uuid.uuid4().hex
cmd_output1 = json.loads(self.openstack(
'network create -f json ' +
@ -283,6 +408,9 @@ class NetworkTests(base.TestCase):
def test_network_set(self):
"""Tests create options, set, show, delete"""
if self.haz_network:
self.skipTest("No Network service present")
name = uuid.uuid4().hex
cmd_output = json.loads(self.openstack(
'network create -f json ' +

30
openstackclient/tests/unit/api/test_compute_v2.py
View File

@ -185,7 +185,7 @@ class TestNetwork(TestComputeAPIv2):
def test_network_create_default(self):
self.requests_mock.register_uri(
'POST',
FAKE_URL + '/os-tenant-networks',
FAKE_URL + '/os-networks',
json={'network': self.FAKE_NETWORK_RESP},
status_code=200,
)
@ -195,7 +195,7 @@ class TestNetwork(TestComputeAPIv2):
def test_network_create_options(self):
self.requests_mock.register_uri(
'POST',
FAKE_URL + '/os-tenant-networks',
FAKE_URL + '/os-networks',
json={'network': self.FAKE_NETWORK_RESP},
status_code=200,
)
@ -208,13 +208,13 @@ class TestNetwork(TestComputeAPIv2):
def test_network_delete_id(self):
self.requests_mock.register_uri(
'GET',
FAKE_URL + '/os-tenant-networks/1',
FAKE_URL + '/os-networks/1',
json={'network': self.FAKE_NETWORK_RESP},
status_code=200,
)
self.requests_mock.register_uri(
'DELETE',
FAKE_URL + '/os-tenant-networks/1',
FAKE_URL + '/os-networks/1',
status_code=202,
)
ret = self.api.network_delete('1')
@ -224,18 +224,18 @@ class TestNetwork(TestComputeAPIv2):
def test_network_delete_name(self):
self.requests_mock.register_uri(
'GET',
FAKE_URL + '/os-tenant-networks/label1',
FAKE_URL + '/os-networks/label1',
status_code=404,
)
self.requests_mock.register_uri(
'GET',
FAKE_URL + '/os-tenant-networks',
FAKE_URL + '/os-networks',
json={'networks': self.LIST_NETWORK_RESP},
status_code=200,
)
self.requests_mock.register_uri(
'DELETE',
FAKE_URL + '/os-tenant-networks/1',
FAKE_URL + '/os-networks/1',
status_code=202,
)
ret = self.api.network_delete('label1')
@ -245,12 +245,12 @@ class TestNetwork(TestComputeAPIv2):
def test_network_delete_not_found(self):
self.requests_mock.register_uri(
'GET',
FAKE_URL + '/os-tenant-networks/label3',
FAKE_URL + '/os-networks/label3',
status_code=404,
)
self.requests_mock.register_uri(
'GET',
FAKE_URL + '/os-tenant-networks',
FAKE_URL + '/os-networks',
json={'networks': self.LIST_NETWORK_RESP},
status_code=200,
)
@ -263,7 +263,7 @@ class TestNetwork(TestComputeAPIv2):
def test_network_find_id(self):
self.requests_mock.register_uri(
'GET',
FAKE_URL + '/os-tenant-networks/1',
FAKE_URL + '/os-networks/1',
json={'network': self.FAKE_NETWORK_RESP},
status_code=200,
)
@ -273,12 +273,12 @@ class TestNetwork(TestComputeAPIv2):
def test_network_find_name(self):
self.requests_mock.register_uri(
'GET',
FAKE_URL + '/os-tenant-networks/label2',
FAKE_URL + '/os-networks/label2',
status_code=404,
)
self.requests_mock.register_uri(
'GET',
FAKE_URL + '/os-tenant-networks',
FAKE_URL + '/os-networks',
json={'networks': self.LIST_NETWORK_RESP},
status_code=200,
)
@ -288,12 +288,12 @@ class TestNetwork(TestComputeAPIv2):
def test_network_find_not_found(self):
self.requests_mock.register_uri(
'GET',
FAKE_URL + '/os-tenant-networks/label3',
FAKE_URL + '/os-networks/label3',
status_code=404,
)
self.requests_mock.register_uri(
'GET',
FAKE_URL + '/os-tenant-networks',
FAKE_URL + '/os-networks',
json={'networks': self.LIST_NETWORK_RESP},
status_code=200,
)
@ -306,7 +306,7 @@ class TestNetwork(TestComputeAPIv2):
def test_network_list_no_options(self):
self.requests_mock.register_uri(
'GET',
FAKE_URL + '/os-tenant-networks',
FAKE_URL + '/os-networks',
json={'networks': self.LIST_NETWORK_RESP},
status_code=200,
)

18
openstackclient/tests/unit/network/v2/test_network_compute.py
View File

@ -132,6 +132,24 @@ class TestCreateNetworkCompute(TestNetworkCompute):
verifylist,
)
def test_network_create_missing_options(self, net_mock):
net_mock.return_value = self._network
arglist = [
self._network['label'],
]
verifylist = [
('name', self._network['label']),
]
# Missing required args should raise exception here
self.assertRaises(
tests_utils.ParserException,
self.check_parser,
self.cmd,
arglist,
verifylist,
)
def test_network_create_default_options(self, net_mock):
net_mock.return_value = self._network
arglist = [