compute: Migrate 'agent *' to SDK

These are not supported by SDK natively (intentionally so) so we use raw
HTTP requests to manage this migration.

Change-Id: I72fa0d6f87899537a24090995b1ba884bc5f9d4d
Signed-off-by: Stephen Finucane <stephenfin@redhat.com>

openstackclient/compute/v2/agent.py

@@ -17,6 +17,7 @@
 
 import logging
 
+from openstack import exceptions as sdk_exceptions
 from osc_lib.command import command
 from osc_lib import exceptions
 from osc_lib import utils
@@ -55,16 +56,26 @@ class CreateAgent(command.ShowOne):
         return parser
 
     def take_action(self, parsed_args):
-        compute_client = self.app.client_manager.compute
+        compute_client = self.app.client_manager.sdk_connection.compute
-        args = (
+
-            parsed_args.os,
+        # doing this since openstacksdk has decided not to support this
-            parsed_args.architecture,
+        # deprecated command
-            parsed_args.version,
+        data = {
-            parsed_args.url,
+            'agent': {
-            parsed_args.md5hash,
+                'hypervisor': parsed_args.hypervisor,
-            parsed_args.hypervisor,
+                'os': parsed_args.os,
+                'architecture': parsed_args.architecture,
+                'version': parsed_args.version,
+                'url': parsed_args.url,
+                'md5hash': parsed_args.md5hash,
+            },
+        }
+        response = compute_client.post(
+            '/os-agents', json=data, microversion='2.1'
         )
-        agent = compute_client.agents.create(*args)._info.copy()
+        sdk_exceptions.raise_from_response(response)
+        agent = response.json().get('agent')
+
         return zip(*sorted(agent.items()))
 
 
@@ -84,11 +95,16 @@ class DeleteAgent(command.Command):
         return parser
 
     def take_action(self, parsed_args):
-        compute_client = self.app.client_manager.compute
+        compute_client = self.app.client_manager.sdk_connection.compute
         result = 0
         for id in parsed_args.id:
             try:
-                compute_client.agents.delete(id)
+                # doing this since openstacksdk has decided not to support this
+                # deprecated command
+                response = compute_client.delete(
+                    f'/os-agents/{id}', microversion='2.1'
+                )
+                sdk_exceptions.raise_from_response(response)
             except Exception as e:
                 result += 1
                 LOG.error(
@@ -123,7 +139,7 @@ class ListAgent(command.Lister):
         return parser
 
     def take_action(self, parsed_args):
-        compute_client = self.app.client_manager.compute
+        compute_client = self.app.client_manager.sdk_connection.compute
         columns = (
             "Agent ID",
             "Hypervisor",
@@ -133,30 +149,36 @@ class ListAgent(command.Lister):
             "Md5Hash",
             "URL",
         )
-        data = compute_client.agents.list(parsed_args.hypervisor)
+
-        return (
+        # doing this since openstacksdk has decided not to support this
-            columns,
+        # deprecated command
-            (
+        path = '/os-agents'
-                utils.get_item_properties(
+        if parsed_args.hypervisor:
-                    s,
+            path += f'?hypervisor={parsed_args.hypervisor}'
-                    columns,
+
-                )
+        response = compute_client.get(path, microversion='2.1')
-                for s in data
+        sdk_exceptions.raise_from_response(response)
-            ),
+        agents = response.json().get('agents')
-        )
+
+        return columns, (utils.get_dict_properties(s, columns) for s in agents)
 
 
 class SetAgent(command.Command):
     """Set compute agent properties.
 
-    The compute agent functionality is hypervisor specific and is only
+    The compute agent functionality is hypervisor-specific and is only
     supported by the XenAPI hypervisor driver. It was removed from nova in the
     23.0.0 (Wallaby) release.
     """
 
     def get_parser(self, prog_name):
         parser = super().get_parser(prog_name)
-        parser.add_argument("id", metavar="<id>", help=_("ID of the agent"))
+        parser.add_argument(
+            "id",
+            metavar="<id>",
+            type=int,
+            help=_("ID of the agent"),
+        )
         parser.add_argument(
             "--agent-version",
             dest="version",
@@ -172,30 +194,34 @@ class SetAgent(command.Command):
         return parser
 
     def take_action(self, parsed_args):
-        compute_client = self.app.client_manager.compute
+        compute_client = self.app.client_manager.sdk_connection.compute
-        data = compute_client.agents.list(hypervisor=None)
-        agent = {}
 
-        for s in data:
+        response = compute_client.get('/os-agents', microversion='2.1')
-            if s.agent_id == int(parsed_args.id):
+        sdk_exceptions.raise_from_response(response)
-                agent['version'] = s.version
+        agents = response.json().get('agents')
-                agent['url'] = s.url
+        data = {}
-                agent['md5hash'] = s.md5hash
+        for agent in agents:
-        if agent == {}:
+            if agent['agent_id'] == parsed_args.id:
+                data['version'] = agent['version']
+                data['url'] = agent['url']
+                data['md5hash'] = agent['md5hash']
+                break
+        else:
             msg = _("No agent with a ID of '%(id)s' exists.")
-            raise exceptions.CommandError(msg % parsed_args.id)
+            raise exceptions.CommandError(msg % {'id': parsed_args.id})
 
         if parsed_args.version:
-            agent['version'] = parsed_args.version
+            data['version'] = parsed_args.version
         if parsed_args.url:
-            agent['url'] = parsed_args.url
+            data['url'] = parsed_args.url
         if parsed_args.md5hash:
-            agent['md5hash'] = parsed_args.md5hash
+            data['md5hash'] = parsed_args.md5hash
 
-        args = (
+        data = {'para': data}
-            parsed_args.id,
+
-            agent['version'],
+        # doing this since openstacksdk has decided not to support this
-            agent['url'],
+        # deprecated command
-            agent['md5hash'],
+        response = compute_client.put(
+            f'/os-agents/{parsed_args.id}', json=data, microversion='2.1'
         )
-        compute_client.agents.update(*args)
+        sdk_exceptions.raise_from_response(response)

openstackclient/tests/unit/compute/v2/test_agent.py

@@ -11,153 +11,162 @@
 #   WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
 #   License for the specific language governing permissions and limitations
 #   under the License.
-#
 
+import http
+import random
 from unittest import mock
-from unittest.mock import call
+import uuid
 
 from osc_lib import exceptions
 
 from openstackclient.compute.v2 import agent
 from openstackclient.tests.unit.compute.v2 import fakes as compute_fakes
+from openstackclient.tests.unit import fakes
 from openstackclient.tests.unit import utils as tests_utils
 
 
-class TestAgent(compute_fakes.TestComputev2):
+def _generate_fake_agent():
-    attr = {}
+    return {
-    attr['agent_id'] = 1
+        'agent_id': random.randint(1, 1000),
-    fake_agent = compute_fakes.create_one_agent(attr)
+        'os': 'agent-os-' + uuid.uuid4().hex,
+        'architecture': 'agent-architecture',
+        'version': '8.0',
+        'url': 'http://127.0.0.1',
+        'md5hash': 'agent-md5hash',
+        'hypervisor': 'hypervisor',
+    }
 
-    columns = (
-        'agent_id',
-        'architecture',
-        'hypervisor',
-        'md5hash',
-        'os',
-        'url',
-        'version',
-    )
-
-    data = (
-        fake_agent.agent_id,
-        fake_agent.architecture,
-        fake_agent.hypervisor,
-        fake_agent.md5hash,
-        fake_agent.os,
-        fake_agent.url,
-        fake_agent.version,
-    )
 
+class TestAgentCreate(compute_fakes.TestComputev2):
     def setUp(self):
         super().setUp()
 
-        self.agents_mock = self.compute_client.agents
+        self._agent = _generate_fake_agent()
-        self.agents_mock.reset_mock()
+        self.columns = (
+            'agent_id',
+            'architecture',
+            'hypervisor',
+            'md5hash',
+            'os',
+            'url',
+            'version',
+        )
+        self.data = (
+            self._agent['agent_id'],
+            self._agent['architecture'],
+            self._agent['hypervisor'],
+            self._agent['md5hash'],
+            self._agent['os'],
+            self._agent['url'],
+            self._agent['version'],
+        )
 
-
+        self.compute_sdk_client.post.return_value = fakes.FakeResponse(
-class TestAgentCreate(TestAgent):
+            data={'agent': self._agent}
-    def setUp(self):
+        )
-        super().setUp()
-
-        self.agents_mock.create.return_value = self.fake_agent
         self.cmd = agent.CreateAgent(self.app, None)
 
     def test_agent_create(self):
         arglist = [
-            self.fake_agent.os,
+            self._agent['os'],
-            self.fake_agent.architecture,
+            self._agent['architecture'],
-            self.fake_agent.version,
+            self._agent['version'],
-            self.fake_agent.url,
+            self._agent['url'],
-            self.fake_agent.md5hash,
+            self._agent['md5hash'],
-            self.fake_agent.hypervisor,
+            self._agent['hypervisor'],
+        ]
+        verifylist = [
+            ('os', self._agent['os']),
+            ('architecture', self._agent['architecture']),
+            ('version', self._agent['version']),
+            ('url', self._agent['url']),
+            ('md5hash', self._agent['md5hash']),
+            ('hypervisor', self._agent['hypervisor']),
         ]
 
-        verifylist = [
-            ('os', self.fake_agent.os),
-            ('architecture', self.fake_agent.architecture),
-            ('version', self.fake_agent.version),
-            ('url', self.fake_agent.url),
-            ('md5hash', self.fake_agent.md5hash),
-            ('hypervisor', self.fake_agent.hypervisor),
-        ]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
         columns, data = self.cmd.take_action(parsed_args)
-        self.agents_mock.create.assert_called_with(
+
-            parsed_args.os,
+        self.compute_sdk_client.post.assert_called_with(
-            parsed_args.architecture,
+            '/os-agents',
-            parsed_args.version,
+            json={
-            parsed_args.url,
+                'agent': {
-            parsed_args.md5hash,
+                    'hypervisor': parsed_args.hypervisor,
-            parsed_args.hypervisor,
+                    'os': parsed_args.os,
+                    'architecture': parsed_args.architecture,
+                    'version': parsed_args.version,
+                    'url': parsed_args.url,
+                    'md5hash': parsed_args.md5hash,
+                },
+            },
+            microversion='2.1',
         )
 
         self.assertEqual(self.columns, columns)
         self.assertEqual(self.data, data)
 
 
-class TestAgentDelete(TestAgent):
+class TestAgentDelete(compute_fakes.TestComputev2):
-    fake_agents = compute_fakes.create_agents(count=2)
-
     def setUp(self):
         super().setUp()
 
-        self.agents_mock.get.return_value = self.fake_agents
+        self.compute_sdk_client.delete.return_value = fakes.FakeResponse(
+            status_code=http.HTTPStatus.NO_CONTENT
+        )
+
         self.cmd = agent.DeleteAgent(self.app, None)
 
     def test_delete_one_agent(self):
-        arglist = [self.fake_agents[0].agent_id]
+        arglist = ['123']
-
         verifylist = [
-            ('id', [self.fake_agents[0].agent_id]),
+            ('id', ['123']),
         ]
 
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
         result = self.cmd.take_action(parsed_args)
-        self.agents_mock.delete.assert_called_with(
+
-            self.fake_agents[0].agent_id
+        self.compute_sdk_client.delete.assert_called_once_with(
+            '/os-agents/123',
+            microversion='2.1',
         )
         self.assertIsNone(result)
 
     def test_delete_multiple_agents(self):
-        arglist = []
+        arglist = ['1', '2', '3']
-        for n in self.fake_agents:
-            arglist.append(n.agent_id)
         verifylist = [
-            ('id', arglist),
+            ('id', ['1', '2', '3']),
         ]
 
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
         result = self.cmd.take_action(parsed_args)
 
-        calls = []
+        calls = [
-        for n in self.fake_agents:
+            mock.call(f'/os-agents/{x}', microversion='2.1') for x in arglist
-            calls.append(call(n.agent_id))
+        ]
-        self.agents_mock.delete.assert_has_calls(calls)
+        self.compute_sdk_client.delete.assert_has_calls(calls)
         self.assertIsNone(result)
 
     def test_delete_multiple_agents_exception(self):
-        arglist = [
+        arglist = ['1', '2', '999']
-            self.fake_agents[0].agent_id,
-            self.fake_agents[1].agent_id,
-            'x-y-z',
-        ]
         verifylist = [
-            ('id', arglist),
+            ('id', ['1', '2', '999']),
         ]
 
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
 
-        ret_delete = [None, None, exceptions.NotFound('404')]
+        self.compute_sdk_client.delete.side_effect = [
-        self.agents_mock.delete = mock.Mock(side_effect=ret_delete)
+            fakes.FakeResponse(status_code=http.HTTPStatus.NO_CONTENT),
+            fakes.FakeResponse(status_code=http.HTTPStatus.NO_CONTENT),
+            fakes.FakeResponse(status_code=http.HTTPStatus.NOT_FOUND),
+        ]
 
         self.assertRaises(
             exceptions.CommandError, self.cmd.take_action, parsed_args
         )
         calls = [
-            call(self.fake_agents[0].agent_id),
+            mock.call(f'/os-agents/{x}', microversion='2.1') for x in arglist
-            call(self.fake_agents[1].agent_id),
         ]
-        self.agents_mock.delete.assert_has_calls(calls)
+        self.compute_sdk_client.delete.assert_has_calls(calls)
 
     def test_agent_delete_no_input(self):
         arglist = []
@@ -171,36 +180,37 @@ class TestAgentDelete(TestAgent):
         )
 
 
-class TestAgentList(TestAgent):
+class TestAgentList(compute_fakes.TestComputev2):
-    agents = compute_fakes.create_agents(count=3)
-    list_columns = (
-        "Agent ID",
-        "Hypervisor",
-        "OS",
-        "Architecture",
-        "Version",
-        "Md5Hash",
-        "URL",
-    )
-
-    list_data = []
-    for _agent in agents:
-        list_data.append(
-            (
-                _agent.agent_id,
-                _agent.hypervisor,
-                _agent.os,
-                _agent.architecture,
-                _agent.version,
-                _agent.md5hash,
-                _agent.url,
-            )
-        )
-
     def setUp(self):
         super().setUp()
 
-        self.agents_mock.list.return_value = self.agents
+        _agents = [_generate_fake_agent() for _ in range(3)]
+
+        self.columns = (
+            "Agent ID",
+            "Hypervisor",
+            "OS",
+            "Architecture",
+            "Version",
+            "Md5Hash",
+            "URL",
+        )
+        self.data = [
+            (
+                _agent['agent_id'],
+                _agent['hypervisor'],
+                _agent['os'],
+                _agent['architecture'],
+                _agent['version'],
+                _agent['md5hash'],
+                _agent['url'],
+            )
+            for _agent in _agents
+        ]
+
+        self.compute_sdk_client.get.return_value = fakes.FakeResponse(
+            data={'agents': _agents},
+        )
         self.cmd = agent.ListAgent(self.app, None)
 
     def test_agent_list(self):
@@ -210,8 +220,12 @@ class TestAgentList(TestAgent):
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
         columns, data = self.cmd.take_action(parsed_args)
 
-        self.assertEqual(self.list_columns, columns)
+        self.assertEqual(self.columns, columns)
-        self.assertEqual(self.list_data, list(data))
+        self.assertEqual(self.data, list(data))
+        self.compute_sdk_client.get.assert_called_once_with(
+            '/os-agents',
+            microversion='2.1',
+        )
 
     def test_agent_list_with_hypervisor(self):
         arglist = [
@@ -225,101 +239,129 @@ class TestAgentList(TestAgent):
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
         columns, data = self.cmd.take_action(parsed_args)
 
-        self.assertEqual(self.list_columns, columns)
+        self.assertEqual(self.columns, columns)
-        self.assertEqual(self.list_data, list(data))
+        self.assertEqual(self.data, list(data))
+        self.compute_sdk_client.get.assert_called_once_with(
+            '/os-agents?hypervisor=hypervisor',
+            microversion='2.1',
+        )
 
 
-class TestAgentSet(TestAgent):
+class TestAgentSet(compute_fakes.TestComputev2):
     def setUp(self):
         super().setUp()
 
-        self.agents_mock.update.return_value = self.fake_agent
+        self.agent = _generate_fake_agent()
-        self.agents_mock.list.return_value = [self.fake_agent]
+        self.compute_sdk_client.get.return_value = fakes.FakeResponse(
+            data={'agents': [self.agent]},
+        )
+        self.compute_sdk_client.put.return_value = fakes.FakeResponse()
+
         self.cmd = agent.SetAgent(self.app, None)
 
     def test_agent_set_nothing(self):
         arglist = [
-            '1',
+            str(self.agent['agent_id']),
         ]
         verifylist = [
-            ('id', '1'),
+            ('id', self.agent['agent_id']),
         ]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
         result = self.cmd.take_action(parsed_args)
 
-        self.agents_mock.update.assert_called_with(
+        self.compute_sdk_client.put.assert_called_once_with(
-            parsed_args.id,
+            f'/os-agents/{self.agent["agent_id"]}',
-            self.fake_agent.version,
+            json={
-            self.fake_agent.url,
+                'para': {
-            self.fake_agent.md5hash,
+                    'version': self.agent['version'],
+                    'url': self.agent['url'],
+                    'md5hash': self.agent['md5hash'],
+                },
+            },
+            microversion='2.1',
         )
         self.assertIsNone(result)
 
     def test_agent_set_version(self):
         arglist = [
-            '1',
+            str(self.agent['agent_id']),
             '--agent-version',
             'new-version',
         ]
 
         verifylist = [
-            ('id', '1'),
+            ('id', self.agent['agent_id']),
             ('version', 'new-version'),
         ]
 
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
         result = self.cmd.take_action(parsed_args)
 
-        self.agents_mock.update.assert_called_with(
+        self.compute_sdk_client.put.assert_called_once_with(
-            parsed_args.id,
+            f'/os-agents/{self.agent["agent_id"]}',
-            parsed_args.version,
+            json={
-            self.fake_agent.url,
+                'para': {
-            self.fake_agent.md5hash,
+                    'version': parsed_args.version,
+                    'url': self.agent['url'],
+                    'md5hash': self.agent['md5hash'],
+                },
+            },
+            microversion='2.1',
         )
         self.assertIsNone(result)
 
     def test_agent_set_url(self):
         arglist = [
-            '1',
+            str(self.agent['agent_id']),
             '--url',
             'new-url',
         ]
 
         verifylist = [
-            ('id', '1'),
+            ('id', self.agent['agent_id']),
             ('url', 'new-url'),
         ]
 
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
         result = self.cmd.take_action(parsed_args)
 
-        self.agents_mock.update.assert_called_with(
+        self.compute_sdk_client.put.assert_called_once_with(
-            parsed_args.id,
+            f'/os-agents/{self.agent["agent_id"]}',
-            self.fake_agent.version,
+            json={
-            parsed_args.url,
+                'para': {
-            self.fake_agent.md5hash,
+                    'version': self.agent['version'],
+                    'url': parsed_args.url,
+                    'md5hash': self.agent['md5hash'],
+                },
+            },
+            microversion='2.1',
         )
         self.assertIsNone(result)
 
     def test_agent_set_md5hash(self):
         arglist = [
-            '1',
+            str(self.agent['agent_id']),
             '--md5hash',
             'new-md5hash',
         ]
 
         verifylist = [
-            ('id', '1'),
+            ('id', self.agent['agent_id']),
             ('md5hash', 'new-md5hash'),
         ]
 
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
         result = self.cmd.take_action(parsed_args)
 
-        self.agents_mock.update.assert_called_with(
+        self.compute_sdk_client.put.assert_called_once_with(
-            parsed_args.id,
+            f'/os-agents/{self.agent["agent_id"]}',
-            self.fake_agent.version,
+            json={
-            self.fake_agent.url,
+                'para': {
-            parsed_args.md5hash,
+                    'version': self.agent['version'],
+                    'url': self.agent['url'],
+                    'md5hash': parsed_args.md5hash,
+                },
+            },
+            microversion='2.1',
         )
         self.assertIsNone(result)

releasenotes/notes/migrate-agent-commands-1c50ffcb75f91418.yaml

@@ -0,0 +1,4 @@
+---
+upgrade:
+  - |
+    The ``compute agent *`` commands have been migrated to SDK.