machine: add machine list&machine modify (#6602)

* machine: add `machine list`&`machine modify`

1. add new command `dvc machine list`
2. complete `dvc machine modify`
3. add new unit tests for this two commands to ensure the call.
4. add functionality tests for two commands

* Update dvc/config_schema.py

Co-authored-by: Peter Rowlands (변기호) <[email protected]>

* Update tests/func/machine/test_machine_config.py

Co-authored-by: Peter Rowlands (변기호) <[email protected]>

* Some reviewed problems

1. capitalize global variables.
2. remove precision restriction.
3. solve windows path seperator problem.

Co-authored-by: Peter Rowlands (변기호) <[email protected]>

dvc/command/machine.py

@@ -69,6 +69,22 @@ class CmdMachineRemove(CmdMachineConfig):
         return 0
 
 
+class CmdMachineList(CmdMachineConfig):
+    def run(self):
+        levels = [self.args.level] if self.args.level else self.config.LEVELS
+        for level in levels:
+            conf = self.config.read(level)["machine"]
+            if self.args.name:
+                conf = conf.get(self.args.name, {})
+            prefix = self._config_file_prefix(
+                self.args.show_origin, self.config, level
+            )
+            configs = list(self._format_config(conf, prefix))
+            if configs:
+                ui.write("\n".join(configs))
+        return 0
+
+
 class CmdMachineModify(CmdMachineConfig):
     def run(self):
         from dvc.config import merge
@@ -219,6 +235,27 @@ def add_parser(subparsers, parent_parser):
     )
     machine_default_parser.set_defaults(func=CmdMachineDefault)
 
