Skip to content

cmd/initContainer: Simplify code by removing a function parameter#1356

Merged
debarshiray merged 2 commits intocontainers:mainfrom
debarshiray:wip/rishi/cmd-initContainer-configureUsers-remove-parameter
Aug 22, 2023
Merged

cmd/initContainer: Simplify code by removing a function parameter#1356
debarshiray merged 2 commits intocontainers:mainfrom
debarshiray:wip/rishi/cmd-initContainer-configureUsers-remove-parameter

Conversation

@debarshiray
Copy link
Copy Markdown
Member

Until now, configureUsers() was pushing the burden of deciding whether
to add a new user or modify an existing one on the callers, even though
it can trivially decide itself. Involving the caller loosens the
encapsulation of the user configuration logic by spreading it across
configureUsers() and it's caller, and adds an extra function parameter
that needs to be carefully set and is vulnerable to programmer errors.

Fallout from 9ea6fe5

On new builds of GNOME OS [1], the host's / is mounted with 'nodev,...'
and those flags are also inherited by /etc because it's not a separate
mount point.  This leads to the same problem with /etc/machine-id that
was seen before with /var/lib/flatpak, /var/lib/systemd/coredump and
/var/log/journal [2].

Therefore, use the same approach [2] to handle /etc/machine-id.

[1] https://gitlab.gnome.org/GNOME/gnome-build-meta/-/issues/718

[2] Commit 1cc9e07
    containers@1cc9e07b7c36fe9f
    containers#1340

containers#911
containers#1354

Signed-off-by: Jordan Petridis <jordan@centricular.com>
Until now, configureUsers() was pushing the burden of deciding whether
to add a new user or modify an existing one on the callers, even though
it can trivially decide itself.  Involving the caller loosens the
encapsulation of the user configuration logic by spreading it across
configureUsers() and it's caller, and adds an extra function parameter
that needs to be carefully set and is vulnerable to programmer errors.

Fallout from 9ea6fe5

containers#1356
@debarshiray debarshiray force-pushed the wip/rishi/cmd-initContainer-configureUsers-remove-parameter branch from 3bb24d4 to 6bd7c87 Compare August 22, 2023 20:47
@softwarefactory-project-zuul
Copy link
Copy Markdown

Build failed.
https://softwarefactory-project.io/zuul/t/local/buildset/c21d447336ed4f73a407ce2b096dc007

unit-test RETRY_LIMIT in 33s
✔️ unit-test-migration-path-for-coreos-toolbox SUCCESS in 3m 03s
unit-test-restricted RETRY_LIMIT in 33s
system-test-fedora-rawhide RETRY_LIMIT in 34s
✔️ system-test-fedora-38 SUCCESS in 30m 26s
✔️ system-test-fedora-37 SUCCESS in 28m 37s

@debarshiray
Copy link
Copy Markdown
Member Author

debarshiray commented Aug 22, 2023

The tests run on Fedora Rawhide nodes are failing because of the same reasons as in #1344 and #1331 , and the root cause appears to be rsync: https://bugzilla.redhat.com/show_bug.cgi?id=2229654

So, I am going to temporarily ignore these test failures on Fedora Rawhide.

@debarshiray debarshiray merged commit 6bd7c87 into containers:main Aug 22, 2023
@debarshiray debarshiray deleted the wip/rishi/cmd-initContainer-configureUsers-remove-parameter branch August 22, 2023 22:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants