Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 6 commits into from
Oct 4, 2023
Merged

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

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e7b797a
Make specs for containers configurable, clean up arguments for spack …
climbfuji Oct 2, 2023
6c02740
Fix style errors in spack-stack extensions
climbfuji Oct 3, 2023
afd8a10
Remove unused 'import copy' from lib/jcsda-emc/spack-stack/stack/cont…
climbfuji Oct 3, 2023
734e565
Update unit tests for creating containers
climbfuji Oct 3, 2023
bea1111
Bug fix for unit tests and formatting changes in lib/jcsda-emc/spack-…
climbfuji Oct 3, 2023
1f4e7c5
Use better logic to filter README.md in lib/jcsda-emc/spack-stack/sta…
climbfuji Oct 3, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 57 additions & 26 deletions lib/jcsda-emc/spack-stack/stack/cmd/stack_cmds/create.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,44 +51,86 @@ def container_config_help():
help_string = "Pre-configured container." + os.linesep
help_string += "Available options are: " + os.linesep
for config in container_configs:
if config == "README.md":
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternatively, we could use the logic from lib/jcsda-emc/spack-stack/tests/test_stack_create.py:

        _, _, containers = next(os.walk(container_path))
        # Exclude files like "README.md"
        containers = [x for x in containers if x.endswith(".yaml")]

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this alternate logic would be better as long as we know that config is always going to be in files with a .yaml extension. That way we don't pick up an undesirable file if something besides README.md shows up.

Would there be cases of config files using a .yml extension, ie is it worthwhile to add the check x.endswith(".yml") to the test in the alternate logic.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, the alternate logic is better. The spack convention is to always use .yaml, I don't think we need .yml. (Also, the logic in the create container code assumes .yaml, because that's what it appends to the arguments.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

continue
help_string += "\t" + config.rstrip(".yaml") + os.linesep
return help_string


def container_specs_help():
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The diffs in this file are confusing. It may be better checking out the current code and this PR and verify side by side in meld (which sometimes does a better job with diffs). Most of the common arguments on the left moved down into the env section, and ctr got its own arguments.

_, _, 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(
Expand All @@ -106,17 +148,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()
)
Expand Down Expand Up @@ -145,7 +176,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):
Expand Down
59 changes: 32 additions & 27 deletions lib/jcsda-emc/spack-stack/stack/container_env.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from spack.extensions.stack.stack_paths import (
common_path,
container_path,
template_path,
container_specs_path,
)


Expand All @@ -16,48 +16,50 @@ 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
#template_env = os.path.join(self.template_path, "spack.yaml")
climbfuji marked this conversation as resolved.
Show resolved Hide resolved
#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:
filedata = f.read()
filedata = filedata.replace("::", ":")
template_yaml = syaml.load_config(filedata)
container_yaml = syaml.load_config(filedata)

with open(self.container_path, "r") as f:
container_yaml = syaml.load_config(f)
with open(self.specs_path, "r") as f:
specs_yaml = syaml.load_config(f)

# Create copy so we can modify it
original_yaml = copy.deepcopy(container_yaml)
Expand All @@ -74,11 +76,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)

Expand Down
1 change: 1 addition & 0 deletions lib/jcsda-emc/spack-stack/stack/stack_paths.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,5 @@ def stack_path(*paths):
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')
Loading