use Kurtosis instead of the run script #649

h4ck3rk3y · 2023-05-12T12:37:47Z

Why this should be merged

This replaces the current run script with Kurtosis which is dockerized & cleans up after itself.

How this works

Installs Kurtosis in the CI and then runs test using Kurtosis. This forces the node thats spinning up to expose ports on 9650, so that we don't have to edit the existing testsuite too much

How this was tested

This is a test; CI runs fine on my fork.

How is this documented

Not sure where we document such things

h4ck3rk3y · 2023-05-12T12:49:39Z

@aaronbuchwald Hey! I took over Tedi's project to replace the script with Kurtosis as he stopped working part time a few days ago.

.github/workflows/ci.yml

tests/utils/node_launcher.go

aaronbuchwald · 2023-05-22T23:27:07Z

tests/utils/node_launcher.go

+	firstNodeId        = "node-0"
+)
+
+func SpinupAvalancheNode() (string, func(), error) {


What would it take to change this from SpinupAvalancheNode to spin up a network of N nodes and how would you specify configs of each individual node?

To run multiple nodes; we'll have to change the args to include node_count like

forceExposeOn9650 = `{"test_mode": true, "node_count": 5}`

This would run 5 different nodes. Note Kurtosis by default tries to expose ephemeral ports but we're using the "test_mode": true to force public ports to be a certain number(first node rpc gets 9650, second gets 9652 and so forth). Ideally we remove the test_mode and pass around the ephemeral port generated inside these tests.

Re config - At the moment all nodes start with the same config except for bootstrap information. The second bootstraps from the first one; the third from the first two. What other config would you like to tweak? It shouldn't be too hard to set it; but I wanted to get something minimal out

Cool that makes sense. AvalancheGo requires uses two ports the HTTP port (https://docs.avax.network/nodes/maintain/avalanchego-config-flags#--http-port-int) and the Staking port (https://docs.avax.network/nodes/maintain/avalanchego-config-flags#--staking-port-int).

If it's easier for Kurtosis to use ephemeral ports, you may just want to assign those two ports via these flags.

As in the documentation, the staking port is where the node listens for incoming p2p connections and the http port is used to run an API server.

So we have one container per Avalanche Node inside the package. Within the Docker network the ports are set to the defaults 9650 and 9651; Kurtosis then produces (by default) an ephemeral port on localhost. That is the port that the tests interact with; with test_mode true I am forcing Kurtosis to produce local port mappings that are expected

So 9650,9651 on the first node will map to 9650 and 9651 on localhost
For the second avalanche node they map to 9652 and 9653
.. so on and so forth!

I am using test_mode as the tests seem to hardcode

var ( DefaultLocalNodeURI = "http://127.0.0.1:9650" NodeURIs = []string{DefaultLocalNodeURI, "http://127.0.0.1:9652", "http://127.0.0.1:9654", "http://127.0.0.1:9656", "http://127.0.0.1:9658"} )

and I didn't want to change your code base too much! There's not much extra effort for Kurtosis here!

Perhaps I can make the load test a multi node load test based on what I see here

tests/load/load_test.go 56: rpcEndpoints := make([]string, 0, len(utils.NodeURIs)) 57: for _, uri := range []string{utils.DefaultLocalNodeURI} { // TODO: use NodeURIs instead, hack until fixing multi node in a network behavior

I have changed the load test slightly and now I pass utils.NodeURIs & the test seems to pass.

I have reverted the multi node load test as it flaked on one run - https://github.com/h4ck3rk3y/subnet-evm/actions/runs/5093958012/jobs/9157181604

## Description: Currently Starlark run remote package, run package and run script blocking calls on the SDK have the following behavior: 1. If an error happens on the APIC side, we return `(*StarlarkRunResult == nil, error != nil)` 2. If an error happens on the Starlark side, we return `(*StarlarkRunResult != nil, error == nil)`, with either `StarlarkRunResult.[InterpretationError, ValidationErrors, ExecutionError] != nil` 3. If no errors happen on the Starlark side, we return `(*StarlarkRunResult != nil, error == nil)` with all `StarlarkRunResult.[InterpretationError, ValidationErrors, ExecutionError] == nil` This behavior is not very ergonomic, given that most people are only interested if the Starlark succeeded or not, they should be able to do this with a simple `err != nil` check, which is the Go idiomatic way of dong it. If the user wants to investigate further the interpreted instructions, the breakdown of which phase failed, etc, this will still be accessible via the `StarlarkRunResult`. ## Is this change user facing? YES   ## References (if applicable):  ava-labs/subnet-evm#649

aaronbuchwald · 2023-05-25T18:02:55Z

tests/utils/node_launcher.go

+		fmt.Println(fmt.Printf("Destroying enclave with id '%v'", enclaveId))
+		if err = kurtosisCtx.StopEnclave(ctx, enclaveId); err != nil {
+			fmt.Printf("An error occurred while stopping the enclave with id '%v'\n", enclaveId)
+		}
+		if err = kurtosisCtx.DestroyEnclave(ctx, enclaveId); err != nil {
+			fmt.Printf("An error occurred while cleaning up the enclave with id '%v'\n", enclaveId)
+		}


should these errors be propagated somewhere?

I think failure to teardown should be treated as unexpected behavior and cause the test to fail. If this causes tests to flake intermittently, then I'd consider it a bug.

Makes sense! I am now asserting that the teardown has no errors.

aaronbuchwald · 2023-05-25T19:03:30Z

tests/utils/node_launcher.go

+const (
+	isPartitioningEnabled    = false
+	enclaveIdPrefix          = "test"
+	avalancheStarlarkPackage = "github.com/kurtosis-tech/avalanche-package"


Is this running a static version of AvalancheGo/Subnet-EVM? For CI, we would want to build a Docker image of AvalancheGo that has the VM binary present in the plugin directory and launch a node running that otherwise it seems that this will not actually be testing the latest code.

I've added image building as a step in the CI test and I pass the built image in the avalanche package now using the :test tag via the avalanchego_image argument

aaronbuchwald · 2023-05-25T19:15:14Z

scripts/run_ginkgo.sh


 # This script assumes that an AvalancheGo and Subnet-EVM binaries are available in the standard location
 # within the $GOPATH
-# The AvalancheGo and PluginDir paths can be specified via the environment variables used in ./scripts/run.sh.


Since this comes with new requirements (install kurtosis cli), would it be possible to add or link instructions that are necessary to run the e2e tests with this change?

I have added instructions about installing Kurtosis & restarting the engine. Let me know what you think and if there's a better spot to put this apart from this script.

Signed-off-by: Gyanendra Mishra <[email protected]>

Co-authored-by: aaronbuchwald <[email protected]> Signed-off-by: Gyanendra Mishra <[email protected]>

Signed-off-by: Gyanendra Mishra <[email protected]>

aaronbuchwald · 2023-09-12T15:23:31Z

Closing in favor of migrating to https://github.com/ava-labs/avalanchego/tree/master/tests/e2e#avalanche-e2e-test-suites

* add regression test * apply fix * use cmp * reorder equal params * improve readability by removing extra var (#650)

h4ck3rk3y requested review from aaronbuchwald, anusha-ctrl, ceyonur and darioush as code owners May 12, 2023 12:37

aaronbuchwald reviewed May 22, 2023

View reviewed changes

.github/workflows/ci.yml Outdated Show resolved Hide resolved

aaronbuchwald reviewed May 22, 2023

View reviewed changes

tests/utils/node_launcher.go Outdated Show resolved Hide resolved

h4ck3rk3y requested a review from aaronbuchwald May 22, 2023 21:20

aaronbuchwald reviewed May 22, 2023

View reviewed changes

victorcolombo mentioned this pull request May 23, 2023

feat: Return error on SDK if Starlark run on any step kurtosis-tech/kurtosis#634

Merged

h4ck3rk3y requested a review from aaronbuchwald May 24, 2023 10:21

aaronbuchwald reviewed May 25, 2023

View reviewed changes

h4ck3rk3y force-pushed the gyani/kurtosis branch from 0241846 to d082994 Compare May 26, 2023 13:50

h4ck3rk3y and others added 11 commits May 26, 2023 14:51

use Kurtosis to spin up nodes instead of script

f004dd4

cleanup ci

250317a

added new line

f4d2b99

Signed-off-by: Gyanendra Mishra <[email protected]>

use start instead of restart

571b1f2

Signed-off-by: Gyanendra Mishra <[email protected]>

error is the last thing to be returned

c133539

fixed framing of KT job

11450f2

Update tests/utils/node_launcher.go

a24ee17

Co-authored-by: aaronbuchwald <[email protected]> Signed-off-by: Gyanendra Mishra <[email protected]>

propagate various error types

2b2bc20

use the single error abstraction

2d86c0d

pin 0.77.0

74a1dea

ran go mod tidy

29c6cb4

h4ck3rk3y force-pushed the gyani/kurtosis branch from d082994 to 29c6cb4 Compare May 26, 2023 13:51

h4ck3rk3y added 2 commits May 26, 2023 14:57

try running after building image instead

255b8d7

fix arguments

c9de7fb

h4ck3rk3y added 15 commits May 26, 2023 15:48

fix json

c77873f

fix json for real

14d5bc0

correctly populate node id

86d6da8

actually correctly populate node id

34338c7

this should work

2741b09

test out multi node behavior

d0dac57

added note about cli installation

71f1c3d

fix node count argument

20a64ab

use a constant instead

19478f7

use the right parameter for image passing

406f885

propagate tear down errors

ebf8ee4

Add notes about test docker image

89ab874

test health of every node in multi node test

5eda8ed

revert load test to single node due to flake

269ec3c

Update run_ginkgo.sh

de07e1f

Signed-off-by: Gyanendra Mishra <[email protected]>

h4ck3rk3y requested a review from aaronbuchwald May 26, 2023 21:33

Update node_launcher.go

a9824c2

Signed-off-by: Gyanendra Mishra <[email protected]>

aaronbuchwald closed this Sep 12, 2023

ceyonur added a commit that referenced this pull request Mar 4, 2025

fix modifying common.Big1 (#649)

2fec97b

* add regression test * apply fix * use cmp * reorder equal params * improve readability by removing extra var (#650)

use Kurtosis instead of the run script #649

use Kurtosis instead of the run script #649

Uh oh!

Conversation

h4ck3rk3y commented May 12, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why this should be merged

How this works

How this was tested

How is this documented

Uh oh!

h4ck3rk3y commented May 12, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

h4ck3rk3y May 26, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

h4ck3rk3y May 26, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

h4ck3rk3y May 26, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aaronbuchwald commented Sep 12, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

h4ck3rk3y commented May 12, 2023 •

edited

Loading

h4ck3rk3y May 26, 2023 •

edited

Loading

h4ck3rk3y May 26, 2023 •

edited

Loading

h4ck3rk3y May 26, 2023 •

edited

Loading