From a8005e173ce0b3c517f1822227bcfe094fc373bb Mon Sep 17 00:00:00 2001
From: Michael Hanselmann <hansmi@google.com>
Date: Mon, 24 Aug 2009 14:30:42 +0200
Subject: [PATCH] Change scripts to use new argument definitions

This can be used to generate the bash completion script automatically.
In the future it may allow for better command line validation as well.

Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>
---
 lib/cli.py           | 100 ++++++++++++++++++++++++++++++++-----------
 scripts/gnt-backup   |  10 +++--
 scripts/gnt-cluster  |  48 +++++++++++----------
 scripts/gnt-debug    |   8 ++--
 scripts/gnt-instance |  54 ++++++++++++-----------
 scripts/gnt-job      |  29 ++++++-------
 scripts/gnt-node     |  70 +++++++++++++++++++-----------
 scripts/gnt-os       |   4 +-
 8 files changed, 199 insertions(+), 124 deletions(-)

diff --git a/lib/cli.py b/lib/cli.py
index 8065f3da7..13c13801c 100644
--- a/lib/cli.py
+++ b/lib/cli.py
@@ -46,7 +46,6 @@ __all__ = ["DEBUG_OPT", "NOHDR_OPT", "SEP_OPT", "GenericMain",
            "SubmitOpCode", "GetClient",
            "cli_option",
            "GenerateTable", "AskUser",
-           "ARGS_NONE", "ARGS_FIXED", "ARGS_ATLEAST", "ARGS_ANY", "ARGS_ONE",
            "USEUNITS_OPT", "FIELDS_OPT", "FORCE_OPT", "SUBMIT_OPT",
            "ListTags", "AddTags", "RemoveTags", "TAG_SRC_OPT",
            "FormatError", "SplitNodeOption", "SubmitOrSend",
@@ -279,21 +278,6 @@ _DRY_RUN_OPT = make_option("--dry-run", default=False,
                           " check steps and verify it it could be executed")
 
 
-def ARGS_FIXED(val):
-  """Macro-like function denoting a fixed number of arguments"""
-  return -val
-
-
-def ARGS_ATLEAST(val):
-  """Macro-like function denoting a minimum number of arguments"""
-  return val
-
-
-ARGS_NONE = None
-ARGS_ONE = ARGS_FIXED(1)
-ARGS_ANY = ARGS_ATLEAST(0)
-
-
 def check_unit(option, opt, value):
   """OptParsers custom converter for units.
 
@@ -464,27 +448,91 @@ def _ParseArgs(argv, commands, aliases):
 
     cmd = aliases[cmd]
 
-  func, nargs, parser_opts, usage, description = commands[cmd]
+  func, args_def, parser_opts, usage, description = commands[cmd]
   parser = OptionParser(option_list=parser_opts + [_DRY_RUN_OPT],
                         description=description,
                         formatter=TitledHelpFormatter(),
                         usage="%%prog %s %s" % (cmd, usage))
   parser.disable_interspersed_args()
   options, args = parser.parse_args()
-  if nargs is None:
-    if len(args) != 0:
-      ToStderr("Error: Command %s expects no arguments", cmd)
-      return None, None, None
-  elif nargs < 0 and len(args) != -nargs:
-    ToStderr("Error: Command %s expects %d argument(s)", cmd, -nargs)
-    return None, None, None
-  elif nargs >= 0 and len(args) < nargs:
-    ToStderr("Error: Command %s expects at least %d argument(s)", cmd, nargs)
+
+  if not _CheckArguments(cmd, args_def, args):
     return None, None, None
 
   return func, options, args
 
 
+def _CheckArguments(cmd, args_def, args):
+  """Verifies the arguments using the argument definition.
+
+  Algorithm:
+
+    1. Abort with error if values specified by user but none expected.
+
+    1. For each argument in definition
+
+      1. Keep running count of minimum number of values (min_count)
+      1. Keep running count of maximum number of values (max_count)
+      1. If it has an unlimited number of values
+
+        1. Abort with error if it's not the last argument in the definition
+
+    1. If last argument has limited number of values
+
+      1. Abort with error if number of values doesn't match or is too large
+
+    1. Abort with error if user didn't pass enough values (min_count)
+
+  """
+  if args and not args_def:
+    ToStderr("Error: Command %s expects no arguments", cmd)
+    return False
+
+  min_count = None
+  max_count = None
+  check_max = None
+
+  last_idx = len(args_def) - 1
+
+  for idx, arg in enumerate(args_def):
+    if min_count is None:
+      min_count = arg.min
+    elif arg.min is not None:
+      min_count += arg.min
+
+    if max_count is None:
+      max_count = arg.max
+    elif arg.max is not None:
+      max_count += arg.max
+
+    if idx == last_idx:
+      check_max = (arg.max is not None)
+
+    elif arg.max is None:
+      raise errors.ProgrammerError("Only the last argument can have max=None")
+
+  if check_max:
+    # Command with exact number of arguments
+    if (min_count is not None and max_count is not None and
+        min_count == max_count and len(args) != min_count):
+      ToStderr("Error: Command %s expects %d argument(s)", cmd, min_count)
+      return False
+
+    # Command with limited number of arguments
+    if max_count is not None and len(args) > max_count:
+      ToStderr("Error: Command %s expects only %d argument(s)",
+               cmd, max_count)
+      return False
+
+  # Command with some required arguments
+  if min_count is not None and len(args) < min_count:
+    ToStderr("Error: Command %s expects at least %d argument(s)",
+             cmd, min_count)
+    return False
+
+  return True
+
+
 def SplitNodeOption(value):
   """Splits the value of a --node option.
 
diff --git a/scripts/gnt-backup b/scripts/gnt-backup
index e005d9147..cb06160aa 100755
--- a/scripts/gnt-backup
+++ b/scripts/gnt-backup
@@ -35,6 +35,7 @@ from ganeti import utils
 
 _VALUE_TRUE = "true"
 
+
 def PrintExportList(opts, args):
   """Prints a list of all the exported system images.
 
@@ -259,14 +260,14 @@ import_opts = [
   ]
 
 commands = {
-  'list': (PrintExportList, ARGS_NONE,
+  'list': (PrintExportList, [],
            [DEBUG_OPT,
             make_option("--node", dest="nodes", default=[], action="append",
                         help="List only backups stored on this node"
                              " (can be used multiple times)"),
             ],
            "", "Lists instance exports available in the ganeti cluster"),
-  'export': (ExportInstance, ARGS_ONE,
+  'export': (ExportInstance, [ArgInstance(min=1, max=1)],
              [DEBUG_OPT, FORCE_OPT,
               make_option("-n", "--node", dest="node", help="Target node",
                           metavar="<node>"),
@@ -275,15 +276,16 @@ commands = {
                           help="Don't shutdown the instance (unsafe)"), ],
              "-n <target_node> [opts...] <name>",
              "Exports an instance to an image"),
-  'import': (ImportInstance, ARGS_ONE, import_opts,
+  'import': (ImportInstance, [ArgInstance(min=1, max=1)], import_opts,
              ("[...] -t disk-type -n node[:secondary-node]"
               " <name>"),
              "Imports an instance from an exported image"),
-  'remove': (RemoveExport, ARGS_ONE,
+  'remove': (RemoveExport, [ArgUnknown(min=1, max=1)],
              [DEBUG_OPT],
              "<name>",
              "Remove exports of named instance from the filesystem."),
   }
 
+
 if __name__ == '__main__':
   sys.exit(GenericMain(commands))
diff --git a/scripts/gnt-cluster b/scripts/gnt-cluster
index 60f7a4a4d..5a68f7a74 100755
--- a/scripts/gnt-cluster
+++ b/scripts/gnt-cluster
@@ -542,7 +542,7 @@ node_option = make_option("-n", "--node", action="append", dest="nodes",
                           metavar="<node>", default=[])
 
 commands = {
-  'init': (InitCluster, ARGS_ONE,
+  'init': (InitCluster, [ArgUnknown(min=1, max=1)],
            [DEBUG_OPT,
             make_option("-s", "--secondary-ip", dest="secondary_ip",
                         help="Specify the secondary ip for this node;"
@@ -603,62 +603,66 @@ commands = {
             ],
            "[opts...] <cluster_name>",
            "Initialises a new cluster configuration"),
-  'destroy': (DestroyCluster, ARGS_NONE,
+  'destroy': (DestroyCluster, [],
               [DEBUG_OPT,
                make_option("--yes-do-it", dest="yes_do_it",
                            help="Destroy cluster",
                            action="store_true"),
               ],
               "", "Destroy cluster"),
-  'rename': (RenameCluster, ARGS_ONE, [DEBUG_OPT, FORCE_OPT],
-               "<new_name>",
-               "Renames the cluster"),
-  'redist-conf': (RedistributeConfig, ARGS_NONE, [DEBUG_OPT, SUBMIT_OPT],
+  'rename': (RenameCluster, [ArgUnknown(min=1, max=1)],
+             [DEBUG_OPT, FORCE_OPT],
+             "<new_name>",
+             "Renames the cluster"),
+  'redist-conf': (RedistributeConfig, [], [DEBUG_OPT, SUBMIT_OPT],
                   "",
                   "Forces a push of the configuration file and ssconf files"
                   " to the nodes in the cluster"),
-  'verify': (VerifyCluster, ARGS_NONE, [DEBUG_OPT,
+  'verify': (VerifyCluster, [], [DEBUG_OPT,
              make_option("--no-nplus1-mem", dest="skip_nplusone_mem",
                          help="Skip N+1 memory redundancy tests",
                          action="store_true",
                          default=False,),
              ],
              "", "Does a check on the cluster configuration"),
-  'verify-disks': (VerifyDisks, ARGS_NONE, [DEBUG_OPT],
+  'verify-disks': (VerifyDisks, [], [DEBUG_OPT],
                    "", "Does a check on the cluster disk status"),
-  'repair-disk-sizes': (RepairDiskSizes, ARGS_ANY, [DEBUG_OPT],
+  'repair-disk-sizes': (RepairDiskSizes, [ArgInstance()], [DEBUG_OPT],
                    "", "Updates mismatches in recorded disk sizes"),
-  'masterfailover': (MasterFailover, ARGS_NONE, [DEBUG_OPT,
+  'masterfailover': (MasterFailover, [], [DEBUG_OPT,
                      make_option("--no-voting", dest="no_voting",
                                  help="Skip node agreement check (dangerous)",
                                  action="store_true",
                                  default=False,),
                      ],
                      "", "Makes the current node the master"),
-  'version': (ShowClusterVersion, ARGS_NONE, [DEBUG_OPT],
+  'version': (ShowClusterVersion, [], [DEBUG_OPT],
               "", "Shows the cluster version"),
-  'getmaster': (ShowClusterMaster, ARGS_NONE, [DEBUG_OPT],
+  'getmaster': (ShowClusterMaster, [], [DEBUG_OPT],
                 "", "Shows the cluster master"),
-  'copyfile': (ClusterCopyFile, ARGS_ONE, [DEBUG_OPT, node_option],
+  'copyfile': (ClusterCopyFile, [ArgFile(min=1, max=1)],
+               [DEBUG_OPT, node_option],
                "[-n node...] <filename>",
                "Copies a file to all (or only some) nodes"),
-  'command': (RunClusterCommand, ARGS_ATLEAST(1), [DEBUG_OPT, node_option],
+  'command': (RunClusterCommand, [ArgCommand(min=1)], [DEBUG_OPT, node_option],
               "[-n node...] <command>",
               "Runs a command on all (or only some) nodes"),
-  'info': (ShowClusterConfig, ARGS_NONE, [DEBUG_OPT],
-                 "", "Show cluster configuration"),
-  'list-tags': (ListTags, ARGS_NONE,
+  'info': (ShowClusterConfig, [], [DEBUG_OPT],
+           "", "Show cluster configuration"),
+  'list-tags': (ListTags, [],
                 [DEBUG_OPT], "", "List the tags of the cluster"),
-  'add-tags': (AddTags, ARGS_ANY, [DEBUG_OPT, TAG_SRC_OPT],
+  'add-tags': (AddTags, [ArgUnknown()], [DEBUG_OPT, TAG_SRC_OPT],
                "tag...", "Add tags to the cluster"),
-  'remove-tags': (RemoveTags, ARGS_ANY, [DEBUG_OPT, TAG_SRC_OPT],
+  'remove-tags': (RemoveTags, [ArgUnknown()], [DEBUG_OPT, TAG_SRC_OPT],
                   "tag...", "Remove tags from the cluster"),
-  'search-tags': (SearchTags, ARGS_ONE,
+  'search-tags': (SearchTags, [ArgUnknown(min=1, max=1)],
                   [DEBUG_OPT], "", "Searches the tags on all objects on"
                   " the cluster for a given pattern (regex)"),
-  'queue': (QueueOps, ARGS_ONE, [DEBUG_OPT],
+  'queue': (QueueOps,
+            [ArgChoice(min=1, max=1, choices=["drain", "undrain", "info"])],
+            [DEBUG_OPT],
             "drain|undrain|info", "Change queue properties"),
-  'modify': (SetClusterParams, ARGS_NONE,
+  'modify': (SetClusterParams, [],
              [DEBUG_OPT,
               make_option("-g", "--vg-name", dest="vg_name",
                           help="Specify the volume group name "
diff --git a/scripts/gnt-debug b/scripts/gnt-debug
index 3a12b2b7a..91f2e102c 100755
--- a/scripts/gnt-debug
+++ b/scripts/gnt-debug
@@ -131,7 +131,7 @@ def TestAllocator(opts, args):
 
 
 commands = {
-  'delay': (Delay, ARGS_ONE,
+  'delay': (Delay, [ArgUnknown(min=1, max=1)],
             [DEBUG_OPT,
              make_option("--no-master", dest="on_master", default=True,
                          action="store_false",
@@ -141,12 +141,10 @@ commands = {
                          help="Select nodes to sleep on"),
              ],
             "[opts...] <duration>", "Executes a TestDelay OpCode"),
-  'submit-job': (GenericOpCodes, ARGS_ATLEAST(1),
-                 [DEBUG_OPT,
-                  ],
+  'submit-job': (GenericOpCodes, [ArgFile(min=1)], [DEBUG_OPT],
                  "<op_list_file...>", "Submits jobs built from json files"
                  " containing a list of serialized opcodes"),
-  'allocator': (TestAllocator, ARGS_ONE,
+  'allocator': (TestAllocator, [ArgInstance(min=1, max=1)],
                 [DEBUG_OPT,
                  make_option("--dir", dest="direction",
                              default="in", choices=["in", "out"],
diff --git a/scripts/gnt-instance b/scripts/gnt-instance
index 2d0fe8c4c..5484ff2e6 100755
--- a/scripts/gnt-instance
+++ b/scripts/gnt-instance
@@ -1417,21 +1417,21 @@ add_opts = [
   ]
 
 commands = {
-  'add': (AddInstance, ARGS_ONE, add_opts,
+  'add': (AddInstance, [ArgUnknown(min=1, max=1)], add_opts,
           "[...] -t disk-type -n node[:secondary-node] -o os-type <name>",
           "Creates and adds a new instance to the cluster"),
-  'batch-create': (BatchCreate, ARGS_ONE,
+  'batch-create': (BatchCreate, [ArgFile(min=1, max=1)],
                    [DEBUG_OPT],
                    "<instances_file.json>",
                    "Create a bunch of instances based on specs in the file."),
-  'console': (ConnectToInstanceConsole, ARGS_ONE,
+  'console': (ConnectToInstanceConsole, [ArgInstance(min=1, max=1)],
               [DEBUG_OPT,
                make_option("--show-cmd", dest="show_command",
                            action="store_true", default=False,
                            help=("Show command instead of executing it"))],
               "[--show-cmd] <instance>",
               "Opens a console on the specified instance"),
-  'failover': (FailoverInstance, ARGS_ONE,
+  'failover': (FailoverInstance, [ArgInstance(min=1, max=1)],
                [DEBUG_OPT, FORCE_OPT,
                 make_option("--ignore-consistency", dest="ignore_consistency",
                             action="store_true", default=False,
@@ -1442,7 +1442,7 @@ commands = {
                "[-f] <instance>",
                "Stops the instance and starts it on the backup node, using"
                " the remote mirror (only for instances of type drbd)"),
-  'migrate': (MigrateInstance, ARGS_ONE,
+  'migrate': (MigrateInstance, [ArgInstance(min=1, max=1)],
                [DEBUG_OPT, FORCE_OPT,
                 make_option("--non-live", dest="live",
                             default=True, action="store_false",
@@ -1462,7 +1462,7 @@ commands = {
                "[-f] <instance>",
                "Migrate instance to its secondary node"
                " (only for instances of type drbd)"),
-  'move': (MoveInstance, ARGS_ONE,
+  'move': (MoveInstance, [ArgInstance(min=1, max=1)],
            [DEBUG_OPT, FORCE_OPT, SUBMIT_OPT,
             make_option("-n", "--new-node", dest="target_node",
                         help="Destinattion node", metavar="NODE",
@@ -1471,7 +1471,7 @@ commands = {
            "[-f] <instance>",
            "Move instance to an arbitrary node"
            " (only for instances of type file and lv)"),
-  'info': (ShowInstanceConfig, ARGS_ANY,
+  'info': (ShowInstanceConfig, [ArgInstance()],
            [DEBUG_OPT,
             make_option("-s", "--static", dest="static",
                         action="store_true", default=False,
@@ -1482,7 +1482,7 @@ commands = {
                         " This can take a long time to run, use wisely."),
             ], "[-s] {--all | <instance>...}",
            "Show information on the specified instance(s)"),
-  'list': (ListInstances, ARGS_ANY,
+  'list': (ListInstances, [ArgInstance()],
            [DEBUG_OPT, NOHDR_OPT, SEP_OPT, USEUNITS_OPT, FIELDS_OPT, SYNC_OPT],
            "[<instance>...]",
            "Lists the instances and their status. The available fields are"
@@ -1493,7 +1493,7 @@ commands = {
            " The default field"
            " list is (in order): %s." % ", ".join(_LIST_DEF_FIELDS),
            ),
-  'reinstall': (ReinstallInstance, ARGS_ANY,
+  'reinstall': (ReinstallInstance, [ArgInstance(min=1)],
                 [DEBUG_OPT, FORCE_OPT, os_opt,
                  m_force_multi,
                  m_node_opt, m_pri_node_opt, m_sec_node_opt,
@@ -1505,7 +1505,7 @@ commands = {
                  SUBMIT_OPT,
                  ],
                 "[-f] <instance>", "Reinstall a stopped instance"),
-  'remove': (RemoveInstance, ARGS_ONE,
+  'remove': (RemoveInstance, [ArgInstance(min=1, max=1)],
              [DEBUG_OPT, FORCE_OPT,
               make_option("--ignore-failures", dest="ignore_failures",
                           action="store_true", default=False,
@@ -1515,7 +1515,8 @@ commands = {
               SUBMIT_OPT,
               ],
              "[-f] <instance>", "Shuts down the instance and removes it"),
-  'rename': (RenameInstance, ARGS_FIXED(2),
+  'rename': (RenameInstance,
+             [ArgInstance(min=1, max=1), ArgUnknown(min=1, max=1)],
              [DEBUG_OPT,
               make_option("--no-ip-check", dest="ignore_ip",
                           help="Do not check that the IP of the new name"
@@ -1524,7 +1525,7 @@ commands = {
               SUBMIT_OPT,
               ],
              "<instance> <new_name>", "Rename the instance"),
-  'replace-disks': (ReplaceDisks, ARGS_ONE,
+  'replace-disks': (ReplaceDisks, [ArgInstance(min=1, max=1)],
                     [DEBUG_OPT,
                      make_option("-n", "--new-secondary", dest="new_secondary",
                                  help=("New secondary node (for secondary"
@@ -1556,7 +1557,7 @@ commands = {
                      ],
                     "[-s|-p|-n NODE|-I NAME] <instance>",
                     "Replaces all disks for the instance"),
-  'modify': (SetInstanceParams, ARGS_ONE,
+  'modify': (SetInstanceParams, [ArgInstance(min=1, max=1)],
              [DEBUG_OPT, FORCE_OPT,
               cli_option("-H", "--hypervisor", type="keyval",
                          default={}, dest="hypervisor",
@@ -1575,13 +1576,13 @@ commands = {
               SUBMIT_OPT,
               ],
              "<instance>", "Alters the parameters of an instance"),
-  'shutdown': (ShutdownInstance, ARGS_ANY,
+  'shutdown': (ShutdownInstance, [ArgInstance(min=1)],
                [DEBUG_OPT, m_node_opt, m_pri_node_opt, m_sec_node_opt,
                 m_clust_opt, m_inst_opt, m_force_multi,
                 SUBMIT_OPT,
                 ],
                "<instance>", "Stops an instance"),
-  'startup': (StartupInstance, ARGS_ANY,
+  'startup': (StartupInstance, [ArgInstance(min=1)],
               [DEBUG_OPT, FORCE_OPT, m_force_multi,
                m_node_opt, m_pri_node_opt, m_sec_node_opt,
                m_clust_opt, m_inst_opt,
@@ -1594,8 +1595,7 @@ commands = {
                            help="Temporary backend parameters"),
                ],
               "<instance>", "Starts an instance"),
-
-  'reboot': (RebootInstance, ARGS_ANY,
+  'reboot': (RebootInstance, [ArgInstance(min=1)],
               [DEBUG_OPT, m_force_multi,
                make_option("-t", "--type", dest="reboot_type",
                            help="Type of reboot: soft/hard/full",
@@ -1609,7 +1609,7 @@ commands = {
                SUBMIT_OPT,
                ],
             "<instance>", "Reboots an instance"),
-  'activate-disks': (ActivateDisks, ARGS_ONE,
+  'activate-disks': (ActivateDisks, [ArgInstance(min=1, max=1)],
                      [DEBUG_OPT, SUBMIT_OPT,
                       make_option("--ignore-size", dest="ignore_size",
                                   default=False, action="store_true",
@@ -1619,10 +1619,11 @@ commands = {
                       ],
                      "<instance>",
                      "Activate an instance's disks"),
-  'deactivate-disks': (DeactivateDisks, ARGS_ONE, [DEBUG_OPT, SUBMIT_OPT],
+  'deactivate-disks': (DeactivateDisks, [ArgInstance(min=1, max=1)],
+                       [DEBUG_OPT, SUBMIT_OPT],
                        "<instance>",
                        "Deactivate an instance's disks"),
-  'recreate-disks': (RecreateDisks, ARGS_ONE,
+  'recreate-disks': (RecreateDisks, [ArgInstance(min=1, max=1)],
                      [DEBUG_OPT, SUBMIT_OPT,
                      make_option("--disks", dest="disks", default=None,
                                  help="Comma-separated list of disks"
@@ -1631,7 +1632,9 @@ commands = {
                       ],
                      "<instance>",
                      "Recreate an instance's disks"),
-  'grow-disk': (GrowDisk, ARGS_FIXED(3),
+  'grow-disk': (GrowDisk,
+                [ArgInstance(min=1, max=1), ArgUnknown(min=1, max=1),
+                 ArgUnknown(min=1, max=1)],
                 [DEBUG_OPT, SUBMIT_OPT,
                  make_option("--no-wait-for-sync",
                              dest="wait_for_sync", default=True,
@@ -1639,11 +1642,13 @@ commands = {
                              help="Don't wait for sync (DANGEROUS!)"),
                  ],
                 "<instance> <disk> <size>", "Grow an instance's disk"),
-  'list-tags': (ListTags, ARGS_ONE, [DEBUG_OPT],
+  'list-tags': (ListTags, [ArgInstance(min=1, max=1)], [DEBUG_OPT],
                 "<instance_name>", "List the tags of the given instance"),
-  'add-tags': (AddTags, ARGS_ATLEAST(1), [DEBUG_OPT, TAG_SRC_OPT],
+  'add-tags': (AddTags, [ArgInstance(min=1, max=1), ArgUnknown()],
+               [DEBUG_OPT, TAG_SRC_OPT],
                "<instance_name> tag...", "Add tags to the given instance"),
-  'remove-tags': (RemoveTags, ARGS_ATLEAST(1), [DEBUG_OPT, TAG_SRC_OPT],
+  'remove-tags': (RemoveTags, [ArgInstance(min=1, max=1), ArgUnknown()],
+                  [DEBUG_OPT, TAG_SRC_OPT],
                   "<instance_name> tag...", "Remove tags from given instance"),
   }
 
@@ -1655,6 +1660,7 @@ aliases = {
   'stop': 'shutdown',
   }
 
+
 if __name__ == '__main__':
   sys.exit(GenericMain(commands, aliases=aliases,
                        override={"tag_type": constants.TAG_INSTANCE}))
diff --git a/scripts/gnt-job b/scripts/gnt-job
index 1402583e6..89f246766 100755
--- a/scripts/gnt-job
+++ b/scripts/gnt-job
@@ -340,32 +340,31 @@ def WatchJob(opts, args):
 
 
 commands = {
-  'list': (ListJobs, ARGS_ANY,
-            [DEBUG_OPT, NOHDR_OPT, SEP_OPT, FIELDS_OPT],
-            "[job_id ...]",
+  'list': (ListJobs, [ArgJobId()],
+           [DEBUG_OPT, NOHDR_OPT, SEP_OPT, FIELDS_OPT],
+           "[job_id ...]",
            "List the jobs and their status. The available fields are"
            " (see the man page for details): id, status, op_list,"
            " op_status, op_result."
            " The default field"
            " list is (in order): %s." % ", ".join(_LIST_DEF_FIELDS)),
-  'archive': (ArchiveJobs, ARGS_ANY,
-              [DEBUG_OPT],
+  'archive': (ArchiveJobs, [ArgJobId(min=1)], [DEBUG_OPT],
               "<job-id> [<job-id> ...]",
               "Archive specified jobs"),
-  'autoarchive': (AutoArchiveJobs, ARGS_ONE,
-              [DEBUG_OPT],
-              "<age>",
-              "Auto archive jobs older than the given age"),
-  'cancel': (CancelJobs, ARGS_ANY,
-             [DEBUG_OPT],
+  'autoarchive': (AutoArchiveJobs,
+                  [ArgSuggest(min=1, max=1, choices=["1d", "1w", "4w"])],
+                  [DEBUG_OPT],
+                  "<age>",
+                  "Auto archive jobs older than the given age"),
+  'cancel': (CancelJobs, [ArgJobId(min=1)], [DEBUG_OPT],
              "<job-id> [<job-id> ...]",
              "Cancel specified jobs"),
-  'info': (ShowJobs, ARGS_ANY, [DEBUG_OPT],
+  'info': (ShowJobs, [ArgJobId(min=1)], [DEBUG_OPT],
            "<job-id> [<job-id> ...]",
            "Show detailed information about the specified jobs"),
-  'watch': (WatchJob, ARGS_ONE, [DEBUG_OPT],
-           "<job-id>",
-           "Follows a job and prints its output as it arrives"),
+  'watch': (WatchJob, [ArgJobId(min=1, max=1)], [DEBUG_OPT],
+            "<job-id>",
+            "Follows a job and prints its output as it arrives"),
   }
 
 
diff --git a/scripts/gnt-node b/scripts/gnt-node
index 29c9a1e05..00c2d3899 100755
--- a/scripts/gnt-node
+++ b/scripts/gnt-node
@@ -67,6 +67,21 @@ _USER_STORAGE_TYPE = {
   constants.ST_LVM_VG: "lvm-vg",
   }
 
+_STORAGE_TYPE_OPT = \
+  cli_option("--storage-type",
+             dest="user_storage_type",
+             choices=_USER_STORAGE_TYPE.keys(),
+             default=None,
+             metavar="STORAGE_TYPE",
+             help=("Storage type (%s)" %
+                   utils.CommaJoin(_USER_STORAGE_TYPE.keys())))
+
+_REPAIRABLE_STORAGE_TYPES = \
+  [st for st, so in constants.VALID_STORAGE_OPERATIONS.iteritems()
+   if constants.SO_FIX_CONSISTENCY in so]
+
+_MODIFIABLE_STORAGE_TYPES = constants.MODIFIABLE_STORAGE_FIELDS.keys()
+
 
 def ConvertStorageType(user_storage_type):
   """Converts a user storage type to its internal name.
@@ -608,7 +623,7 @@ def SetNodeParams(opts, args):
 
 
 commands = {
-  'add': (AddNode, ARGS_ONE,
+  'add': (AddNode, [ArgUnknown(min=1, max=1)],
           [DEBUG_OPT,
            make_option("-s", "--secondary-ip", dest="secondary_ip",
                        help="Specify the secondary ip for the node",
@@ -622,7 +637,7 @@ commands = {
            ],
           "[-s ip] [--readd] [--no-ssh-key-check] <node_name>",
           "Add a node to the cluster"),
-  'evacuate': (EvacuateNode, ARGS_ONE,
+  'evacuate': (EvacuateNode, [ArgNode(min=1, max=1)],
                [DEBUG_OPT, FORCE_OPT,
                 make_option("-n", "--new-secondary", dest="dst_node",
                             help="New secondary node", metavar="NODE",
@@ -636,7 +651,7 @@ commands = {
                "[-f] {-I <iallocator> | -n <dst>} <node>",
                "Relocate the secondary instances from a node"
                " to other nodes (only for instances with drbd disk template)"),
-  'failover': (FailoverNode, ARGS_ONE,
+  'failover': (FailoverNode, [ArgNode(min=1, max=1)],
                [DEBUG_OPT, FORCE_OPT,
                 make_option("--ignore-consistency", dest="ignore_consistency",
                             action="store_true", default=False,
@@ -646,7 +661,7 @@ commands = {
                "[-f] <node>",
                "Stops the primary instances on a node and start them on their"
                " secondary node (only for instances with drbd disk template)"),
-  'migrate': (MigrateNode, ARGS_ONE,
+  'migrate': (MigrateNode, [ArgNode(min=1, max=1)],
                [DEBUG_OPT, FORCE_OPT,
                 make_option("--non-live", dest="live",
                             default=True, action="store_false",
@@ -658,16 +673,16 @@ commands = {
                "[-f] <node>",
                "Migrate all the primary instance on a node away from it"
                " (only for instances of type drbd)"),
-  'info': (ShowNodeConfig, ARGS_ANY, [DEBUG_OPT],
+  'info': (ShowNodeConfig, [ArgNode()], [DEBUG_OPT],
            "[<node_name>...]", "Show information about the node(s)"),
-  'list': (ListNodes, ARGS_ANY,
+  'list': (ListNodes, [ArgNode()],
            [DEBUG_OPT, NOHDR_OPT, SEP_OPT, USEUNITS_OPT, FIELDS_OPT, SYNC_OPT],
            "[nodes...]",
            "Lists the nodes in the cluster. The available fields"
            " are (see the man page for details): %s"
            " The default field list is (in order): %s." %
            (", ".join(_LIST_HEADERS), ", ".join(_LIST_DEF_FIELDS))),
-  'modify': (SetNodeParams, ARGS_ONE,
+  'modify': (SetNodeParams, [ArgNode(min=1, max=1)],
              [DEBUG_OPT, FORCE_OPT,
               SUBMIT_OPT,
               make_option("-C", "--master-candidate", dest="master_candidate",
@@ -681,28 +696,25 @@ commands = {
                           choices=('yes', 'no'), default=None,
                           help="Set the drained flag on the node"),
               ],
-             "<instance>", "Alters the parameters of an instance"),
-  'powercycle': (PowercycleNode, ARGS_ONE, [DEBUG_OPT, FORCE_OPT, CONFIRM_OPT],
+             "<node>", "Alters the parameters of a node"),
+  'powercycle': (PowercycleNode, [ArgNode(min=1, max=1)],
+                 [DEBUG_OPT, FORCE_OPT, CONFIRM_OPT],
                  "<node_name>", "Tries to forcefully powercycle a node"),
-  'remove': (RemoveNode, ARGS_ONE, [DEBUG_OPT],
+  'remove': (RemoveNode, [ArgNode(min=1, max=1)], [DEBUG_OPT],
              "<node_name>", "Removes a node from the cluster"),
-  'volumes': (ListVolumes, ARGS_ANY,
+  'volumes': (ListVolumes, [ArgNode()],
               [DEBUG_OPT, NOHDR_OPT, SEP_OPT, USEUNITS_OPT, FIELDS_OPT],
               "[<node_name>...]", "List logical volumes on node(s)"),
-  'physical-volumes': (ListPhysicalVolumes, ARGS_ANY,
+  'physical-volumes': (ListPhysicalVolumes, [ArgNode()],
                        [DEBUG_OPT, NOHDR_OPT, SEP_OPT, USEUNITS_OPT,
-                        FIELDS_OPT,
-                        make_option("--storage-type",
-                                    dest="user_storage_type",
-                                    choices=_USER_STORAGE_TYPE.keys(),
-                                    default=None,
-                                    metavar="STORAGE_TYPE",
-                                    help=("Storage type (%s)" %
-                                          utils.CommaJoin(_USER_STORAGE_TYPE.keys()))),
-                       ],
+                        FIELDS_OPT, _STORAGE_TYPE_OPT],
                        "[<node_name>...]",
                        "List physical volumes on node(s)"),
-  'modify-volume': (ModifyVolume, ARGS_FIXED(3),
+  'modify-volume': (ModifyVolume,
+                    [ArgNode(min=1, max=1),
+                     ArgChoice(min=1, max=1,
+                               choices=_MODIFIABLE_STORAGE_TYPES),
+                     ArgFile(min=1, max=1)],
                     [DEBUG_OPT,
                      make_option("--allocatable", dest="allocatable",
                                  choices=["yes", "no"], default=None,
@@ -711,15 +723,21 @@ commands = {
                      ],
                     "<node_name> <storage_type> <name>",
                     "Modify storage volume on a node"),
-  'repair-volume': (RepairVolume, ARGS_FIXED(3),
+  'repair-volume': (RepairVolume,
+                    [ArgNode(min=1, max=1),
+                     ArgChoice(min=1, max=1,
+                               choices=_REPAIRABLE_STORAGE_TYPES),
+                     ArgFile(min=1, max=1)],
                     [DEBUG_OPT],
                     "<node_name> <storage_type> <name>",
                     "Repairs a storage volume on a node"),
-  'list-tags': (ListTags, ARGS_ONE, [DEBUG_OPT],
+  'list-tags': (ListTags, [ArgNode(min=1, max=1)], [DEBUG_OPT],
                 "<node_name>", "List the tags of the given node"),
-  'add-tags': (AddTags, ARGS_ATLEAST(1), [DEBUG_OPT, TAG_SRC_OPT],
+  'add-tags': (AddTags, [ArgNode(min=1, max=1), ArgUnknown()],
+               [DEBUG_OPT, TAG_SRC_OPT],
                "<node_name> tag...", "Add tags to the given node"),
-  'remove-tags': (RemoveTags, ARGS_ATLEAST(1), [DEBUG_OPT, TAG_SRC_OPT],
+  'remove-tags': (RemoveTags, [ArgNode(min=1, max=1), ArgUnknown()],
+                  [DEBUG_OPT, TAG_SRC_OPT],
                   "<node_name> tag...", "Remove tags from the given node"),
   }
 
diff --git a/scripts/gnt-os b/scripts/gnt-os
index 4cfa07bf0..41d3dca4d 100755
--- a/scripts/gnt-os
+++ b/scripts/gnt-os
@@ -146,9 +146,9 @@ def DiagnoseOS(opts, args):
 
 
 commands = {
-  'list': (ListOS, ARGS_NONE, [DEBUG_OPT, NOHDR_OPT], "",
+  'list': (ListOS, [], [DEBUG_OPT, NOHDR_OPT], "",
            "Lists all valid OSes on the master"),
-  'diagnose': (DiagnoseOS, ARGS_NONE, [DEBUG_OPT], "",
+  'diagnose': (DiagnoseOS, [], [DEBUG_OPT], "",
                "Diagnose all OSes"),
   }
 
-- 
GitLab