Merge "Modify compute agent set command"

6 files changed, 125 insertions, 30 deletions

diff --git a/doc/source/backwards-incompatible.rst b/doc/source/backwards-incompatible.rst
index da3c1b64..4f38aa54 100644
--- a/doc/source/backwards-incompatible.rst
+++ b/doc/source/backwards-incompatible.rst
@@ -170,6 +170,19 @@ Releases Before 3.0
   * Bug: https://bugs.launchpad.net/python-openstackclient/+bug/1546065
   * Commit: https://review.openstack.org/#/c/281088/
 
+12. <version> <url> <md5hash> should be optional for command `openstack compute agent set`
+
+  Previously, the command was `openstack compute agent set <id> <version> <url> <md5hash>`,
+  whereas now it is: `openstack compute agent set <id> --version <version>
+                                                       --url <url>
+                                                       --md5hash <md5hash>`.
+
+  * In favor of: making <version> <url> <md5hash> optional.
+  * As of: NA
+  * Removed in: NA
+  * Bug: NA
+  * Commit: https://review.openstack.org/#/c/328819/
+
 13. `aggregate set` commands will no longer return the modified resource
 
   Previously, modifying an aggregate would result in the new aggregate being
diff --git a/doc/source/command-objects/compute-agent.rst b/doc/source/command-objects/compute-agent.rst
index 55540c8d..239a24e8 100644
--- a/doc/source/command-objects/compute-agent.rst
+++ b/doc/source/command-objects/compute-agent.rst
@@ -79,21 +79,24 @@ Set compute agent properties
 .. code:: bash
 
     os compute agent set
-        <id> <version> <url> <md5hash>
+        [--agent-version <version>]
+        [--url <url]
+        [--md5hash <md5hash>]
+        <id>
 
 .. _compute_agent-set:
-.. describe:: <id>
+.. option:: --agent-version <version>
 
-    ID of the agent
+    Version of the agent
 
-.. describe:: <version>
+.. option:: --url <url>
 
-    Version of the agent
+    URL of the agent
 
-.. describe:: <url>
+.. option:: --md5hash <md5hash>
 
-    URL
+    MD5 hash of the agent
 
-.. describe:: <md5hash>
+.. describe:: <id>
 
-    MD5 hash
+    Agent to modify (ID only)
diff --git a/functional/tests/compute/v2/test_agent.py b/functional/tests/compute/v2/test_agent.py
index ad2e8253..d432768d 100644
--- a/functional/tests/compute/v2/test_agent.py
+++ b/functional/tests/compute/v2/test_agent.py
@@ -64,10 +64,11 @@ class ComputeAgentTests(test.TestCase):
         url = "http://openstack"
         md5hash = hashlib.md5().hexdigest()
 
-        raw_output = self.openstack('compute agent set ' +
-                                    self.ID + ' ' + ver + ' ' +
-                                    url + ' ' + md5hash)
-        self.assertEqual('', raw_output)
+        self.openstack('compute agent set '
+                       + self.ID
+                       + ' --agent-version ' + ver
+                       + ' --url ' + url
+                       + ' --md5hash ' + md5hash)
 
         raw_output = self.openstack('compute agent list')
         self.assertIn(self.ID, raw_output)
diff --git a/openstackclient/compute/v2/agent.py b/openstackclient/compute/v2/agent.py
index 4d923955..76c1b3b7 100644
--- a/openstackclient/compute/v2/agent.py
+++ b/openstackclient/compute/v2/agent.py
@@ -152,28 +152,48 @@ class SetAgent(command.Command):
             help=_("ID of the agent")
         )
         parser.add_argument(
-            "version",
+            "--agent-version",
+            dest="version",
             metavar="<version>",
             help=_("Version of the agent")
         )
         parser.add_argument(
-            "url",
+            "--url",
             metavar="<url>",
-            help=_("URL")
+            help=_("URL of the agent")
         )
         parser.add_argument(
-            "md5hash",
+            "--md5hash",
             metavar="<md5hash>",
-            help=_("MD5 hash")
+            help=_("MD5 hash of the agent")
         )
         return parser
 
     def take_action(self, parsed_args):
         compute_client = self.app.client_manager.compute