+    machine_LIST_HELP = "List the configuration of one/all machines."
+    machine_list_parser = machine_subparsers.add_parser(
+        "list",
+        parents=[parent_config_parser, parent_parser],
+        description=append_doc_link(machine_LIST_HELP, "machine/list"),
+        help=machine_LIST_HELP,
+        formatter_class=argparse.RawDescriptionHelpFormatter,
+    )
+    machine_list_parser.add_argument(
+        "--show-origin",
+        default=False,
+        action="store_true",
+        help="Show the source file containing each config value.",
+    )
+    machine_list_parser.add_argument(
+        "name",
+        nargs="?",
+        type=str,
+        help="name of machine to specify",
+    )
+    machine_list_parser.set_defaults(func=CmdMachineList)
     machine_MODIFY_HELP = "Modify the configuration of an machine."
     machine_modify_parser = machine_subparsers.add_parser(
         "modify",

dvc/config_schema.py

@@ -229,7 +229,17 @@ SCHEMA = {
     "machine": {
         str: {
             "cloud": All(Lower, Choices("aws", "azure")),
+            "region": All(
+                Lower, Choices("us-west", "us-east", "eu-west", "eu-north")
+            ),
+            "image": str,
+            "name": str,
+            "spot": Bool,
+            "spot_price": Coerce(float),
+            "instance_hdd_size": Coerce(int),
             "instance_type": Lower,
+            "instance_gpu": Lower,
+            "ssh_private": str,
             "startup_script": str,
         },
     },

tests/func/machine/test_machine_config.py

@@ -0,0 +1,123 @@
+import os
+import textwrap
+
+import pytest
+
+from dvc.main import main
+
+CONFIG_TEXT = textwrap.dedent(
+    """\
+        [feature]
+            machine = true
+        ['machine \"foo\"']
+            cloud = aws
+    """
+)
+
+
+@pytest.mark.parametrize(
+    "slot,value",
+    [
+        ("region", "us-west"),
+        ("image", "iterative-cml"),
+        ("name", "iterative_test"),
+        ("spot", "True"),
+        ("spot_price", "1.2345"),
+        ("spot_price", "12345"),
+        ("instance_hdd_size", "10"),
+        ("instance_type", "l"),
+        ("instance_gpu", "tesla"),
+        ("ssh_private", "secret"),
+    ],
+)
+def test_machine_modify_susccess(tmp_dir, dvc, slot, value):
+    (tmp_dir / ".dvc" / "config").write_text(CONFIG_TEXT)
+    assert main(["machine", "modify", "foo", slot, value]) == 0
+    assert (
+        tmp_dir / ".dvc" / "config"
+    ).read_text() == CONFIG_TEXT + f"    {slot} = {value}\n"
+    assert main(["machine", "modify", "--unset", "foo", slot]) == 0
+    assert (tmp_dir / ".dvc" / "config").read_text() == CONFIG_TEXT
+
+
+def test_machine_modify_startup_script(tmp_dir, dvc):
+    slot, value = "startup_script", "start.sh"
+    (tmp_dir / ".dvc" / "config").write_text(CONFIG_TEXT)
+    assert main(["machine", "modify", "foo", slot, value]) == 0
+    assert (
+        tmp_dir / ".dvc" / "config"
+    ).read_text() == CONFIG_TEXT + f"    {slot} = ../{value}\n"
+    assert main(["machine", "modify", "--unset", "foo", slot]) == 0
+    assert (tmp_dir / ".dvc" / "config").read_text() == CONFIG_TEXT
+
+
+@pytest.mark.parametrize(
+    "slot,value,msg",
+    [
+        (
+            "region",
+            "other-west",
+            "expected one of us-west, us-east, eu-west, eu-north",
+        ),
+        ("spot_price", "NUM", "expected float"),
+        ("instance_hdd_size", "BIG", "expected int"),
+    ],
+)
+def test_machine_modify_fail(tmp_dir, dvc, caplog, slot, value, msg):
+    (tmp_dir / ".dvc" / "config").write_text(CONFIG_TEXT)
+
+    assert main(["machine", "modify", "foo", slot, value]) == 251
+    assert (tmp_dir / ".dvc" / "config").read_text() == CONFIG_TEXT
+    assert msg in caplog.text
+
+
+FULL_CONFIG_TEXT = textwrap.dedent(
+    """\
+        [feature]
+            machine = true
+        ['machine \"bar\"']
+            cloud = azure
+        ['machine \"foo\"']
+            cloud = aws
+            region = us-west
+            image = iterative-cml
+            name = iterative_test
+            spot = True
+            spot_price = 1.2345
+            instance_hdd_size = 10
+            instance_type = l
+            instance_gpu = tesla
+            ssh_private = secret
+            startup_script = {}
+    """.format(
+        os.path.join("..", "start.sh")
+    )
+)
+
+
+def test_machine_list(tmp_dir, dvc, capsys):
+    (tmp_dir / ".dvc" / "config").write_text(FULL_CONFIG_TEXT)
+
+    assert main(["machine", "list"]) == 0
+    cap = capsys.readouterr()
+    assert "cloud=azure" in cap.out
+
+    assert main(["machine", "list", "foo"]) == 0
+    cap = capsys.readouterr()
+    assert "cloud=azure" not in cap.out
+    assert "cloud=aws" in cap.out
+    assert "region=us-west" in cap.out
+    assert "image=iterative-cml" in cap.out
+    assert "name=iterative_test" in cap.out
+    assert "spot=True" in cap.out
+    assert "spot_price=1.2345" in cap.out
+    assert "instance_hdd_size=10" in cap.out
+    assert "instance_type=l" in cap.out
+    assert "instance_gpu=tesla" in cap.out
+    assert "ssh_private=secret" in cap.out
+    assert (
+        "startup_script={}".format(
+            os.path.join(tmp_dir, ".dvc", "..", "start.sh")
+        )
+        in cap.out
+    )

tests/unit/command/test_machine.py

@@ -5,10 +5,23 @@ from dvc.command.machine import (
     CmdMachineAdd,
     CmdMachineCreate,
     CmdMachineDestroy,
+    CmdMachineList,
+    CmdMachineModify,
     CmdMachineRemove,
     CmdMachineSsh,
 )
 
+DATA = {
+    ".dvc": {
+        "config": (
+            "[feature]\n"
+            "  machine = true\n"
+            "['machine \"foo\"']\n"
+            "  cloud = aws"
+        )
+    }
+}
+
 
 def test_add(tmp_dir):
     tmp_dir.gen({".dvc": {"config": "[feature]\n  machine = true"}})
@@ -21,18 +34,7 @@ def test_add(tmp_dir):
 
 
 def test_remove(tmp_dir):
-    tmp_dir.gen(
-        {
-            ".dvc": {
-                "config": (
-                    "[feature]\n"
-                    "  machine = true\n"
-                    "['machine \"foo\"']\n"
-                    "  cloud = aws"
-                )
-            }
-        }
-    )
+    tmp_dir.gen(DATA)
     cli_args = parse_args(["machine", "remove", "foo"])
     assert cli_args.func == CmdMachineRemove
     cmd = cli_args.func(cli_args)
@@ -78,3 +80,25 @@ def test_ssh(tmp_dir, dvc, mocker):
 
     assert cmd.run() == 0
     m.assert_called_once_with("foo")
+
+
+def test_list(tmp_dir, mocker):
+    from dvc.ui import ui
+
+    tmp_dir.gen(DATA)
+    cli_args = parse_args(["machine", "list", "foo"])
+    assert cli_args.func == CmdMachineList
+    cmd = cli_args.func(cli_args)
+    m = mocker.patch.object(ui, "write", autospec=True)
+    assert cmd.run() == 0
+    m.assert_called_once_with("cloud=aws")
+
+
+def test_modified(tmp_dir):
+    tmp_dir.gen(DATA)
+    cli_args = parse_args(["machine", "modify", "foo", "cloud", "azure"])
+    assert cli_args.func == CmdMachineModify
+    cmd = cli_args.func(cli_args)
+    assert cmd.run() == 0
+    config = configobj.ConfigObj(str(tmp_dir / ".dvc" / "config"))
+    assert config['machine "foo"']["cloud"] == "azure"