nixos/auto-upgrade: add persistent option #107957

tex · 2020-12-30T11:06:24Z

Motivation for this change

I am complaining about auto-upgrade not correctly working in laptop environment where laptop is turned on and off unpredictable causing those timers never expire and this service never executed.

Here is my fix.

I think that current situation is making this service unusable on laptops. Since it is auto upgrade that could lead to security issues as user might expect getting automatic updates while he is really not.

One can add this to his configuration.nix: systemd.timers.auto-upgrade.timerConfig.Persistent = true; but it is not apparent for users...

#15689
#75861

#107805

Things done

bbjubjub2494

code LGTM. Tested in the field, all my hosts updated.

Commit message and CONTRIBUTING.md, not to excessively nitpick, but this should be "nixos/auto-upgrade: add persistent option", otherwise it doesn't say what we're adding the option to. See most other commits on services for reference

tex · 2020-12-30T11:40:49Z

Oh, sure. Updated. Thanks!

thiagokokada · 2020-12-30T12:29:27Z

nixos/modules/tasks/auto-upgrade.nix

        type = types.str;
+        default = "daily";


Why change the old default?

But if we do need to change it anyway, I think this may needs some documentation in the changelog for the next release 🤔 ?

Because of security. Currently if you just enable auto-upgrade it will never run on laptop (unless you keep it running over night, but who does it with laptops?). So user might expect to get security fixes, but he doesn't.

I thought just setting persistent = true fixed this issue. Does it not?

Well, I read systemd manual, but I am not sure if persistent applies to a specific date/time. It looked it applies only to intervals, but I might be wrong. Do you know it better?

Can confirm it does apply to specific date/times based on a month of using it in my config.

Ok, I'll revert to original specific date/times. One last question, if set to 4:40, that is very early morning, does it mean that for laptop it always tries to run it as soon as it finishes booting? Would like to know how it behaves with daily. I'd like to avoid every such timer expiring right after boots finishes... Would be better to spread it out more... But eh, probably just trying to find good spot where there are none, right?

I think the manpage give us the answer:

Persistent=

Takes a boolean argument. If true, the time when the service unit was last triggered is stored on disk. When the timer is activated, the service unit is triggered immediately if it would have been triggered at least once during the time when the timer was inactive. Such triggering is nonetheless subject to the delay imposed by RandomizedDelaySec=. This is useful to catch up on missed runs of the service when the system was powered down. Note that this setting only has an effect on timers configured with OnCalendar=. Defaults to false.

Use systemctl clean --what=state … on the timer unit to remove the timestamp file maintained by this option from disk. In particular, use this command before uninstalling a timer unit. See systemctl(1) for details.

I'll return defaults back to original value. Do you agree?

thiagokokada · 2020-12-30T12:30:36Z

nixos/modules/tasks/auto-upgrade.nix

+        default = true;
+        type = types.bool;
+        example = false;
+        description = ''
+          Takes a boolean argument. If true, the time when the service
+          unit was last triggered is stored on disk. When the timer is
+          activated, the service unit is triggered immediately if it
+          would have been triggered at least once during the time when
+          the timer was inactive. Such triggering is nonetheless
+          subject to the delay imposed by RandomizedDelaySec=. This is
+          useful to catch up on missed runs of the service when the
+          system was powered down.
+        '';


Same here, think it needs documentation, but I think this is less controversial since persistent = true makes more sense for this service.

Do you know what documentation (where) exactly? If I can, I would update it.

I don't know exactly, I think maybe it is only filled after the release is tagged 🤔 ?

Release notes for Okapi are over there:

nixpkgs/nixos/doc/manual/release-notes/rl-2103.xml

Line 86 in 7b968c7

<title>Backward Incompatibilities</title>

thiagokokada

I think the systemd.services.nixos-upgrade needs to be altered too to include after = [ "network-online.target" ]; (and maybe "network.target" too? I am not sure if only "network-online.target" is sufficient), otherwise the service will fail during restart because of the lack of network access.

tex · 2020-12-30T16:00:38Z

I think the systemd.services.nixos-upgrade needs to be altered too to include after = [ "network-online.target" ]; (and maybe "network.target" too? I am not sure if only "network-online.target" is sufficient), otherwise the service will fail during restart because of the lack of network access.

https://www.freedesktop.org/wiki/Software/systemd/NetworkTarget/ says:

Cut the crap! How do I make sure that my service starts after the network is really online?

Well, that depends on your setup and the services you plan to run after it (see above). If you need to delay you service after the network is up, include

After=network-online.target
Wants=network-online.target

in the .service file.

So, I do what it says.

thiagokokada

LGTM.

nixos-discourse · 2020-12-31T02:57:50Z

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

