feat: Add Nix Builder + Add remote runner example plan by MarcoPolo · Pull Request #1425 · testground/testground

MarcoPolo · 2022-08-19T23:41:58Z

This PR does a couple things:

Introduces a Nix builder that lets us build binaries, containers, buildpacks and OSs for multiple architectures. (you can actually define what the OS looks like and deploy it as part of the test!)
a. Adds an integration test (that will fail until we add Nix to Circle CI)
Tweaks local:exec to allow you to run any executable, not just Go binaries.
Adds a new example plan that demonstrates how to support remote runners with ssh and the Nix builder. It leverages Nix to build for different architectures and efficiently/correctly deploy the application to the remote machine – even if the application is dynamically linked (caveat that the remote machine must have Nix installed).
a. This plan tests a simple server/client ping across machines.

In the long term we'll probably want some sort of first class support for remote runners in testground, but I think this is a usable solution in the short term that will let us set up some reproducible benchmark environments. But I may be missing something in my understanding of how Testground works, so apologies if that's the case.

r? @laurentsenta

p-shahi · 2022-10-20T17:59:21Z

@laurentsenta and I discussed this PR today and what it would enable for the libp2p maintainers.

We specifically talked about this proposal

That being said. I can add to the benchmark milestone and move F.1 to Q4 (or early Q1): wdyt? F.1 - Benchmarking using nix-builders (dependency: remote machines need Nix installed) F.2 - Benchmarking using first class support for remote runners (using remote:exec) in Testground

This sound good. I think F.1 will give us >80% of the benefit of F.2. And also inform what F.2 should look like. Also worth noting that we can do F.1 today. We aren't blocked on anything (except time and prioritization).

For that reason, this PR has been prioritized and added to both the test-plans and go-libp2p roadmaps (for Q2 November) to bootstrap the go-libp2p benchmarking efforts.
It will also enable libp2p/test-plans#65

The questions we had were:

If this PR achieves 80% of what we need, what is the remaining 20% that needs to be addressed afterwards?¹

A question Laurent had with regard to the Testground team's project prioritization was:

Is this PR enough to unblock the libp2p teams, and allow the Testground maintainers to prioritize the set of network simulation features (used currently in CI) to do RTT testing as a part of multi-dimenstional testing and deprioritize EPIC: Testground remote:docker and remote:exec runners #1392 ²

Next steps: both of those will be topics for discussion when people are together in Lisbon.

cc: @BigLep @TinyTb

Note: it's important to callout that

Toolchain changes: this introduces Nix (not sure if it's ever been used at PL -- having used it, I think it's great and not hard to use/setup.) We could use this effort to identify where else it can be introduced (for reproducible builds and deployments.)
Maintainability: Not sure how many people can help Marco maintain it. So, maintaining this would be go-libp2p maintainers responsibility, not Testground maintainers.

My thoughts were, I think we will likely have a better idea of the "20% gap" after merging and using this in Testground. It largely accomplishes what remote:exec is trying to do "we build a binary, scp to the remote machine, and start the binary there" ↩
I think yes it's enough to allow the Testground team to focus on the network simulation features. ↩

p-shahi · 2022-10-20T18:41:48Z

plans/remote-runner/README.md

+   You may need to set up [remote
+   builders](https://nixos.org/manual/nix/stable/advanced-topics/distributed-builds.html)
+   to properly cross-build the application.
+1. Using a `local:exec` runner to run a shell script that copies the built


Suggested change

1. Using a `local:exec` runner to run a shell script that copies the built

2. Using a `local:exec` runner to run a shell script that copies the built

Numbering is wrong.

Also, would it be better define a new remote:exec runner?

If you keep using 1.s for list items then GitHub Markdown creates a numbered list starting from 1. That's where the numbering comes from.

As for the name, local:exec is an existing construct. Also, the name is fitting here because the shell script that performs the copy is executed locally.

MarcoPolo · 2022-11-14T16:45:53Z

@galargh friendly ping on this

galargh · 2022-11-14T17:12:53Z

@galargh friendly ping on this

It's on my TODO list for this week. I'll have it reviewed on Wednesday the latest - hopefully, tomorrow. Sorry for the delay.

galargh

Looks good :) We'll probably want to update docs at some point but I think we can hold of for a bit and see how we end up using it in practice.

I left a few questions here and there, mostly to make sure I understand it correctly.

🚀

