thanos: init at 0.6.0 & NixOS module #60500

basvandijk · 2019-04-30T16:30:17Z

~~Please don't merge since this is work-in-progress.~~ EDIT: this is now ready for review.

Motivation for this change

I would like to use Thanos in NixOS.

0.4.0 Release notes.

Things done

basvandijk · 2019-04-30T16:31:29Z

@GrahamcOfBorg build thanos

The following contains some preliminary tests for thanos:
@GrahamcOfBorg test prometheus2

basvandijk · 2019-04-30T16:40:56Z

@Mic92 I see you've done some work on buildGoModule. It would be great if you could review commit c93e5ca. The thanos go package needs to have access to the bzr executable when downloading the go module dependencies. This commit adds a way to override the go-modules derivation so that we can add bzr to the nativeBuildInputs.

infinisil · 2019-05-03T04:32:31Z

I really don't like having more such huge modules in nixpkgs. Having options for every config setting has some serious downsides, check out NixOS/rfcs#42 if you haven't seen it already.

basvandijk · 2019-05-10T22:25:20Z

@infinisil thanks for pointing me to that RFC. I didn't see it yet. You raise some interesting points. I guess I'll have to make my mind up what my opinion is on the matter.

basvandijk · 2019-05-10T22:25:58Z

@GrahamcOfBorg test prometheus2

basvandijk · 2019-05-16T23:48:13Z

@GrahamcOfBorg test prometheus2

basvandijk · 2019-06-03T07:19:14Z

@GrahamcOfBorg test prometheus2

basvandijk · 2019-06-03T08:49:55Z

@infinisil regarding NixOS/rfcs#42, I understand the motivation for it: NixOS modules that encode every setting as a NixOS option are, to quote the RFC, "time consuming to implement, tedious to review and hard to maintain". This is true. I do think though that the time invested in implementing, reviewing and maintaining the module will benefit the user significantly. She'll get a module which is

discoverable: every setting is a NixOS options which is searchable and documented
safe to use: eval-time syntax- and type-checking for every option.

OTOH reducing the time spent implementing, reviewing and maintaining modules is also important so I'm willing to investigate a config-based approach. Regarding that I have a few questions:

Thanos doesn't work with a configuration file but accepts command line arguments instead. It's unclear to me how to map an option of type types.json to a list of command line arguments.

For most type of options the mapping is not that hard. For example we can have:
{ store.sd-interval = "10m"; store.unhealthy-timeout = "3m"; } ->
[ ''--store.sd-interval="10m"'' ''store.unhealthy-timeout="3m"'' ]

But what to do with repeatable options like:
```
--store=<store> ...        Addresses of statically configured store API
                           servers (repeatable). The scheme may be
                           prefixed with 'dns+' or 'dnssrv+' to detect
                           store API servers through respective DNS
                           lookups.  
```
This PR solves this by having the option:
store.addresses = mkOption { type = types.listOf types.str; ... }
This option will then get converted to a list of multiple --store="<address>" arguments.

Of course we could implement the same kind of logic with the config setting where we would check if config has a store.addresses field of type list of string and then converting that. However, in contrast to the NixOS option, that field would not be discoverable.
Similarly to the above list-based arguments Thanos has key-value-based arguments like:
```
--label=<name>="<value>" ...
                           Labels to be applied to all generated metrics
                           (repeated). Similar to external labels for
                           Prometheus, used to identify ruler and its
                           blocks as unique source.
```
How would we specify and convert these? Maybe by checking for a field named label in the config which should be an attribute set, then converting the name-value pairs to --label=<name>="<value>". This could work but is also not very discoverable.

mmahut · 2019-06-04T11:44:39Z

I've tested the binary and this works as expected.

kalbasit

I have not tested yet, unsure if I can afford to do so myself. This is a pure CR.

nixos/modules/services/monitoring/thanos.nix

kalbasit · 2019-06-04T14:39:03Z

nixos/modules/services/monitoring/thanos.nix

