Add nix configuration #7020

trevorwhitney · 2022-08-31T21:46:47Z

What this PR does / why we need it:

This PR adds nix configuration to the Loki repo to make it easier for people who use nix to integrate building Loki into their existing workflows. For example, I have a direnv hook that uses nix shell to make sure I have the correct version of all dependent binaries (ie. go, golangci-lint, etc) on my $PATH whenever I change into the Loki directory. I find this especially useful for building with CGO enabled as it requires a C compiler with systemd headers to be available on the path.

Furthermore, by having a flake.nix in the root of this repo, it will make it easier for users of the nix package manager to grab a specific version of Loki for their own systems / projects.

This PR does not attempt to replace any existing build processes, this is merely supplemental. All these nix configurations wrap existing Makefile targets, and the addition of these files will have no effect on anyone not using nix.

Please see the README.md included in this PR for more information.

Checklist

Documentation added

trevorwhitney · 2022-08-31T21:52:08Z

nix/build-vars.nix

+{
+  imageTag = "main-5a6d09f";
+  gitRevision = "5a6d09f83";
+  gitBranch = "main";


in general there probably isn't much need to commit changes to this file, so I decided to have it track main. The gitRevision is only used on dirty git trees, and there's a shell hook that will automatically update these values for your local branch when you run nix develop

What is the reasoning to have it in git?

I suspect to still be able to build the derivation when the flake is imported. I would prefer to have a unknown revision instead. As otherwise it is just implying the wrong thing.

so are you suggesting we use either self.rev or unknown (when self.rev is not set, which would mean the local git tree is dirty)?

grafanabot · 2022-08-31T22:20:35Z

./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki

Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell.

+           ingester	0%
+        distributor	0%
+            querier	0%
+ querier/queryrange	0%
+               iter	0%
+            storage	0%
+           chunkenc	0%
+              logql	0%
+               loki	0%

grafanabot · 2022-08-31T22:30:00Z

./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki

Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell.

+           ingester	0%
+        distributor	0%
+            querier	0%
+ querier/queryrange	0%
+               iter	0%
+            storage	0%
+           chunkenc	0%
+              logql	0%
+               loki	0%

simonswine

I welcome this initiative. It is quite useful to have this as standardized environments and maybe in the future we could expand this to also form the basis of our CI/release (if helpful)

I think before we merge we should have some CI around that at least enforced the formatting of nix files and run a build.

For some reason in my local environment (on nixos) I have trouble building the loki derivation and I need that patch:

failed to initialize build cache at /homeless-shelter/.cache/go-build: mkdir /homeless-shelter: permission denied
[...]
failed to initialize build cache at /homeless-shelter/.cache/golangci-lint: mkdir /homeless-shelter: permission denied

--- a/nix/loki.nix
+++ b/nix/loki.nix
@@ -30,11 +30,13 @@ pkgs.stdenv.mkDerivation {
   '';

   buildPhase = ''
+    export GOCACHE=$TMPDIR/go-cache
     make clean loki logcli loki-canary promtail
   '';

   doCheck = true;
   checkPhase = ''
+    export GOCACHE=$TMPDIR/go-cache
     make lint test
   '';

simonswine · 2022-09-01T08:09:41Z

nix/build-vars.nix

+{
+  imageTag = "main-5a6d09f";
+  gitRevision = "5a6d09f83";
+  gitBranch = "main";


What is the reasoning to have it in git?

I suspect to still be able to build the derivation when the flake is imported. I would prefer to have a unknown revision instead. As otherwise it is just implying the wrong thing.

simonswine · 2022-09-01T08:11:25Z

nix/README.md

+
+You will also need to enable the Flakes feature to use Nix with this repo. See this [wiki](https://nixos.wiki/wiki/Flakes) for instructions on enabling Flakes.
+
+## Dealing with .git


Have you tried the approach lined out in: https://stackoverflow.com/questions/67025103/how-inject-git-revision-into-nix-derivation

I think I am using the approach lined out in that link, which if I read correctly is to use self.rev for flakes? Maybe I'm misunderstanding if there's another workaround in that article?

Ahh, I see, the suggestion is to use (builtins.fetchGit args).rev. That will not work, as the url here would be ./.. or self, which in either case points to the source in the nix store which has already been stripped of git info, so fetchGit will complain that this is not a git repo.

flake.nix

grafanabot · 2022-09-01T13:09:59Z

./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki

Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell.

+           ingester	0%
+        distributor	0%
+            querier	0%
+ querier/queryrange	0%
+               iter	0%
+            storage	0%
+           chunkenc	0%
+              logql	0%
+               loki	0%

trevorwhitney · 2022-09-01T13:28:17Z

What is the reasoning to have it in git?

I suspect to still be able to build the derivation when the flake is imported. I would prefer to have a unknown revision > > instead. As otherwise it is just implying the wrong thing.

So, if you import the flake it will not use the value of gitRevision from this file, it will instead use self.rev, which will be correct. However, maybe we could put only gitBranch in this file, and use either self.rev or unknown or dirty as the sha? That would make it so we should never have to commit changes to this file.

trevorwhitney · 2022-09-01T20:52:39Z

Build is currently broken, waiting on: #7029

grafanabot · 2022-09-01T22:06:05Z

./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki

Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell.

+           ingester	0%
+        distributor	0%
+            querier	0%
+ querier/queryrange	0%
+               iter	0%
+            storage	0%
+           chunkenc	0%
+              logql	0%
+               loki	0%

grafanabot · 2022-09-02T15:01:15Z

./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki

Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell.

+           ingester	0%
+        distributor	0%
+            querier	0%
+ querier/queryrange	0%
+               iter	0%
+            storage	0%
+           chunkenc	0%
+              logql	0%
+               loki	0%

grafanabot · 2022-09-02T15:06:44Z

./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki

Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell.

+           ingester	0%
+        distributor	0%
+            querier	0%
+ querier/queryrange	0%
+               iter	0%
+            storage	0%
+           chunkenc	0%
+              logql	0%
+               loki	0%

grafanabot · 2022-09-02T15:49:04Z

./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki

Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell.

+           ingester	0%
+        distributor	0%
+            querier	0%
+ querier/queryrange	0%
+               iter	0%
+            storage	0%
+           chunkenc	0%
+              logql	0%
+               loki	0%

**What this PR does / why we need it**: The build is currently dependent upon you building in an environment with internet connectivity and GCP credentials. This PR fixes the few parts of our code and test that rely on these external dependencies so it can be built in an environment without internet or GCP credentials (such as when building a [nix flake](#7020)) **Which issue(s) this PR fixes**: Fixes #7028

Co-authored-by: Christian Simon <simon@swine.de>

grafanabot · 2022-09-02T19:16:35Z

./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki

Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell.

+           ingester	0%
+        distributor	0%
+            querier	0%
+ querier/queryrange	0%
+               iter	0%
+            storage	0%
+           chunkenc	0%
+              logql	0%
+               loki	0%

grafanabot · 2022-09-02T19:47:23Z

./tools/diff_coverage.sh ../loki-main/test_results.txt test_results.txt ingester,distributor,querier,querier/queryrange,iter,storage,chunkenc,logql,loki

Change in test coverage per package. Green indicates 0 or positive change, red indicates that test coverage for a package fell.

+           ingester	0%
+        distributor	0%
+            querier	0%
+ querier/queryrange	0%
+               iter	0%
+            storage	0%
+           chunkenc	0%
+              logql	0%
+               loki	0%

**What this PR does / why we need it**: The build is currently dependent upon you building in an environment with internet connectivity and GCP credentials. This PR fixes the few parts of our code and test that rely on these external dependencies so it can be built in an environment without internet or GCP credentials (such as when building a [nix flake](grafana#7020)) **Which issue(s) this PR fixes**: Fixes grafana#7028

**What this PR does / why we need it**: This PR adds nix configuration to the Loki repo to make it easier for people who use nix to integrate building Loki into their existing workflows. For example, I have a `direnv` hook that uses nix shell to make sure I have the correct version of all dependent binaries (ie. `go`, `golangci-lint`, etc) on my `$PATH` whenever I change into the Loki directory. I find this especially useful for building with `CGO` enabled as it requires a C compiler with systemd headers to be available on the path. Furthermore, by having a `flake.nix` in the root of this repo, it will make it easier for users of the nix package manager to grab a specific version of Loki for their own systems / projects. This PR does _not_ attempt to replace any existing build processes, this is merely supplemental. All these nix configurations wrap existing `Makefile` targets, and the addition of these files will have no effect on anyone not using nix. Please see the README.md included in this PR for more information. **Checklist** - [X] Documentation added Co-authored-by: Christian Simon <simon@swine.de>

trevorwhitney requested a review from a team as a code owner August 31, 2022 21:46

pull-request-size bot added the size/L label Aug 31, 2022

trevorwhitney commented Aug 31, 2022

View reviewed changes

owen-d approved these changes Aug 31, 2022

View reviewed changes

simonswine approved these changes Sep 1, 2022

View reviewed changes

trevorwhitney mentioned this pull request Sep 1, 2022

Enable build in env w/o internet or GCP creds #7029

Merged

4 tasks

trevorwhitney and others added 14 commits September 2, 2022 13:11

Add nix configuration

b5c1831

Refactor nix simply flake.nix

107556d

Always run loki lint and tests

ee86f6e

Smarter imageTag generation, fix shellcheck

afc8805

Image tag wants the short sha

efd1731

Modify image tag version to match script

b5a78a8

Add WIP to imageTag

e588a44

Update flake.nix

2236c3e

Co-authored-by: Christian Simon <simon@swine.de>

Refactor to remove multiple nixpkgs and add CI

3e92e5a

Add linting step for nix

7f8d57e

Try setting env vars to fix /homeless-shelter issue

392bd8d

Set GOLANGCI_LINT_CACHE

73e92ea

Stream build logs on CI

6bdcd99

More descriptive CI task name

3e93d2f

trevorwhitney force-pushed the add-nix branch from fd16fbd to 3e93d2f Compare September 2, 2022 19:11

trevorwhitney added 2 commits September 2, 2022 13:41

Remove additional nixpkgs, make apps for binaries

8fc6ee6

Remove accidental switch of branch

98243e0

trevorwhitney merged commit a369189 into grafana:main Sep 6, 2022

trevorwhitney deleted the add-nix branch September 6, 2022 17:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add nix configuration #7020

Add nix configuration #7020

trevorwhitney commented Aug 31, 2022 •

edited

Loading

trevorwhitney Aug 31, 2022

simonswine Sep 1, 2022

trevorwhitney Sep 1, 2022 •

edited

Loading

grafanabot commented Aug 31, 2022

grafanabot commented Aug 31, 2022

simonswine left a comment

simonswine Sep 1, 2022

simonswine Sep 1, 2022

trevorwhitney Sep 1, 2022

trevorwhitney Sep 1, 2022

grafanabot commented Sep 1, 2022

trevorwhitney commented Sep 1, 2022

trevorwhitney commented Sep 1, 2022

grafanabot commented Sep 1, 2022

grafanabot commented Sep 2, 2022

grafanabot commented Sep 2, 2022

grafanabot commented Sep 2, 2022

grafanabot commented Sep 2, 2022

grafanabot commented Sep 2, 2022


		You will also need to enable the Flakes feature to use Nix with this repo. See this [wiki](https://nixos.wiki/wiki/Flakes) for instructions on enabling Flakes.

		## Dealing with .git

Add nix configuration #7020

Add nix configuration #7020

Conversation

trevorwhitney commented Aug 31, 2022 • edited Loading

trevorwhitney Aug 31, 2022

Choose a reason for hiding this comment

simonswine Sep 1, 2022

Choose a reason for hiding this comment

trevorwhitney Sep 1, 2022 • edited Loading

Choose a reason for hiding this comment

grafanabot commented Aug 31, 2022

grafanabot commented Aug 31, 2022

simonswine left a comment

Choose a reason for hiding this comment

simonswine Sep 1, 2022

Choose a reason for hiding this comment

simonswine Sep 1, 2022

Choose a reason for hiding this comment

trevorwhitney Sep 1, 2022

Choose a reason for hiding this comment

trevorwhitney Sep 1, 2022

Choose a reason for hiding this comment

grafanabot commented Sep 1, 2022

trevorwhitney commented Sep 1, 2022

trevorwhitney commented Sep 1, 2022

grafanabot commented Sep 1, 2022

grafanabot commented Sep 2, 2022

grafanabot commented Sep 2, 2022

grafanabot commented Sep 2, 2022

grafanabot commented Sep 2, 2022

grafanabot commented Sep 2, 2022

trevorwhitney commented Aug 31, 2022 •

edited

Loading

trevorwhitney Sep 1, 2022 •

edited

Loading