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.

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

stages(machine-id): add a new "machine-id" stage #1452

Merged
merged 1 commit into from
Nov 30, 2023

Conversation

mvo5
Copy link
Contributor

@mvo5 mvo5 commented Nov 15, 2023

This is a draft for a new machien-id stage. it would allow us to create image that are in the sytemd "ConditionFirstBoot" state.

Draft for now to ensure it follows the vision/ideas of Tom and the team.

See also #960 for more context (and the commit messages here).

@mvo5 mvo5 requested a review from teg November 15, 2023 18:05
stages/org.osbuild.machine-id Outdated Show resolved Hide resolved
@teg
Copy link
Member

teg commented Nov 28, 2023

Makes sense to me! The question of whether or not to enable FirstBoot mode, and when is correctly deferred to osbuild/images, so I see no reason not to land this.

teg
teg previously approved these changes Nov 28, 2023
@mvo5 mvo5 marked this pull request as ready for review November 28, 2023 12:59
stages/org.osbuild.machine-id Show resolved Hide resolved
stages/org.osbuild.machine-id Show resolved Hide resolved
stages/org.osbuild.machine-id Outdated Show resolved Hide resolved
stages/test/test_machine-id.py Outdated Show resolved Hide resolved
stages/test/test_machine-id.py Outdated Show resolved Hide resolved
stages/test/test_machine-id.py Outdated Show resolved Hide resolved
@mvo5 mvo5 force-pushed the stages/machine-id branch 4 times, most recently from a395d7f to d13f1d5 Compare November 28, 2023 20:40
test/run/test_stages.py Outdated Show resolved Hide resolved
@mvo5 mvo5 force-pushed the stages/machine-id branch 2 times, most recently from ed94a89 to 54dbc12 Compare November 29, 2023 09:02
supakeen
supakeen previously approved these changes Nov 29, 2023
Copy link
Member

@supakeen supakeen left a comment

Choose a reason for hiding this comment

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

Nice.

This is a variation of PR osbuild#960
that put the machine-id handling into it's own stage and adds
explicit handling what should happen with it.

For machine-id(5) we essentially want the following three states
implemented:

1. `first-boot: yes` will ensure that /etc/machine-id is
   in the "uninitialized" state. This means on boot the systemd
   `ConditionFirstBoot` is triggered and a new id in `/etc/machine-id`
   is created. This will work for systemd v247+.
2. `first-boot: no` will ensure that /etc/machine-id exists but
   is empty. This will trigger the creation of a new machine-id but
   will *not* trigger `ConditionFirstBoot`.
3. `first-boot: preserve` will just keep the existing machine-id.
   Note that it will error if there is no /etc/machine-id

Note that the `org.osbuild.rpm` will also create a
`{tree}/etc/machine-id` while it runs to ensure that postinst
scripts will not fail that rely on this file. This is an
implementation detail but unfortunately the rpm stage will
leave an empty machine-id file if it was missing. So we cannot
just remove /etc/machine-id because any following rpm stage
would re-create it again (and we cannot change that without
breaking backward compatiblity). Thanks to the special semantic
that a missing /etc/machine-id and an /etc/machine-id with
the `uninitialized` string are equivalent we don't care.

To support systemd versions below v247 we could offer an option
to remove /etc/machine-id. But the downside of this is that
it would only work if the org.osbuild.machine-id stage is after
the rpm stage.

See also the discussion in PR#960.

Thanks to Tom, Christian for the PR and the background.
@mvo5
Copy link
Contributor Author

mvo5 commented Nov 29, 2023

I had to remove the integration test for the feature from this commit because I need to figure out how to run systemd-nspawn inside our test container first :-/ I put it into https://github.com/osbuild/osbuild/compare/main...mvo5:stages/machine-id-integration-test?expand=1 for now and will get back to it. But this should be unblocked now.

Copy link
Member

@achilleas-k achilleas-k left a comment

Choose a reason for hiding this comment

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

I think this is the perfect solution to our old problem. Like Tom said, it puts the decision in the hands of the manifest generator, which should know when the stage is needed and if an option is unsupported (based on distro/systemd version).

I'm looking forward to writing proper first-boot stages with ConditionFirstBoot in our image definitions with this now instead of fiddling with our own marker files.

@achilleas-k achilleas-k merged commit 0a2e0bb into osbuild:main Nov 30, 2023
75 checks passed
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.

None yet

4 participants