+    (mkIf cfg.sidecar.enable {
+      systemd.services.thanos-sidecar = {
+        wantedBy = [ "multi-user.target" ];
+        after    = [ "network.target" "prometheus2.service" ];


should we enable prometheus (services.prometheus.enable) or write an assertion for it?

I think an assertion is an excellent idea! I'll add it.

Awesome! Thank you for adding the assertions. Not a blocker, but it would be great to add a test for the assertions.

kalbasit · 2019-06-04T14:56:24Z

pkgs/servers/monitoring/thanos/default.nix

+    runCommandNoCC "bzr-no-cert-validation" {
+      inherit bazaar;
+      buildInputs = [ makeWrapper ];
+    } "makeWrapper $bazaar/bin/bzr $out/bin/bzr --add-flags -Ossl.cert_reqs=none";


is there a way this can go into go-modules instead?

We could add it to nativeBuildInputs by default but that would cause an unnecessary build dependency for all go-modules (except for thanos of course).

However git is also added by default so there's precedent. OTOH git is used by most go-modules while bzr isn't.

In general I think it's worthwhile to have support for overriding go-modules.

I understand the need to be able to support overriding the go-modules and I would leave that. I'm just thinking that any package using bzr will have to copy your hack over to their package and most likely spend time figuring out why they're getting a TLS error in the first place. We could maybe control it with a flag defaulting to false? Something like enable-bzr ? false?

Apparently thanos-0.5.0 doesn't depend on bzr anymore so I removed the overrideModAttrs setting.

It would still be nice to add a enableBzr flag but this can be done in a separate PR.

pkgs/servers/monitoring/thanos/default.nix

basvandijk · 2019-06-16T13:56:06Z

@GrahamcOfBorg test prometheus2

basvandijk · 2019-06-16T14:20:10Z

The test fails on aarch64-linux because of insufficient disk space:

store# [  126.911144] thanos[739]: level=error ts=2019-06-16T14:00:26.59870538Z caller=main.go:182 msg="running command failed" err="error executing compaction: first pass of downsampling failed: create dir: mkdir /var/lib/thanos-compact/downsample: no space left on device"
store# [  126.942655] systemd[1]: thanos-compact.service: Main process exited, code=exited, status=1/FAILURE

I'll increase the disk space on store and run the test again...

basvandijk · 2019-06-16T14:24:18Z

@GrahamcOfBorg test prometheus2

basvandijk · 2019-07-28T10:18:25Z

@GrahamcOfBorg test prometheus2

I'll leave in the support for overrideModAttrs because that can be useful for other go packages.

This is to fix the following error in the test on aarch64-linux: store# [ 126.911144] thanos[739]: level=error ts=2019-06-16T14:00:26.59870538Z caller=main.go:182 msg="running command failed" err="error executing compaction: first pass of downsampling failed: create dir: mkdir /var/lib/thanos-compact/downsample: no space left on device" store# [ 126.942655] systemd[1]: thanos-compact.service: Main process exited, code=exited, status=1/FAILURE

basvandijk · 2019-07-28T11:29:28Z

@GrahamcOfBorg test prometheus2

basvandijk added the 2.status: work-in-progress label Apr 30, 2019

basvandijk requested a review from infinisil as a code owner April 30, 2019 16:30

basvandijk self-assigned this Apr 30, 2019

ofborg bot added 6.topic: golang 6.topic: nixos 8.has: module (update) labels Apr 30, 2019

ofborg bot added the 8.has: package (new) label Apr 30, 2019

ofborg bot requested a review from kalbasit April 30, 2019 16:43

ofborg bot added 11.by: package-maintainer 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 labels Apr 30, 2019

basvandijk force-pushed the thanos-init branch 3 times, most recently from 930bccf to 8ef1718 Compare May 2, 2019 15:28

basvandijk changed the title ~~WIP: thanos: init at 0.4.0-rc.1 & NixOS module~~ thanos: init at 0.4.0-rc.1 & NixOS module May 2, 2019

basvandijk removed the 2.status: work-in-progress label May 2, 2019

basvandijk removed their assignment May 2, 2019

basvandijk force-pushed the thanos-init branch from 8ef1718 to 15b1dd9 Compare May 10, 2019 22:21

basvandijk changed the title ~~thanos: init at 0.4.0-rc.1 & NixOS module~~ thanos: init at 0.4.0 & NixOS module May 10, 2019

basvandijk force-pushed the thanos-init branch 2 times, most recently from 4aeb105 to 13c264b Compare May 16, 2019 23:26

basvandijk force-pushed the thanos-init branch from 13c264b to ff0350a Compare June 3, 2019 07:17

basvandijk changed the title ~~thanos: init at 0.4.0 & NixOS module~~ thanos: init at 0.5.0-rc.0 & NixOS module Jun 3, 2019

kalbasit requested changes Jun 4, 2019

View reviewed changes

ofborg bot requested a review from kalbasit June 5, 2019 11:24

basvandijk changed the title ~~thanos: init at 0.5.0-rc.0 & NixOS module~~ thanos: init at 0.5.0 & NixOS module Jun 16, 2019

basvandijk changed the title ~~thanos: init at 0.5.0 & NixOS module~~ thanos: init at 0.6.0 & NixOS module Jul 28, 2019

basvandijk added 12 commits July 28, 2019 13:28

buildGoModule: support overriding the go-modules derivation

8d6fbef

thanos: init at 0.4.0

4a6e471

nixos/thanos: add module for the thanos service

ebc65a5

thanos: 0.4.0 -> 0.5.0-rc.0

2d0243c

nixos/thanos: allow overriding arguments to the thanos subcommands

13da811

thanos: don't use URL literals (RFC-0045)

7f093a2

nixos/thanos: assert that prometheus2 is running and has labels set

e32e0e6

nixos/thanos: code style: don't use a space before a colon

dc69b3e

thanos: 0.5.0-rc.0 -> 0.5.0

aefb0aa

thanos: remove bzr because 0.5.0 apparently doesn't need it anymore

e753322

I'll leave in the support for overrideModAttrs because that can be useful for other go packages.

thanos: 0.5.0 -> 0.6.0

0a59be7

basvandijk force-pushed the thanos-init branch from 5a0035a to 0a59be7 Compare July 28, 2019 11:29

basvandijk merged commit 9ff408a into NixOS:master Jul 28, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

thanos: init at 0.6.0 & NixOS module #60500

thanos: init at 0.6.0 & NixOS module #60500

basvandijk commented Apr 30, 2019 •

edited

basvandijk commented Apr 30, 2019

basvandijk commented Apr 30, 2019

infinisil commented May 3, 2019

basvandijk commented May 10, 2019

basvandijk commented May 10, 2019

basvandijk commented May 16, 2019

basvandijk commented Jun 3, 2019

basvandijk commented Jun 3, 2019 •

edited

mmahut commented Jun 4, 2019

kalbasit left a comment

kalbasit Jun 4, 2019

basvandijk Jun 5, 2019

kalbasit Jun 5, 2019

kalbasit Jun 4, 2019

basvandijk Jun 5, 2019

kalbasit Jun 5, 2019 •

edited

basvandijk Jun 16, 2019

basvandijk commented Jun 16, 2019

basvandijk commented Jun 16, 2019

basvandijk commented Jun 16, 2019

basvandijk commented Jul 28, 2019

basvandijk commented Jul 28, 2019

thanos: init at 0.6.0 & NixOS module #60500

thanos: init at 0.6.0 & NixOS module #60500

Conversation

basvandijk commented Apr 30, 2019 • edited

Motivation for this change

Things done

basvandijk commented Apr 30, 2019

basvandijk commented Apr 30, 2019

infinisil commented May 3, 2019

basvandijk commented May 10, 2019

basvandijk commented May 10, 2019

basvandijk commented May 16, 2019

basvandijk commented Jun 3, 2019

basvandijk commented Jun 3, 2019 • edited

mmahut commented Jun 4, 2019

kalbasit left a comment

Choose a reason for hiding this comment

kalbasit Jun 4, 2019

Choose a reason for hiding this comment

basvandijk Jun 5, 2019

Choose a reason for hiding this comment

kalbasit Jun 5, 2019

Choose a reason for hiding this comment

kalbasit Jun 4, 2019

Choose a reason for hiding this comment

basvandijk Jun 5, 2019

Choose a reason for hiding this comment

kalbasit Jun 5, 2019 • edited

Choose a reason for hiding this comment

basvandijk Jun 16, 2019

Choose a reason for hiding this comment

basvandijk commented Jun 16, 2019

basvandijk commented Jun 16, 2019

basvandijk commented Jun 16, 2019

basvandijk commented Jul 28, 2019

basvandijk commented Jul 28, 2019

basvandijk commented Apr 30, 2019 •

edited

basvandijk commented Jun 3, 2019 •

edited

kalbasit Jun 5, 2019 •

edited