diff --git a/openstackclient/tests/unit/volume/v2/test_service.py b/openstackclient/tests/unit/volume/v2/test_service.py index a553985647..e230a39a9a 100644 --- a/openstackclient/tests/unit/volume/v2/test_service.py +++ b/openstackclient/tests/unit/volume/v2/test_service.py @@ -12,108 +12,83 @@ # under the License. # +from unittest import mock + +from openstack.block_storage.v2 import service as _service +from openstack.test import fakes as sdk_fakes from osc_lib import exceptions from openstackclient.tests.unit.volume.v2 import fakes as volume_fakes from openstackclient.volume.v2 import service -class TestService(volume_fakes.TestVolume): +class TestServiceList(volume_fakes.TestVolume): def setUp(self): super().setUp() - # Get a shortcut to the ServiceManager Mock - self.service_mock = self.volume_client.services - self.service_mock.reset_mock() + self.service = sdk_fakes.generate_fake_resource(_service.Service) + self.volume_sdk_client.services.return_value = [self.service] - -class TestServiceList(TestService): - # The service to be listed - services = volume_fakes.create_one_service() - - def setUp(self): - super().setUp() - - self.service_mock.list.return_value = [self.services] - - # Get the command object to test self.cmd = service.ListService(self.app, None) def test_service_list(self): arglist = [ '--host', - self.services.host, + self.service.host, '--service', - self.services.binary, + self.service.binary, ] verifylist = [ - ('host', self.services.host), - ('service', self.services.binary), + ('host', self.service.host), + ('service', self.service.binary), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) - # In base command class Lister in cliff, abstract method take_action() - # returns a tuple containing the column names and an iterable - # containing the data to be listed. columns, data = self.cmd.take_action(parsed_args) - expected_columns = [ + expected_columns = ( 'Binary', 'Host', 'Zone', 'Status', 'State', 'Updated At', - ] - - # confirming if all expected columns are present in the result. - self.assertEqual(expected_columns, columns) - + ) datalist = ( ( - self.services.binary, - self.services.host, - self.services.zone, - self.services.status, - self.services.state, - self.services.updated_at, + self.service.binary, + self.service.host, + self.service.availability_zone, + self.service.status, + self.service.state, + self.service.updated_at, ), ) - - # confirming if all expected values are present in the result. + self.assertEqual(expected_columns, columns) self.assertEqual(datalist, tuple(data)) - - # checking if proper call was made to list services - self.service_mock.list.assert_called_with( - self.services.host, - self.services.binary, + self.volume_sdk_client.services.assert_called_with( + host=self.service.host, + binary=self.service.binary, ) - # checking if prohibited columns are present in output - self.assertNotIn("Disabled Reason", columns) - self.assertNotIn(self.services.disabled_reason, tuple(data)) - def test_service_list_with_long_option(self): arglist = [ '--host', - self.services.host, + self.service.host, '--service', - self.services.binary, + self.service.binary, '--long', ] verifylist = [ - ('host', self.services.host), - ('service', self.services.binary), + ('host', self.service.host), + ('service', self.service.binary), ('long', True), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) - # In base command class Lister in cliff, abstract method take_action() - # returns a tuple containing the column names and an iterable - # containing the data to be listed. columns, data = self.cmd.take_action(parsed_args) - expected_columns = [ + expected_columns = ( 'Binary', 'Host', 'Zone', @@ -121,41 +96,34 @@ class TestServiceList(TestService): 'State', 'Updated At', 'Disabled Reason', - ] - - # confirming if all expected columns are present in the result. - self.assertEqual(expected_columns, columns) - + ) datalist = ( ( - self.services.binary, - self.services.host, - self.services.zone, - self.services.status, - self.services.state, - self.services.updated_at, - self.services.disabled_reason, + self.service.binary, + self.service.host, + self.service.availability_zone, + self.service.status, + self.service.state, + self.service.updated_at, + self.service.disabled_reason, ), ) - - # confirming if all expected values are present in the result. + self.assertEqual(expected_columns, columns) self.assertEqual(datalist, tuple(data)) - - self.service_mock.list.assert_called_with( - self.services.host, - self.services.binary, + self.volume_sdk_client.services.assert_called_with( + host=self.service.host, + binary=self.service.binary, ) -class TestServiceSet(TestService): - service = volume_fakes.create_one_service() - +class TestServiceSet(volume_fakes.TestVolume): def setUp(self): super().setUp() - self.service_mock.enable.return_value = self.service - self.service_mock.disable.return_value = self.service - self.service_mock.disable_log_reason.return_value = self.service + self.service = sdk_fakes.generate_fake_resource(_service.Service) + self.service.enable = mock.Mock(autospec=True) + self.service.disable = mock.Mock(autospec=True) + self.volume_sdk_client.find_service.return_value = self.service self.cmd = service.SetService(self.app, None) @@ -171,9 +139,8 @@ class TestServiceSet(TestService): parsed_args = self.check_parser(self.cmd, arglist, verifylist) result = self.cmd.take_action(parsed_args) - self.service_mock.enable.assert_not_called() - self.service_mock.disable.assert_not_called() - self.service_mock.disable_log_reason.assert_not_called() + self.service.enable.assert_not_called() + self.service.disable.assert_not_called() self.assertIsNone(result) def test_service_set_enable(self): @@ -191,11 +158,8 @@ class TestServiceSet(TestService): result = self.cmd.take_action(parsed_args) - self.service_mock.enable.assert_called_with( - self.service.host, self.service.binary - ) - self.service_mock.disable.assert_not_called() - self.service_mock.disable_log_reason.assert_not_called() + self.service.enable.assert_called_with(self.volume_sdk_client) + self.service.disable.assert_not_called() self.assertIsNone(result) def test_service_set_disable(self): @@ -213,11 +177,10 @@ class TestServiceSet(TestService): result = self.cmd.take_action(parsed_args) - self.service_mock.disable.assert_called_with( - self.service.host, self.service.binary + self.service.enable.assert_not_called() + self.service.disable.assert_called_with( + self.volume_sdk_client, reason=None ) - self.service_mock.enable.assert_not_called() - self.service_mock.disable_log_reason.assert_not_called() self.assertIsNone(result) def test_service_set_disable_with_reason(self): @@ -239,8 +202,9 @@ class TestServiceSet(TestService): result = self.cmd.take_action(parsed_args) - self.service_mock.disable_log_reason.assert_called_with( - self.service.host, self.service.binary, reason + self.service.enable.assert_not_called() + self.service.disable.assert_called_with( + self.volume_sdk_client, reason=reason ) self.assertIsNone(result) @@ -258,6 +222,7 @@ class TestServiceSet(TestService): ('service', self.service.binary), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) + try: self.cmd.take_action(parsed_args) self.fail("CommandError should be raised.") @@ -284,6 +249,7 @@ class TestServiceSet(TestService): ('service', self.service.binary), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) + try: self.cmd.take_action(parsed_args) self.fail("CommandError should be raised.") diff --git a/openstackclient/tests/unit/volume/v3/test_service.py b/openstackclient/tests/unit/volume/v3/test_service.py index e5aa46758c..53027fcb58 100644 --- a/openstackclient/tests/unit/volume/v3/test_service.py +++ b/openstackclient/tests/unit/volume/v3/test_service.py @@ -12,109 +12,83 @@ # under the License. # -from cinderclient import api_versions +from unittest import mock + +from openstack.block_storage.v3 import service as _service +from openstack.test import fakes as sdk_fakes from osc_lib import exceptions from openstackclient.tests.unit.volume.v3 import fakes as volume_fakes from openstackclient.volume.v3 import service -class TestService(volume_fakes.TestVolume): +class TestServiceList(volume_fakes.TestVolume): def setUp(self): super().setUp() - # Get a shortcut to the ServiceManager Mock - self.service_mock = self.volume_client.services - self.service_mock.reset_mock() + self.service = sdk_fakes.generate_fake_resource(_service.Service) + self.volume_sdk_client.services.return_value = [self.service] - -class TestServiceList(TestService): - # The service to be listed - services = volume_fakes.create_one_service() - - def setUp(self): - super().setUp() - - self.service_mock.list.return_value = [self.services] - - # Get the command object to test self.cmd = service.ListService(self.app, None) def test_service_list(self): arglist = [ '--host', - self.services.host, + self.service.host, '--service', - self.services.binary, + self.service.binary, ] verifylist = [ - ('host', self.services.host), - ('service', self.services.binary), + ('host', self.service.host), + ('service', self.service.binary), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) - # In base command class Lister in cliff, abstract method take_action() - # returns a tuple containing the column names and an iterable - # containing the data to be listed. columns, data = self.cmd.take_action(parsed_args) - expected_columns = [ + expected_columns = ( 'Binary', 'Host', 'Zone', 'Status', 'State', 'Updated At', - ] - - # confirming if all expected columns are present in the result. - self.assertEqual(expected_columns, columns) - + ) datalist = ( ( - self.services.binary, - self.services.host, - self.services.zone, - self.services.status, - self.services.state, - self.services.updated_at, + self.service.binary, + self.service.host, + self.service.availability_zone, + self.service.status, + self.service.state, + self.service.updated_at, ), ) - - # confirming if all expected values are present in the result. + self.assertEqual(expected_columns, columns) self.assertEqual(datalist, tuple(data)) - - # checking if proper call was made to list services - self.service_mock.list.assert_called_with( - self.services.host, - self.services.binary, + self.volume_sdk_client.services.assert_called_with( + host=self.service.host, + binary=self.service.binary, ) - # checking if prohibited columns are present in output - self.assertNotIn("Disabled Reason", columns) - self.assertNotIn(self.services.disabled_reason, tuple(data)) - def test_service_list_with_long_option(self): arglist = [ '--host', - self.services.host, + self.service.host, '--service', - self.services.binary, + self.service.binary, '--long', ] verifylist = [ - ('host', self.services.host), - ('service', self.services.binary), + ('host', self.service.host), + ('service', self.service.binary), ('long', True), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) - # In base command class Lister in cliff, abstract method take_action() - # returns a tuple containing the column names and an iterable - # containing the data to be listed. columns, data = self.cmd.take_action(parsed_args) - expected_columns = [ + expected_columns = ( 'Binary', 'Host', 'Zone', @@ -122,55 +96,43 @@ class TestServiceList(TestService): 'State', 'Updated At', 'Disabled Reason', - ] - - # confirming if all expected columns are present in the result. - self.assertEqual(expected_columns, columns) - + ) datalist = ( ( - self.services.binary, - self.services.host, - self.services.zone, - self.services.status, - self.services.state, - self.services.updated_at, - self.services.disabled_reason, + self.service.binary, + self.service.host, + self.service.availability_zone, + self.service.status, + self.service.state, + self.service.updated_at, + self.service.disabled_reason, ), ) - - # confirming if all expected values are present in the result. + self.assertEqual(expected_columns, columns) self.assertEqual(datalist, tuple(data)) - - self.service_mock.list.assert_called_with( - self.services.host, - self.services.binary, + self.volume_sdk_client.services.assert_called_with( + host=self.service.host, + binary=self.service.binary, ) def test_service_list_with_cluster(self): - self.volume_client.api_version = api_versions.APIVersion('3.7') - cluster = {'cluster': 'fake-cluster'} - cluster_service = volume_fakes.create_one_service(attrs=cluster) - self.service_mock.list.return_value = [cluster_service] + self.set_volume_api_version('3.7') arglist = [ '--host', - cluster_service.host, + self.service.host, '--service', - cluster_service.binary, + self.service.binary, ] verifylist = [ - ('host', cluster_service.host), - ('service', cluster_service.binary), + ('host', self.service.host), + ('service', self.service.binary), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) - # In base command class Lister in cliff, abstract method take_action() - # returns a tuple containing the column names and an iterable - # containing the data to be listed. columns, data = self.cmd.take_action(parsed_args) - expected_columns = [ + expected_columns = ( 'Binary', 'Host', 'Zone', @@ -178,60 +140,43 @@ class TestServiceList(TestService): 'State', 'Updated At', 'Cluster', - ] - - # confirming if all expected columns are present in the result. - self.assertEqual(expected_columns, columns) - + ) datalist = ( ( - cluster_service.binary, - cluster_service.host, - cluster_service.zone, - cluster_service.status, - cluster_service.state, - cluster_service.updated_at, - cluster_service.cluster, + self.service.binary, + self.service.host, + self.service.availability_zone, + self.service.status, + self.service.state, + self.service.updated_at, + self.service.cluster, ), ) - - # confirming if all expected values are present in the result. + self.assertEqual(expected_columns, columns) self.assertEqual(datalist, tuple(data)) - - # checking if proper call was made to list services - self.service_mock.list.assert_called_with( - cluster_service.host, - cluster_service.binary, + self.volume_sdk_client.services.assert_called_with( + host=self.service.host, + binary=self.service.binary, ) - # checking if prohibited columns are present in output - self.assertNotIn("Disabled Reason", columns) - self.assertNotIn(cluster_service.disabled_reason, tuple(data)) - def test_service_list_with_backend_state(self): - self.volume_client.api_version = api_versions.APIVersion('3.49') - backend_state = {'cluster': 'fake-cluster', 'backend_state': 'up'} - backend_service = volume_fakes.create_one_service(attrs=backend_state) - self.service_mock.list.return_value = [backend_service] + self.set_volume_api_version('3.49') arglist = [ '--host', - backend_service.host, + self.service.host, '--service', - backend_service.binary, + self.service.binary, ] verifylist = [ - ('host', backend_service.host), - ('service', backend_service.binary), + ('host', self.service.host), + ('service', self.service.binary), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) - # In base command class Lister in cliff, abstract method take_action() - # returns a tuple containing the column names and an iterable - # containing the data to be listed. columns, data = self.cmd.take_action(parsed_args) - expected_columns = [ + expected_columns = ( 'Binary', 'Host', 'Zone', @@ -240,47 +185,35 @@ class TestServiceList(TestService): 'Updated At', 'Cluster', 'Backend State', - ] - - # confirming if all expected columns are present in the result. - self.assertEqual(expected_columns, columns) - + ) datalist = ( ( - backend_service.binary, - backend_service.host, - backend_service.zone, - backend_service.status, - backend_service.state, - backend_service.updated_at, - backend_service.cluster, - backend_service.backend_state, + self.service.binary, + self.service.host, + self.service.availability_zone, + self.service.status, + self.service.state, + self.service.updated_at, + self.service.cluster, + self.service.backend_state, ), ) - - # confirming if all expected values are present in the result. + self.assertEqual(expected_columns, columns) self.assertEqual(datalist, tuple(data)) - - # checking if proper call was made to list services - self.service_mock.list.assert_called_with( - backend_service.host, - backend_service.binary, + self.volume_sdk_client.services.assert_called_with( + host=self.service.host, + binary=self.service.binary, ) - # checking if prohibited columns are present in output - self.assertNotIn("Disabled Reason", columns) - self.assertNotIn(backend_service.disabled_reason, tuple(data)) - - -class TestServiceSet(TestService): - service = volume_fakes.create_one_service() +class TestServiceSet(volume_fakes.TestVolume): def setUp(self): super().setUp() - self.service_mock.enable.return_value = self.service - self.service_mock.disable.return_value = self.service - self.service_mock.disable_log_reason.return_value = self.service + self.service = sdk_fakes.generate_fake_resource(_service.Service) + self.service.enable = mock.Mock(autospec=True) + self.service.disable = mock.Mock(autospec=True) + self.volume_sdk_client.find_service.return_value = self.service self.cmd = service.SetService(self.app, None) @@ -296,9 +229,8 @@ class TestServiceSet(TestService): parsed_args = self.check_parser(self.cmd, arglist, verifylist) result = self.cmd.take_action(parsed_args) - self.service_mock.enable.assert_not_called() - self.service_mock.disable.assert_not_called() - self.service_mock.disable_log_reason.assert_not_called() + self.service.enable.assert_not_called() + self.service.disable.assert_not_called() self.assertIsNone(result) def test_service_set_enable(self): @@ -316,11 +248,8 @@ class TestServiceSet(TestService): result = self.cmd.take_action(parsed_args) - self.service_mock.enable.assert_called_with( - self.service.host, self.service.binary - ) - self.service_mock.disable.assert_not_called() - self.service_mock.disable_log_reason.assert_not_called() + self.service.enable.assert_called_with(self.volume_sdk_client) + self.service.disable.assert_not_called() self.assertIsNone(result) def test_service_set_disable(self): @@ -338,11 +267,10 @@ class TestServiceSet(TestService): result = self.cmd.take_action(parsed_args) - self.service_mock.disable.assert_called_with( - self.service.host, self.service.binary + self.service.enable.assert_not_called() + self.service.disable.assert_called_with( + self.volume_sdk_client, reason=None ) - self.service_mock.enable.assert_not_called() - self.service_mock.disable_log_reason.assert_not_called() self.assertIsNone(result) def test_service_set_disable_with_reason(self): @@ -364,8 +292,9 @@ class TestServiceSet(TestService): result = self.cmd.take_action(parsed_args) - self.service_mock.disable_log_reason.assert_called_with( - self.service.host, self.service.binary, reason + self.service.enable.assert_not_called() + self.service.disable.assert_called_with( + self.volume_sdk_client, reason=reason ) self.assertIsNone(result) @@ -383,6 +312,7 @@ class TestServiceSet(TestService): ('service', self.service.binary), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) + try: self.cmd.take_action(parsed_args) self.fail("CommandError should be raised.") @@ -409,6 +339,7 @@ class TestServiceSet(TestService): ('service', self.service.binary), ] parsed_args = self.check_parser(self.cmd, arglist, verifylist) + try: self.cmd.take_action(parsed_args) self.fail("CommandError should be raised.") diff --git a/openstackclient/volume/v2/service.py b/openstackclient/volume/v2/service.py index 2a33fc0b9a..418ecec427 100644 --- a/openstackclient/volume/v2/service.py +++ b/openstackclient/volume/v2/service.py @@ -45,33 +45,34 @@ class ListService(command.Lister): return parser def take_action(self, parsed_args): - service_client = self.app.client_manager.volume + volume_client = self.app.client_manager.sdk_connection.volume + + columns: tuple[str, ...] = ( + "binary", + "host", + "availability_zone", + "status", + "state", + "updated_at", + ) + column_names: tuple[str, ...] = ( + "Binary", + "Host", + "Zone", + "Status", + "State", + "Updated At", + ) if parsed_args.long: - columns = [ - "Binary", - "Host", - "Zone", - "Status", - "State", - "Updated At", - "Disabled Reason", - ] - else: - columns = [ - "Binary", - "Host", - "Zone", - "Status", - "State", - "Updated At", - ] + columns += ("disabled_reason",) + column_names += ("Disabled Reason",) - data = service_client.services.list( - parsed_args.host, parsed_args.service + data = volume_client.services( + host=parsed_args.host, binary=parsed_args.service ) return ( - columns, + column_names, ( utils.get_item_properties( s, @@ -87,7 +88,11 @@ class SetService(command.Command): def get_parser(self, prog_name): parser = super().get_parser(prog_name) - parser.add_argument("host", metavar="<host>", help=_("Name of host")) + parser.add_argument( + "host", + metavar="<host>", + help=_("Name of host"), + ) parser.add_argument( "service", metavar="<service>", @@ -118,19 +123,17 @@ class SetService(command.Command): ) raise exceptions.CommandError(msg) - service_client = self.app.client_manager.volume + volume_client = self.app.client_manager.sdk_connection.volume + + service = volume_client.find_service( + host=parsed_args.host, service=parsed_args.service + ) + if parsed_args.enable: - service_client.services.enable( - parsed_args.host, parsed_args.service - ) + service.enable(volume_client) + if parsed_args.disable: - if parsed_args.disable_reason: - service_client.services.disable_log_reason( - parsed_args.host, - parsed_args.service, - parsed_args.disable_reason, - ) - else: - service_client.services.disable( - parsed_args.host, parsed_args.service - ) + service.disable( + volume_client, + reason=parsed_args.disable_reason, + ) diff --git a/openstackclient/volume/v3/service.py b/openstackclient/volume/v3/service.py index 51055dc7ef..fb41b1f472 100644 --- a/openstackclient/volume/v3/service.py +++ b/openstackclient/volume/v3/service.py @@ -14,7 +14,7 @@ """Service action implementations""" -from cinderclient import api_versions +from openstack import utils as sdk_utils from osc_lib.command import command from osc_lib import exceptions from osc_lib import utils @@ -46,29 +46,40 @@ class ListService(command.Lister): return parser def take_action(self, parsed_args): - service_client = self.app.client_manager.volume + volume_client = self.app.client_manager.sdk_connection.volume - columns = [ + columns: tuple[str, ...] = ( + "binary", + "host", + "availability_zone", + "status", + "state", + "updated_at", + ) + column_names: tuple[str, ...] = ( "Binary", "Host", "Zone", "Status", "State", "Updated At", - ] + ) - if service_client.api_version >= api_versions.APIVersion('3.7'): - columns.append("Cluster") - if service_client.api_version >= api_versions.APIVersion('3.49'): - columns.append("Backend State") + if sdk_utils.supports_microversion(volume_client, '3.7'): + columns += ("cluster",) + column_names += ("Cluster",) + if sdk_utils.supports_microversion(volume_client, '3.49'): + columns += ("backend_state",) + column_names += ("Backend State",) if parsed_args.long: - columns.append("Disabled Reason") + columns += ("disabled_reason",) + column_names += ("Disabled Reason",) - data = service_client.services.list( - parsed_args.host, parsed_args.service + data = volume_client.services( + host=parsed_args.host, binary=parsed_args.service ) return ( - columns, + column_names, ( utils.get_item_properties( s, @@ -84,7 +95,11 @@ class SetService(command.Command): def get_parser(self, prog_name): parser = super().get_parser(prog_name) - parser.add_argument("host", metavar="<host>", help=_("Name of host")) + parser.add_argument( + "host", + metavar="<host>", + help=_("Name of host"), + ) parser.add_argument( "service", metavar="<service>", @@ -115,19 +130,17 @@ class SetService(command.Command): ) raise exceptions.CommandError(msg) - service_client = self.app.client_manager.volume + volume_client = self.app.client_manager.sdk_connection.volume + + service = volume_client.find_service( + host=parsed_args.host, service=parsed_args.service + ) + if parsed_args.enable: - service_client.services.enable( - parsed_args.host, parsed_args.service - ) + service.enable(volume_client) + if parsed_args.disable: - if parsed_args.disable_reason: - service_client.services.disable_log_reason( - parsed_args.host, - parsed_args.service, - parsed_args.disable_reason, - ) - else: - service_client.services.disable( - parsed_args.host, parsed_args.service - ) + service.disable( + volume_client, + reason=parsed_args.disable_reason, + )