Merge pull request #333 from climbfuji/feature/container_env_updates

Update spack stack extension for containers: make specs configurable, clean up arguments for env/ctr

lib/jcsda-emc/spack-stack/stack/cmd/stack_cmds/create.py

@@ -48,47 +48,83 @@ def template_help():
 
 def container_config_help():
     _, _, container_configs = next(os.walk(stack_path("configs", "containers")))
+    # Exclude files like "README.md"
+    container_configs = [x for x in container_configs if x.endswith(".yaml")]
     help_string = "Pre-configured container." + os.linesep
     help_string += "Available options are: " + os.linesep
     for config in container_configs:
         help_string += "\t" + config.rstrip(".yaml") + os.linesep
     return help_string
 
 
+def container_specs_help():
+    _, _, specs_lists = next(os.walk(stack_path("configs", "containers", "specs")))
+    help_string = "List of specs to build in container." + os.linesep
+    help_string += "Available options are: " + os.linesep
+    for specs_list in specs_lists:
+        help_string += "\t" + specs_list.rstrip(".yaml") + os.linesep
+    return help_string
+
+
 def setup_common_parser_args(subparser):
     """Shared CLI args for container and environment subcommands"""
+
     subparser.add_argument(
-        "--template",
-        type=str,
+        "--overwrite",
+        action="store_true",
         required=False,
-        dest="template",
-        default="empty",
-        help=template_help(),
+        default=False,
+        help="Overwrite existing environment if it exists." " Warning this is dangerous.",
     )
 
+
+def setup_ctr_parser(subparser):
+    """create container-specific parsing options"""
+
+    subparser.add_argument("--container", required=True, help=container_config_help())
+
+    subparser.add_argument("--specs", required=True, help=container_specs_help())
+
     subparser.add_argument(
-        "--name",
+        "--dir",
         type=str,
         required=False,
-        default=None,
-        help='Environment name, defaults to "{}".'.format(default_env_name),
+        default=default_env_path,
+        help="Environment will be placed in <dir>/container/."
+        " Default is {}/container/.".format(default_env_path),
     )
 
+    setup_common_parser_args(subparser)
+
+
+def setup_env_parser(subparser):
+    """create environment-specific parsing options"""
+    setup_common_parser_args(subparser)
+
     subparser.add_argument(
         "--dir",
         type=str,
         required=False,
         default=default_env_path,
-        help="Environment will be placed in <dir>/<name>/."
-        " Default is {}/<name>/.".format(default_env_path),
+        help="Environment will be placed in <dir>/<env-name>/."
+        " Default is {}/<env-name>/.".format(default_env_path),
     )
 
     subparser.add_argument(
-        "--overwrite",
-        action="store_true",
+        "--name",
+        type=str,
         required=False,
-        default=False,
-        help="Overwrite existing environment if it exists." " Warning this is dangerous.",
+        default=None,
+        help="Environment name, defaults to <template>.<site>",
+    )
+
+    subparser.add_argument(
+        "--template",
+        type=str,
+        required=False,
+        dest="template",
+        default="empty",
+        help=template_help(),
     )
 
     subparser.add_argument(
@@ -106,17 +142,6 @@ def setup_common_parser_args(subparser):
         help="Include upstream environment (/path/to/spack-stack-x.y.z/envs/unified-env/install)",
     )
 
-
-def setup_ctr_parser(subparser):
-    """create container-specific parsing options"""
-    subparser.add_argument("container", help=container_config_help())
-
-    setup_common_parser_args(subparser)
-
-
-def setup_env_parser(subparser):
-    """create environment-specific parsing options"""
-    setup_common_parser_args(subparser)
     subparser.add_argument(
         "--site", type=str, required=False, default=default_site(), help=site_help()
     )
@@ -145,7 +170,7 @@ def setup_create_parser(subparser):
 def container_create(args):
     """Create pre-configured container"""
 
-    container = StackContainer(args.container, args.template, args.name, args.dir, args.packages)
+    container = StackContainer(args.container, args.dir, args.specs)
 
     env_dir = container.env_dir
     if os.path.exists(env_dir):

lib/jcsda-emc/spack-stack/stack/container_env.py

@@ -1,13 +1,8 @@
-import copy
 import os
 
 import spack
 import spack.util.spack_yaml as syaml
-from spack.extensions.stack.stack_paths import (
-    common_path,
-    container_path,
-    template_path,
-)
+from spack.extensions.stack.stack_paths import common_path, container_path, container_specs_path
 
 
 class StackContainer:
