From ad3369ed1fdad73b5d457a40df7df8ad55eb69cd Mon Sep 17 00:00:00 2001 From: Artem Goncharov Date: Mon, 2 Nov 2020 13:25:59 +0100 Subject: [PATCH] Fix formatting of the flavor properties Do not stringify flavor properties to allow proper output formatting to json/yaml/etc Change-Id: I9f4c42acb85b726af87123134dd19de98fe95074 --- lower-constraints.txt | 4 +- openstackclient/compute/v2/flavor.py | 59 +++++++++++++++---- .../functional/compute/v2/test_flavor.py | 43 ++++++++------ .../tests/unit/compute/v2/test_flavor.py | 26 ++++---- ...vor-props-formatting-d21e97745543caa7.yaml | 4 ++ requirements.txt | 4 +- 6 files changed, 94 insertions(+), 46 deletions(-) create mode 100644 releasenotes/notes/fix-flavor-props-formatting-d21e97745543caa7.yaml diff --git a/lower-constraints.txt b/lower-constraints.txt index 74eac6403..52766b229 100644 --- a/lower-constraints.txt +++ b/lower-constraints.txt @@ -5,7 +5,7 @@ asn1crypto==0.23.0 bandit==1.1.0 cachetools==2.0.0 cffi==1.14.0 -cliff==2.8.0 +cliff==3.4.0 cmd2==0.8.0 contextlib2==0.4.0 coverage==4.0 @@ -48,7 +48,7 @@ netifaces==0.10.4 openstacksdk==0.48.0 os-service-types==1.7.0 os-testr==1.0.0 -osc-lib==2.0.0 +osc-lib==2.2.0 osc-placement==1.7.0 oslo.concurrency==3.26.0 oslo.config==5.2.0 diff --git a/openstackclient/compute/v2/flavor.py b/openstackclient/compute/v2/flavor.py index 805e919ea..00431b7b7 100644 --- a/openstackclient/compute/v2/flavor.py +++ b/openstackclient/compute/v2/flavor.py @@ -18,6 +18,7 @@ import logging from novaclient import api_versions +from osc_lib.cli import format_columns from osc_lib.cli import parseractions from osc_lib.command import command from osc_lib import exceptions @@ -30,6 +31,31 @@ from openstackclient.identity import common as identity_common LOG = logging.getLogger(__name__) +_formatters = { + 'extra_specs': format_columns.DictColumn, + # Unless we finish switch to use SDK resources this need to be doubled this + # way + 'properties': format_columns.DictColumn, + 'Properties': format_columns.DictColumn +} + + +def _get_flavor_columns(item): + # To maintain backwards compatibility we need to rename sdk props to + # whatever OSC was using before + column_map = { + 'extra_specs': 'properties', + 'ephemeral': 'OS-FLV-EXT-DATA:ephemeral', + 'is_disabled': 'OS-FLV-DISABLED:disabled', + 'is_public': 'os-flavor-access:is_public' + + } + hidden_columns = ['links', 'location'] + + return utils.get_osc_show_columns_for_sdk_resource( + item, column_map, hidden_columns) + + def _find_flavor(compute_client, flavor): try: return compute_client.flavors.get(flavor) @@ -191,10 +217,16 @@ class CreateFlavor(command.ShowOne): LOG.error(_("Failed to set flavor property: %s"), e) flavor_info = flavor._info.copy() - flavor_info.pop("links") - flavor_info['properties'] = utils.format_dict(flavor.get_keys()) + flavor_info['properties'] = flavor.get_keys() - return zip(*sorted(flavor_info.items())) + display_columns, columns = _get_flavor_columns(flavor_info) + data = utils.get_dict_properties( + flavor_info, columns, + formatters=_formatters, + mixed_case_fields=['OS-FLV-DISABLED:disabled', + 'OS-FLV-EXT-DATA:ephemeral']) + + return (display_columns, data) class DeleteFlavor(command.Command): @@ -309,7 +341,7 @@ class ListFlavor(command.Lister): return (column_headers, (utils.get_item_properties( - s, columns, formatters={'Properties': utils.format_dict}, + s, columns, formatters=_formatters, ) for s in data)) @@ -428,11 +460,8 @@ class ShowFlavor(command.ShowOne): try: flavor_access = compute_client.flavor_access.list( flavor=resource_flavor.id) - projects = [utils.get_field(access, 'tenant_id') - for access in flavor_access] - # TODO(Huanxuan Ao): This format case can be removed after - # patch https://review.opendev.org/#/c/330223/ merged. - access_projects = utils.format_list(projects) + access_projects = [utils.get_field(access, 'tenant_id') + for access in flavor_access] except Exception as e: msg = _("Failed to get access projects list " "for flavor '%(flavor)s': %(e)s") @@ -442,11 +471,17 @@ class ShowFlavor(command.ShowOne): flavor.update({ 'access_project_ids': access_projects }) - flavor.pop("links", None) - flavor['properties'] = utils.format_dict(resource_flavor.get_keys()) + flavor['properties'] = resource_flavor.get_keys() - return zip(*sorted(flavor.items())) + display_columns, columns = _get_flavor_columns(flavor) + data = utils.get_dict_properties( + flavor, columns, + formatters=_formatters, + mixed_case_fields=['OS-FLV-DISABLED:disabled', + 'OS-FLV-EXT-DATA:ephemeral']) + + return (display_columns, data) class UnsetFlavor(command.Command): diff --git a/openstackclient/tests/functional/compute/v2/test_flavor.py b/openstackclient/tests/functional/compute/v2/test_flavor.py index c274adf2e..162d42878 100644 --- a/openstackclient/tests/functional/compute/v2/test_flavor.py +++ b/openstackclient/tests/functional/compute/v2/test_flavor.py @@ -115,8 +115,8 @@ class FlavorTests(base.TestCase): self.assertFalse( cmd_output["os-flavor-access:is_public"], ) - self.assertEqual( - "a='b2', b='d2'", + self.assertDictEqual( + {"a": "b2", "b": "d2"}, cmd_output["properties"], ) @@ -133,12 +133,18 @@ class FlavorTests(base.TestCase): "flavor list -f json " + "--long" )) - col_name = [x["Name"] for x in cmd_output] - col_properties = [x['Properties'] for x in cmd_output] - self.assertIn(name1, col_name) - self.assertIn("a='b', c='d'", col_properties) - self.assertNotIn(name2, col_name) - self.assertNotIn("b2', b='d2'", col_properties) + # We have list of complex json objects + # Iterate through the list setting flags + found_expected = False + for rec in cmd_output: + if rec['Name'] == name1: + found_expected = True + self.assertEqual('b', rec['Properties']['a']) + self.assertEqual('d', rec['Properties']['c']) + elif rec['Name'] == name2: + # We should have not seen private flavor + self.assertFalse(True) + self.assertTrue(found_expected) # Test list --public cmd_output = json.loads(self.openstack( @@ -201,8 +207,8 @@ class FlavorTests(base.TestCase): self.assertFalse( cmd_output["os-flavor-access:is_public"], ) - self.assertEqual( - "a='first', b='second'", + self.assertDictEqual( + {"a": "first", "b": "second"}, cmd_output["properties"], ) @@ -223,9 +229,14 @@ class FlavorTests(base.TestCase): cmd_output["id"], ) self.assertEqual( - "a='third and 10', b='second', g='fourth'", - cmd_output['properties'], - ) + 'third and 10', + cmd_output['properties']['a']) + self.assertEqual( + 'second', + cmd_output['properties']['b']) + self.assertEqual( + 'fourth', + cmd_output['properties']['g']) raw_output = self.openstack( "flavor unset " + @@ -238,7 +249,5 @@ class FlavorTests(base.TestCase): "flavor show -f json " + name1 )) - self.assertEqual( - "a='third and 10', g='fourth'", - cmd_output["properties"], - ) + + self.assertNotIn('b', cmd_output['properties']) diff --git a/openstackclient/tests/unit/compute/v2/test_flavor.py b/openstackclient/tests/unit/compute/v2/test_flavor.py index 4732cc822..2828d74e3 100644 --- a/openstackclient/tests/unit/compute/v2/test_flavor.py +++ b/openstackclient/tests/unit/compute/v2/test_flavor.py @@ -17,8 +17,8 @@ from unittest import mock from unittest.mock import call import novaclient +from osc_lib.cli import format_columns from osc_lib import exceptions -from osc_lib import utils from openstackclient.compute.v2 import flavor from openstackclient.tests.unit.compute.v2 import fakes as compute_fakes @@ -70,7 +70,7 @@ class TestFlavorCreate(TestFlavor): flavor.id, flavor.name, flavor.is_public, - utils.format_dict(flavor.properties), + format_columns.DictColumn(flavor.properties), flavor.ram, flavor.rxtx_factor, flavor.swap, @@ -111,7 +111,7 @@ class TestFlavorCreate(TestFlavor): self.flavors_mock.create.assert_called_once_with(*default_args) self.assertEqual(self.columns, columns) - self.assertEqual(self.data, data) + self.assertItemEqual(self.data, data) def test_flavor_create_all_options(self): @@ -165,7 +165,7 @@ class TestFlavorCreate(TestFlavor): self.flavor.get_keys.assert_called_once_with() self.assertEqual(self.columns, columns) - self.assertEqual(self.data, data) + self.assertItemEqual(self.data, data) def test_flavor_create_other_options(self): @@ -226,7 +226,7 @@ class TestFlavorCreate(TestFlavor): {'key1': 'value1', 'key2': 'value2'}) self.flavor.get_keys.assert_called_with() self.assertEqual(self.columns, columns) - self.assertEqual(self.data, data) + self.assertItemEqual(self.data, data) def test_public_flavor_create_with_project(self): arglist = [ @@ -300,7 +300,7 @@ class TestFlavorCreate(TestFlavor): self.flavors_mock.create.assert_called_once_with(*args) self.assertEqual(self.columns, columns) - self.assertEqual(self.data, data) + self.assertItemEqual(self.data, data) def test_flavor_create_with_description_api_older(self): arglist = [ @@ -429,7 +429,7 @@ class TestFlavorList(TestFlavor): data_long = (data[0] + ( flavors[0].swap, flavors[0].rxtx_factor, - u'property=\'value\'' + format_columns.DictColumn(flavors[0].properties) ), ) def setUp(self): @@ -583,7 +583,7 @@ class TestFlavorList(TestFlavor): ) self.assertEqual(self.columns_long, columns) - self.assertEqual(tuple(self.data_long), tuple(data)) + self.assertListItemEqual(self.data_long, tuple(data)) class TestFlavorSet(TestFlavor): @@ -817,7 +817,7 @@ class TestFlavorShow(TestFlavor): flavor.id, flavor.name, flavor.is_public, - utils.format_dict(flavor.get_keys()), + format_columns.DictColumn(flavor.get_keys()), flavor.ram, flavor.rxtx_factor, flavor.swap, @@ -854,7 +854,7 @@ class TestFlavorShow(TestFlavor): columns, data = self.cmd.take_action(parsed_args) self.assertEqual(self.columns, columns) - self.assertEqual(self.data, data) + self.assertItemEqual(self.data, data) def test_private_flavor_show(self): private_flavor = compute_fakes.FakeFlavor.create_one_flavor( @@ -874,13 +874,13 @@ class TestFlavorShow(TestFlavor): data_with_project = ( private_flavor.disabled, private_flavor.ephemeral, - self.flavor_access.tenant_id, + [self.flavor_access.tenant_id], private_flavor.description, private_flavor.disk, private_flavor.id, private_flavor.name, private_flavor.is_public, - utils.format_dict(private_flavor.get_keys()), + format_columns.DictColumn(private_flavor.get_keys()), private_flavor.ram, private_flavor.rxtx_factor, private_flavor.swap, @@ -894,7 +894,7 @@ class TestFlavorShow(TestFlavor): self.flavor_access_mock.list.assert_called_with( flavor=private_flavor.id) self.assertEqual(self.columns, columns) - self.assertEqual(data_with_project, data) + self.assertItemEqual(data_with_project, data) class TestFlavorUnset(TestFlavor): diff --git a/releasenotes/notes/fix-flavor-props-formatting-d21e97745543caa7.yaml b/releasenotes/notes/fix-flavor-props-formatting-d21e97745543caa7.yaml new file mode 100644 index 000000000..2a026b3f7 --- /dev/null +++ b/releasenotes/notes/fix-flavor-props-formatting-d21e97745543caa7.yaml @@ -0,0 +1,4 @@ +--- +fixes: + Fix '-f json' output of the flavor properties to return valid json object + instead of stringying it. diff --git a/requirements.txt b/requirements.txt index ee6b6241a..99d409bf5 100644 --- a/requirements.txt +++ b/requirements.txt @@ -3,10 +3,10 @@ # process, which may cause wedges in the gate later. pbr!=2.1.0,>=2.0.0 # Apache-2.0 -cliff!=2.9.0,>=2.8.0 # Apache-2.0 +cliff>=3.4.0 # Apache-2.0 iso8601>=0.1.11 # MIT openstacksdk>=0.48.0 # Apache-2.0 -osc-lib>=2.0.0 # Apache-2.0 +osc-lib>=2.2.0 # Apache-2.0 oslo.i18n>=3.15.3 # Apache-2.0 python-keystoneclient>=3.22.0 # Apache-2.0 python-novaclient>=15.1.0 # Apache-2.0