volume: Add v3-specific volume service module

stephenfin · stephenfin · commit 1ee3ef33d7a8 · 2025-05-19T13:20:09.000+01:00
Ease migration.

Change-Id: Ibcdb157ba1bf370c63320d3a1afcf3c400370624
Signed-off-by: Stephen Finucane &lt;stephenfin@redhat.com&gt;
diff --git a/openstackclient/tests/unit/volume/v3/test_service.py b/openstackclient/tests/unit/volume/v3/test_service.py
@@ -13,6 +13,7 @@
 #
 
 from cinderclient import api_versions
+from osc_lib import exceptions
 
 from openstackclient.tests.unit.volume.v3 import fakes as volume_fakes
 from openstackclient.volume.v3 import service
@@ -269,3 +270,151 @@ def test_service_list_with_backend_state(self):
         # 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()
+
+    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.cmd = service.SetService(self.app, None)
+
+    def test_service_set_nothing(self):
+        arglist = [
+            self.service.host,
+            self.service.binary,
+        ]
+        verifylist = [
+            ('host', self.service.host),
+            ('service', self.service.binary),
+        ]
+        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.assertIsNone(result)
+
+    def test_service_set_enable(self):
+        arglist = [
+            '--enable',
+            self.service.host,
+            self.service.binary,
+        ]
+        verifylist = [
+            ('enable', True),
+            ('host', self.service.host),
+            ('service', self.service.binary),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        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.assertIsNone(result)
+
+    def test_service_set_disable(self):
+        arglist = [
+            '--disable',
+            self.service.host,
+            self.service.binary,
+        ]
+        verifylist = [
+            ('disable', True),
+            ('host', self.service.host),
+            ('service', self.service.binary),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        result = self.cmd.take_action(parsed_args)
+
+        self.service_mock.disable.assert_called_with(
+            self.service.host, self.service.binary
+        )
+        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):
+        reason = 'earthquake'
+        arglist = [
+            '--disable',
+            '--disable-reason',
+            reason,
+            self.service.host,
+            self.service.binary,
+        ]
+        verifylist = [
+            ('disable', True),
+            ('disable_reason', reason),
+            ('host', self.service.host),
+            ('service', self.service.binary),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        result = self.cmd.take_action(parsed_args)
+
+        self.service_mock.disable_log_reason.assert_called_with(
+            self.service.host, self.service.binary, reason
+        )
+        self.assertIsNone(result)
+
+    def test_service_set_only_with_disable_reason(self):
+        reason = 'earthquake'
+        arglist = [
+            '--disable-reason',
+            reason,
+            self.service.host,
+            self.service.binary,
+        ]
+        verifylist = [
+            ('disable_reason', reason),
+            ('host', self.service.host),
+            ('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.")
+        except exceptions.CommandError as e:
+            self.assertEqual(
+                "Cannot specify option --disable-reason without "
+                "--disable specified.",
+                str(e),
+            )
+
+    def test_service_set_enable_with_disable_reason(self):
+        reason = 'earthquake'
+        arglist = [
+            '--enable',
+            '--disable-reason',
+            reason,
+            self.service.host,
+            self.service.binary,
+        ]
+        verifylist = [
+            ('enable', True),
+            ('disable_reason', reason),
+            ('host', self.service.host),
+            ('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.")
+        except exceptions.CommandError as e:
+            self.assertEqual(
+                "Cannot specify option --disable-reason without "
+                "--disable specified.",
+                str(e),
+            )
diff --git a/openstackclient/volume/v3/service.py b/openstackclient/volume/v3/service.py
@@ -15,12 +15,36 @@
 """Service action implementations"""
 
 from cinderclient import api_versions
+from osc_lib.command import command
+from osc_lib import exceptions
 from osc_lib import utils
 
-from openstackclient.volume.v2 import service as service_v2
+from openstackclient.i18n import _
 
 
-class ListService(service_v2.ListService):
+class ListService(command.Lister):
+    _description = _("List service command")
+
+    def get_parser(self, prog_name):
+        parser = super().get_parser(prog_name)
+        parser.add_argument(
+            "--host",
+            metavar="<host>",
+            help=_("List services on specified host (name only)"),
+        )
+        parser.add_argument(
+            "--service",
+            metavar="<service>",
+            help=_("List only specified service (name only)"),
+        )
+        parser.add_argument(
+            "--long",
+            action="store_true",
+            default=False,
+            help=_("List additional fields in output"),
+        )
+        return parser
+
     def take_action(self, parsed_args):
         service_client = self.app.client_manager.volume
 
@@ -53,3 +77,57 @@ def take_action(self, parsed_args):
                 for s in data
             ),
         )
+
+
+class SetService(command.Command):
+    _description = _("Set volume service properties")
+
+    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(
+            "service",
+            metavar="<service>",
+            help=_("Name of service (Binary name)"),
+        )
+        enabled_group = parser.add_mutually_exclusive_group()
+        enabled_group.add_argument(
+            "--enable", action="store_true", help=_("Enable volume service")
+        )
+        enabled_group.add_argument(
+            "--disable", action="store_true", help=_("Disable volume service")
+        )
+        parser.add_argument(
+            "--disable-reason",
+            metavar="<reason>",
+            help=_(
+                "Reason for disabling the service "
+                "(should be used with --disable option)"
+            ),
+        )
+        return parser
+
+    def take_action(self, parsed_args):
+        if parsed_args.disable_reason and not parsed_args.disable:
+            msg = _(
+                "Cannot specify option --disable-reason without "
+                "--disable specified."
+            )
+            raise exceptions.CommandError(msg)
+
+        service_client = self.app.client_manager.volume
+        if parsed_args.enable:
+            service_client.services.enable(
+                parsed_args.host, parsed_args.service
+            )
+        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
+                )
diff --git a/setup.cfg b/setup.cfg
@@ -804,7 +804,7 @@ openstack.volume.v3 =
     volume_qos_unset = openstackclient.volume.v2.qos_specs:UnsetQos
 
     volume_service_list = openstackclient.volume.v3.service:ListService
-    volume_service_set = openstackclient.volume.v2.service:SetService
+    volume_service_set = openstackclient.volume.v3.service:SetService
 
     volume_transfer_request_accept = openstackclient.volume.v3.volume_transfer_request:AcceptTransferRequest
     volume_transfer_request_create = openstackclient.volume.v3.volume_transfer_request:CreateTransferRequest
