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/self-deploy: init #120940

Merged
merged 3 commits into from May 18, 2021

Conversation

lambdadog
Copy link
Contributor

Add self-deploy service to facilitate continuous deployment of a NixOS system derivation from a git repository to the local system. This is based on an internal NixOS module of the same name we've been using at Awake Security that we've generalized for open source use.

Motivation for this change

At Awake Security we've found our internal version of this service useful in ensuring infrastructure is always up to date and felt it could be useful for others.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • 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 nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

Add `self-deploy` service to facilitate continuous deployment of NixOS
configuration from a git repository.
nixos/modules/services/system/self-deploy.nix Outdated Show resolved Hide resolved
nixos/modules/services/system/self-deploy.nix Outdated Show resolved Hide resolved
nixos/modules/services/system/self-deploy.nix Outdated Show resolved Hide resolved
Changes:
 - Correct `services.self-deploy.branch` description
 - Use `systemd.services.<name>.restartIfChanged`
 - Switch to using empty git repo and fetching without adding remote
 - Move `systemctl reboot` to after cleanup tasks
@SuperSandro2000
Copy link
Member

@ofborg eval

Copy link
Member

@SuperSandro2000 SuperSandro2000 left a comment

Choose a reason for hiding this comment

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

Can you format the file with nixpkgs-fmt? I don't want to write a suggestion for every little indentation.

Also some of these options could require an example value like sshKeyFile, nixArgs or startAt.

nixos/modules/services/system/self-deploy.nix Outdated Show resolved Hide resolved
nixos/modules/services/system/self-deploy.nix Show resolved Hide resolved
Changes:
 - /var/lib/{self-deploy->nixos-self-deploy}
 - run nixpkgs-fmt
@lambdadog
Copy link
Contributor Author

lambdadog commented May 3, 2021

@SuperSandro2000 made proposed changes other than network-online.target requirement, since I would argue it's "more correct" to require it even if it strictly shouldn't be necessary.

@SuperSandro2000
Copy link
Member

SuperSandro2000 commented May 3, 2021

@SuperSandro2000 made proposed changes other than network-online.target requirement, since I would argue it's "more correct" to require it even if it strictly shouldn't be necessary.

That has nothing to do with being correct or not. Network-online delays the start unecessary and it also does not mean you have internet connection necessarily. I am not a big fan of it and have it disabled on all my machines.

@Gabriella439
Copy link
Contributor

Gabriella439 commented May 3, 2021

@SuperSandro2000: The self-deploy service runs hourly by default, so if the first run fails (due to the network not being online), then the first run that succeeds might be as late as an hour after boot. Rather than optimizing for "time to first (possibly failed) run" we should be optimizing for "time to first successful run" and adding the network-online.target helps for the latter criterion.

Yes, network-online.target does not completely guarantee that the network is available, but it's still a decent approximation and improves the likelihood of success.

@lambdadog
Copy link
Contributor Author

lambdadog commented May 3, 2021

As Gabriel mentions, my understanding is that, while the network-online target doesn't ensure that you necessarily have an internet connection, the inverse (excluding exceptions like your intentional disabling of it) is true. When network-online is down you can fundamentally assume that you have no internet connection.

By this argument, any time that self-deploy runs when network-online is down on a system where network-online isn't specifically disabled is ensured to fail. It seems like by removing it we'd be specifically optimizing for a case that will near 100% of the time produce a failed run (unless the network comes up between the service starting and the fetch, and this is quite unlikely IMO, since it's near the first thing self-deploy does).

@Gabriella439
Copy link
Contributor

@SuperSandro2000: Do you have any other feedback before we merge this?

@SuperSandro2000
Copy link
Member

not right now

@Gabriella439 Gabriella439 merged commit 903665f into NixOS:master May 18, 2021
@Gabriella439 Gabriella439 deleted the ashlynn/self-deploy-service branch May 18, 2021 15:29
@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/how-to-use-services-self-deploy/20696/1

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

Successfully merging this pull request may close these issues.

None yet

5 participants