Merge pull request #1636 from buildtesters/remove_executors

remove executors via command line 'buildtest config executors remove'

bash_completion.sh

@@ -56,6 +56,11 @@ _avail_executors ()
   buildtest config executors list
 }
 
+_all_executors()
+{
+  buildtest config executors list --all
+}
+
 # list of available compilers
 _avail_compilers ()
 {
@@ -302,13 +307,16 @@ _buildtest ()
           esac
           ;;
         executors|ex)
-          local opts="--help --disabled --invalid --json --yaml -d -h -i -j -y"
-          local cmds="list"
+          local cmds="list rm remove"
 
-          case "$prev" in
+          case ${COMP_WORDS[3+offset]} in
               list)
+                  local opts="--help --all --disabled --invalid --json --yaml -a -d -h -i -j -y"
                   COMPREPLY=( $( compgen -W "$opts" -- "${cur}" ) )
                   ;;
+              rm|remove)
+                  COMPREPLY=( $( compgen -W "$(_all_executors)" -- "${cur}" ) )
+                  ;;
               *)
                   COMPREPLY=( $( compgen -W "${cmds}" -- "${cur}" ) )
                   ;;

buildtest/cli/__init__.py

@@ -1373,8 +1373,26 @@ def config_menu(self):
                                     "help": "Show invalid executors",
                                 },
                             ),