https://discourse.nixos.org/t/prs-already-reviewed/2617/317

thiagokokada · 2021-01-03T15:06:02Z

I decided to give a try and test the PR to make sure everything is alright. My /etc/nixos/configuration.nix:

{
  # Backport module from unstable.
  imports = [
    /home/thiagoko/Projects/nixpkgs/nixos/modules/tasks/auto-upgrade.nix
  ];
  disabledModules = [ "tasks/auto-upgrade.nix" ];
  system.autoUpgrade = {
    enable = true;
    dates = "daily";
  };
}

And this is the result:

~/Projects/nixpkgs nixos/auto-upgrade 14s
❯ systemctl cat nixos-upgrade.timer               
# /nix/store/qcv8x0xp01yn5zpz0nffk6y6zx7qdjn3-unit-nixos-upgrade.timer/nixos-upgrade.timer
[Unit]

[Timer]
OnCalendar=daily
Persistent=true
RandomizedDelaySec=0


~/Projects/nixpkgs nixos/auto-upgrade
❯ systemctl cat nixos-upgrade.service
# /nix/store/ivsfnlkw0ckf2s3qh7d12f795yq83hgj-unit-nixos-upgrade.service/nixos-upgrade.service
[Unit]
After=network-online.target
Description=NixOS Upgrade
Wants=network-online.target
X-StopOnRemoval=false

[Service]
Environment="HOME=/root"
Environment="LOCALE_ARCHIVE=/nix/store/a2px4kdz1jm03f8ifr1pzir0csfmyrlv-glibc-locales-2.31/lib/locale/locale-archive"
Environment="NIX_PATH=nixpkgs=/nix/var/nix/profiles/per-user/root/channels/nixos:nixos-config=/etc/nixos/configuration.nix:/nix/var/nix/>
Environment="PATH=/nix/store/z1qvlavy35wanw5k54fvvfffws5bvigj-coreutils-8.31/bin:/nix/store/xlfmnfbqryr6s52xgaqfaljmiccrv2rk-gnutar-1.32>
Environment="TZDIR=/nix/store/w1g27pgslf28nh1py1szj7lk4xksdhqq-tzdata-2020c/share/zoneinfo"

X-RestartIfChanged=false


ExecStart=/nix/store/h45gf360a2h7pbq86b2kvqmyxn9dsz75-unit-script-nixos-upgrade-start/bin/nixos-upgrade-start 
Type=oneshot

So this seems 💯 .

thiagokokada · 2021-01-31T01:49:26Z

@tex Can you fix the merge conflict?

thiagokokada · 2021-01-31T11:06:42Z

@tex AFAIK we generally don't fix merge conflicts with merging other branches, but with a rebase.

tex · 2021-01-31T12:11:45Z

And I though I get from this easily by doing web-based merge conflict solve :)

tex · 2021-01-31T12:29:06Z

So here you are and please merge it.

thiagokokada · 2021-02-28T14:56:14Z

@tex Now that #107959 can you fix the merge conflict again?

@SuperSandro2000 Do you know someone that can help reviewing this PR?

stale · 2021-09-03T01:14:06Z

I marked this as stale due to inactivity. → More info

thiagokokada · 2021-09-03T13:34:28Z

Still relevant.

SuperSandro2000 · 2021-09-21T20:13:19Z

@SuperSandro2000 Do you know someone that can help reviewing this PR?

Not really.

stale · 2022-04-16T04:15:56Z

I marked this as stale due to inactivity. → More info

thiagokokada · 2022-04-16T10:04:20Z

I am still interested in this PR. I can give a review/merge now.

@tex Sorry for the inconvenience. If you can rebase this PR I can review/merge ASAP.

Lassulus

Lgtm

Lassulus · 2022-04-16T10:26:35Z

But needs a rebase. After that i can merge it. Maybe i can rebase myself later today

Lassulus · 2022-04-16T17:28:10Z

@thiagokokada feel free to review and merge please :)

thiagokokada · 2022-04-16T22:06:37Z

Thanks @Lassulus for the rebase.

Thanks @tex for the original PR.

ofborg bot added 6.topic: nixos 8.has: module (update) labels Dec 30, 2020

tex requested a review from bbjubjub2494 December 30, 2020 11:13

ofborg bot added 10.rebuild-darwin: 0 10.rebuild-linux: 1-10 10.rebuild-linux: 1 labels Dec 30, 2020

bbjubjub2494 suggested changes Dec 30, 2020

View reviewed changes

tex force-pushed the nixos/auto-upgrade branch from 4c3405c to 7ed9eec Compare December 30, 2020 11:38

tex changed the title ~~add persistent option~~ nixos/auto-upgrade: add persistent option Dec 30, 2020