@@ -16,51 +11,43 @@ class StackContainer:
     its packages.yaml versions then writes out a merged file.
     """
 
-    def __init__(self, container, template, name, dir, base_packages) -> None:
-        self.template = template
+    def __init__(self, container, dir, specs) -> None:
         self.container = container
+        self.specs = specs
 
-        test_path = os.path.join(container_path, container + ".yaml")
+        test_path = os.path.join(container_path, self.container + ".yaml")
         if os.path.exists(test_path):
             self.container_path = test_path
         elif os.path.isabs(container):
-            self.container_path = container
+            self.container_path = self.container
         else:
             raise Exception("Invalid container {}".format(self.container))
 
-        if os.path.isabs(self.template):
-            self.template_path = self.template
-        elif os.path.exists(os.path.join(template_path, self.template)):
-            self.template_path = os.path.join(template_path, self.template)
+        test_path = os.path.join(container_specs_path, self.specs + ".yaml")
+        if os.path.exists(test_path):
+            self.specs_path = test_path
+        elif os.path.isabs(specs):
+            self.specs_path = self.specs
         else:
-            raise Exception("Invalid application template")
-
-        self.name = name if name else "{}".format(container)
+            raise Exception("Invalid specs list {}".format(self.specs))
 
+        self.name = self.container
         self.dir = dir
         self.env_dir = os.path.join(self.dir, self.name)
-        if base_packages:
-            self.base_packages = base_packages
-        else:
-            self.base_packages = os.path.join(common_path, "packages.yaml")
+        self.base_packages = os.path.join(common_path, "packages.yaml")
 
     def write(self):
         """Merge base packages and app's spack.yaml into
         output container file.
         """
-        template_env = os.path.join(self.template_path, "spack.yaml")
-        with open(template_env, "r") as f:
-            # Spack uses :: to override settings.
-            # but it's not understood when used in a spack.yaml
-            filedata = f.read()
-            filedata = filedata.replace("::", ":")
-            template_yaml = syaml.load_config(filedata)
 
         with open(self.container_path, "r") as f:
-            container_yaml = syaml.load_config(f)
+            filedata = f.read()
+            filedata = filedata.replace("::", ":")
+            container_yaml = syaml.load_config(filedata)
 
-        # Create copy so we can modify it
-        original_yaml = copy.deepcopy(container_yaml)
+        with open(self.specs_path, "r") as f:
+            specs_yaml = syaml.load_config(f)
 
         with open(self.base_packages, "r") as f:
             filedata = f.read()
@@ -74,11 +61,14 @@ def write(self):
             container_yaml["spack"]["packages"], packages_yaml["packages"]
         )
 
-        container_yaml = spack.config.merge_yaml(container_yaml, template_yaml)
-        # Merge the original back in so it takes precedence
-        container_yaml = spack.config.merge_yaml(container_yaml, original_yaml)
+        if "specs" not in container_yaml["spack"]:
+            container_yaml["spack"]["specs"] = {}
+
+        container_yaml["spack"]["specs"] = spack.config.merge_yaml(
+            container_yaml["spack"]["specs"], specs_yaml["specs"]
+        )
 
-        container_yaml["spack"]["container"]["labels"]["app"] = self.template
+        container_yaml["spack"]["container"]["labels"]["app"] = self.specs
 
         os.makedirs(self.env_dir, exist_ok=True)
 

lib/jcsda-emc/spack-stack/stack/stack_paths.py

@@ -5,7 +5,7 @@
 # Hidden file in top-level spack-stack dir so this module can
 # find relative config files. Assuming Spack is a submodule of
 # spack-stack.
-check_file = '.spackstack'
+check_file = ".spackstack"
 
 
 # Find spack-stack directory assuming this Spack instance
@@ -14,12 +14,13 @@ def stack_path(*paths):
     stack_dir = os.path.dirname(spack.paths.spack_root)
 
     if not os.path.exists(os.path.join(stack_dir, check_file)):
-        raise Exception('Not a submodule of spack-stack')
+        raise Exception("Not a submodule of spack-stack")
 
     return os.path.join(stack_dir, *paths)
 
 
-common_path = stack_path('configs', 'common')
-site_path = stack_path('configs', 'sites')
-container_path = stack_path('configs', 'containers')
-template_path = stack_path('configs', 'templates')
+common_path = stack_path("configs", "common")
+site_path = stack_path("configs", "sites")
+container_path = stack_path("configs", "containers")
+container_specs_path = stack_path("configs", "containers", "specs")
+template_path = stack_path("configs", "templates")

lib/jcsda-emc/spack-stack/tests/test_stack_create.py

@@ -51,6 +51,17 @@ def all_containers():
         return None
 
 
+def all_specs():
+    specs_path = stack_path("configs", "containers", "specs")
+    if specs_path:
+        _, _, specs = next(os.walk(specs_path))
+        # Exclude files like "README.md"
+        specs = [x for x in specs if x.endswith(".yaml")]
+        return specs
+    else:
+        return None
+
+
 @pytest.mark.extension("stack")
 @pytest.mark.parametrize("template", all_templates())
 @pytest.mark.filterwarnings("ignore::UserWarning")
@@ -71,9 +82,21 @@ def test_sites(site):
 
 @pytest.mark.extension("stack")
 @pytest.mark.parametrize("container", all_containers())
-@pytest.mark.filterwarnings("ignore::UserWarning")
-def test_containers(container):
-    if not container:
+@pytest.mark.parametrize("spec", all_specs())
+# @pytest.mark.filterwarnings("ignore::UserWarning")
+def test_containers(container, spec):
+    if not container or not spec:
         return
     container_wo_ext = os.path.splitext(container)[0]
-    stack_create("create", "ctr", container_wo_ext, "--dir", test_dir, "--overwrite")
+    spec_wo_ext = os.path.splitext(spec)[0]
+    stack_create(
+        "create",
+        "ctr",
+        "--container",
+        container_wo_ext,
+        "--spec",
+        spec_wo_ext,
+        "--dir",
+        test_dir,
+        "--overwrite",
+    )