But the migration would fail. Would that failure reach the user in a sensible fashion?
Automated migrations are almost always fraught with ugly potentialities… and that’s one of the reasons we have stateVersion.
Well, that’s a completely different point. I was personally only commenting on the “mv is not atomic” comment, not the issue of whether migrations as a whole are feasible. But, IMO, without going into detail, failures in edge cases like that IMO are acceptable, and typically it’s reasonable to make the failure pretty obvious. Like in this case the migration would probably simply happen if it can easily, or fail the service if it can’t. Obviously ExecStartPre=-mv ... doesn’t fail the service, but again I think this example was just off the cuff and not what would be used in reality.
1 Like
But even in such a simple case, we’ve already discussed a number of naive implementations that wouldn’t have the right characteristics. Could you get those things right if you were writing the module? Probably. But how many of the people writing and reviewing module changes are as good at this as you or me? Is “probably” really good enough considering the potential of catastrophic data loss? (Yes, everyone should have backups. Lots of people don’t. “NixOS ate my data” is a really bad thing for people to be legitimately saying, regardless.)
If it can only be done robustly by super-programmers, it’s not a reasonable thing for nixpkgs to force module writers to do. Migrations are just inherently risky in this sense, since bugs in the process fundamentally can irreversibly destroy things far more easily than almost anything else we might write.
3 Likes
Idk mv /var/lib/{jellyseerr,seer} in ExecPreStart for everyone seems like a bad idea in general. Sometimes after upgrading you notice that enough stuff is broken that you have to rollback to the previous build but now you can’t do that easily because some stateful scripts messed with the system. Just tell the users to set a homeDir in the config when you asked them to rename the service config. Maybe even make the option required for a release cycle.
5 Likes
Maybe even a switch-time sanity check that refuses to switch if a jellyseer directory is found, plus a break-the-glass option? I do agree that there are probably less risky ways to handle these types of state changes.
This all seems a bit off-topic, though, this is a specific edge case. Better to discuss in a PR that tries to blanket-undo these types of uses of system.stateVersion, or at least this specific one?
This is why I like @ElvishJerricco 's initial suggestion, though. It’s a quick win in terms of useability, and clearly marks services which might need some thinking to be able to deprecate system.stateVersion.
Maybe it just causes a rush of people thinking about how to avoid adding these extra options to various modules to prevent that change, but that’d be ideal.
1 Like
stateVersion or a moral equivalent is clearly necessary as long as there is on-disk state to manage; automagic migration is clearly a bad idea to even attempt outside totally trivial cases.
instead of deprecating the mechanism I’d prefer for there to be a reliable way to query “which modules are going to be affected if I change stateVersion to be x.y" (but I’ve no idea how to do that, of course).
2 Likes
To be clear, my feeling on that idea is that it’s useful when stateVersion as a concept is actually necessary. But I think @emily has done a good job explaining that the real-world cases in nixpkgs where it’s necessary are very few (possibly zero), and that the alternatives (at least in most cases) are comparably difficult to implement.
3 Likes
This thread has meandered a lot, but this is one of the problems the top-level proposal directly addresses, specifically with the (computed, not user-written) system.moduleStateVersions option. The idea is that you will be able to compare the value of that option before and after changing system.stateVersion in order to see which modules are affected, and then you will have the option of migrating the affected modules incrementally by setting each individual module stateVersion instead of having to do them all at once by actually committing the system.stateVersion change.
(A relative disadvantage is that, while the benefits of being able to migrate individual modules will be available to those modules as they incrementally adopt this pattern, the benefit of being able to assume system.moduleStateVersions is comprehensive would only be realized if every use of system.stateVersion goes through this pattern, which will take longer if only because a larger proportion of high-influence committers will need to be convinced in order to make this a project-wide requirement.)
got me. I was making to many assumptions that come from my own systems.
But with the big if that it cannot be cross moinpoints it just feels to risky to generally do and I would not like it in review.
7 Likes
Could this maybe be useful to figure out what would change? Diffing NixOS configurations at the config-level
In the case of the mv discussion, do we even want a default for such directories? I’m an impermanence user, so I’m used to being hyper aware of the directories each module uses. I would prefer simply having a mandatory “state directory” module option and letting the user set that to whatever (of course, there might be more to the seer situation that I’m missing, in which case feel free to ignore this).
This (as well as the aforementioned “mandatory package option pattern”) essentially makes the state explicit in the user’s config. This is, in my opinion, the better solution (in most cases) over trying to automatically handle it for the user.
3 Likes
You’re going to end up with a bunch of users setting this to random directories because they don’t understand the purpose of /var/lib. There are security implications to this, users might not choose directories with good permissions.
Personally, I think a policy of “simply don’t change default directories” makes more sense. There’s no real reason for it to be renamed, why break backwards compatibility at all? If a user wants to rename it they can choose to do so any time.
This doesn’t solve all possible cases, of course, but I think the number of required breaking changes to state could be <5 if we tried to avoid them, and the remaining small handful could be handled with release notes and manually-run migration scripts.
Regardless, @rhendric 's PR is the way to get there anyway. Without it the real impact/need is a little unknowable.
2 Likes
Isn’t the right solution there to add a validation script that crashes the service in such cases? The problem you’re pointing out could be argued to already exist for any service that allows configuring the state directory.
Sure, but right now users just never need to consider this, sidestepping the whole issue. You have to go out of your way to break the sane defaults. Making the option mandatory would mean there are no sane defaults; it even removes the (presumably) sane defaults set by upstream projects by making options mandatory that are usually not mandatory in the upstream configuration.
I think exposing backwards-compatibility routines imposed by upstreams through package.version makes sense, but we shouldn’t add additional complexity where it isn’t needed.
1 Like
systemd already accounts for the need for setup tasks to require elevated privileges; ExecStartPre=+… will run the given command as root. So this shouldn’t be an issue; something like ExecStartPre=+-mv -T /var/lib/private/old /var/lib/private/new.
I expect you probably can do the symlink, given the above 
In general, I would say that rollbacks are never guaranteed to work perfectly when it comes to mutable state; lots of programs change their state format over time and do automatic migrations to handle it. They’re useful for rolling back manual configuration changes, or at a pinch downgrading a short distance back when something goes really wrong, but in general it’s not something we can ensure always works perfectly to restore the old system state without any manual intervention; your software necessarily changes its state, and so rolling back to old versions will not necessarily give you the same state you had before.
But here are some other alternative things you could do to ease the migration here in case you don’t like the symlink, that are nicer than the ExecStopPost=:
-
Do a two‐stage migration: for one release cycle, add
ExecStartPre=+-mv -T /var/lib/private/new /var/lib/private/oldto prepare for rollbacks. Then swap it toExecStartPre=+-mv -T /var/lib/private/old /var/lib/private/new. This will handle migrations across the divide seamlessly (and rolling back several releases is definitely not something you can expect to work reliably anyway). It takes longer to roll out fully, of course. For the existing module, this could be accomplished by conditionalizing such migration logic onstateVersionfor now, and then eliminating that conditional once a release has passed. -
Have a
stateDiroption that needs to be explicitly set, in the same way thatpackageoptions with state incompatibility ought to be handled. This is probably not ideal for every service that has state, but some modules with relatively complex data storage needs have explicit configuration for this anyway. -
Just don’t rename the state directory; it’s largely cosmetic, and changing it could break other references that people have to it. If it really bothers someone, they can override it in the unit definition. I suspect that this is how this is generally handled, which is why the
seerrmodule is doing something relatively unusual here. -
Teach
switch-to-configurationto handle migration ofStateDirectory=names – seamlessly migrating the system state from one generation to another is its job, after all. I don’t propose this as a serious short‐term option, and it has some limitations (you need to handle the migration when booting a different generation too), but if we want changingStateDirectory=to be supported practice, then treating it as part of the service configurations to be switched between is the way to go in the long run. (Probably as part of convincing systemd to do more of theswitch-to-configurationjob for us, since it knows more than us about the relevant pieces of state here, and upstreaming that annoying custom logic is a long‐term desire people have.)
Hopefully at least one of these is inspiring enough to restore your faith in killing stateVersion entirely 
(If we had editions/actual migration tooling, then it would be simple to handle: we switch the default, and people migrating from an earlier edition get systemd.services.the-service.serviceConfig.StateDirectory = "/var/lib/old"; set explicitly in their configuration. But as I said, I don’t think we need this to handle the existing stateVersion uses, and don’t propose anyone block on it.)
(As pointed out, that’s just not true. Renaming is one of the few things Unix can do atomically. The rest of the discussion about mount points doesn’t seem super relevant, since StateDirectory= is already managed automatically by systemd.)
6 Likes
I tried this; it works without DynamicUser, but with it, even with root permissions, I couldn’t get it to work. Manipulating neither /var/lib nor /var/lib/private seems to be possible in that case.
And then, even without DynamicUser, I could not in fact get the symlink trick to help with rollbacks.
Here's my test harness; feel free to play with it yourself.
{
nixpkgs ? <nixpkgs>,
dynamicUser ? true,
}:
(import nixpkgs { overlays = [ ]; }).testers.nixosTest {
name = "migration-test";
nodes.machine =
{ lib, pkgs, ... }:
{
specialisation = lib.pipe 4 [
(builtins.genList (testPhase: {
name = "phase${toString testPhase}";
value.configuration.systemd.services.sut = {
serviceConfig = {
Type = "oneshot";
RemainAfterExit = true;
StateDirectory = if testPhase == 0 then "old" else "new";
DynamicUser = dynamicUser;
# Here is the migration logic; good luck making it work!
${if testPhase == 0 || testPhase == 3 then null else "ExecStartPre"} =
let
s = if dynamicUser then "%S/private" else "%S";
in
if testPhase == 1 then
[
"+-${lib.getExe' pkgs.coreutils "mv"} -T ${s}/old ${s}/new"
"+-${lib.getExe' pkgs.coreutils "ln"} -s ${s}/new ${s}/old"
]
else
[
"+-${lib.getExe' pkgs.coreutils "rm"} -rf ${s}/old"
];
};
script = ''
state=''${STATE_DIRECTORY##*:}
if [[ -e "$state/previous-version" ]]; then
read v < "$state/previous-version"
else
v=none
fi
echo "$v"
echo "${toString testPhase}" > "$state/previous-version"
'';
};
}))
builtins.listToAttrs
];
};
testScript =
{ nodes, ... }:
''
def check_output(expected):
machine.wait_for_unit("sut.service")
iid = machine.systemctl("show -p InvocationID --value sut.service")[1]
actual = machine.succeed(f"journalctl -u sut.service _SYSTEMD_INVOCATION_ID={iid}")
actual = actual.rsplit(' ', 1)[1]
expected = f"{expected}\n"
assert actual == expected, f"{actual!r} != {expected!r}"
def test_migration(p0, p1):
machine.succeed(f"${nodes.machine.system.build.toplevel}/specialisation/phase{p1}/bin/switch-to-configuration test")
check_output(p0)
machine.systemctl("restart sut.service")
check_output(p1)
# Comment out the below if you want to ignore reverse-migrating on
# rollbacks, but ideally they should work as well.
machine.succeed(f"${nodes.machine.system.build.toplevel}/specialisation/phase{p0}/bin/switch-to-configuration test")
check_output(p1)
machine.succeed(f"${nodes.machine.system.build.toplevel}/specialisation/phase{p1}/bin/switch-to-configuration test")
check_output(p0)
machine.start()
machine.succeed("${nodes.machine.system.build.toplevel}/specialisation/phase0/bin/switch-to-configuration test")
machine.systemctl("start sut.service")
check_output("none")
test_migration(0, 1)
test_migration(1, 2)
test_migration(2, 3)
'';
}
Granted, but if we are going to implement automated migrations, I don’t think we should knowingly introduce rollback footguns that can go off without the user doing anything other than a version update. The fact that not all of our packaged software manages forward compatibility correctly doesn’t excuse us from doing it in NixOS modules!
Perhaps that’s too strong. Maybe in the fullness of time, after everyone’s had a chance to consider all of the angles, we ultimately decide that a few more footguns are an acceptable price to pay to eradicate stateVersion entirely. That’s a plausible trade-off to make, even if I’m not yet convinced that it’s the best trade-off.
The point I want to make is that discussing user-facing trade-offs with all of the various stakeholders will necessarily take far more time than implementing a plan that, from the user’s perspective, strictly dominates the status quo. It’s time-consuming enough for me to be convincing you that this incremental improvement is something you should cosign; but if I succeed, what remains is work that can’t impact users in any negative way, because it only adds documentation and optional expressiveness and changes no default behavior. Consequently, if maintainers are a little unresponsive, then I’ll have fewer qualms about self-merging and asking for forgiveness if they later object.
I’m afraid I balk at the idea of undertaking the project of convincing, individually, the maintainers of several dozen modules that their users should accept some loss of functionality (be it related to the usability of rollbacks or anything else) in order to gain something else. If that’s something the core team wants to do, then good luck to you; but I’m not signing up for that! And it’d be great folly for me to assume that I can proceed with trade-offs like that, on behalf of users of modules many of which I myself have never used, without the module maintainers’ approval.
So I’m asking that you show me that there’s a migration strategy that dominates the status quo the same way that my proposed plan does. Breaking rollbacks without user awareness is a flaw not present when requiring manual migrations. Other flaws relative to the status quo include:
- waiting an extra six months before adapting to changes in services
- requiring users to do mindless busy-work when adopting a module for the first time
- ignoring changes in services, which will unnecessarily add to the dissonance between how things work on NixOS and on other distros
I guess this is its own thing. I don’t know how much of the problem space would be covered by specifically addressing StateDirectory, but as you said, it’s not a serious short-term option, and doesn’t really help with deciding whether it’s okay to do something less ambitious in the short term.
Again: my point is not that I can’t imagine that any of these costs would be worth paying in order to drop stateVersion from a given module or from all modules. My point is that I don’t have the energy to have that debate thirty times, even if I end up convinced that we really would be better off without stateVersion at all.
If all the other strategies we can come up with are trade-offs of various values against the status quo instead of dominating the status quo, then I’m asking you to let me make things marginally better on a realistic timeline instead of letting perfect be the enemy of good!
9 Likes
There should be some indication on the file-system to indicate the version, so packages can run their migrations on activation…
Let’s try both: incremental split of stateVersion, and also a migration mitigation (my pref is to teach switch-to-configuration to handle migration of StateDirectory= names).
I don’t see that these are necessarily incompatible, and may even be complimentary to each other.
The issue is that migration mitigation requires policy discussion and ecosystem-wide agreement. We’ve outlined quite a few strategies for dealing with specific migration scenarios.
The hard work is to put all of that in an RFC, agree on it, implement helpers, modify all nixpkgs modules to follow them, and then enforce them.
If you’re interested, I’d suggest starting that work, probably with a separate “pre-RFC” topic experimenting with the various strategies in real nixpkgs use cases.
I don’t think there’s much point in continuing the discussion without some active work; we’re squarely at the “ok, let’s do it” stage.
12 Likes