+        data = compute_client.agents.list(hypervisor=None)
+        agent = {}
+
+        for s in data:
+            if s.agent_id == int(parsed_args.id):
+                agent['version'] = s.version
+                agent['url'] = s.url
+                agent['md5hash'] = s.md5hash
+        if agent == {}:
+            msg = _("No agent with a ID of '%(id)s' exists.")
+            raise exceptions.CommandError(msg % parsed_args.id)
+
+        if parsed_args.version:
+            agent['version'] = parsed_args.version
+        if parsed_args.url:
+            agent['url'] = parsed_args.url
+        if parsed_args.md5hash:
+            agent['md5hash'] = parsed_args.md5hash
+
         args = (
             parsed_args.id,
-            parsed_args.version,
-            parsed_args.url,
-            parsed_args.md5hash
+            agent['version'],
+            agent['url'],
+            agent['md5hash'],
         )
         compute_client.agents.update(*args)
diff --git a/openstackclient/tests/compute/v2/test_agent.py b/openstackclient/tests/compute/v2/test_agent.py
index da329728..7695ee41 100644
--- a/openstackclient/tests/compute/v2/test_agent.py
+++ b/openstackclient/tests/compute/v2/test_agent.py
@@ -25,7 +25,9 @@ from openstackclient.tests import utils as tests_utils
 
 class TestAgent(compute_fakes.TestComputev2):
 
-    fake_agent = compute_fakes.FakeAgent.create_one_agent()
+    attr = {}
+    attr['agent_id'] = 1
+    fake_agent = compute_fakes.FakeAgent.create_one_agent(attr)
 
     columns = (
         'agent_id',
@@ -238,21 +240,34 @@ class TestAgentSet(TestAgent):
         super(TestAgentSet, self).setUp()
 
         self.agents_mock.update.return_value = self.fake_agent
+        self.agents_mock.list.return_value = [self.fake_agent]
         self.cmd = agent.SetAgent(self.app, None)
 
-    def test_agent_set(self):
+    def test_agent_set_nothing(self):
         arglist = [
-            'id',
-            'new-version',
-            'new-url',
-            'new-md5hash',
+            '1',
+        ]
+        verifylist = [
+            ('id', '1'),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+        result = self.cmd.take_action(parsed_args)
+
+        self.agents_mock.update.assert_called_with(parsed_args.id,
+                                                   self.fake_agent.version,
+                                                   self.fake_agent.url,
+                                                   self.fake_agent.md5hash)
+        self.assertIsNone(result)
+
+    def test_agent_set_version(self):
+        arglist = [
+            '1',
+            '--agent-version', 'new-version',
         ]
 
         verifylist = [
-            ('id', 'id'),
+            ('id', '1'),
             ('version', 'new-version'),
-            ('url', 'new-url'),
-            ('md5hash', 'new-md5hash'),
         ]
 
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
@@ -260,6 +275,46 @@ class TestAgentSet(TestAgent):
 
         self.agents_mock.update.assert_called_with(parsed_args.id,
                                                    parsed_args.version,
+                                                   self.fake_agent.url,
+                                                   self.fake_agent.md5hash)
+        self.assertIsNone(result)
+
+    def test_agent_set_url(self):
+        arglist = [
+            '1',
+            '--url', 'new-url',
+        ]
+
+        verifylist = [
+            ('id', '1'),
+            ('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(parsed_args.id,
+                                                   self.fake_agent.version,
                                                    parsed_args.url,
+                                                   self.fake_agent.md5hash)
+        self.assertIsNone(result)
+
+    def test_agent_set_md5hash(self):
+        arglist = [
+            '1',
+            '--md5hash', 'new-md5hash',
+        ]
+
+        verifylist = [
+            ('id', '1'),
+            ('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(parsed_args.id,
+                                                   self.fake_agent.version,
+                                                   self.fake_agent.url,
                                                    parsed_args.md5hash)
         self.assertIsNone(result)
diff --git a/releasenotes/notes/modify-compute-agent-set-77ff894ef62ebbc7.yaml b/releasenotes/notes/modify-compute-agent-set-77ff894ef62ebbc7.yaml
new file mode 100644
index 00000000..28ab3624
--- /dev/null
+++ b/releasenotes/notes/modify-compute-agent-set-77ff894ef62ebbc7.yaml
@@ -0,0 +1,3 @@
+---
+upgrade:
+  - Migrate command ``compute agent set`` arguments to be optional.