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

Update nix configuration #8452

Merged
merged 13 commits into from
Nov 16, 2023
Merged

Update nix configuration #8452

merged 13 commits into from
Nov 16, 2023

Conversation

angaz
Copy link
Contributor

@angaz angaz commented Feb 7, 2023

Hi.

I found the nix configuration here to be a bit broken, it looks like it hasn't been updated in a while, so I had a look at fixing some of the issues. It creates a workable dev environment now. I also fixed the linter errors I came accross.

One problem I have run into is that some of the tests are failing, so I can't actually build the packages with this setup. I tried it without running the tests, and it does build, but the default is to run the tests. We can disable that if we don't want to run the tests at build time.

Checklist

  • Reviewed the CONTRIBUTING.md guide (required)
  • Documentation added
  • Tests updated
  • CHANGELOG.md updated
  • Changes that require user attention or interaction to upgrade are documented in docs/sources/upgrading/_index.md

@angaz angaz requested a review from a team as a code owner February 7, 2023 18:20
@angaz
Copy link
Contributor Author

angaz commented Feb 8, 2023

I've figured out why those two tests are failing, they seem to rely on the fact that the variables are captured and in parallel. Is this expected?

@trevorwhitney
Copy link
Collaborator

@angaz I clearly missed this, so sorry it's been sitting for so long. I'll try to take a look today.

@angaz
Copy link
Contributor Author

angaz commented Sep 21, 2023

It's been such a long time. Fixing the tests was done already, so that's not relevant anymore. Should I update the branch again?

@angaz
Copy link
Contributor Author

angaz commented Sep 21, 2023

I'm running the tests locally because the CI is quite slow on GH and I see there are tests failing trying to find addresses for all the interfaces, which is interesting. I'm not sure what to do about it, but it seems like the test is possibly wrong.

--- FAIL: TestInstanceCountDelegate_CorrectlyInvokesOtherDelegates (0.00s)
    instance_count_test.go:107:
         Error Trace:    /build/z5wlqr5j57n2i2jn8cdmmvqiw4x40z0d-source/pkg/distributor/instance_count_test.go:107
         Error:          Received unexpected error:
                         no useable address found for interfaces [lo]
         Test:           TestInstanceCountDelegate_CorrectlyInvokesOtherDelegates
FAIL
--- FAIL: TestCompactor_RunCompaction (0.01s)
    compactor_test.go:53:
         Error Trace:    /build/z5wlqr5j57n2i2jn8cdmmvqiw4x40z0d-source/pkg/storage/stores/indexshipper/compactor/compactor_test.go:53
                                                 /build/z5wlqr5j57n2i2jn8cdmmvqiw4x40z0d-source/pkg/storage/stores/indexshipper/compactor/compactor_test.go:94
         Error:          Received unexpected error:
                         no useable address found for interfaces [eth0 en0 lo]
                         invalid ring lifecycler config
                         github.com/grafana/loki/pkg/storage/stores/indexshipper/compactor.NewCompactor
                                 /build/z5wlqr5j57n2i2jn8cdmmvqiw4x40z0d-source/pkg/storage/stores/indexshipper/compactor/compactor.go:228
                         github.com/grafana/loki/pkg/storage/stores/indexshipper/compactor.setupTestCompactor
                                 /build/z5wlqr5j57n2i2jn8cdmmvqiw4x40z0d-source/pkg/storage/stores/indexshipper/compactor/compactor_test.go:50
                         github.com/grafana/loki/pkg/storage/stores/indexshipper/compactor.TestCompactor_RunCompaction
                                 /build/z5wlqr5j57n2i2jn8cdmmvqiw4x40z0d-source/pkg/storage/stores/indexshipper/compactor/compactor_test.go:94
                         testing.tRunner
                                 /nix/store/3wpbn0nv1yhb47njlb3z3vicj20cp2mj-go-1.20.2/share/go/src/testing/testing.go:1576
                         runtime.goexit
                                 /nix/store/3wpbn0nv1yhb47njlb3z3vicj20cp2mj-go-1.20.2/share/go/src/runtime/asm_amd64.s:1598
         Test:           TestCompactor_RunCompaction
