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

Jump to bottom

fix handling of static/volume dir #20088

Merged
merged 1 commit into from
Sep 25, 2023
Merged

fix handling of static/volume dir #20088

merged 1 commit into from
Sep 25, 2023

Commits on Sep 25, 2023

  1. fix handling of static/volume dir 

    The processing and setting of the static and volume directories was
scattered across the code base (including c/common) leading to subtle
errors that surfaced in containers#19938.

There were multiple issues that I try to summarize below:

 - c/common loaded the graphroot from c/storage to set the defaults for
   static and volume dir.  That ignored Podman's --root flag and
   surfaced in containers#19938 and other bugs.  c/common does not set the
   defaults anymore which gives Podman the ability to detect when the
   user/admin configured a custom directory (not empty value).

 - When parsing the CLI, Podman (ab)uses containers.conf structures to
   set the defaults but also to override them in case the user specified
   a flag.  The --root flag overrode the static dir which is wrong and
   broke a couple of use cases.  Now there is a dedicated field for in
   the "PodmanConfig" which also includes a containers.conf struct.

 - The defaults for static and volume dir and now being set correctly
   and adhere to --root.

 - The CONTAINERS_CONF_OVERRIDE env variable has not been passed to the
   cleanup process.  I believe that _all_ env variables should be passed
   to conmon to avoid such subtle bugs.

Overall I find that the code and logic is scattered and hard to
understand and follow.  I refrained from larger refactorings as I really
just want to get containers#19938 fixed and then go back to other priorities.

containers/common#1659 broke three pkg/machine
tests.  Those have been commented out until getting fixed.

Fixes: containers#19938
Signed-off-by: Valentin Rothberg <vrothberg@redhat.com>
    @vrothberg
    vrothberg committed Sep 25, 2023
    Configuration menu
    Copy the full SHA
    6293ec2 View commit details
    Browse the repository at this point in the history