[RM-12171] Make pkg --install and --remove mutually exclusive

The code could not handle both being present, so make sure that we
don't allow that on the CLI.

Add unit tests for pkg.

Remove nargs=?. Using '?' doesn't make any sense unless a default
or const flag is also provided, which they aren't.  We want to make
sure there is one string provided, which is the default behavior.

Fixes: #12171
Signed-off-by: Travis Rhoden <trhoden@redhat.com>

diff --git a/ceph_deploy/pkg.py b/ceph_deploy/pkg.py
index d0ec93adbd714517c9a40dd4c711e26dbe99f237..00557c9901c2d352630b20f4e330219d262a77e5 100644 (file)
--- a/ceph_deploy/pkg.py
+++ b/ceph_deploy/pkg.py
@@ -50,16 +50,16 @@ def make(parser):
     Manage packages on remote hosts.
     """
 
-    parser.add_argument(
+    action = parser.add_mutually_exclusive_group()
+
+    action.add_argument(
         '--install',
-        nargs='?',
         metavar='PKG(s)',
         help='Comma-separated package(s) to install',
     )
 
-    parser.add_argument(
+    action.add_argument(
         '--remove',
-        nargs='?',
         metavar='PKG(s)',
         help='Comma-separated package(s) to remove',
     )

diff --git a/ceph_deploy/tests/parser/test_pkg.py b/ceph_deploy/tests/parser/test_pkg.py
new file mode 100644 (file)
index 0000000..6259e58
--- /dev/null
+++ b/ceph_deploy/tests/parser/test_pkg.py
@@ -0,0 +1,65 @@
+import pytest
+
+from ceph_deploy.cli import get_parser
+
+
+class TestParserPkg(object):
+
+    def setup(self):
+        self.parser = get_parser()
+
+    def test_pkg_help(self, capsys):
+        with pytest.raises(SystemExit):
+            self.parser.parse_args('pkg --help'.split())
+        out, err = capsys.readouterr()
+        assert 'usage: ceph-deploy pkg' in out
+        assert 'positional arguments:' in out
+        assert 'optional arguments:' in out
+
+    def test_pkg_install_host_required(self, capsys):
+        with pytest.raises(SystemExit):
+            self.parser.parse_args('pkg --install pkg1'.split())
+        out, err = capsys.readouterr()
+        assert "error: too few arguments" in err
+
+    def test_pkg_install_one_host(self):
+        args = self.parser.parse_args('pkg --install pkg1 host1'.split())
+        assert args.hosts == ['host1']
+        assert args.install == "pkg1"
+
+    def test_pkg_install_multiple_hosts(self):
+        hostnames = ['host1', 'host2', 'host3']
+        args = self.parser.parse_args('pkg --install pkg1'.split() + hostnames)
+        assert args.hosts == hostnames
+        assert args.install == "pkg1"
+
+    def test_pkg_install_muliple_pkgs(self):
+        args = self.parser.parse_args('pkg --install pkg1,pkg2 host1'.split())
+        assert args.install == "pkg1,pkg2"
+
+    def test_pkg_remove_host_required(self, capsys):
+        with pytest.raises(SystemExit):
+            self.parser.parse_args('pkg --remove pkg1'.split())
+        out, err = capsys.readouterr()
+        assert "error: too few arguments" in err
+
+    def test_pkg_remove_one_host(self):
+        args = self.parser.parse_args('pkg --remove pkg1 host1'.split())
+        assert args.hosts == ['host1']
+        assert args.remove == "pkg1"
+
+    def test_pkg_remove_multiple_hosts(self):
+        hostnames = ['host1', 'host2', 'host3']
+        args = self.parser.parse_args('pkg --remove pkg1'.split() + hostnames)
+        assert args.hosts == hostnames
+        assert args.remove == "pkg1"
+
+    def test_pkg_remove_muliple_pkgs(self):
+        args = self.parser.parse_args('pkg --remove pkg1,pkg2 host1'.split())
+        assert args.remove == "pkg1,pkg2"
+
+    def test_pkg_install_remove_are_mutex(self, capsys):
+        with pytest.raises(SystemExit):
+            self.parser.parse_args('pkg --install pkg2 --remove pkg1 host1'.split())
+        out, err = capsys.readouterr()
+        assert "argument --remove: not allowed with argument --install" in err