Skip to content

Do not load config files in re-exec process#6711

Merged
Luap99 merged 3 commits intocontainers:mainfrom
Luap99:init
Mar 10, 2026
Merged

Do not load config files in re-exec process#6711
Luap99 merged 3 commits intocontainers:mainfrom
Luap99:init

Conversation

@Luap99
Copy link
Member

@Luap99 Luap99 commented Mar 9, 2026

What type of PR is this?

/kind api-change
/kind bug
/kind cleanup
/kind deprecation
/kind design
/kind documentation
/kind failing-test
/kind feature
/kind flake
/kind other

What this PR does / why we need it:

It fixes storage.conf parsing errors with my storage.conf rework in #6708

How to verify it

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

None

@dosubot dosubot bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Mar 9, 2026
@Luap99 Luap99 added the No New Tests Allow PR to proceed without adding regression tests label Mar 9, 2026
Copy link
Member

@nalind nalind left a comment

Choose a reason for hiding this comment

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

Got some typos in the commit messages ("cotnainers.conf", "signle", "refernce"), otherwise LGTM.

Luap99 added 3 commits March 9, 2026 18:32
There is one large problem with using init() functions here, they run
every single time the executable is started even in cases where we do
not need the buildah cli. The main point being the reexec logic which
bypasses all of the cli and may be in a totally different rootfs.

Currently the init() function needs containers.conf and storage.conf so
these files gets parsed in wrong contexts doing extra work that is not
required.

For reference I used the following script:
```
for FILE in $(find ./cmd/buildah/ -type f -not -name '*_test.go'); do

BASENAME=$(basename "$FILE")
FILENAME_NO_EXT="${BASENAME%.*}"

sed -i "s/func init() {/func ${FILENAME_NO_EXT}Init() {/g" "$FILE"
echo "${FILENAME_NO_EXT}Init()"

done
```

And then used the output to be added in main() after the reexec call.

Signed-off-by: Paul Holzinger <pholzing@redhat.com>
In order to avoid parsing containers.conf inside the reexec child
process just resolve it before.

Signed-off-by: Paul Holzinger <pholzing@redhat.com>
This is a partial revert of commit 04847f5 ("Set CONTAINERS_CONF in the
chroot-mount-flags integration test").

With the changes from the prior commits we should not longer load
containers.conf in the re-exec processes and as such leaking the env is
no longer needed to work around the test issue.

Signed-off-by: Paul Holzinger <pholzing@redhat.com>
@Luap99
Copy link
Member Author

Luap99 commented Mar 9, 2026

Thanks, fixed.

Copy link
Member

@nalind nalind left a comment

Choose a reason for hiding this comment

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

LGTM

@Luap99
Copy link
Member Author

Luap99 commented Mar 9, 2026

@Honny1 @TomSweeneyRedHat PTAL and merge

Copy link
Member

@Honny1 Honny1 left a comment

Choose a reason for hiding this comment

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

LGTM, from what I can tell.

@Luap99 Luap99 merged commit 5974554 into containers:main Mar 10, 2026
40 checks passed
@Luap99 Luap99 deleted the init branch March 10, 2026 12:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

No New Tests Allow PR to proceed without adding regression tests size:M This PR changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants