Merge ~rgildein/juju-verify:bug/1920914 into ~canonical-solutions-engineering/juju-verify:master

[Robert Gildein]

Add support for multiple units/machines arguments

This change provides support for using multiple units/machines as
arguments. Without these changes, only the last argument was taken into
account, so `--units ceph-osd/0 --units ceph-osd/1` used only
"ceph-osd/1".

Closes-bug: LP#1920914

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

This merge proposal is being monitored by mergebot. Change the status to Approved to merge.

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

A CI job is currently in progress. A follow up comment will be added when it completes.

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

FAILED: Continuous integration, rev:10c90b430c6fbd74c8df2a60a4c43eaae40fab96
https://jenkins.canonical.com/bootstack/job/juju-verify-test/68/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/bootstack/job/generic-make-test/89/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/68/

Click here to trigger a rebuild:
https://jenkins.canonical.com/bootstack/job/juju-verify-test/68//rebuild

review: Needs Fixing (continuous-integration)

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

A CI job is currently in progress. A follow up comment will be added when it completes.

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

PASSED: Continuous integration, rev:7327b64dd3fc5bf4c46bf7d1462c36e6cc0c7563
https://jenkins.canonical.com/bootstack/job/juju-verify-test/69/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/bootstack/job/generic-make-test/90/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/69/

Click here to trigger a rebuild:
https://jenkins.canonical.com/bootstack/job/juju-verify-test/69//rebuild

review: Approve (continuous-integration)

[Martin Kalcok (martin-kalcok)]

Looks good, +1. Thanks.

review: Approve

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

Change successfully merged at revision 59e1dd5d0aae75789574285ab2f073ab192c1d05

diff --git a/juju_verify/juju_verify.py b/juju_verify/juju_verify.py
index 7d3d86e..704fb09 100644
--- a/juju_verify/juju_verify.py
+++ b/juju_verify/juju_verify.py
@@ -22,6 +22,7 @@ import argparse
 import logging
 import os
 import sys
+import typing
 from typing import List, Union
 
 from juju import errors, loop
@@ -91,11 +92,27 @@ async def connect_model(model_name: Union[str, None]) -> Model:
     return model
 
 
+class ExtendAction(argparse.Action):  # pylint: disable=too-few-public-methods
+    """Extend action for argparse.
+
+    NOTE (rgildein): This action should be removed after python 3.6 support ends, because
+                     since Python 3.8 the "extend" is available directly in stdlib.
+    """
+
+    @typing.no_type_check
+    def __call__(self, parser, namespace, values, option_string=None):
+        """Extend existing items with values."""
+        items = getattr(namespace, self.dest) or []
+        items = [*items, *values]  # extend list of items
+        setattr(namespace, self.dest, items)
+
+
 def parse_args() -> argparse.Namespace:
     """Parse cli arguments."""
     description = "Verify that it's safe to perform selected action on " \
                   "specified units"
     parser = argparse.ArgumentParser(description=description)
+    parser.register("action", "extend", ExtendAction)
 
     parser.add_argument('--model', '-m', required=False,
                         help='Connect to specific model.')
@@ -106,9 +123,9 @@ def parse_args() -> argparse.Namespace:
                         default='info', choices=['trace', 'debug', 'info'])
 
     target = parser.add_mutually_exclusive_group(required=True)
-    target.add_argument('--units', '-u', nargs='+', type=str,
+    target.add_argument('--units', '-u', action="extend", nargs='+', type=str,
                         help='Units to check.')
-    target.add_argument('--machines', '-M', nargs='+', type=str,
+    target.add_argument('--machines', '-M', action="extend", nargs='+', type=str,
                         help='Check all units on the machine.')
     return parser.parse_args()
 
diff --git a/tests/unit/test_juju_verify.py b/tests/unit/test_juju_verify.py
index 308843d..2aa0a7a 100644
--- a/tests/unit/test_juju_verify.py
+++ b/tests/unit/test_juju_verify.py
@@ -16,10 +16,10 @@
 # this program. If not, see https://www.gnu.org/licenses/.
 """Test suite for the entrypoint function and helpers."""
 
-import argparse
 import logging
 import os
 import sys
+from argparse import Namespace
 from asyncio import Future
 from unittest.mock import ANY, MagicMock
 
@@ -166,13 +166,38 @@ def test_unsupported_log_levels(fail, log_level):
     fail.assert_called_with(expected_msg)
 
 
-def test_parse_args(mocker):
-    """Rudimentary test for argument parsing."""
-    parser = mocker.patch.object(argparse.ArgumentParser, 'parse_args')
-
-    juju_verify.parse_args()
-
-    parser.assert_called_once()
[e-mail]("args, exp_args", [
+    (["reboot", "--units", "ceph-osd/0", "ceph-osd/1"],
+     dict(check="reboot", machines=None, units=["ceph-osd/0", "ceph-osd/1"])),
+    (["reboot", "--machines", "0", "1"],
+     dict(check="reboot", units=None, machines=["0", "1"])),
+    (["reboot", "--machines", "0", "--machines", "1"],
+     dict(check="reboot", units=None, machines=["0", "1"])),
+    (["reboot", "--machine", "0", "--machine", "1"],
+     dict(check="reboot", units=None, machines=["0", "1"])),
+    (["reboot", "--machine", "0", "--machine", "1", "2"],
+     dict(check="reboot", units=None, machines=["0", "1", "2"])),
+])
+def test_parse_args(args, exp_args, mocker):
+    """Test for argument parsing."""
+    mocker.patch("sys.argv", ["juju-verify", *args])
+    exp_result = Namespace(**exp_args, log_level="info", model=None)
+
+    result = juju_verify.parse_args()
+    assert result == exp_result
+
+
[e-mail]("args", [
+    ["--units", "ceph-osd/0"],
+    ["--machines", "0"],
+    ["reboot", "--units", "ceph-osd/0", "--machines", "0"]
+])
+def test_parse_args_error(args, mocker):
+    """Test for argument parsing raise error."""
+    mocker.patch("sys.argv", ["juju-verify", *args])
+
+    with pytest.raises(SystemExit):
+        juju_verify.parse_args()
 
 
 def test_main_entrypoint_target_units(mocker):