rename oni/lotus-soup imports to testplans/lotus-soup and go get lotus@master on trigger #4995

nonsense · 2020-11-24T18:25:47Z

This PR is:

Locking a specific Lotus version in go.mod to be used with the testplans ran on TaaS (currently based on go get lotus@master but could be changed to use the commit from CircleCI as well).
Removing imports from filecoin-project/oni.
Removing go.mod replace to Lotus as in replace github.com/filecoin-project/lotus => ../../ since the parent folder is not included with the testplan, and this doesn't work with the test plan.
Upgrading sectors from 2KiB to 8MiB - computationally sealing is similarly expensive, but this allows for future tests where we cancel transfer and restart nodes/miners in the middle of a transfer.
Upgrade deal size from 1600 bytes to 5MB.

It's worth noting that without making too many changes to the tests from one version of Lotus to another, we are sometimes getting different flakyness / different success rates. These tests were very stable for a few months, then they got broken for a month, and now they seem to pass some of the time. It is worth integrating them and running them soon as part of the Lotus test suite, so that we get them to a stable state, which is not easy since most of the logic really happens behind multiple APIs and is abstracted away from the test itself.

I am not sure how much of this is timing issues due to much lower block delays / propagation delays / etc. vs actual undergoing work on Lotus / go-fil-markets / etc.

PRIOR TO MERGE:

remove lock-lotus-version-in-testplan branch from .circleci/config.yaml

TODO:

1. e2e test is still flaky - sometimes clients detect fork (although not all clients detect it):

2020-12-07T13:45:09.016Z        WARN    chain   chain/sync.go:1418      (fork detected) synced header chain ([bafy2bzaceay2gcgqbzqumy7meq3u6yjjghyk4svdt6fetvdgpdkr6inaa7tp2] - 19) does not link to our best block ([bafy2bzacebp3ut76gfxygwqocacgykreiegg5pdqhfxqx6yfl2gsoadxtyjcc] - 1)
2020-12-07T13:45:09.017Z        WARN    chain   chain/sync.go:1418      (fork detected) synced header chain ([bafy2bzacea5dzdbsj3azff7dvdulqffedw5eekk6jbs5wcqrcadbfzhhmqsey] - 19) does not link to our best block ([bafy2bzacebp3ut76gfxygwqocacgykreiegg5pdqhfxqx6yfl2gsoadxtyjcc] - 1)
2020-12-07T13:45:09.021Z        ERROR   chain   chain/sync_manager.go:253       error during sync in [bafy2bzacea5dzdbsj3azff7dvdulqffedw5eekk6jbs5wcqrcadbfzhhmqsey]: collectChain failed: failed to sync fork: synced chain forked at genesis, refusing to sync; incoming: [bafy2bzacedcxdw6n62i32dmjjf6dr4ujzfflzr3zhfmjkxcre44aoj4aonxxa bafy2bzaceadvi5vs5e6pkzeolhwsel6n3mkknxm4ipkiflcfwskjco5sohxyg]
2020-12-07T13:45:09.021Z        ERROR   chain   chain/sync_manager.go:253       error during sync in [bafy2bzaceay2gcgqbzqumy7meq3u6yjjghyk4svdt6fetvdgpdkr6inaa7tp2]: collectChain failed: failed to sync fork: synced chain forked at genesis, refusing to sync; incoming: [bafy2bzacedcxdw6n62i32dmjjf6dr4ujzfflzr3zhfmjkxcre44aoj4aonxxa bafy2bzaceadvi5vs5e6pkzeolhwsel6n3mkknxm4ipkiflcfwskjco5sohxyg]

2. e2e test is still flaky - sometimes miners supposedly fail to lock funds as pledge:

miner

2020-12-07T13:31:27.425Z        WARN    vm      vm/runtime.go:332       Abortf: failed to lock balance: failed to lock client funds: not enough balance to lock for addr t0102: escrow balance 0 < locked 0 + required 2561092000000          2020-12-07T13:31:27.425Z        WARN    vm      vm/runtime.go:145       VM.Call failure: failed to lock balance: failed to lock client funds: not enough balance to lock for addr t0102: escrow balance 0 < locked 0 + required 2561092000000
(RetCode=19):
    github.com/filecoin-project/specs-actors/v2/actors/builtin.RequireNoErr
        /go/pkg/mod/github.com/filecoin-project/specs-actors/v2@v2.3.2/actors/builtin/shared.go:60
2020-12-07T13:31:27.425Z        WARN    vm      vm/vm.go:528    Send actor error        {"from": "t0104", "to": "t05", "nonce": 1, "method": "4", "height": "28", "error": "failed to lock balance: failed to lock client funds: not enough ba
lance to lock for addr t0102: escrow balance 0 < locked 0 + required 2561092000000 (RetCode=19):\n    github.com/filecoin-project/specs-actors/v2/actors/builtin.RequireNoErr\n        /go/pkg/mod/github.com/filecoin-project/specs-actors/v2
@v2.3.2/actors/builtin/shared.go:60"}

client

{"ts":1607347882314916921,"msg":"","group_id":"clients","run_id":"bv72sdd5p7acvjljrr9g","event":{"message_event":{"message":"deal state: StorageDealClientFunding"}}}
2020-12-07T13:31:27.019Z        WARN    chainstore      store/store.go:282      chain head sub exit loop
2020-12-07T13:31:27.019Z        WARN    chainstore      store/store.go:282      chain head sub exit loop
2020-12-07T13:31:28.155Z        ERROR   storagemarket_impl      clientstates/client_states.go:304       deal bafyreif447ug2lstf5abwxzzjnsrh3flmhfcdq2sra3jhnhc36xv5zagie failed: deal failed: (State=26) error calling node: publishing deal: GasEstimateMessageGas error: estimating gas used: message execution failed: exit 19, reason: failed to lock balance: failed to lock client funds: not enough balance to lock for addr t0102: escrow balance 0 < locked 0 + required 2561092000000 (RetCode=19)
{"ts":1607347889019236712,"msg":"","group_id":"clients","run_id":"bv72sdd5p7acvjljrr9g","event":{"message_event":{"message":"got tipset: height 28"}}}

3. e2e test is flaky: sometimes one of the clients blocks in StorageDealAwaitingPreCommit state
4. WindowPoSt deadlines appear to have a one-off error with the configs in init.go. (This is not a problem for any of the existing tests, but worth figuring out and fixing. For the current tests, usually by the time the first WindowPoSt is to be submitted the deals should have been sealed by miners and retrieved from the clients).

nonsense · 2020-11-24T18:27:25Z

testplans/lotus-soup/go.mod

@@ -43,5 +43,3 @@ require (
 replace github.com/filecoin-project/filecoin-ffi => ../../extern/filecoin-ffi

 replace github.com/supranational/blst => ../../extern/blst
-
-replace github.com/filecoin-project/lotus => ../../


I am not sure this plays well with Testground. filecoin-ffi and blst are bundled with the base image (as they don't change often), but we are not including ../../ when we ship the test plan to Testground, so I am a bit puzzled how this compiled and worked over the last few days.

nonsense · 2020-12-07T14:55:53Z

I am marking this as ready for review, because I think it is an improvement to master, even if the deals e2e is still flaky (issues described in the TODO list).

I think that many of the issues described are not directly related to the test plan, and something that might involve a bit more effort to address in the underlying APIs within Lotus.

For reference the lotus-soup/deals e2e test is very similar to the TestDealFlow test in Lotus, but AFAICT TestDealFlow is also not using multiple clients/miners, so might as well not hit the same problems as are visible here.

nonsense · 2020-12-07T14:57:12Z

testplans/docker-images/Dockerfile.oni-buildbase


 FROM golang:${GO_VERSION}-buster

 RUN apt-get update && apt-get install -y ca-certificates llvm clang mesa-opencl-icd ocl-icd-opencl-dev jq gcc git pkg-config bzr libhwloc-dev

-ARG FILECOIN_FFI_COMMIT=1985275547f222e8c97a8ab70b5cc26bc1fa50b1
+ARG FILECOIN_FFI_COMMIT=1d9cb3e8ff53f51f9318fc57e5d00bc79bdc0128


We might want to extract this from the Docker image and compile on every test run. filecoin-ffi library is changing much more often than originally anticipated, so the fact that we have to maintain the correct version in the base image we use for the testplan is not ideal.

Yes, agree. In fact, it's already outdated since you submitted the PR and I'm reviewing this. We can rely on docker caching.

We can rely on docker caching.

Not sure what you mean with that. Could you elaborate?

I think we should just rely on Lotus and clone Lotus for a given commit. We discarded that option early on, but time and time again Lotus is changing underneath these tests and we have to replicate the Lotus setup - for example we no longer need blst to be included as an extern.

nonsense · 2020-12-07T14:58:23Z

testplans/docker-images/Dockerfile.oni-runtime-debug


-ARG LOTUS_COMMIT=1a170e18a
+ARG LOTUS_COMMIT=b13226bc2f8a0233b3beb9bd18ab266547bc29c7


This is necessary for the debug image as we bundle lotus-miner and lotus binaries, so that we can inspect miners and clients while tests are running. This is NOT the version under test, but if it gets old enough, lotus and lotus-miner might stop working (due to CBOR serialisation changes for example).

Would it be worth rescuing the change log from README-old-from-oni.md? To keep tabs on the changes we're making.

It would be useful to document what this image is for. Maybe add comments to the top of the Dockerfile?

(also to the others)

Would it be worth rescuing the change log from README-old-from-oni.md? To keep tabs on the changes we're making.

I'd rather lock these tests with a specific Lotus commit. The initial thinking that "filecoin-ffi" does not change often is just not correct at this stage, we have updated it tens of times, it should be regarded as any other dependency in the go.mod of Lotus in my opinion.

nonsense · 2020-12-07T14:59:02Z

testplans/lotus-soup/_compositions/baseline-k8s-1-1.toml

@@ -0,0 +1,67 @@
+[metadata]


I am bundling a few more composition files for easier debugging, when we notice that tests are broken / flaky.

nonsense · 2020-12-07T14:59:32Z

testplans/lotus-soup/_compositions/baseline-k8s-3-2.toml

@@ -26,7 +26,7 @@
  miners = "2"
  genesis_timestamp_offset = "0"
  balance = "20000000" # These balances will work for maximum 100 nodes, as TotalFilecoin is 2B
-  sectors = "10"
+  sectors = "4"


We are moving to 8MiB sectors, so no need to preseal 10 sectors when starting the miners, as it takes unnecessary time.

nonsense · 2020-12-07T15:00:35Z

testplans/lotus-soup/init.go

+	_ = log.SetLogLevel("*", "DEBUG")
+	_ = log.SetLogLevel("dht", "WARN")
+	_ = log.SetLogLevel("swarm2", "WARN")
+	_ = log.SetLogLevel("addrutil", "WARN")
+	_ = log.SetLogLevel("stats", "WARN")


I've enabled DEBUG logging by default, so that we can monitor TaaS and try to fix flakiness continuously, and also reduced verbosity on some of the louder subsystems.

nonsense · 2020-12-07T15:04:37Z

Marking @vyzo and @raulk as they are familiar with these tests.

raulk

LGTM. Needs updating FFI and possibly BLST in the images (we really do need to find a better system; as @nonsense mentioned, we expected these modules to be mostly static, but they change more frequently than anticipated).

Also, needs a merge from master and a dependency upgrade, I think.

raulk · 2021-01-26T12:52:34Z

testplans/docker-images/Dockerfile.oni-buildbase


 FROM golang:${GO_VERSION}-buster

 RUN apt-get update && apt-get install -y ca-certificates llvm clang mesa-opencl-icd ocl-icd-opencl-dev jq gcc git pkg-config bzr libhwloc-dev

-ARG FILECOIN_FFI_COMMIT=1985275547f222e8c97a8ab70b5cc26bc1fa50b1
+ARG FILECOIN_FFI_COMMIT=1d9cb3e8ff53f51f9318fc57e5d00bc79bdc0128


Yes, agree. In fact, it's already outdated since you submitted the PR and I'm reviewing this. We can rely on docker caching.

raulk · 2021-01-26T12:53:19Z

testplans/docker-images/Dockerfile.oni-buildbase


 FROM golang:${GO_VERSION}-buster

 RUN apt-get update && apt-get install -y ca-certificates llvm clang mesa-opencl-icd ocl-icd-opencl-dev jq gcc git pkg-config bzr libhwloc-dev

-ARG FILECOIN_FFI_COMMIT=1985275547f222e8c97a8ab70b5cc26bc1fa50b1
+ARG FILECOIN_FFI_COMMIT=1d9cb3e8ff53f51f9318fc57e5d00bc79bdc0128
 ARG FFI_DIR=/extern/filecoin-ffi

 ARG BLST_COMMIT=1cbb16ed9580dcd3e9593b71221fcf2a048faaef


Same as above? (for FFI)

raulk · 2021-01-26T12:55:37Z

testplans/docker-images/Dockerfile.oni-runtime-debug


-ARG LOTUS_COMMIT=1a170e18a
+ARG LOTUS_COMMIT=b13226bc2f8a0233b3beb9bd18ab266547bc29c7


Would it be worth rescuing the change log from README-old-from-oni.md? To keep tabs on the changes we're making.

raulk · 2021-01-26T12:56:19Z

testplans/docker-images/Dockerfile.oni-runtime-debug


-ARG LOTUS_COMMIT=1a170e18a
+ARG LOTUS_COMMIT=b13226bc2f8a0233b3beb9bd18ab266547bc29c7


It would be useful to document what this image is for. Maybe add comments to the top of the Dockerfile?

raulk · 2021-01-26T13:01:06Z

testplans/docker-images/Dockerfile.oni-runtime-debug


-ARG LOTUS_COMMIT=1a170e18a
+ARG LOTUS_COMMIT=b13226bc2f8a0233b3beb9bd18ab266547bc29c7


(also to the others)

raulk · 2021-01-26T13:03:14Z

testplans/lotus-soup/deals_e2e.go

+	// give some time to the miner, otherwise, we get errors like:
+	// deal errored deal failed: (State=26) error calling node: publishing deal: GasEstimateMessageGas
+	// error: estimating gas used: message execution failed: exit 19, reason: failed to lock balance: failed to lock client funds: not enough balance to lock for addr t0102: escrow balance 0 < locked 0 + required 640297000 (RetCode=19)
+	time.Sleep(50 * time.Second)


Ideally the miner would be able to publish an event when it's ready to start accepting deals.

…in-testplan

.circleci/config.yml

magik6k · 2021-01-30T11:42:33Z

testplans/Makefile

-	docker build -t "iptestground/oni-runtime:v5" -f "docker-images/Dockerfile.oni-runtime" "docker-images"
+	docker build -t "iptestground/oni-buildbase:v13-lotus" -f "docker-images/Dockerfile.oni-buildbase" "docker-images"
+	docker build -t "iptestground/oni-runtime:v7" -f "docker-images/Dockerfile.oni-runtime" "docker-images"
+	docker build -t "iptestground/oni-runtime:v8-debug" -f "docker-images/Dockerfile.oni-runtime-debug" "docker-images"


Do we want to keep this -debug version?

Yes, we want to keep it, -debug includes the lotus binary and the lotus-miner binary, so it is useful to have it. Actually at the moment the manifest.toml of the testplan refers to the debug version.

Co-authored-by: Łukasz Magiera <magik6k@users.noreply.github.com>

…otus-version-in-testplan rename oni/lotus-soup imports to testplans/lotus-soup and go get lotus@master on trigger

nonsense commented Nov 24, 2020

View reviewed changes

nonsense added 6 commits December 7, 2020 11:48

rename oni/lotus-soup to testplans/lotus-soup and lock lotus version

c543a10

update base image with latest filecoin-ffi

86af05b

add more time to miner to bootstrap

a53fd82

dummy delay for everyone but first miner

d0599ff

8MB sectors ; 5MB deal

b1dcf5f

baseline k8s configs

2829245

nonsense force-pushed the lock-lotus-version-in-testplan branch from b0fdf9a to bd6d814 Compare December 7, 2020 13:06

nonsense added 2 commits December 7, 2020 14:14

update debug image to use 8MiB sectors

064b036

reduce sectors ; less verbose logging for some subsystems

1cfd955

nonsense force-pushed the lock-lotus-version-in-testplan branch 4 times, most recently from 12e9e8a to 301e2d0 Compare December 7, 2020 14:41

go get lotus@master in circleci

3c291dd

nonsense changed the title ~~rename oni/lotus-soup imports to testplans/lotus-soup and lock lotus version~~ rename oni/lotus-soup imports to testplans/lotus-soup and go get lotus@master on trigger Dec 7, 2020

nonsense marked this pull request as ready for review December 7, 2020 14:52

nonsense requested review from Kubuxu, magik6k and whyrusleeping as code owners December 7, 2020 14:52

nonsense commented Dec 7, 2020

View reviewed changes

nonsense requested review from raulk and vyzo December 7, 2020 15:03

resolve conflicts with master

4d69947

nonsense force-pushed the lock-lotus-version-in-testplan branch from 301e2d0 to 4d69947 Compare December 11, 2020 09:13

baseline 1 miner 1 cluster composition for local:docker

6552771

nonsense mentioned this pull request Jan 26, 2021

fix stale imports in testplans/lotus-soup. #5425

Merged

raulk approved these changes Jan 26, 2021

View reviewed changes

raulk mentioned this pull request Jan 28, 2021

Snake a context through the Chain-blockstore creation #5282

Merged

magik6k and others added 5 commits January 28, 2021 19:50

Merge remote-tracking branch 'origin/master' into lock-lotus-version-…

4f128ac

…in-testplan

remove blst replace; update go.mod to use lotus@master

c095719

bump up go to 1.15

8ed726f

upgrade docker; upgrade filecoin-ffi; bump up image versions

1cca0ff

merge master; resolve conflicts

52a6106

magik6k reviewed Jan 30, 2021

View reviewed changes

Update .circleci/config.yml

6c045b4

Co-authored-by: Łukasz Magiera <magik6k@users.noreply.github.com>

magik6k approved these changes Feb 8, 2021

View reviewed changes

magik6k merged commit 0afe732 into master Feb 8, 2021

magik6k deleted the lock-lotus-version-in-testplan branch February 8, 2021 14:39

nonsense mentioned this pull request Feb 8, 2021

Error running lotus-soup testplans #5548

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rename oni/lotus-soup imports to testplans/lotus-soup and go get lotus@master on trigger #4995

rename oni/lotus-soup imports to testplans/lotus-soup and go get lotus@master on trigger #4995

nonsense commented Nov 24, 2020 •

edited

Loading

nonsense Nov 24, 2020

nonsense commented Dec 7, 2020

nonsense Dec 7, 2020

raulk Jan 26, 2021

nonsense Jan 26, 2021

nonsense Dec 7, 2020

raulk Jan 26, 2021

raulk Jan 26, 2021

raulk Jan 26, 2021

nonsense Jan 26, 2021

nonsense Dec 7, 2020

nonsense Dec 7, 2020

nonsense Dec 7, 2020

nonsense commented Dec 7, 2020

raulk left a comment

raulk Jan 26, 2021

raulk Jan 26, 2021

raulk Jan 26, 2021

raulk Jan 26, 2021

raulk Jan 26, 2021

raulk Jan 26, 2021

magik6k Jan 30, 2021

nonsense Jan 30, 2021


		ARG LOTUS_COMMIT=1a170e18a
		ARG LOTUS_COMMIT=b13226bc2f8a0233b3beb9bd18ab266547bc29c7

rename oni/lotus-soup imports to testplans/lotus-soup and go get lotus@master on trigger #4995

rename oni/lotus-soup imports to testplans/lotus-soup and go get lotus@master on trigger #4995

Conversation

nonsense commented Nov 24, 2020 • edited Loading

Choose a reason for hiding this comment

nonsense commented Dec 7, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nonsense commented Dec 7, 2020

raulk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nonsense commented Nov 24, 2020 •

edited

Loading