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

locatedb: fix startup fail due to systemd path capabilities #30312

Merged
merged 2 commits into from Oct 11, 2017
Merged

locatedb: fix startup fail due to systemd path capabilities #30312

merged 2 commits into from Oct 11, 2017

Conversation

florianjacob
Copy link
Contributor

Motivation for this change

On fresh NixOS 17.09 installs, update-locatedb.service continuously fails with:

update-locatedb.service: Failed at step NAMESPACE spawning /nix/store/bad6dlkkpkbk6qjdw8qhfbr1710bl0f9-unit-script/bin/update-locatedb-start: No such file or directory

This is because of the ReadWritePaths=/var/cache namespace restriction, as on new installs, /var/cache does not exist and therefore results in the no such file or directory. To reproduce this, delete your /var/cache directory.

I try to fix this by pulling out the mkdir part in a separate service. Note that this still fails on the first invocation with the same error, but creates the directory and then works on all subsequent invocations. I don't know why that happens. Maybe because the namespace restrictions are evaluated instantly when the unit is started, or because I did something wrong with the depency specification?

In any case, working the second day after being enabled is an improvement over always failing until the user creates /var/cache manually.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

@florianjacob florianjacob mentioned this pull request Oct 11, 2017
Closed
Mic92
Mic92 reviewed Oct 11, 2017
View reviewed changes
nixos/modules/misc/locate.nix Outdated
mkdir -m 0755 -p ${dirOf cfg.output}
'';
requiredBy = [ "update-locatedb.service" ];
before = [ "update-locatedb.service" ];
Copy link
Member

Choose a reason for hiding this comment

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

I would probably make this of type oneshot and RemainAfterExit=true.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh I forgot. This was probably causing the first invocation problems.

Mic92
Mic92 reviewed Oct 11, 2017
View reviewed changes
nixos/modules/misc/locate.nix Outdated
{ description = "Create locatedb state on first run";
script = ''
mkdir -m 0755 -p ${dirOf cfg.output}
'';
Copy link
Member

Choose a reason for hiding this comment

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

The following should save building a dedicated script derivation:

serviceConfig.ExecStart = "${pkgs.coreutils}/bin/mkdir -m 0755 -p ${dirOf cfg.output}"

Mic92
Mic92 reviewed Oct 11, 2017
View reviewed changes
nixos/modules/misc/locate.nix Outdated
serviceConfig.ReadWriteDirectories = dirOf cfg.output;
serviceConfig.ReadOnlyPaths = "/";
# TODO: one could try to further reduce this to cfg.output by pre-creating the file in locatedb-setup
serviceConfig.ReadWritePaths = dirOf cfg.output;
Copy link
Member

Choose a reason for hiding this comment

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

ConditionPathExists= might also lead to better error messages.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just tested it, sadly neither ConditionPathExists nor AssertPathExists seem to have any effect, it's always just the NAMESPACE error. 😞

@Mic92
Copy link
Member

Mic92 commented Oct 11, 2017 •
edited

I wonder, if we just could use systemd-tmpfiles instead of creating a dedicated service.

@Mic92 Mic92 added the 9.needs: port to stable A PR needs a backport to the stable release. label Oct 11, 2017
@florianjacob
nixos/locatedb: fix first run when /var/cache doesn't exist
70c3f56 
by using systemd-tmpfiles.
Also document what's happening there.
@florianjacob florianjacob force-pushed the locatedb-fix-systemd-path-capabilities branch from 7372279 to 70c3f56 Compare October 11, 2017 12:59
@florianjacob
Copy link
Contributor Author

Forgot about systemd-tmpfiles, turned out to be much simpler, I could remove the extra unit again and the first invocation problem vanished as well, of course.

@Mic92 thanks alot for the hint, just updated the PR.

@Mic92 Mic92 merged commit 659c748 into NixOS:master Oct 11, 2017
@Mic92
Copy link
Member

Mic92 commented Oct 11, 2017

also in 17.09 now.

@florianjacob florianjacob deleted the locatedb-fix-systemd-path-capabilities branch October 11, 2017 15:21
@samueldr samueldr removed the 9.needs: port to stable A PR needs a backport to the stable release. label Apr 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Mic92 Mic92 Mic92 left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@florianjacob @Mic92 @samueldr