Integration test fails inside Nix sandbox

[flokli]

I tried packaging grafana/agent for NixOS, and ran into an issue while running the tests:

=== RUN   TestIntegrationRegistration
--- PASS: TestIntegrationRegistration (0.00s)
PASS
ok      github.com/grafana/agent/pkg/integrations       2.548s
=== RUN   TestNodeExporter
    node_exporter_test.go:43:
                Error Trace:    node_exporter_test.go:43
                Error:          Received unexpected error:
                                failed to create node_exporter: failed to open sysfs: could not read /sys: stat /sys: no such file or directory
                Test:           TestNodeExporter
                Messages:       failed to setup node_exporter
--- FAIL: TestNodeExporter (0.00s)
=== RUN   TestNodeExporter_IgnoredFlags

Apparently the node exporter integration test tries to access /sys, which doesn't quite work in that sandboxed environment. Could the test be skipped in such cases?

[rfratto]

I'd be happy to add a build tag to make tests for that package skippable. I won't be able to work on this for a few more days, but if you want to take a shot at it, you're more than welcome to!

[flokli]

For now, while packaging this in nixpkgs I simply removed pkg/integrations/node_exporter/node_exporter_test.go before running the tests.

Inside Nix sandboxed builds, /sys simply doesn't exist. Maybe that test can skip if it detects /sys not being present.

[stale]

This issue has been automatically marked as stale because it has not had any activity in the past 30 days. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

[flokli]

still relevant.

[stale]

This issue has been automatically marked as stale because it has not had any activity in the past 30 days. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

[rfratto]

Hey @flokli, is this still a problem? Are there any other tests that are failing inside Nix that we can filter out?

[flokli]

Okay, build flags seem to be the way to add conditional tests.

We could add a integration flag, filter these out, and set it in the drone config and Makefile.

But I'm not about the naming. I don't want to disable all these tests. Some integrations tests seem to work, even inside the sandbox. Should we skip them too, or call this flag with_node_exporter_integration_tests?

Why does the redis integration test not fail? I do see time="2021-08-19T17:05:03Z" level=error msg="Couldn't connect to redis instance" in the logs, but it still seem to succeed…

go test -v -p 12 ./pkg/integrations/redis_exporter
=== RUN   TestRedisCases
=== RUN   TestRedisCases/Default_config
time="2021-08-19T17:05:03Z" level=error msg="Couldn't connect to redis instance"
=== RUN   TestRedisCases/Include_exporter_metrics
time="2021-08-19T17:05:03Z" level=error msg="Couldn't connect to redis instance"
=== RUN   TestRedisCases/Lua_script_read_OK
time="2021-08-19T17:05:03Z" level=error msg="Couldn't connect to redis instance"
=== RUN   TestRedisCases/Lua_script_read_fail
=== RUN   TestRedisCases/no_address_given
=== RUN   TestRedisCases/valid_password_file
time="2021-08-19T17:05:03Z" level=error msg="Couldn't connect to redis instance"
=== RUN   TestRedisCases/invalid_password_file
--- PASS: TestRedisCases (0.01s)
    --- PASS: TestRedisCases/Default_config (0.00s)
    --- PASS: TestRedisCases/Include_exporter_metrics (0.00s)
    --- PASS: TestRedisCases/Lua_script_read_OK (0.00s)
    --- PASS: TestRedisCases/Lua_script_read_fail (0.00s)
    --- PASS: TestRedisCases/no_address_given (0.00s)
    --- PASS: TestRedisCases/valid_password_file (0.00s)
    --- PASS: TestRedisCases/invalid_password_file (0.00s)
PASS

[rfratto]

Sorry for taking so long to get back to you @flokli, things have been busy and I lost track of some of these issues.

This is a really interesting problem to have, and I'm not sure how to break this down. What are the limitations of the sandbox? No network, no access to host folders like /sys or /dev that something like node_exporter might use?

It's starting to sound more and more like we might just need a sandbox tag, but I'm starting to question whether it's worthwhile running these tests in a restricted environment anyway. It'd ensure some base behavior of the Agent is functional, but could miss out on other things being broken. What do you think?

[flokli]

Sorry for taking so long to get back to you @flokli, things have been busy and I lost track of some of these issues.

No worries, thanks for getting back to me! :-)

What are the limitations of the sandbox? No network, no access to host folders like /sys or /dev that something like node_exporter might use?

Yeah, certainly no network access, only private networking. I assume the sandbox also prevents access to some of these folders, to rule out hardware-specific impurities leaking into builds. /dev and /proc are also very minimal, not sure about /sys.

It's not super well documented, but https://github.com/NixOS/nix/blob/bedd12ec14062bb23bbd87dd892219b84abbcce9/src/libstore/build/local-derivation-goal.cc#L1580 should get you an idea if you want to know exactly.

--

More generally: What are the tests supposed to test? Why do the redis ones succeed, even though there's obviously no redis database started, and why does the node-exporter one fail even though it might only not be able to get some metrics?

[rfratto]

Yeah, certainly no network access, only private networking. I assume the sandbox also prevents access to some of these folders, to rule out hardware-specific impurities leaking into builds. /dev and /proc are also very minimal, not sure about /sys.

It's not super well documented, but https://github.com/NixOS/nix/blob/bedd12ec14062bb23bbd87dd892219b84abbcce9/src/libstore/build/local-derivation-goal.cc#L1580 should get you an idea if you want to know exactly.

Thanks, this is helpful!

More generally: What are the tests supposed to test? Why do the redis ones succeed, even though there's obviously no redis database started, and why does the node-exporter one fail even though it might only not be able to get some metrics?

I don't think we've been very consistent in testing these, but for integrations in particular, I'd say there's a few things we'd be interested in looking at:

1. Testing the translation from the agent's integration to the exporter's config (if a translation exists, sometimes the exporter's config is sufficient).
2. A test where where we test the entire integration (including the exporter)

The first case can probably just be performed as a unit test safely within a sandbox. The second case almost always requires network access (or root FS access for node_exporter), and it sounds like that's where we'd want to start using the sandbox flag.

[flokli]

Yeah, this sounds like a good plan 👍

[rfratto]

I'd like to close this as part of our normal bug cleanup process. If we run into similar issues again, we can open a new issue for tracking.