bbjubjub2494 approved these changes Dec 30, 2020

View reviewed changes

thiagokokada reviewed Dec 30, 2020

View reviewed changes

thiagokokada mentioned this pull request Dec 30, 2020

nixos/nix-gc: add persistent and randomizeDelaySec options #107959

Merged

10 tasks

tex force-pushed the nixos/auto-upgrade branch from 7ed9eec to 406d7de Compare December 30, 2020 18:12

ofborg bot added 8.has: changelog 8.has: documentation labels Dec 30, 2020

thiagokokada approved these changes Dec 30, 2020

View reviewed changes

otavio approved these changes Dec 31, 2020

View reviewed changes

SuperSandro2000 added the 2.status: merge conflict label Jan 18, 2021

ofborg bot removed the 2.status: merge conflict label Jan 31, 2021

tex force-pushed the nixos/auto-upgrade branch 2 times, most recently from f8ef5e9 to 276f280 Compare January 31, 2021 12:23

SuperSandro2000 added the 2.status: merge conflict label Mar 5, 2021

SuperSandro2000 requested review from Mic92 and Lassulus March 5, 2021 02:36

stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Sep 3, 2021

TredwellGit removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Sep 3, 2021

stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Apr 16, 2022

stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Apr 16, 2022

Lassulus approved these changes Apr 16, 2022

View reviewed changes

nixos/auto-upgrade: add persistent option

a5fb565

Lassulus force-pushed the nixos/auto-upgrade branch from 276f280 to a5fb565 Compare April 16, 2022 17:27

ofborg bot removed 2.status: merge conflict 10.rebuild-linux: 1 labels Apr 16, 2022

thiagokokada approved these changes Apr 16, 2022

View reviewed changes

thiagokokada merged commit 99b20f5 into NixOS:master Apr 16, 2022

bjornfor mentioned this pull request May 26, 2023

autoupgrade not run when the machine doesn't run all the time #15689

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

nixos/auto-upgrade: add persistent option #107957

nixos/auto-upgrade: add persistent option #107957

tex commented Dec 30, 2020 •

edited

bbjubjub2494 left a comment

tex commented Dec 30, 2020

thiagokokada Dec 30, 2020

tex Dec 30, 2020

thiagokokada Dec 30, 2020

tex Dec 30, 2020

bbjubjub2494 Dec 30, 2020

tex Dec 30, 2020

thiagokokada Dec 30, 2020 •

edited

tex Dec 30, 2020

thiagokokada Dec 30, 2020

tex Dec 30, 2020

thiagokokada Dec 30, 2020

bbjubjub2494 Dec 30, 2020

thiagokokada left a comment •

edited

tex commented Dec 30, 2020

thiagokokada left a comment

nixos-discourse commented Dec 31, 2020

thiagokokada commented Jan 3, 2021

thiagokokada commented Jan 31, 2021

thiagokokada commented Jan 31, 2021

tex commented Jan 31, 2021

tex commented Jan 31, 2021

thiagokokada commented Feb 28, 2021

stale bot commented Sep 3, 2021

thiagokokada commented Sep 3, 2021

SuperSandro2000 commented Sep 21, 2021

stale bot commented Apr 16, 2022

thiagokokada commented Apr 16, 2022

Lassulus left a comment

Lassulus commented Apr 16, 2022

Lassulus commented Apr 16, 2022

thiagokokada commented Apr 16, 2022

nixos/auto-upgrade: add persistent option #107957

nixos/auto-upgrade: add persistent option #107957

Conversation

tex commented Dec 30, 2020 • edited

Motivation for this change

Things done

bbjubjub2494 left a comment

Choose a reason for hiding this comment

tex commented Dec 30, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thiagokokada Dec 30, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thiagokokada left a comment • edited

Choose a reason for hiding this comment

tex commented Dec 30, 2020

thiagokokada left a comment

Choose a reason for hiding this comment

nixos-discourse commented Dec 31, 2020

thiagokokada commented Jan 3, 2021

thiagokokada commented Jan 31, 2021

thiagokokada commented Jan 31, 2021

tex commented Jan 31, 2021

tex commented Jan 31, 2021

thiagokokada commented Feb 28, 2021

stale bot commented Sep 3, 2021

thiagokokada commented Sep 3, 2021

SuperSandro2000 commented Sep 21, 2021

stale bot commented Apr 16, 2022

thiagokokada commented Apr 16, 2022

Lassulus left a comment

Choose a reason for hiding this comment

Lassulus commented Apr 16, 2022

Lassulus commented Apr 16, 2022

thiagokokada commented Apr 16, 2022

tex commented Dec 30, 2020 •

edited

thiagokokada Dec 30, 2020 •

edited

thiagokokada left a comment •

edited