galargh · 2022-11-16T09:37:43Z

pkg/build/nix_generic.go

+	AttributeToBuild string `toml:"attr"`
+}
+
+// Build builds a testplan written in Go and outputs an executable.


Am I right in saying it builds whatever flake.nix specifies so it doesn't necessarily have to be Go?

galargh · 2022-11-16T09:40:00Z

pkg/build/nix_generic.go

+)
+
+// NixBuilder (id: "nix:generic") is a builder that build the test plan using
+// Nix. The resulting artifact can be a binary or a container.


Do we care about distinguishing between binary/container artifacts? Should we flag it somehow?

galargh · 2022-11-16T09:40:40Z

pkg/build/nix_generic.go

+
+	decoder := json.NewDecoder(&stdout)
+	type nixBuildOutput struct {
+		DrvPath string


What's this for?

galargh · 2022-11-16T09:47:23Z

pkg/runner/local_exec.go

+	// BinPath is the path of the binary relative to the artifact path. Use this
+	// if the builder doesn't put the binary as the artifact path
+	BinPath string `toml:"bin_path"`


Just to confirm I understand this. We need this for nix. It outputs an artifact. We pass the artifact path to local exec runner. We want to execute a specific binary within that artifact. Is it correct?

galargh · 2022-11-16T11:32:37Z

@laurentsenta What was the trick for go mod tidy not to look into directories with different go versions? It seems that's the problem here.

laurentsenta · 2022-12-01T07:29:44Z

@galargh I used a _ prefix,

https://pkg.go.dev/cmd/go#hdr-Package_lists_and_patterns

Directory and file names that begin with "." or "_" are ignored by the go tool, as are directories named "testdata".

@MarcoPolo is this still priority if we plan on moving the building out of testground in libp2p/test-plans?

MarcoPolo · 2022-12-06T23:32:07Z

Closing this. I think we want to get out of the building game. And I think we need to make testground simpler to enable remote workers properly.

MarcoPolo mentioned this pull request Oct 7, 2022

add milestones and reorganize libp2p/go-libp2p#1804

Merged

p-shahi reviewed Oct 20, 2022

View reviewed changes

This was referenced Oct 20, 2022

Create test to show libp2p's performance compared to http libp2p/test-plans#27

Open

[tracking issue] Interop test-plans for all existing/developing libp2p transports libp2p/test-plans#61

Open

laurentsenta mentioned this pull request Nov 8, 2022

Milestone 2023.3: Support libp2p's interoperability testing story and ProbeLabs work as a way to drive "critical" Testground improvements #1514

Open

galargh self-assigned this Nov 14, 2022

Add Nix builder and remote runner

8de0332

galargh force-pushed the marco/with-nix branch from a0c3ba2 to 8de0332 Compare November 16, 2022 09:15

galargh approved these changes Nov 16, 2022

View reviewed changes

galargh force-pushed the marco/with-nix branch from 951e5b5 to 8de0332 Compare November 16, 2022 11:31

laurentsenta added the hint/needs-author-input Hint: Needs Author Input label Dec 1, 2022

MarcoPolo closed this Dec 6, 2022

	1. Using a `local:exec` runner to run a shell script that copies the built
	2. Using a `local:exec` runner to run a shell script that copies the built

Conversation

MarcoPolo commented Aug 19, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

p-shahi commented Oct 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Footnotes

Uh oh!

p-shahi Oct 20, 2022

Choose a reason for hiding this comment

Uh oh!

galargh Nov 16, 2022

Choose a reason for hiding this comment

Uh oh!

MarcoPolo commented Nov 14, 2022

Uh oh!

galargh commented Nov 14, 2022

Uh oh!

galargh left a comment

Choose a reason for hiding this comment

Uh oh!

galargh Nov 16, 2022

Choose a reason for hiding this comment

Uh oh!

galargh Nov 16, 2022

Choose a reason for hiding this comment

Uh oh!

galargh Nov 16, 2022

Choose a reason for hiding this comment

Uh oh!

galargh Nov 16, 2022

Choose a reason for hiding this comment

Uh oh!

galargh commented Nov 16, 2022

Uh oh!

laurentsenta commented Dec 1, 2022

Uh oh!

MarcoPolo commented Dec 6, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

MarcoPolo commented Aug 19, 2022 •

edited

Loading

p-shahi commented Oct 20, 2022 •

edited

Loading