Improved NixOS module for Hydra (WIP) #501

taktoa · 2017-08-03T20:16:47Z

I'm working on making the Hydra configuration more declarative; I think the current state of affairs where you literally have to reverse-engineer the Perl source to figure out what options you can give in services.hydra.extraConfig is embarrassing; you'd think that Hydra, of all projects, would be really usable with NixOS.

My ultimate plan is what I call "Fully Declarative Hydra": every aspect of Hydra's state should be specified in the NixOS module except the build and evaluation history.

taktoa · 2017-08-03T20:19:53Z

hydra-module.nix

@@ -64,7 +67,7 @@ with rec {
      };

      package = mkOption {
-        type = types.path;
+        type = types.package;


The NixOS module documentation recommends types.package over types.path, but there may be some people taking advantage of the current state of affairs; feel free to let me know if this is the case.

domenkozar

This essentially (minus all the styling changes that are hard to track) adds following NixOS options for Hydra:

googleClientID
private
storeMode

Obviously needs documentation :)

domenkozar · 2017-08-07T14:29:05Z

hydra-module.nix

+  env = {
+    NIX_REMOTE = "daemon";
+    # FIXME: why isn't this removed yet? we've been on 17.03 for a while now
+    SSL_CERT_FILE = "/etc/ssl/certs/ca-certificates.crt"; # Remove in 16.03


Let's remove it.

taktoa · 2017-08-07T19:22:55Z

@domenkozar I have a lot more options added locally; I'll push later today

Ericson2314 · 2017-08-07T20:46:40Z

We could do the indentation modernization in a separate PR too just to get that out of the way.

taktoa · 2017-08-09T23:28:33Z

@domenkozar: I just sent up all the stuff I've written; it's messy and probably doesn't evaluate yet

Gabriella439 · 2017-08-25T13:55:46Z

hydra-module.nix

+    });
+
+  types = lib.types // {
+    oneof = list: (


It seems like oneof is not used anywhere else within the pull request

Also, oneof seems very similar to types.enum

oneof is not the same as enum, as enum cannot take any NixOS option type; it can only take values.

Gabriella439 · 2017-08-25T13:56:24Z

hydra-module.nix

+    };
+
+    {
+      foldr   = lib.lists.fold;


nixpkgs masteralready hasfoldr`

Yeah, I was testing this on a nixpkgs version without foldr.

Gabriella439 · 2017-08-25T13:57:45Z

hydra-module.nix

+
+    {
+      foldr   = lib.lists.fold;
+      foldr1  = foldToFold1 lists.foldr;


All of the fold*1 utilities seem like they are unused in this pull request. The only exception is foldr1 but that is used within oneof which is in turn not used elsewhere

Gabriella439 · 2017-08-25T13:58:46Z

hydra-module.nix

+      foldr1  = foldToFold1 lists.foldr;
+      foldl   = lib.lists.foldl;
+      foldl1  = foldToFold1 lists.foldl;
+      foldl'  = lib.lists.foldl';


Why does this add foldl' to lib.lists when it's already an attribute of lib.lists? Same comment for foldl

Gabriella439 · 2017-08-25T14:00:04Z

hydra-module.nix

+  forceDeep = x: builtins.deepSeq x x;
+
+  strings = lib.strings // {
+    intercalate = str: list: lib.concatStrings (lib.intersperse str list);


intercalate already exists in the standard library as lib.concatMapStringsSep

Good to know!

Gabriella439 · 2017-08-25T14:02:32Z

hydra-module.nix


+  forceWHNF = x: builtins.seq x x;
+  forceDeep = x: builtins.deepSeq x x;


This pull request does not use forceWHNF and forceDeep

Also forceWHNF does not do anything because seq x x = x

Good point on forceWHNF.

Gabriella439 · 2017-08-25T14:07:23Z

hydra-module.nix

@@ -62,15 +130,16 @@ in
      };

      package = mkOption {
-        type = types.path;
-        #default = pkgs.hydra;
+        type = types.package;


You should probably avoid making unrelated breaking changes when submitting a large PR, because if this change breaks anything we can't easily roll back

Gabriella439 · 2017-08-25T14:08:28Z

hydra-module.nix


-  };
+      googleClientID = mkOption {


Is this googleClientID a secret or is it safe to store in the configuration?

I think it is a secret.

Would it be possible to template the Hydra configuration at run-time during the systemd.services.hydra-init.preStart by reading the secret from a preconfigured system path (i.e. /etc/hydra/googleClientID or maybe something under /run)? That way you don't need to worry about the secret being publicly readable inside the /nix/store and the secret key file can have appropriate permissions set so that only the hydra daemon can read the file

(Same question for any other secrets these changes may require)

This is not a bad idea. Will investigate.

+1. I think it's currently impossible to configure Hydra with the current NixOS module without putting e.g. github auth tokens in the nix store.

@ElvishJerricco: I have an example NixOps deploy of Hydra that patches Hydra to fix this. You can see the deploy here:

https://github.com/dhall-lang/dhall-lang/tree/master/nixops

... and the patch here:

https://github.com/dhall-lang/dhall-lang/blob/master/nixops/hydra.patch

Gabriella439 · 2017-08-25T14:09:47Z

hydra-module.nix

+        default = 100;
+        example = 1000;
+        description = ''
+          FIXME: lol


Did you intend to keep this FIXME? :)

Gabriella439 · 2017-09-02T14:19:56Z

hydra-module.nix


-  };
+      googleClientID = mkOption {


Would it be possible to template the Hydra configuration at run-time during the systemd.services.hydra-init.preStart by reading the secret from a preconfigured system path (i.e. /etc/hydra/googleClientID or maybe something under /run)? That way you don't need to worry about the secret being publicly readable inside the /nix/store and the secret key file can have appropriate permissions set so that only the hydra daemon can read the file

(Same question for any other secrets these changes may require)

Gabriella439 · 2017-09-02T14:30:56Z

hydra-module.nix

+           parameter key.
+        '';
+        # FIXME: ensure that the above is actually true
+        # https://github.com/NixOS/nix/blob/2fd8f8bb99a2832b3684878c020ba47322e79332/src/libstore/store-api.hh#L693-L718


Could you create a GitHub issue against the nix repository for this and link that issue here?

Gabriella439 · 2017-09-02T14:32:05Z

hydra-module.nix

-        group = "hydra";
-        useDefaultShell = true;
-        home = "${baseDir}/queue-runner"; # really only to keep SSH happy
+      storeURI = mkOption {


It looks like this option is not used yet

Gabriella439 · 2017-09-02T14:40:08Z

temporary.nix

+  # then you should set `auth = "foo";`.
+  #
+  # There can be multiple <githubstatus> stanzas.
+  makeGithubStatusStanza = (


It looks like these make*Stanza are not used yet?

the intention is to fold the semantics defined by these functions into hydra-module.nix and then delete temporary.nix altogether

taktoa · 2017-09-04T12:11:58Z

Related issue: NixOS/nix#1556

domenkozar · 2017-10-08T10:56:29Z

@taktoa sorry I just have a bit of time over weekends, is this ready for review?

taktoa · 2017-10-08T10:58:35Z

Sorry, not yet, I've been busy with other things.

dtzWill · 2018-02-20T05:24:38Z

Gentle but enthusiastic ping! :)

ElvishJerricco · 2018-03-02T12:56:33Z

Seconding that ping!

taktoa · 2018-03-02T17:20:18Z

yep, was actually working on this a couple days ago

gilligan · 2020-05-11T19:06:14Z

@taktoa this hasn’t been touched in a while. I think several people had shown interest in this and you apparently invested quite some work. Do you have any intentions to pick this up again at some point?

Maybe you could try to break this down instead into smaller changes that are easier and more feasible to get done and merged?

It had been 2 years but maybe there is still some interest on your side.

taktoa · 2020-05-11T19:07:37Z

Sorry, but I just don't have the cycles these days. I'd be happy to let someone take this work over.

taktoa commented Aug 3, 2017

View reviewed changes

domenkozar requested changes Aug 7, 2017

View reviewed changes

taktoa mentioned this pull request Aug 10, 2017

Private Hydra instances do not have API support #503

Open

Gabriella439 reviewed Aug 25, 2017

View reviewed changes

Gabriella439 reviewed Sep 2, 2017

View reviewed changes

taktoa and others added 17 commits September 17, 2017 12:22

hydra-module.nix: whitespace changes

20c95b5

hydra-module.nix: fit doc messages in 80 column limit

d7bdfd3

hydra-module.nix: more formatting stuff

c8d0094

hydra-module.nix: refactoring that may affect semantics

77ba9f9

hydra-module.nix: added options: googleClientID, private, storeMode

2f1e541

rebaseme: send progress upstream for now

a8b2180

rebaseme: added hydra plugin options

6e01f75

rebaseme: a bunch of work on documentation

79f9f39

rebaseme: more documentation

721ae8e

rebaseme: added mkLink helper

dfa5858

rebaseme: added max* options

06ba2bc

rebaseme: deleted cruft

80137cb

rebaseme: reverted type change of services.hydra.package

2849442

rebaseme: added my work on hydra-repave.py

849c454

rebaseme: added hydraConfGen utilities

89eaad9

rebaseme: slight documentation improvements

235922e

rebaseme: more work on documentation

8a714cf

taktoa force-pushed the taktoa/module branch from 4dbe2ea to 8a714cf Compare September 17, 2017 17:22

taktoa mentioned this pull request Feb 4, 2018

Take a look at Fully Declarative Hydra pull request gilligan/groking-hydra#22

Open

taktoa closed this May 11, 2020


		forceWHNF = x: builtins.seq x x;
		forceDeep = x: builtins.deepSeq x x;

Improved NixOS module for Hydra (WIP) #501

Improved NixOS module for Hydra (WIP) #501

Conversation

taktoa commented Aug 3, 2017

Choose a reason for hiding this comment

domenkozar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

taktoa commented Aug 7, 2017

Ericson2314 commented Aug 7, 2017

taktoa commented Aug 9, 2017

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

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

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

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

taktoa commented Sep 4, 2017

domenkozar commented Oct 8, 2017

taktoa commented Oct 8, 2017

dtzWill commented Feb 20, 2018

ElvishJerricco commented Mar 2, 2018

taktoa commented Mar 2, 2018

gilligan commented May 11, 2020

taktoa commented May 11, 2020