diff --git a/.github/workflows/test-mlc-apptainer-core.yml b/.github/workflows/test-mlc-apptainer-core.yml index b8e16f5aa..61eb187c5 100644 --- a/.github/workflows/test-mlc-apptainer-core.yml +++ b/.github/workflows/test-mlc-apptainer-core.yml @@ -6,6 +6,7 @@ on: paths: - '.github/workflows/test-mlc-apptainer-core.yml' - 'mlc/**' + - 'automation/**' - 'tests/**' - 'pyproject.toml' - '!**.md' diff --git a/.github/workflows/test-unit.yml b/.github/workflows/test-unit.yml index fc2713228..57b4f1480 100644 --- a/.github/workflows/test-unit.yml +++ b/.github/workflows/test-unit.yml @@ -9,6 +9,7 @@ on: paths: - 'mlc/**' - 'tests/**' + - 'automation/**' - '.github/workflows/test-unit.yml' - 'pyproject.toml' push: @@ -16,6 +17,7 @@ on: paths: - 'mlc/**' - 'tests/**' + - 'automation/**' - '.github/workflows/test-unit.yml' - 'pyproject.toml' diff --git a/automation/script/apptainer.py b/automation/script/apptainer.py index c2897b3f2..c63b8b7cf 100644 --- a/automation/script/apptainer.py +++ b/automation/script/apptainer.py @@ -8,9 +8,10 @@ def apptainerfile(self_module, input_params): - # Step 1: Prune and prepare input + # Step 1: Prune and prepare input (remove both apptainer_ and docker_ + # prefixed keys) prune_result = prune_input( - {'input': input_params, 'extra_keys_starts_with': ['apptainer_']}) + {'input': input_params, 'extra_keys_starts_with': ['apptainer_', 'docker_']}) if prune_result['return'] > 0: return prune_result @@ -56,13 +57,15 @@ def apptainerfile(self_module, input_params): run_state = self_module.run_state - apptainer_settings = run_state.get('docker', {}) + apptainer_settings = copy.deepcopy(run_state.get('docker', {})) + utils.merge_dicts({'dict1': apptainer_settings, 'dict2': run_state.get('apptainer', {}), + 'append_lists': True, 'append_unique': True}) apptainer_settings_default_env = apptainer_settings.get('default_env', {}) for key in apptainer_settings_default_env: env.setdefault(key, apptainer_settings_default_env[key]) if not apptainer_settings.get('run', True) and not input_params.get( - 'apptainer_run_override', False): + 'apptainer_run_override', input_params.get('docker_run_override', False)): logger.info("Apptainer 'run' is set to False in meta.yaml") return {'return': 0, 'warning': 'Apptainer run is set to false in script meta'} @@ -86,7 +89,9 @@ def apptainerfile(self_module, input_params): if update_state_result['return'] > 0: return update_state_result - apptainer_settings = run_state.get('docker', {}) + apptainer_settings = copy.deepcopy(run_state.get('docker', {})) + utils.merge_dicts({'dict1': apptainer_settings, 'dict2': run_state.get('apptainer', {}), + 'append_lists': True, 'append_unique': True}) # Prune temporary environment variables run_command = copy.deepcopy(run_command_arc) @@ -102,7 +107,7 @@ def apptainerfile(self_module, input_params): 'tags': script_tags, 'fake_run': True, 'docker_settings': apptainer_settings, - 'docker_run_cmd_prefix': input_params.get('apptainer_run_cmd_prefix', apptainer_settings.get('run_cmd_prefix', '')) + 'docker_run_cmd_prefix': input_params.get('apptainer_run_cmd_prefix', input_params.get('docker_run_cmd_prefix', apptainer_settings.get('run_cmd_prefix', ''))) }) if regenerate_result['return'] > 0: return regenerate_result @@ -155,10 +160,10 @@ def apptainerfile(self_module, input_params): apptainer_v = False apptainer_s = False if is_true(input_params.get( - 'apptainer_v', input_params.get('apptainer_verbose', False))): + 'apptainer_v', input_params.get('apptainer_verbose', input_params.get('docker_v', input_params.get('docker_verbose', False))))): apptainer_v = True if is_true(input_params.get( - 'apptainer_s', input_params.get('apptainer_silent', False))): + 'apptainer_s', input_params.get('apptainer_silent', input_params.get('docker_s', input_params.get('docker_silent', False))))): apptainer_s = True if apptainer_s and apptainer_v: @@ -213,11 +218,16 @@ def apptainer_run(self_module, i): if quiet: env['MLC_QUIET'] = 'yes' - regenerate_def_file = not i.get('apptainer_noregenerate', False) - rebuild_apptainer_image = i.get('apptainer_rebuild', False) + regenerate_def_file = not i.get( + 'apptainer_noregenerate', i.get( + 'docker_noregenerate', False)) + rebuild_apptainer_image = i.get( + 'apptainer_rebuild', i.get( + 'docker_rebuild', False)) - # Prune unnecessary Apptainer-related input keys - r = prune_input({'input': i, 'extra_keys_starts_with': ['apptainer_']}) + # Prune unnecessary Apptainer- and Docker-related input keys + r = prune_input( + {'input': i, 'extra_keys_starts_with': ['apptainer_', 'docker_']}) f_run_cmd = r['new_input'] # Save current directory and prepare to search for scripts @@ -259,7 +269,7 @@ def apptainer_run(self_module, i): mounts = copy.deepcopy( i.get( 'apptainer_mounts', - [])) + i.get('docker_mounts', []))) variations = meta.get('variations', {}) if not hasattr(self_module, 'run_state'): @@ -273,7 +283,9 @@ def apptainer_run(self_module, i): run_state = self_module.run_state - apptainer_settings = run_state.get('docker', {}) + apptainer_settings = copy.deepcopy(run_state.get('docker', {})) + utils.merge_dicts({'dict1': apptainer_settings, 'dict2': run_state.get('apptainer', {}), + 'append_lists': True, 'append_unique': True}) apptainer_settings_default_env = apptainer_settings.get('default_env', {}) for key in apptainer_settings_default_env: @@ -297,7 +309,7 @@ def apptainer_run(self_module, i): # Skip scripts marked as non-runnable if not apptainer_settings.get('run', True) and not i.get( - 'apptainer_run_override', False): + 'apptainer_run_override', i.get('docker_run_override', False)): logger.info("apptainer.run set to False in meta.yaml") return {'return': 0, 'warning': 'Apptainer run is set to false in script meta'} @@ -443,14 +455,15 @@ def prepare_apptainer_inputs(input_params, apptainer_settings, "network", "security_opt" ] - # Collect inputs - apptainer_inputs = { - key: input_params.get( - f"apptainer_{key}", apptainer_settings.get( - key, get_apptainer_default(key))) - for key in keys - if (value := input_params.get(f"apptainer_{key}", apptainer_settings.get(key, get_apptainer_default(key)))) is not None - } + # Collect inputs: apptainer_X > docker_X > meta settings > default + apptainer_inputs = {} + for key in keys: + value = input_params.get( + f"apptainer_{key}", + input_params.get( + f"docker_{key}", apptainer_settings.get(key, get_apptainer_default(key)))) + if value is not None: + apptainer_inputs[key] = value # Convert boolean values to 'yes'/'no' strings for MLC input mapping for key in list(apptainer_inputs.keys()): diff --git a/automation/script/meta_schema.py b/automation/script/meta_schema.py index 17d3bcbe3..be4062647 100644 --- a/automation/script/meta_schema.py +++ b/automation/script/meta_schema.py @@ -86,6 +86,7 @@ # Docker "docker": DICT, # dict - see DOCKER_SCHEMA + "apptainer": DICT, # dict - apptainer overrides; merges with docker settings # Output / debugging "print_env_at_the_end": DICT, # dict[str, list[str]] @@ -164,6 +165,7 @@ "state": DICT, "const": DICT, "docker": DICT, + "apptainer": DICT, "alias": STR, "default_version": STR_OR_FLOAT, "required_disk_space": INT, @@ -248,6 +250,7 @@ "default_env": DICT, "default_variations": DICT, "docker": DICT, + "apptainer": DICT, "adr": DICT, "ad": DICT, } @@ -403,6 +406,20 @@ def validate_meta(data, file_path=""): errors.append( f"{prefix}docker.{dk} has type '{actual}', expected {allowed}") + # Validate apptainer section (same schema as docker; apptainer overrides docker) + apptainer = data.get("apptainer") + if isinstance(apptainer, dict): + for ak, av in apptainer.items(): + if ak not in DOCKER_SCHEMA: + warnings.append( + f"{prefix}apptainer: unknown key '{ak}'") + continue + actual = type(av).__name__ + allowed = DOCKER_SCHEMA[ak] + if actual not in allowed: + errors.append( + f"{prefix}apptainer.{ak} has type '{actual}', expected {allowed}") + # Validate tests section tests = data.get("tests") if isinstance(tests, dict): diff --git a/automation/script/module.py b/automation/script/module.py index 1686b7ed8..d5f414c2b 100644 --- a/automation/script/module.py +++ b/automation/script/module.py @@ -151,7 +151,7 @@ def init_run_state(self, run_state): for f in ['fake_deps', 'cache']: run_state.setdefault(f, False) - for d in ['input_mapping', 'docker', 'remote_run']: + for d in ['input_mapping', 'docker', 'remote_run', 'apptainer']: run_state.setdefault(d, {}) for l in ['deps', 'post_deps', 'prehook_deps', 'posthook_deps', @@ -5768,6 +5768,14 @@ def _apply_conditional_meta_updates(update_meta_if_env, default_env, env, const, 'append_lists': True, 'append_unique': True}) + if c_meta.get('apptainer', {}): + if not run_state.get('apptainer', {}): + run_state['apptainer'] = {} + utils.merge_dicts({'dict1': run_state['apptainer'], + 'dict2': c_meta['apptainer'], + 'append_lists': True, + 'append_unique': True}) + c_add_deps_info = c_meta.get('ad', {}) if not c_add_deps_info: c_add_deps_info = c_meta.get('add_deps', {}) @@ -5975,6 +5983,13 @@ def update_state_from_meta(meta, env, state, const, const_state, run_state, i): if remote_run_settings.get('deps', []): update_deps(remote_run_settings['deps'], add_deps_info, False, env) + new_apptainer_settings = meta.get('apptainer') + if new_apptainer_settings: + utils.merge_dicts({'dict1': run_state['apptainer'], + 'dict2': new_apptainer_settings, + 'append_lists': True, + 'append_unique': True}) + new_env_keys_from_meta = meta.get('new_env_keys', []) if new_env_keys_from_meta: run_state['new_env_keys'] += new_env_keys_from_meta diff --git a/mlc/meta_schema.py b/mlc/meta_schema.py index 500670f31..57a6b3eea 100644 --- a/mlc/meta_schema.py +++ b/mlc/meta_schema.py @@ -86,6 +86,7 @@ # Docker "docker": DICT, # dict - see DOCKER_SCHEMA + "apptainer": DICT, # dict - apptainer overrides; merges with docker settings # Output / debugging "print_env_at_the_end": DICT, # dict[str, list[str]] @@ -164,6 +165,7 @@ "state": DICT, "const": DICT, "docker": DICT, + "apptainer": DICT, "alias": STR, "default_version": STR_OR_FLOAT, "required_disk_space": INT, @@ -248,6 +250,7 @@ "default_env": DICT, "default_variations": DICT, "docker": DICT, + "apptainer": DICT, "adr": DICT, "ad": DICT, } @@ -403,6 +406,20 @@ def validate_meta(data, file_path=""): errors.append( f"{prefix}docker.{dk} has type '{actual}', expected {allowed}") + # Validate apptainer section (same schema as docker; apptainer overrides docker) + apptainer = data.get("apptainer") + if isinstance(apptainer, dict): + for ak, av in apptainer.items(): + if ak not in DOCKER_SCHEMA: + warnings.append( + f"{prefix}apptainer: unknown key '{ak}'") + continue + actual = type(av).__name__ + allowed = DOCKER_SCHEMA[ak] + if actual not in allowed: + errors.append( + f"{prefix}apptainer.{ak} has type '{actual}', expected {allowed}") + # Validate tests section tests = data.get("tests") if isinstance(tests, dict): diff --git a/mlc/script_action.py b/mlc/script_action.py index f0cf8e8dd..125b99cd1 100644 --- a/mlc/script_action.py +++ b/mlc/script_action.py @@ -441,6 +441,38 @@ def apptainer_run(self, run_args): mlc apptainer script --tags=detect,os -j mlca detect,os -j + Flags Available (--apptainer_X takes priority over --docker_X for each option): + + 1. --apptainer_rebuild / --docker_rebuild: + Force rebuild of the Apptainer image even if it already exists. + + 2. --apptainer_noregenerate / --docker_noregenerate: + Skip regenerating the Apptainer definition file before running. + + 3. --apptainer_mounts / --docker_mounts: + List of bind mounts to pass to the container (host:container format). + + 4. --apptainer_run_cmd_prefix / --docker_run_cmd_prefix: + Command prefix to prepend before the mlcr command inside the container. + + 5. --apptainer_verbose / --apptainer_v / --docker_verbose / --docker_v: + Enable verbose output inside the container. + + 6. --apptainer_silent / --apptainer_s / --docker_silent / --docker_s: + Enable silent output inside the container. + + 7. --apptainer_run_override / --docker_run_override: + Force apptainer execution even if 'run' is set to False in script meta. + + All --docker_X options listed above are accepted as defaults when the + corresponding --apptainer_X option is not provided. Docker-only options + (e.g. --docker_dt, --docker_cache, --docker_shm_size) are not applicable + to Apptainer and are ignored. + + Script meta.yaml keys: + - ``docker``: base container settings (used by both mlcd and mlca). + - ``apptainer``: apptainer-specific overrides; merged over ``docker`` settings. + """ return self.call_script_module_function("apptainer", run_args) diff --git a/tests/test_apptainer_docker_unification.py b/tests/test_apptainer_docker_unification.py new file mode 100644 index 000000000..6e09a3802 --- /dev/null +++ b/tests/test_apptainer_docker_unification.py @@ -0,0 +1,286 @@ +""" +Tests for the unification of mlcd and mlca options (issue #312). + +Verifies that: +1. --docker_X CLI options act as fallbacks when --apptainer_X is not given. +2. script meta.yaml `apptainer` key reaches run_state via update_state_from_meta + and overrides `docker` key for apptainer runs. +3. meta_schema validates the `apptainer` section using the same rules as `docker`. +4. mlcd --help and mlca --help work as expected. +""" +import copy +import unittest +import mlc.utils as mlc_utils + + +# --------------------------------------------------------------------------- +# 1. run_state population from meta.yaml — calls real mlc.utils.merge_dicts +# to exercise the same code path as update_state_from_meta in module.py +# --------------------------------------------------------------------------- + +class RunStateApptainerKeyTest(unittest.TestCase): + """ + Verifies that the apptainer key in meta.yaml reaches run_state via the + same merge_dicts pattern used in update_state_from_meta. + This mirrors module.py:update_state_from_meta exactly. + """ + + def _apply_meta_to_run_state(self, meta, run_state=None): + """Replicate the apptainer and docker blocks in update_state_from_meta.""" + if run_state is None: + run_state = {'docker': {}, 'apptainer': {}} + + new_docker = meta.get('docker') + if new_docker: + mlc_utils.merge_dicts({'dict1': run_state.get('docker', {}), + 'dict2': new_docker, + 'append_lists': True, 'append_unique': True}) + + new_apptainer = meta.get('apptainer') + if new_apptainer: + mlc_utils.merge_dicts({'dict1': run_state.get('apptainer', {}), + 'dict2': new_apptainer, + 'append_lists': True, 'append_unique': True}) + return run_state + + def _effective_settings(self, run_state): + """Replicate the merge in apptainer.py: docker overridden by apptainer.""" + effective = copy.deepcopy(run_state.get('docker', {})) + mlc_utils.merge_dicts({'dict1': effective, + 'dict2': run_state.get('apptainer', {}), + 'append_lists': True, 'append_unique': True}) + return effective + + def test_apptainer_base_image_overrides_docker(self): + meta = { + 'docker': {'os': 'ubuntu', 'base_image': 'ubuntu:22.04'}, + 'apptainer': {'base_image': 'docker://ubuntu:20.04'}, + } + run_state = self._apply_meta_to_run_state(meta) + self.assertEqual( + run_state['apptainer']['base_image'], + 'docker://ubuntu:20.04') + effective = self._effective_settings(run_state) + self.assertEqual(effective['base_image'], 'docker://ubuntu:20.04') + self.assertEqual(effective['os'], 'ubuntu') + + def test_docker_settings_used_when_no_apptainer_key(self): + meta = {'docker': {'os': 'ubuntu', 'base_image': 'ubuntu:22.04'}} + run_state = self._apply_meta_to_run_state(meta) + effective = self._effective_settings(run_state) + self.assertEqual(effective['os'], 'ubuntu') + self.assertEqual(effective['base_image'], 'ubuntu:22.04') + + def test_apptainer_key_missing_from_meta_leaves_run_state_empty(self): + meta = {'docker': {'os': 'ubuntu'}} + run_state = self._apply_meta_to_run_state(meta) + self.assertEqual(run_state['apptainer'], {}) + + def test_deep_merge_preserves_docker_subkeys_not_in_apptainer(self): + """apptainer.default_env.B overrides docker.default_env.B; A is preserved.""" + meta = { + 'docker': {'default_env': {'A': '1', 'B': '2'}}, + 'apptainer': {'default_env': {'B': 'override'}}, + } + run_state = self._apply_meta_to_run_state(meta) + effective = self._effective_settings(run_state) + self.assertEqual(effective['default_env']['A'], '1') + self.assertEqual(effective['default_env']['B'], 'override') + + def test_init_run_state_seeds_apptainer_key(self): + """init_run_state in module.py must initialise run_state['apptainer'] to {}.""" + import sys + import importlib.util + import os + + # Load automation/utils.py as 'utils' so module.py can do `from utils + # import *` + automation_path = os.path.join( + os.path.dirname(os.path.dirname(__file__)), 'automation') + utils_spec = importlib.util.spec_from_file_location( + 'utils', os.path.join(automation_path, 'utils.py')) + utils_mod = importlib.util.module_from_spec(utils_spec) + sys.modules.setdefault('utils', utils_mod) + utils_spec.loader.exec_module(utils_mod) + + # Also add automation/script to sys.path so `from script.X import *` + # works + script_path = os.path.join(automation_path, 'script') + for p in [automation_path, script_path]: + if p not in sys.path: + sys.path.insert(0, p) + + # Dynamically load automation/script/module.py + module_spec = importlib.util.spec_from_file_location( + '_test_module', + os.path.join(automation_path, 'script', 'module.py')) + module_mod = importlib.util.module_from_spec(module_spec) + module_spec.loader.exec_module(module_mod) + + sa = module_mod.ScriptAutomation.__new__(module_mod.ScriptAutomation) + result = sa.init_run_state(None) + self.assertIn('apptainer', result, + "init_run_state must seed run_state['apptainer']") + self.assertEqual(result['apptainer'], {}) + + +# --------------------------------------------------------------------------- +# 2. CLI fallback: docker_X used when apptainer_X is absent +# Tests the three-level lookup: apptainer_X > docker_X > settings > default +# --------------------------------------------------------------------------- + +class ApptainerDockerCliFallbackTest(unittest.TestCase): + + def _lookup(self, key, input_params, settings, default=None): + """Replicates the lookup used in prepare_apptainer_inputs.""" + return input_params.get( + f"apptainer_{key}", + input_params.get(f"docker_{key}", settings.get(key, default)) + ) + + def test_apptainer_prefix_wins(self): + val = self._lookup( + "os", { + "apptainer_os": "debian", "docker_os": "centos"}, {}) + self.assertEqual(val, "debian") + + def test_docker_prefix_fallback(self): + val = self._lookup("os", {"docker_os": "centos"}, {}) + self.assertEqual(val, "centos") + + def test_settings_fallback(self): + val = self._lookup("os", {}, {"os": "ubuntu"}) + self.assertEqual(val, "ubuntu") + + def test_default_fallback(self): + val = self._lookup("os", {}, {}, "alpine") + self.assertEqual(val, "alpine") + + def test_noregenerate_docker_fallback(self): + i = {"docker_noregenerate": True} + regenerate_def_file = not i.get( + "apptainer_noregenerate", i.get( + "docker_noregenerate", False)) + self.assertFalse(regenerate_def_file) + + def test_noregenerate_apptainer_overrides_docker(self): + i = {"apptainer_noregenerate": False, "docker_noregenerate": True} + regenerate_def_file = not i.get( + "apptainer_noregenerate", i.get( + "docker_noregenerate", False)) + self.assertTrue(regenerate_def_file) + + def test_rebuild_docker_fallback(self): + i = {"docker_rebuild": True} + rebuild = i.get("apptainer_rebuild", i.get("docker_rebuild", False)) + self.assertTrue(rebuild) + + def test_mounts_docker_fallback(self): + i = {"docker_mounts": ["/host:/container"]} + mounts = i.get("apptainer_mounts", i.get("docker_mounts", [])) + self.assertEqual(mounts, ["/host:/container"]) + + def test_mounts_apptainer_overrides_docker(self): + i = {"apptainer_mounts": ["/a:/a"], "docker_mounts": ["/b:/b"]} + mounts = i.get("apptainer_mounts", i.get("docker_mounts", [])) + self.assertEqual(mounts, ["/a:/a"]) + + +# --------------------------------------------------------------------------- +# 3. meta_schema: apptainer key validation (calls real validate_meta) +# --------------------------------------------------------------------------- + +class MetaSchemaApptainerKeyTest(unittest.TestCase): + + def _validate(self, data): + from mlc.meta_schema import validate_meta + base = { + "alias": "test-script", + "uid": "aabbccdd11223344", + "automation_alias": "script", + "automation_uid": "5b4aa8f95024c2a5", + } + base.update(data) + return validate_meta(base, "test.yaml") + + def test_apptainer_key_accepted_with_dict(self): + errors, warnings = self._validate({"apptainer": {"os": "ubuntu"}}) + self.assertEqual(errors, []) + + def test_apptainer_unknown_key_produces_warning(self): + _, warnings = self._validate({"apptainer": {"unknown_key_xyz": "val"}}) + self.assertTrue(any("unknown key" in w for w in warnings)) + + def test_apptainer_wrong_type_produces_error(self): + errors, _ = self._validate({"apptainer": {"run": "not-a-bool"}}) + self.assertTrue(any("apptainer.run" in e for e in errors)) + + def test_apptainer_run_bool_accepted(self): + errors, _ = self._validate({"apptainer": {"run": False}}) + self.assertEqual(errors, []) + + def test_docker_and_apptainer_both_accepted(self): + errors, _ = self._validate({ + "docker": {"os": "ubuntu"}, + "apptainer": {"os": "centos"}, + }) + self.assertEqual(errors, []) + + def test_apptainer_base_image_str_accepted(self): + errors, _ = self._validate( + {"apptainer": {"base_image": "ubuntu:22.04"}}) + self.assertEqual(errors, []) + + +# --------------------------------------------------------------------------- +# 4. CLI smoke tests: mlcd --help and mlca --help +# --------------------------------------------------------------------------- + +class CliHelpSmokeTest(unittest.TestCase): + """mlcd and mlca --help exit cleanly and include expected content.""" + + def test_mlcd_help_exits_successfully(self): + import subprocess + import sys + result = subprocess.run( + [sys.executable, "-c", + "import sys; sys.argv=['mlcd','--help']; " + "from mlc.main import mlcd; mlcd()"], + capture_output=True, text=True + ) + combined = result.stdout + result.stderr + self.assertIn("docker", combined.lower(), + msg=f"Expected docker help text, got: {combined[:500]}") + + def test_mlca_help_exits_successfully(self): + import subprocess + import sys + result = subprocess.run( + [sys.executable, "-c", + "import sys; sys.argv=['mlca','--help']; " + "from mlc.main import mlca; mlca()"], + capture_output=True, text=True + ) + combined = result.stdout + result.stderr + self.assertIn("apptainer", combined.lower(), + msg=f"Expected apptainer help text, got: {combined[:500]}") + + def test_mlca_help_mentions_docker_fallback(self): + """mlca --help documents that --docker_X options are accepted.""" + import subprocess + import sys + result = subprocess.run( + [sys.executable, "-c", + "import sys; sys.argv=['mlca','--help']; " + "from mlc.main import mlca; mlca()"], + capture_output=True, text=True + ) + combined = result.stdout + result.stderr + self.assertIn("docker_rebuild", combined, + msg=f"Expected docker_rebuild in mlca help, got: {combined[:500]}") + self.assertIn("docker_noregenerate", combined, + msg=f"Expected docker_noregenerate in mlca help, got: {combined[:500]}") + + +if __name__ == "__main__": + unittest.main()