Skip to content

Comments

Local Overlay Store#8397

Merged
Ericson2314 merged 175 commits intoNixOS:masterfrom
NixLayeredStore:overlayfs-store
Apr 8, 2024
Merged

Local Overlay Store#8397
Ericson2314 merged 175 commits intoNixOS:masterfrom
NixLayeredStore:overlayfs-store

Conversation

@Ericson2314
Copy link
Member

@Ericson2314 Ericson2314 commented May 25, 2023 •
edited
Loading

Motivation

The motivation for this is given in NixOS/rfcs#152

Context

Implementation notes:

The new store is marked experimental, under a new local-overlay-store experimental feature.

The new store type is a subclass of LocalStore. It lives in a new separate {cc,hh} file pair, so the impact on the rest of the codebase is minimal. This makes it easy to revert if we want to back out of the experiment.

The changes to the rest of the codebase are mainly making to make methods virtual / slightly different so we can override them.

depends on #8774

Checklist for maintainers

Maintainers: tick if completed or explain if not relevant

  • agreed on idea
  • agreed on implementation strategy
  • tests, as appropriate
    • functional tests - tests/**.sh
    • unit tests - src/*/tests
    • integration tests - tests/nixos/*
  • documentation in the manual
  • documentation in the internal API docs
  • code and comments are self-explanatory
  • commit message explains why the change was made
  • new feature or incompatible change: updated release notes

Priorities

Add 👍 to pull requests you find important.

This work is sponsored by Replit ✨

Ericson2314 and others added 30 commits March 21, 2023 10:53
@Ericson2314
Testing overlayfs stores
111aeb9
@Ericson2314
Merge remote-tracking branch 'upstream/master' into overlayfs-store
d62f6da
@Ericson2314
Init local overlay store
f0a176e
@Ericson2314
Merge remote-tracking branch 'upstream/master' into overlayfs-store
d5fe828
@Ericson2314
Progress on tests
f08754a
@Ericson2314
Merge branch 'best-effort-supplementary-groups' into overlayfs-store
9c9f5f0
@Ericson2314
First round of testing, with todos
d80fc2a
@Ericson2314
Improve tests slightly
0193c2a
@Ericson2314
Specialize LocalOverlayStore::registerDrvOutput
31e98ed
@Ericson2314
Specialize LocalOverlayStore::queryPathInfoUncached
5406256
@Ericson2314
Fix LocalOverlayStore::queryPathInfoUncached, FIXME in test
59a8099
@Ericson2314
Convert more methods
b3d320c 
Fixed one test, broke another
@Ericson2314
Specialize more methods, fix tests
ddaf275
@Ericson2314
Specialize LocalOverlayStore::queryPathFromHashPart
e7c3399 
With test
@Ericson2314
Fix recursive ingestion from lower store
5059be5
@Ericson2314
More tests
8339c17
@Ericson2314
Implement more queries
4173743
@benradf
Check that overlay store directory is mounted correctly.
b5591ec 
Nix does not manage the overlayfs mount point itself, but the correct
functioning of the overlay store does depend on this mount point being set up
correctly. Rather than just assume this is the case, check that the lowerdir
and upperdir options are what we expect them to be. This check is on by
default, but can be disabled if needed.
@benradf
Support percent encoded URIs for lower store.
b0989cb
@benradf
Make upper-layer a PathSetting instead of a Setting<string>.
0df37ed
@cidkidnix
Add test for checking that we reject bad local overlay store uris
b7e5aaf
@cidkidnix
Begin to split up overlay-local-store tests
0979a37 
The bad-uris tests are now in their own file.
"Outer" is a bad name, but it will be split up next.
@Ericson2314
Merge remote-tracking branch 'upstream/master' into overlayfs-store
72bb960
@Ericson2314
Fix PS4 for heredocs
b1fba1c
@Ericson2314
Merge branch 'best-effort-supplementary-groups' into overlayfs-store
3496a5e
@Ericson2314
Merge branch 'best-effort-supplementary-groups' into overlayfs-store
1dcb782
@Ericson2314
Create notion of "test group", use for local overlay store
97deb00
@Ericson2314
Split tests some more
5d18120 
Good for parallelism and easier reading.
@Ericson2314
Create local.mk for local-overlay-store tests
0ec7f2f
@benradf
More detailed explanation of check-mount setting.
4d69bd0
@Ericson2314 Ericson2314 marked this pull request as ready for review February 29, 2024 17:09
@Ericson2314
Revert "Merge pull request NixOS#9546 from NixOS/nixos-23.11"
d6d7d2c 
This reverts commit 587c7dc, reversing
changes made to 864fc85.
@Ericson2314
Copy link
Member Author

Ericson2314 commented Feb 29, 2024 •
edited
Loading

Annoying 11th hour CI failure.

I have tried temporarily reverting the Nixpkgs bump just to rule that out.

@Ericson2314
Copy link
Member Author

Ericson2314 commented Feb 29, 2024

That did fix it. Reverting the revert now.

thufschmitt
thufschmitt approved these changes Mar 6, 2024
View reviewed changes
Copy link
Member

@thufschmitt thufschmitt left a comment

Choose a reason for hiding this comment

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

At last found the time for a last review.

That's good, let's go!

@thufschmitt thufschmitt enabled auto-merge March 6, 2024 08:10
@thufschmitt
Copy link
Member

thufschmitt commented Mar 6, 2024

Oh right, forgot about the CI failure :/

@thufschmitt thufschmitt disabled auto-merge March 6, 2024 08:51
thufschmitt
thufschmitt reviewed Mar 6, 2024
View reviewed changes
@benradf
Copy link
Member

benradf commented Mar 6, 2024

It seems to be failing to remount the overlayfs directory for some reason:

    + remountOverlayfs
    + mount -o remount /build/nix-test/local-overlay-store/verify/stores/merged-store/nix/store
    mount: /build/nix-test/local-overlay-store/verify/stores/merged-store/nix/store: mount point not mounted or bad option.
           dmesg(1) may have more information after failed mount system call.

Checking /proc/self/mounts inside the sandbox, that path looks to be a valid mountpoint, so I'm not sure why it's failing.

Did the mount binary we're using change perhaps? E.g. from the util-linux tool to busybox.

@Ericson2314
Copy link
Member Author

Ericson2314 commented Mar 18, 2024

@benradf The nixpkgs bump is what broke it so yes, a changing mount command does seem like a likely culprit.

@Ericson2314 Ericson2314 enabled auto-merge April 8, 2024 02:22
@Ericson2314 Ericson2314 merged commit fef952e into NixOS:master Apr 8, 2024
@nixos-discourse
Copy link

nixos-discourse commented Apr 9, 2024

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/tweag-nix-dev-update-56/43035/1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@edolstra edolstra edolstra left review comments

+1 more reviewer

@thufschmitt thufschmitt thufschmitt approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@edolstra edolstra

Labels

store Issues and pull requests concerning the Nix store with-tests Issues related to testing. PRs with tests have some priority

Projects

Nix team
Archived in project

Milestone

local-overlay-store stabilisation

Development

Successfully merging this pull request may close these issues.

7 participants

@Ericson2314 @nixos-discourse @Fructokinase @thufschmitt @benradf @edolstra @cidkidnix