--- FAIL: TestCompactor_RunCompactionMultipleStores (0.01s)
    compactor_test.go:53:
         Error Trace:    /build/z5wlqr5j57n2i2jn8cdmmvqiw4x40z0d-source/pkg/storage/stores/indexshipper/compactor/compactor_test.go:53
                                                 /build/z5wlqr5j57n2i2jn8cdmmvqiw4x40z0d-source/pkg/storage/stores/indexshipper/compactor/compactor_test.go:165
         Error:          Received unexpected error:
                         no useable address found for interfaces [eth0 en0 lo]
                         invalid ring lifecycler config
                         github.com/grafana/loki/pkg/storage/stores/indexshipper/compactor.NewCompactor
                                 /build/z5wlqr5j57n2i2jn8cdmmvqiw4x40z0d-source/pkg/storage/stores/indexshipper/compactor/compactor.go:228
                         github.com/grafana/loki/pkg/storage/stores/indexshipper/compactor.setupTestCompactor
                                 /build/z5wlqr5j57n2i2jn8cdmmvqiw4x40z0d-source/pkg/storage/stores/indexshipper/compactor/compactor_test.go:50
                         github.com/grafana/loki/pkg/storage/stores/indexshipper/compactor.TestCompactor_RunCompactionMultipleStores
                                 /build/z5wlqr5j57n2i2jn8cdmmvqiw4x40z0d-source/pkg/storage/stores/indexshipper/compactor/compactor_test.go:165
                         testing.tRunner
                                 /nix/store/3wpbn0nv1yhb47njlb3z3vicj20cp2mj-go-1.20.2/share/go/src/testing/testing.go:1576
                         runtime.goexit
                                 /nix/store/3wpbn0nv1yhb47njlb3z3vicj20cp2mj-go-1.20.2/share/go/src/runtime/asm_amd64.s:1598
         Test:           TestCompactor_RunCompactionMultipleStores
FAIL

@angaz
Copy link
Contributor Author

angaz commented Sep 21, 2023

It looks like this is also happening in the CI. I think everything else is good. I don't know what to do about the tests.

@trevorwhitney
Copy link
Collaborator

if you wouldn't mind, it looks pretty much good to go from my perspective, so just a matter of getting the tests to pass

@angaz
Copy link
Contributor Author

angaz commented Sep 22, 2023

I do not understand this code or these tests. Is the code wrong or are the tests wrong? I have no idea. So I can't say if I know how they need to be fixed. Are they just broken for the nix-run tests or are they also broken for everyone else?

@trevorwhitney
Copy link
Collaborator

trevorwhitney commented Sep 22, 2023

I understand the code and the failure, but not how to fix it ☹️

Loki uses your local network interface to find your IP when setting up memberlist communication with the other processes. This normally works fine as there's a default set of interface names that covers the standard nic naming strategies across most OSes. I'm assuming in whatever sandbox nix is building in there is no networking interface, or it's named something totally different, I'm just not sure how to discover that?

@trevorwhitney
Copy link
Collaborator

@angaz sorry for the long delay on this. I'm going to look into this a bit today. FYI I've force pushed to your branch to rebase these changes on the latest in main.

@angaz
Copy link
Contributor Author

angaz commented Nov 14, 2023

Yeah, cool, sounds good. No problem. You can take over if you need to.

Copy link
Collaborator

@trevorwhitney trevorwhitney left a comment

Choose a reason for hiding this comment

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

Thanks for helping fix this! It's easy for this nix stuff to get neglected as it has a smaller user base. I would love continuing help in the future keeping it up to date!

@github-actions github-actions bot added the type/docs Issues related to technical documentation; the Docs Squad uses this label across many repositories label Nov 16, 2023
@github-actions github-actions bot removed the type/docs Issues related to technical documentation; the Docs Squad uses this label across many repositories label Nov 16, 2023
@trevorwhitney trevorwhitney merged commit 258a8b3 into grafana:main Nov 16, 2023
9 of 10 checks passed
jeschkies pushed a commit that referenced this pull request Nov 21, 2023
Fix nix configuration (mainly failing test), and break up various binaries (ie. logcli, promtail, loki, etc.) into their own pakcages.

---------

Co-authored-by: Trevor Whitney <trevorjwhitney@gmail.com>
rhnasc pushed a commit to inloco/loki that referenced this pull request Apr 12, 2024
Fix nix configuration (mainly failing test), and break up various binaries (ie. logcli, promtail, loki, etc.) into their own pakcages.

---------

Co-authored-by: Trevor Whitney <trevorjwhitney@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants