-
Notifications
You must be signed in to change notification settings - Fork 344
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
test: minor version upgrade e2e testing #2797
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work! When I run this locally, I encounter:
$ make test-e2e
--> Running end to end tests
=== RUN TestE2ESimple
simple_test.go:34: Running simple e2e test version latest
simple_test.go:37:
Error Trace: /Users/rootulp/git/rootulp/celestia/celestia-app/test/e2e/simple_test.go:37
Error: Received unexpected error:
retrieving the Kubernetes config: stat /Users/rootulp/.kube/config: no such file or directory
Test: TestE2ESimple
--- FAIL: TestE2ESimple (0.00s)
=== RUN TestMinorVersionCompatibility
upgrade_test.go:27: skipping e2e test: E2E_VERSION not set
--- SKIP: TestMinorVersionCompatibility (0.00s)
What prerequisites do contributors need to perform prior to running make test-e2e locally? It looks like I need some kube config and I'm not sure why the the e2e test is getting skipped
Yes you need a .kube/config setup. You can ask samuel for it. Also you need the E2E environment variable to run the tests (I purposefully didn't want go test ./... to run these tests). It's best to just run |
|
no because this shouldn't be public. Ideally we work out how to support locally running the tests but we don't have that at the moment |
Codecov Report
@@ Coverage Diff @@
## main #2797 +/- ##
==========================================
+ Coverage 19.44% 19.67% +0.22%
==========================================
Files 143 144 +1
Lines 17320 17395 +75
==========================================
+ Hits 3368 3422 +54
- Misses 13633 13653 +20
- Partials 319 320 +1
|
[blocking] in this PR can we please indicate that |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
had some questions, but its super dope we're getting to use knuu for compatibility tests
test/e2e/upgrade_test.go
Outdated
} | ||
newVersion := versions.Random(r).String() | ||
t.Log("Upgrading node", "node", i%numNodes, "version", newVersion) | ||
err := testnet.Node(i % numNodes).Upgrade(newVersion) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does this function block until the node is back up?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I actually don't know. It calls something in knuu (cc @smuu)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes the current implementation blocks until the node is back up
// FIXME: skip the first node because we need them available to | ||
// submit txs | ||
if i%numNodes == 0 { | ||
continue | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is skipping the first node also the mechanism that is testing that the versions are compatible?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't quite understand. We need the first node to always be up because we're constantly submitting transactions. If txsim can't submit a transaction it exits.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see now that the test has been updated for each upgraded node to wait a few heights. I was curious what the mechanism was to ensure that different versions were running concurrently
Co-authored-by: Evan Forbes <[email protected]>
…lestia-app into cal/minor-upgrade-testing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When I run this locally, I observe:
- [not blocking] verbose logging from test e2e simple. Can we please redirect that logging so that the test results are easier to read?
- [blocking] TestMinorVersionCompatibility gets skipped. TBH not sure why b/c the Makefile looks correct but my shell doesn't ever have these values populated.
$ make test-e2e
--> Running end to end tests
=== RUN TestE2ESimple
simple_test.go:34: Running simple e2e test version latest
< verbose logging from test e2e simple>
--- PASS: TestE2ESimple (318.32s)
=== RUN TestMinorVersionCompatibility
upgrade_test.go:29: skipping e2e test: E2E_VERSION not set
--- SKIP: TestMinorVersionCompatibility (0.00s)
=== RUN TestVersionParsing
--- PASS: TestVersionParsing (0.00s)
=== RUN TestFilterMajorVersions
--- PASS: TestFilterMajorVersions (0.00s)
=== RUN TestOrder
--- PASS: TestOrder (0.00s)
=== RUN TestOrderOfReleaseCandidates
--- PASS: TestOrderOfReleaseCandidates (0.00s)
PASS
ok github.com/celestiaorg/celestia-app/test/e2e 318.931s
$ echo $E2E_VERSIONS
$ echo $E2E_VERSION
Yeah I can remove that. That was initially just to let me know it was working but should be fine to silence them now
Let me look into it |
Co-authored-by: Rootul P <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm 👍
@rootulp do you want to try it now |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
testing upgrade test now and it appears to be running 👍
Update: I quit after 10 mins b/c it didn't seem to be making progress
$ make test-e2e
--> Running end to end tests
=== RUN TestE2ESimple
simple_test.go:25:
--- SKIP: TestE2ESimple (0.00s)
=== RUN TestMinorVersionCompatibility
versions v1.0.0 v1.1.0 v1.2.0 v1.3.0 upgrade_test.go:40: Running minor version compatibility test versions v1.0.0 v1.1.0 v1.2.0 v1.3.0
upgrade_test.go:58: Starting node node 0 version v1.1.0
upgrade_test.go:58: Starting node node 1 version v1.3.0
upgrade_test.go:58: Starting node node 2 version v1.0.0
upgrade_test.go:58: Starting node node 3 version v1.2.0
^Csignal: interrupt
FAIL github.com/celestiaorg/celestia-app/test/e2e 585.399s
make: *** [test-e2e] Error 1
Co-authored-by: Rootul P <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great!
Overview
Closes: #313
Ref: #2633
This PR introduces a test for checking compatibility and the running of upgrades of minor versions.
It reads all minor versions tagged in git. Each node begins on a random version.
Each node then individually performs upgrades, going down and spinning back up on the new randomly chosen version (can be downgrades as well)
Currently we only check that their is no app version mismatch and the network is able to continually build blocks
Checklist