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

nixos/systemd: Enable systemd-machine-id-commit.service #351151

Merged
merged 1 commit into from
Oct 26, 2024

Conversation

frederictobiasc
Copy link
Contributor

@frederictobiasc frederictobiasc commented Oct 25, 2024

Prior to this contribution, every boot with a default configuration was considered ConditionFirstBoot=true by systemd, since /etc/machine-id was not commited to disk.

This also extends the systemd with a check for subsequent boots not being considered first boots.

This is a follow-up on #327552

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` labels Oct 25, 2024
@frederictobiasc frederictobiasc force-pushed the systemd-fix-firstboot branch 4 times, most recently from 71c7862 to 3024567 Compare October 25, 2024 12:01
Prior to this contribution, every boot with a default configuration was
considered `ConditionFirstBoot=true` by systemd, since /etc/machine-id
was not commited to disk.

This also extends the systemd with a check for subsequent boots not
being considered first boots.
@arianvp
Copy link
Member

arianvp commented Oct 25, 2024

Oh wow oops. Thanks for catching this

@wegank wegank added the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Oct 26, 2024
@emilazy emilazy merged commit 7f753fb into NixOS:master Oct 26, 2024
33 checks passed
@NickCao
Copy link
Member

NickCao commented Oct 27, 2024

This does not work well with https://github.com/nix-community/impermanence (or more precisely, the other way around), since when /etc/machine-id is persisted by impermanence, it's created as a bind mount, unintentionally triggering systemd-machine-id-commit.service. Some workaround should be implemented on impermanence side.

@mjm
Copy link
Contributor

mjm commented Oct 27, 2024

I've added these lines to my config for hosts where I have /etc/machine-id as a bind mount to persistent storage:

      boot.initrd.systemd.suppressedUnits = [ "systemd-machine-id-commit.service" ];
      systemd.suppressedSystemUnits = [ "systemd-machine-id-commit.service" ];

I'm not sure if both are necessary.

@arianvp
Copy link
Member

arianvp commented Oct 27, 2024

I think maybe the original PR shouldn't have removed the

: >> /etc/machine-id

line but instead it should've done something like:

[ -e /etc/machine-id ] || echo "uninitialized\n" >> /etc/machine-id

I think then all use-cases work?

@WilliButz
Copy link
Member

I looked into this for preservation and think that a good approach would be to create /etc/machine-id when it does not exist and populate it with uninitialized\n as @arianvp suggests.
To properly have the commit service work with the machine-id persisted under a non-standard prefix the service needs to be slightly adapted, otherwise it will fail - either silently or with an error. depending on the exact setup.

for preservation, I came up with this. I'll try to push that soon together with some other changes:

boot.initrd.systemd.tmpfiles.settings.preservation."/sysroot/state/etc/machine-id".f = {
  user = "root";
  group = "root";
  mode = ":0644";
  argument = "uninitialized\\n";
};

systemd.services.systemd-machine-id-commit = {
  unitConfig.ConditionPathIsMountPoint = [
    ""
    "/state/etc/machine-id"
  ];
  serviceConfig.ExecStart = [
    ""
    "systemd-machine-id-setup --commit --root /state"
  ];
};

@WilliButz
Copy link
Member

I forgot to add, this works with /etc/machine-id being a symlink to /state/etc/machine-id. It does not work with a bind-mounted file

@frederictobiasc
Copy link
Contributor Author

frederictobiasc commented Oct 28, 2024

[..] a good approach would be to create /etc/machine-id when it does not exist and populate it with uninitialized\n as @arianvp suggests. To properly have the commit service work with the machine-id persisted under a non-standard prefix the service needs to be slightly adapted, otherwise it will fail - either silently or with an error. depending on the exact setup.

Is the systemd-machine-id-commit.service needed at all when doing that?

@WilliButz
Copy link
Member

WilliButz commented Oct 28, 2024

Is the systemd-machine-id-commit.service needed at all when doing that?

As I understand it, yes. On first boot, /state/etc/machine-id contains uninitialized\n and a symlink /etc/machine-id -> /state/etc/machine-id is created in stage 1. After switch root the machine-id is populated in the tmpfs mount (I think coming from the -setup service edit: something else is creating the tmpfs) and then the commit service is needed to persist that to the underlying fs.

@WilliButz
Copy link
Member

WilliButz commented Oct 28, 2024

But I don't think that needs to be handled in nixpkgs. It should be handled by whatever configuration causes the machine-id file to not be persisted under /etc/machine-id directly, in my case preservation.

@arianvp
Copy link
Member

arianvp commented Oct 28, 2024

We should add the uninitialized\n thing to nixpkgs though

aftix added a commit to aftix/cfg that referenced this pull request Nov 1, 2024
Nixpkgs recently merged NixOS/nixpkgs#351151
which enables systemd-machine-id-commit.service. This service fails
if /etc/machine-id is a bind mount, as it is with the impermenance module,
so I've changed the persistence for it to use a systemd tmpfiles symlink instead.
Enkaiyuegure added a commit to Enkaiyuegure/flakes that referenced this pull request Nov 2, 2024
…ce trigger

The bind mount creation for `/etc/machine-id` by `impermanence` is unintentionally triggering `systemd-machine-id-commit.service`, causing an error.
This commit resolves this issue, see: nix-community/impermanence#229.
And NixOS/nixpkgs#351151 for details.
Enkaiyuegure added a commit to Enkaiyuegure/flakes that referenced this pull request Nov 2, 2024
…emd-machine-id-commit.service

The bind mount creation for `/etc/machine-id` by `impermanence` is unintentionally triggering `systemd-machine-id-commit.service`, causing an error.
This commit resolves this issue, see: nix-community/impermanence#229
and NixOS/nixpkgs#351151 for details.
Enkaiyuegure added a commit to Enkaiyuegure/flakes that referenced this pull request Nov 4, 2024
…emd-machine-id-commit.service

The bind mount creation for `/etc/machine-id` by `impermanence` is unintentionally triggering `systemd-machine-id-commit.service`, causing an error.
This commit resolves this issue, see: nix-community/impermanence#229
and NixOS/nixpkgs#351151 for details.
@frederictobiasc
Copy link
Contributor Author

frederictobiasc commented Nov 8, 2024

We should add the uninitialized\n thing to nixpkgs though

Would you mind explaining the intention of that? I don't understand how this helps in any of the reported issues in the PR.

As I understand, impermanence creates a bindmount for an already persisted /etc/machine-id triggering systemd-machine-id-commit.service That won't change.

dguibert added a commit to dguibert/nix-config that referenced this pull request Dec 16, 2024
@WilliButz
Copy link
Member

Would you mind explaining the intention of that? I don't understand how this helps in any of the reported issues in the PR.

I believe there was some confusion about this, definitely on my end. Right now in nixpkgs we should be fine without any changes, there are no problems and a missing machine-id file is semantically equivalent to one that contains the string uninitialized (the documented newline doesn't matter for the check).

For those interested in preserving the machine-id at another location than /etc/machine-id there are different approaches that seem to work:

  • bind-mount an empty file from a desired location to /etc/machine-id but suppress the systemd-machine-id-commit.service (note: doing this in boot.initrd.systemd is not necessary as this service runs after switch-root)
  • bind-mount a file as above but with the content uninitialized from a desired location to /etc/machine-id to make use of first boot semantics
  • create a symlink at /etc/machine-id targeting an etc directory at a desired prefix and patch the systemd-commit-machine-id.service to operate on the target prefix. This is a bit more complicated than suppressing the unit but it both adheres to firstboot semantics and avoids having a bind-mount targeting the lowerdir of an overlayfs when working with confext images.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 12.approvals: 1 This PR was reviewed and approved by one reputable person
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants