Single-point for reading container configuration

datalad_container/containers_add.py

@@ -22,6 +22,7 @@
 from datalad.support.exceptions import InsufficientArgumentsError
 from datalad.interface.results import get_status_dict
 
+from .utils import get_container_configuration
 from .definitions import definitions
 
 lgr = logging.getLogger("datalad.containers.containers_add")
@@ -205,8 +206,8 @@ def __call__(name, url=None, dataset=None, call_fmt=None, image=None,
                 "Container names can only contain alphanumeric characters "
                 "and '-', got: '{}'".format(name))
 
-        cfgbasevar = "datalad.containers.{}".format(name)
-        if cfgbasevar + ".image" in ds.config:
+        container_cfg = get_container_configuration(ds, name)
+        if 'image' in container_cfg:
             if not update:
                 yield get_status_dict(
                     action="containers_add", ds=ds, logger=lgr,
@@ -219,16 +220,16 @@ def __call__(name, url=None, dataset=None, call_fmt=None, image=None,
             if not (url or image or call_fmt):
                 # No updated values were provided. See if an update url is
                 # configured (currently relevant only for Singularity Hub).
-                url = ds.config.get(cfgbasevar + ".updateurl")
+                url = container_cfg.get("updateurl")
                 if not url:
                     yield get_status_dict(
                         action="containers_add", ds=ds, logger=lgr,
                         status="impossible",
                         message="No values to update specified")
                     return
 
-            call_fmt = call_fmt or ds.config.get(cfgbasevar + ".cmdexec")
-            image = image or ds.config.get(cfgbasevar + ".image")
+            call_fmt = call_fmt or container_cfg.get("cmdexec")
+            image = image or container_cfg.get("image")
 
         if not image:
             loc_cfg_var = "datalad.containers.location"
@@ -329,6 +330,7 @@ def __call__(name, url=None, dataset=None, call_fmt=None, image=None,
             return
 
         # store configs
+        cfgbasevar = "datalad.containers.{}".format(name)
         if imgurl != url:
             # store originally given URL, as it resolves to something
             # different and maybe can be used to update the container

datalad_container/containers_list.py

@@ -19,6 +19,8 @@
 from datalad.coreapi import subdatasets
 from datalad.ui import ui
 
+from datalad_container.utils import get_container_configuration
+
 lgr = logging.getLogger("datalad.containers.containers_list")
 
 
@@ -77,22 +79,7 @@ def __call__(dataset=None, recursive=False, contains=None):
                         yield c
 
         # all info is in the dataset config!
-        var_prefix = 'datalad.containers.'
-        containers = {}
-        for var, value in ds.config.items():
-            if not var.startswith(var_prefix):
-                # not an interesting variable
-                continue
-            var_comps = var[len(var_prefix):].split('.')
-            cname = var_comps[0]
-            ccfgname = '.'.join(var_comps[1:])
-            if not ccfgname:
-                continue
-
-            cinfo = containers.get(cname, {})
-            cinfo[ccfgname] = value
-
-            containers[cname] = cinfo
+        containers = get_container_configuration(ds)
 
         for k, v in containers.items():
             if 'image' not in v:

datalad_container/containers_remove.py

@@ -15,6 +15,8 @@
 from datalad.support.constraints import EnsureStr
 from datalad.interface.results import get_status_dict
 
+from datalad_container.utils import get_container_configuration
+
 lgr = logging.getLogger("datalad.containers.containers_remove")
 
 
@@ -24,15 +26,24 @@ class ContainersRemove(Interface):
     # first docstring line is used a short description in the cmdline help
     # the rest is put in the verbose help and manpage
     """Remove a known container from a dataset
+
+    This command is only removing a container from the committed
+    Dataset configuration (configuration scope ``branch``). It will not
+    modify any other configuration scopes.
+
+    This command is *not* dropping the container image associated with the
+    removed record, because it may still be needed for other dataset versions.
+    In order to drop the container image, use the 'drop' command prior
+    to removing the container configuration.
     """
 
     # parameters of the command, must be exhaustive
     _params_ = dict(
         dataset=Parameter(
             args=("-d", "--dataset"),
-            doc="""specify the dataset to query. If no dataset is given, an
-            attempt is made to identify the dataset based on the current
-            working directory""",
+            doc="""specify the dataset from removing a container. If no dataset
+            is given, an attempt is made to identify the dataset based on the
+            current working directory""",
             constraints=EnsureDataset() | EnsureNone()),
         name=Parameter(
             args=("name",),
@@ -42,7 +53,9 @@ class ContainersRemove(Interface):
         ),
         remove_image=Parameter(
             args=("-i", "--remove-image",),
-            doc="""if set, remove container image as well""",
+            doc="""if set, remove container image as well. Even with this flag,
+            the container image content will not be dropped. Use the 'drop'
+            command explicitly before removing the container configuration.""",
             action="store_true",
         ),
     )
@@ -59,12 +72,11 @@ def __call__(name, dataset=None, remove_image=False):
             action='containers_remove',
             logger=lgr)
 
-        section = 'datalad.containers.{}'.format(name)
-        imagecfg = '{}.image'.format(section)
+        container_cfg = get_container_configuration(ds, name)
 
         to_save = []
-        if remove_image and imagecfg in ds.config:
-            imagepath = ds.config.get(imagecfg)
+        if remove_image and 'image' in container_cfg:
+            imagepath = container_cfg['image']
             if op.lexists(op.join(ds.path, imagepath)):
                 for r in ds.remove(
                         path=imagepath,
@@ -78,10 +90,10 @@ def __call__(name, dataset=None, remove_image=False):
                     yield r
                 to_save.append(imagepath)
 
-        if section in ds.config.sections():
+        if container_cfg:
             ds.config.remove_section(
-                section,
-                where='dataset',
+                f'datalad.containers.{name}',
+                scope='branch',
                 reload=True)
             res['status'] = 'ok'
             to_save.append(op.join('.datalad', 'config'))

datalad_container/containers_run.py

@@ -81,6 +81,14 @@ def __call__(cmd, container_name=None, dataset=None,
         ds = require_dataset(dataset, check_installed=True,
                              purpose='run a containerized command execution')
 
+        # this following block locates the target container. this involves a
+        # configuration look-up. This is not using
+        # get_container_configuration(), because it needs to account for a
+        # wide range of scenarios, including the installation of the dataset(s)
+        # that will eventually provide (the configuration) for the container.
+        # However, internally this is calling `containers_list()`, which is
+        # using get_container_configuration(), so any normalization of
+        # configuration on-read, get still be implemented in this helper.
         container = None
         for res in find_container_(ds, container_name):
             if res.get("action") == "containers":

datalad_container/tests/test_containers.py

@@ -29,27 +29,32 @@
 
 from datalad_container.tests.utils import add_pyscript_image
 
+common_kwargs = {'result_renderer': 'disabled'}
+
 
 @with_tempfile
 def test_add_noop(path=None):
-    ds = Dataset(path).create()
+    ds = Dataset(path).create(**common_kwargs)
     ok_clean_git(ds.path)
     assert_raises(TypeError, ds.containers_add)
     # fails when there is no image
-    assert_status('error', ds.containers_add('name', on_failure='ignore'))
+    assert_status(
+        'error',
+        ds.containers_add('name', on_failure='ignore', **common_kwargs))
     # no config change
     ok_clean_git(ds.path)
     # place a dummy "image" file
     with open(op.join(ds.path, 'dummy'), 'w') as f:
         f.write('some')
-    ds.save('dummy')
+    ds.save('dummy', **common_kwargs)
     ok_clean_git(ds.path)
     # config will be added, as long as there is a file, even when URL access
     # fails
     res = ds.containers_add(
         'broken',
         url='bogus-protocol://bogus-server', image='dummy',
-        on_failure='ignore'
+        on_failure='ignore',
+        **common_kwargs
     )
     assert_status('ok', res)
     assert_result_count(res, 1, action='save', status='ok')
@@ -59,7 +64,7 @@ def test_add_noop(path=None):
 @with_tree(tree={"foo.img": "doesn't matter 0",
                  "bar.img": "doesn't matter 1"})
 def test_add_local_path(path=None, local_file=None):
-    ds = Dataset(path).create()
+    ds = Dataset(path).create(**common_kwargs)
     res = ds.containers_add(name="foobert",
                             url=op.join(local_file, "foo.img"))
     foo_target = op.join(path, ".datalad", "environments", "foobert", "image")
@@ -94,12 +99,12 @@ def test_container_files(ds_path=None, local_file=None, url=None):
     local_file = get_local_file_url(op.join(local_file, 'some_container.img'))
 
     # prepare dataset:
-    ds = Dataset(ds_path).create()
+    ds = Dataset(ds_path).create(**common_kwargs)
     # non-default location:
     ds.config.add("datalad.containers.location",
                   value=op.join(".datalad", "test-environments"),
                   where='dataset')
-    ds.save(message="Configure container mountpoint")
+    ds.save(message="Configure container mountpoint", **common_kwargs)
 
     # no containers yet:
     res = ds.containers_list(**RAW_KWDS)
@@ -108,7 +113,7 @@ def test_container_files(ds_path=None, local_file=None, url=None):
     # add first "image": must end up at the configured default location
     target_path = op.join(
         ds.path, ".datalad", "test-environments", "first", "image")
-    res = ds.containers_add(name="first", url=local_file)
+    res = ds.containers_add(name="first", url=local_file, **common_kwargs)
     ok_clean_git(ds.repo)
 
     assert_result_count(res, 1, status="ok", type="file", path=target_path,
@@ -125,7 +130,7 @@ def test_container_files(ds_path=None, local_file=None, url=None):
     # and kill it again
     # but needs name
     assert_raises(TypeError, ds.containers_remove)
-    res = ds.containers_remove('first', remove_image=True)
+    res = ds.containers_remove('first', remove_image=True, **common_kwargs)
     assert_status('ok', res)
     assert_result_count(ds.containers_list(**RAW_KWDS), 0)
     # image removed
@@ -143,25 +148,28 @@ def test_extra_inputs(ds_path=None):
     overlay2_file  = 'overlay2.img'
 
     # prepare dataset:
-    ds = Dataset(ds_path).create(force=True)
-    ds.save()
+    ds = Dataset(ds_path).create(force=True, **common_kwargs)
+    ds.save(**common_kwargs)
 
     ds.containers_add(
         name="container",
         image=container_file,
         call_fmt="apptainer exec {img} {cmd}",
+        **common_kwargs
     )
     ds.containers_add(
         name="container-with-overlay",
         image=container_file,
         call_fmt="apptainer exec --overlay {img_dirpath}/overlay1.img {img} {cmd}",
-        extra_input=[overlay1_file]
+        extra_input=[overlay1_file],
+        **common_kwargs
     )
     ds.containers_add(
         name="container-with-two-overlays",
         image=container_file,
         call_fmt="apptainer exec --overlay {img_dirpath}/overlay1.img --overlay {img_dirpath}/overlay2.img:ro {img} {cmd}",
-        extra_input=[overlay1_file, overlay2_file]
+        extra_input=[overlay1_file, overlay2_file],
+        **common_kwargs
     )
 
     res = ds.containers_list(**RAW_KWDS)
@@ -181,28 +189,33 @@ def test_container_update(ds_path=None, local_file=None, url=None):
     url_bar = get_local_file_url(op.join(local_file, 'bar.img'))
     img = op.join(".datalad", "environments", "foo", "image")
 
-    ds = Dataset(ds_path).create()
+    ds = Dataset(ds_path).create(**common_kwargs)
 
-    ds.containers_add(name="foo", call_fmt="call-fmt1", url=url_foo)
+    ds.containers_add(name="foo", call_fmt="call-fmt1", url=url_foo,
+                      **common_kwargs)
 
     # Abort without --update flag.
-    res = ds.containers_add(name="foo", on_failure="ignore")
+    res = ds.containers_add(name="foo", on_failure="ignore",
+                            **common_kwargs)
     assert_result_count(res, 1, action="containers_add", status="impossible")
 
     # Abort if nothing to update is specified.
-    res = ds.containers_add(name="foo", update=True, on_failure="ignore")
+    res = ds.containers_add(name="foo", update=True, on_failure="ignore",
+                            **common_kwargs)
     assert_result_count(res, 1, action="containers_add", status="impossible",
                         message="No values to update specified")
 
     # Update call format.
-    ds.containers_add(name="foo", update=True, call_fmt="call-fmt2")
+    ds.containers_add(name="foo", update=True, call_fmt="call-fmt2",
+                      **common_kwargs)
     assert_equal(ds.config.get("datalad.containers.foo.cmdexec"),
                  "call-fmt2")
     ok_file_has_content(op.join(ds.path, img), "foo")
 
     # Update URL/image.
-    ds.drop(img)  # Make sure it works even with absent content.
-    res = ds.containers_add(name="foo", update=True, url=url_bar)
+    ds.drop(img, **common_kwargs)  # Make sure it works even with absent content.
+    res = ds.containers_add(name="foo", update=True, url=url_bar,
+                            **common_kwargs)
     assert_in_results(res, action="remove", status="ok")
     assert_in_results(res, action="save", status="ok")
     ok_file_has_content(op.join(ds.path, img), "bar")
@@ -213,7 +226,8 @@ def test_container_update(ds_path=None, local_file=None, url=None):
     assert_in("Update ", get_commit_msg())
 
     # If we add a new image with update=True should say Configure
-    res = ds.containers_add(name="foo2", update=True, url=url_bar)
+    res = ds.containers_add(name="foo2", update=True, url=url_bar,
+                            **common_kwargs)
     assert_in("Configure ", get_commit_msg())
 
 
@@ -223,16 +237,17 @@ def test_container_update(ds_path=None, local_file=None, url=None):
 def test_container_from_subdataset(ds_path=None, src_subds_path=None, local_file=None):
 
     # prepare a to-be subdataset with a registered container
-    src_subds = Dataset(src_subds_path).create()
-    src_subds.containers_add(name="first",
-                         url=get_local_file_url(op.join(local_file,
-                                                        'some_container.img'))
-                         )
+    src_subds = Dataset(src_subds_path).create(**common_kwargs)
+    src_subds.containers_add(
+        name="first",
+        url=get_local_file_url(op.join(local_file, 'some_container.img')),
+        **common_kwargs
+    )
     # add it as subdataset to a super ds:
-    ds = Dataset(ds_path).create()
-    subds = ds.install("sub", source=src_subds_path)
+    ds = Dataset(ds_path).create(**common_kwargs)
+    subds = ds.install("sub", source=src_subds_path, **common_kwargs)
     # add it again one level down to see actual recursion:
-    subds.install("subsub", source=src_subds_path)
+    subds.install("subsub", source=src_subds_path, **common_kwargs)
 
     # We come up empty without recursive:
     res = ds.containers_list(recursive=False, **RAW_KWDS)
@@ -254,9 +269,9 @@ def test_container_from_subdataset(ds_path=None, src_subds_path=None, local_file
     )
 
     # not installed subdataset doesn't pose an issue:
-    sub2 = ds.create("sub2")
-    assert_result_count(ds.subdatasets(), 2, type="dataset")
-    ds.uninstall("sub2", check=False)
+    sub2 = ds.create("sub2", **common_kwargs)
+    assert_result_count(ds.subdatasets(**common_kwargs), 2, type="dataset")
+    ds.uninstall("sub2", check=False, **common_kwargs)
     from datalad.tests.utils_pytest import assert_false
     assert_false(sub2.is_installed())
 
@@ -285,17 +300,17 @@ def test_container_from_subdataset(ds_path=None, src_subds_path=None, local_file
 
 @with_tempfile
 def test_list_contains(path=None):
-    ds = Dataset(path).create()
-    subds_a = ds.create("a")
-    subds_b = ds.create("b")
-    subds_a_c = subds_a.create("c")
+    ds = Dataset(path).create(**common_kwargs)
+    subds_a = ds.create("a", **common_kwargs)
+    subds_b = ds.create("b", **common_kwargs)
+    subds_a_c = subds_a.create("c", **common_kwargs)
 
     add_pyscript_image(subds_a_c, "in-c", "img")
     add_pyscript_image(subds_a, "in-a", "img")
     add_pyscript_image(subds_b, "in-b", "img")
     add_pyscript_image(ds, "in-top", "img")
 
-    ds.save(recursive=True)
+    ds.save(recursive=True, **common_kwargs)
 
     assert_result_count(ds.containers_list(recursive=True, **RAW_KWDS),
                         4)