From 75ed315885b9f6f433a791bb3e55cf657d01de83 Mon Sep 17 00:00:00 2001 From: Brian Haley Date: Wed, 14 Feb 2024 17:28:04 -0500 Subject: [PATCH] Do not sort subnet dns_nameservers field When using table output format, the dns_nameservers field of a subnet is sorted, but it should not be as the order is important. Created an UnsortedListColumn() class in subnet.py so the output is correct. Updated the unit test accordingly to verify the order is correct when an entry is removed. Change-Id: I60a15a944f83549738305dd025db38ff8e165be7 Closes-bug: #2053201 --- openstackclient/network/v2/subnet.py | 9 ++++++- .../tests/unit/network/v2/test_subnet.py | 24 +++++++++++-------- 2 files changed, 22 insertions(+), 11 deletions(-) diff --git a/openstackclient/network/v2/subnet.py b/openstackclient/network/v2/subnet.py index 950ad2578..a9af74bba 100644 --- a/openstackclient/network/v2/subnet.py +++ b/openstackclient/network/v2/subnet.py @@ -60,9 +60,16 @@ class HostRoutesColumn(cliff_columns.FormattableColumn): ) +class UnsortedListColumn(cliff_columns.FormattableColumn): + # format_columns.ListColumn sorts the output, but for things like + # DNS server addresses the order matters + def human_readable(self): + return ', '.join(self._value) + + _formatters = { 'allocation_pools': AllocationPoolsColumn, - 'dns_nameservers': format_columns.ListColumn, + 'dns_nameservers': UnsortedListColumn, 'host_routes': HostRoutesColumn, 'service_types': format_columns.ListColumn, 'tags': format_columns.ListColumn, diff --git a/openstackclient/tests/unit/network/v2/test_subnet.py b/openstackclient/tests/unit/network/v2/test_subnet.py index 649ba2af7..816026410 100644 --- a/openstackclient/tests/unit/network/v2/test_subnet.py +++ b/openstackclient/tests/unit/network/v2/test_subnet.py @@ -167,7 +167,7 @@ class TestCreateSubnet(TestSubnet): subnet_v2.AllocationPoolsColumn(self._subnet.allocation_pools), self._subnet.cidr, self._subnet.description, - format_columns.ListColumn(self._subnet.dns_nameservers), + subnet_v2.UnsortedListColumn(self._subnet.dns_nameservers), self._subnet.enable_dhcp, self._subnet.gateway_ip, subnet_v2.HostRoutesColumn(self._subnet.host_routes), @@ -190,7 +190,9 @@ class TestCreateSubnet(TestSubnet): ), self._subnet_from_pool.cidr, self._subnet_from_pool.description, - format_columns.ListColumn(self._subnet_from_pool.dns_nameservers), + subnet_v2.UnsortedListColumn( + self._subnet_from_pool.dns_nameservers + ), self._subnet_from_pool.enable_dhcp, self._subnet_from_pool.gateway_ip, subnet_v2.HostRoutesColumn(self._subnet_from_pool.host_routes), @@ -213,7 +215,7 @@ class TestCreateSubnet(TestSubnet): ), self._subnet_ipv6.cidr, self._subnet_ipv6.description, - format_columns.ListColumn(self._subnet_ipv6.dns_nameservers), + subnet_v2.UnsortedListColumn(self._subnet_ipv6.dns_nameservers), self._subnet_ipv6.enable_dhcp, self._subnet_ipv6.gateway_ip, subnet_v2.HostRoutesColumn(self._subnet_ipv6.host_routes), @@ -236,7 +238,7 @@ class TestCreateSubnet(TestSubnet): ), self._subnet_ipv6_pd.cidr, self._subnet_ipv6_pd.description, - format_columns.ListColumn(self._subnet_ipv6_pd.dns_nameservers), + subnet_v2.UnsortedListColumn(self._subnet_ipv6_pd.dns_nameservers), self._subnet_ipv6_pd.enable_dhcp, self._subnet_ipv6_pd.gateway_ip, subnet_v2.HostRoutesColumn(self._subnet_ipv6_pd.host_routes), @@ -837,7 +839,7 @@ class TestListSubnet(TestSubnet): subnet.cidr, subnet.project_id, subnet.enable_dhcp, - format_columns.ListColumn(subnet.dns_nameservers), + subnet_v2.UnsortedListColumn(subnet.dns_nameservers), subnet_v2.AllocationPoolsColumn(subnet.allocation_pools), subnet_v2.HostRoutesColumn(subnet.host_routes), subnet.ip_version, @@ -1489,7 +1491,7 @@ class TestShowSubnet(TestSubnet): subnet_v2.AllocationPoolsColumn(_subnet.allocation_pools), _subnet.cidr, _subnet.description, - format_columns.ListColumn(_subnet.dns_nameservers), + subnet_v2.UnsortedListColumn(_subnet.dns_nameservers), _subnet.enable_dhcp, _subnet.gateway_ip, subnet_v2.HostRoutesColumn(_subnet.host_routes), @@ -1550,9 +1552,10 @@ class TestShowSubnet(TestSubnet): class TestUnsetSubnet(TestSubnet): def setUp(self): super(TestUnsetSubnet, self).setUp() + # Add three dns_nameserver entries so we can verify ordering self._testsubnet = network_fakes.FakeSubnet.create_one_subnet( { - 'dns_nameservers': ['8.8.8.8', '8.8.8.4'], + 'dns_nameservers': ['8.8.8.8', '8.8.8.4', '8.8.4.4'], 'host_routes': [ {'destination': '10.20.20.0/24', 'nexthop': '10.20.20.1'}, {'destination': '10.30.30.30/24', 'nexthop': '10.30.30.1'}, @@ -1578,9 +1581,10 @@ class TestUnsetSubnet(TestSubnet): self.cmd = subnet_v2.UnsetSubnet(self.app, self.namespace) def test_unset_subnet_params(self): + # Remove just the middle dns_nameserver entry, verify still in order arglist = [ '--dns-nameserver', - '8.8.8.8', + '8.8.8.4', '--host-route', 'destination=10.30.30.30/24,gateway=10.30.30.1', '--allocation-pool', @@ -1591,7 +1595,7 @@ class TestUnsetSubnet(TestSubnet): self._testsubnet.name, ] verifylist = [ - ('dns_nameservers', ['8.8.8.8']), + ('dns_nameservers', ['8.8.8.4']), ( 'host_routes', [{"destination": "10.30.30.30/24", "gateway": "10.30.30.1"}], @@ -1605,7 +1609,7 @@ class TestUnsetSubnet(TestSubnet): result = self.cmd.take_action(parsed_args) attrs = { - 'dns_nameservers': ['8.8.8.4'], + 'dns_nameservers': ['8.8.8.8', '8.8.4.4'], 'host_routes': [ {"destination": "10.20.20.0/24", "nexthop": "10.20.20.1"} ],