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/oci-containers can depend on arbitrary systemd services #82102
base: master
Are you sure you want to change the base?
Conversation
This adds It's the same as I use this for creating a one-shot |
I also found that I need a separation between If I have too many containers using So I need to instead have another |
👎 you can add more services by overriding the systemd service with |
That's not what I need. My specific containers need to depend on specific services. Not that docker depends on some service. It would not solve my problem. |
Oof, I totally misinterpreted the patch, sorry! One question though: is there any reason why you don't do Also, I think that if we merge this, we should probably provide options for |
Yea I have it in my own custom module. You won't know the name of the container ahead of time. It would be a leaky abstraction. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @CMCDragonkai ,
We can add this.
git diff /projects/github/nixpkgs_matrixai
diff --git a/nixos/modules/virtualisation/docker-containers.nix b/nixos/modules/virtualisation/docker-containers.nix
index 31d6b1ef745..59f32352da5 100644
--- a/nixos/modules/virtualisation/docker-containers.nix
+++ b/nixos/modules/virtualisation/docker-containers.nix
@@ -184,16 +184,31 @@ let
'';
};
- dependsOnService = mkOption {
+ requiresServices = mkOption {
type = with types; listOf str;
default = [];
description = ''
- Define which other services this one depends on. They will be added to both After and Requires for the unit.
+ Define which other services this one depends on. They will be added to only Requires for the unit.
'';
example = literalExample ''
services.docker-containers = {
node1 = {
- dependsOnService = [ "ncsd.service" ];
+ requiresServices = [ "ncsd.service" ];
+ }
+ }
+ '';
+ };
+
+ afterServices = mkOption {
+ type = with types; listOf str;
+ default = [];
+ description = ''
+ Define which other services this one will delay until they have started. They will be added to only the After for the unit.
+ '';
+ example = literalExample ''
+ services.docker-containers = {
+ node1 = {
+ afterServices = [ "ncsd.service" ];
}
}
'';
@@ -223,8 +238,8 @@ let
mkAfter = map (x: "docker-${x}.service") container.dependsOn;
in rec {
wantedBy = [] ++ optional (container.autoStart) "multi-user.target";
- after = [ "docker.service" "docker.socket" ] ++ mkAfter ++ container.dependsOnService;
- requires = after;
+ after = [ "docker.service" "docker.socket" ] ++ mkAfter ++ container.afterServices;
+ requires = [ "docker.service" "docker.socket" ] ++ mkAfter ++ container.requiresServices;
path = [ pkgs.docker ];
preStart = ''
f386898
to
dc2c06b
Compare
This is now updated with 2 additional options of |
This is ready to be merged. |
dc2c06b
to
ca32d5d
Compare
This has been updated since docker-containers.nix module has changed oci-containers.nix. Someone please merge this. |
Who's responsible for this module? |
ca32d5d
to
f184561
Compare
Updated name of this PR and name of the commit message. |
Wouldn't it be better to make naming deterministic, such that |
Can you elaborate?
…On 24 August 2020 12:18:15 GMT+10:00, Silvan Mosberger ***@***.***> wrote:
Wouldn't it be better to make naming deterministic, such that
`systemd.services.*.{requires,after}` can be used directly?
--
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#82102 (comment)
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
|
Oh wait they already are, never mind. But then you can just do this instead: {
systemd.services.docker-foo.after = [ "docker-bar.service" ];
} Same for I now see that the problem with this is that this might change from diff --git a/nixos/modules/virtualisation/oci-containers.nix b/nixos/modules/virtualisation/oci-containers.nix
index a46dd65eb49..894d9f8a784 100644
--- a/nixos/modules/virtualisation/oci-containers.nix
+++ b/nixos/modules/virtualisation/oci-containers.nix
@@ -310,7 +310,7 @@ in {
config = lib.mkIf (cfg.containers != {}) (lib.mkMerge [
{
- systemd.services = mapAttrs' (n: v: nameValuePair "${cfg.backend}-${n}" (mkService n v)) cfg.containers;
+ systemd.services = mapAttrs' (n: v: nameValuePair "oci-${n}" (mkService n v)) cfg.containers;
}
(lib.mkIf (cfg.backend == "podman") {
virtualisation.podman.enable = true; Then you can do {
systemd.services.oci-foo.after = [ "oci-bar.service" ];
} Without worrying about changing backends. |
I marked this as stale due to inactivity. → More info |
@infinisil if you prefer that, can you send up a PR to ensure the OCI has deterministic naming then? Then we don't have to add an additional option to the container modules. |
I marked this as stale due to inactivity. → More info |
Still relevant. |
Motivation for this change
Fixes #81814
Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./result/bin/
)nix path-info -S
before and after)