+                            (
+                                ("-a", "--all"),
+                                {"action": "store_true", "help": "Show all executors"},
+                            ),
+                        ],
+                    },
+                    {
+                        "name": "remove",
+                        "aliases": ["rm"],
+                        "help": "Remove executor from configuration",
+                        "arguments": [
+                            (
+                                ("executor_names",),
+                                {
+                                    "nargs": "*",
+                                    "help": "Specify an executor name to remove",
+                                },
+                            )
                         ],
-                    }
+                    },
                 ],
             },
             {

buildtest/cli/config.py

@@ -36,13 +36,16 @@ def config_cmd(args, configuration, editor, system):
         buildexecutor = BuildExecutor(configuration)
         if args.executors == "list":
             view_executors(
-                configuration,
-                buildexecutor,
-                args.json,
-                args.yaml,
-                args.disabled,
-                args.invalid,
+                configuration=configuration,
+                buildexecutor=buildexecutor,
+                json_format=args.json,
+                yaml_format=args.yaml,
+                disabled=args.disabled,
+                invalid=args.invalid,
+                all_executors=args.all,
             )
+        if args.executors in ["remove", "rm"]:
+            remove_executors(configuration, args.executor_names)
 
     elif args.config in ["validate", "val"]:
         validate_config(configuration, system.system["moduletool"])
@@ -243,6 +246,7 @@ def view_executors(
     yaml_format=False,
     disabled=False,
     invalid=False,
+    all_executors=False,
 ):
     """Display executors from buildtest configuration. This implements ``buildtest config executors`` command.
 
@@ -253,6 +257,7 @@ def view_executors(
         yaml_format (bool): Display output in yaml format which is specified via ``buildtest config executors --yaml``
         disabled (bool): Display list of disabled executors which is specified via ``buildtest config executors --disabled``
         invalid (bool): Display list of invalid executors which is specified via ``buildtest config executors --invalid``
+        all_executors (bool): Display all executors which is specified via ``buildtest config executors --all``
     """
 
     executor_settings = {"executors": configuration.target_config["executors"]}
@@ -285,6 +290,59 @@ def view_executors(
             console.print(executor)
         return
 
+    if all_executors:
+        names = configuration.get_all_executors()
+        for name in names:
+            print(name)
+        return
     names = buildexecutor.names()
     for name in names:
         print(name)
+
+
+def remove_executors(configuration, executor_names):
+    """Remove executors from buildtest configuration. This implements ``buildtest config executors remove`` command.
+
+    Args:
+        configuration (buildtest.config.SiteConfiguration): An instance of SiteConfiguration class
+        executor_names (list): List of executor names to remove
+    """
+
+    # variable to determine if file needs to be written back to disk
+    write_back = False
+
+    for name in executor_names:
+        exec_type = name.split(".")[1]
+        exec_name = name.split(".")[2]
+
+        if not configuration.target_config["executors"].get(exec_type):
+            console.print(
+                f"Unable to remove executor: {name} because there are no executors of type: {exec_type}"
+            )
+            continue
+
+        if exec_name not in configuration.target_config["executors"][exec_type].keys():
+            console.print(
+                f"Unable to remove executor: {name} because it does not exist"
+            )
+            continue
+
+        del configuration.target_config["executors"][exec_type][exec_name]
+
+        if len(configuration.target_config["executors"][exec_type].keys()) == 0:
+            del configuration.target_config["executors"][exec_type]
+        console.print(f"Removing executor: {name}")
+        write_back = True
+
+    custom_validator(
+        configuration.config, schema_table["settings.schema.json"]["recipe"]
+    )
+
+    # only update the configuration file if we removed an executor
+    if write_back:
+        console.print(f"Updating configuration file: {configuration.file}")
+
+        with open(configuration.file, "w") as fd:
+            yaml.safe_dump(
+                configuration.config, fd, default_flow_style=False, sort_keys=False
+            )

buildtest/cli/show.py

@@ -297,6 +297,13 @@ def print_config_show():
     table.add_row(
         "buildtest config executors list --json", "List all invalid executors"
     )
+    table.add_row(
+        "buildtest config executors list --all", "List all available executors"
+    )
+    table.add_row(
+        "buildtest config executors remove generic.local.bash generic.local.sh",
+        "Remove executor names 'generic.local.bash' and 'generic.local.sh' from configuration file",
+    )
     table.add_row("buildtest config path", "Show path to configuration file")
     table.add_row(
         "buildtest config systems",

buildtest/config.py

@@ -40,6 +40,7 @@ def __init__(self, settings_file=None):
 
         self.disabled_executors = []
         self.invalid_executors = []
+        self.all_executors = []
         self.valid_executors = {
             "local": {},
             "slurm": {},
@@ -155,6 +156,17 @@ def _executor_check(self):
         self._validate_cobalt_executors()
         self._validate_pbs_executors()
 
+        for executor_type in self.target_config["executors"]:
+            if executor_type == "defaults":
+                continue
+
+            for name in self.target_config["executors"][executor_type]:
+                self.all_executors.append(f"{self.name()}.{executor_type}.{name}")
+
+    def get_all_executors(self):
+        """Return list of all executors"""
+        return self.all_executors
+
     def is_executor_disabled(self, executor):
         if executor.get("disable"):
             return True

buildtest/schemas/settings.schema.json

@@ -310,7 +310,8 @@
                 },
                 "additionalProperties": {
                   "$ref": "#/definitions/local"
-                }
+                },
+                "minProperties": 1
               },
               "lsf": {
                 "type": "object",

tests/cli/test_config.py

@@ -7,6 +7,7 @@
 from buildtest.cli.build import BuildTest
 from buildtest.cli.config import (
     list_profiles,
+    remove_executors,
     remove_profiles,
     validate_config,
     view_configuration,
@@ -43,6 +44,24 @@ def test_config_systems():
         view_system(configuration)
 
 
+def test_remove_executors():
+    temp_config_file = tempfile.NamedTemporaryFile(suffix=".yml")
+    shutil.copy(configuration.file, temp_config_file.name)
+
+    print(temp_config_file.name)
+    config = SiteConfiguration(settings_file=temp_config_file.name)
+    config.detect_system()
+    configuration.validate(moduletool=system.system["moduletool"])
+
+    remove_executors(config, executor_names=["generic.local.bash", "generic.local.sh"])
+
+    # remove an invalid executor type
+    remove_executors(config, executor_names=["generic.XYZ.bash"])
+
+    # remove an invalid executor name
+    remove_executors(config, executor_names=["generic.local.bash1234"])
+
+
 @pytest.mark.cli
 def test_view_configuration():
     view_configuration(configuration)
@@ -122,6 +141,7 @@ def test_config_executors():
         yaml_format=False,
         disabled=False,
         invalid=False,
+        all_executors=False,
     )
 
     # buildtest config executors list --yaml
@@ -132,6 +152,18 @@ def test_config_executors():
         yaml_format=True,
         disabled=False,
         invalid=False,
+        all_executors=False,
+    )
+
+    # buildtest config executors list --all
+    view_executors(
+        configuration=configuration,
+        buildexecutor=buildexecutor,
+        json_format=False,
+        yaml_format=False,
+        disabled=False,
+        invalid=False,
+        all_executors=True,
     )
 
     # buildtest config executors list --disabled