R4R: Test that localnet-start makes at least 10 blocks #2018

jackzampolin · 2018-08-14T02:04:47Z

This PR adds the check to the Circle CI script to ensure that the make localnet-start command starts a testnet that can produce at least 10 blocks. cc @ValarDragon

codecov · 2018-08-14T02:56:16Z

Codecov Report

Merging #2018 into develop will not change coverage.
The diff coverage is 0%.

@@           Coverage Diff            @@
##           develop    #2018   +/-   ##
========================================
  Coverage    64.86%   64.86%           
========================================
  Files          115      115           
  Lines         6862     6862           
========================================
  Hits          4451     4451           
  Misses        2127     2127           
  Partials       284      284

ValarDragon · 2018-08-14T03:02:40Z

Looks like the docker container isn't printing out the NUMBER OF BLOCKS. I think its expecting the localnet to be started on the same docker instance. I don't know too much about docker to know if thats doable. But now its building :)

Closes #1955

If dep already sees its scratch directory (.vendor-new), dep ensure fails. This rm -rf's that directory so make get_vendor_deps doesn't fail.

cwgoes

Great idea! Notes:

Can we not rely on docker pull? Depends on an external repository (security threat) & often slow. Would prefer a local image with proper layer caching.
Do we need to use Docker at all?

jackzampolin · 2018-08-14T14:29:44Z

@cwgoes I was trying to reuse the existing make localnet-start command to make this easy and that uses docker. One other option could be to just run the script from CI w/o a docker container. I can try and chase down that approach later. As for the external dependency, here is the repo: https://github.com/jackzampolin/gaiad-poll

Its really just 2 files. If we just run the script in circle, then we could reduce that to just the script. I think the hard part here is figuring out the docker networking. I'll try both approaches.

alexanderbez · 2018-08-14T14:40:56Z

afaik, the docker-compose file just makes it very easy to start/stop a network, tail logs, etc...

ValarDragon · 2018-08-14T15:08:03Z

I think using docker is fine here, it's part of a test. If putting the docker file in the sdk is doable, let's do that. I'd prefer to get this working with docker pull, and we make an issue for fixing that dep later. It's not a huge security concern because worst case is that one of our circle ci tests passes when it shouldn't have. (It's running in its own container and persists nothing)

jackzampolin · 2018-08-14T23:15:07Z

So after doing a bit of testing here and playing around with circle's cool ssh feature (its cool). When running the docker-compose up -d step the containers failed to start. They exited with the message:

The binary gaiad cannot be found. Please add the binary to the shared folder. Please use the BINARY environment variable if the name of the binary is not 'gaiad' E.g.: -e BINARY=gaiad_my_test_version

This is kinda weird because the step before builds the linux binary. When I look where its supposed to be (./build/gaiad), it exists. This lead me to find out that Docker mounts (something we rely on for mounting the built binary) fail in the Circle 2.0 environment. This specific test requires the machine environment which supports docker mounts.

Running it in the machine environment caused a TON of file permissions issues. Womp! After moving the repo path to somewhere w/o the file permissions issues and eliminating the restore from cache piece I started running into $GOPATH issues. Once I started working w/in the go path setup on the build VM and got the containers running I finally got the script working and the containers running! Yay!

Now I'm running into this fun issue which looks like a Makefile issue 😢

LEDGER_ENABLED=false GOOS=linux GOARCH=amd64 make build
make[1]: Entering directory `/home/circleci/.go_workspace/src/github.com/cosmos/cosmos-sdk'
TMP_BUILD_TAGS := netgo ledger
make[1]: TMP_BUILD_TAGS: Command not found
make[1]: *** [check-ledger] Error 127
make[1]: Leaving directory `/home/circleci/.go_workspace/src/github.com/cosmos/cosmos-sdk'
make: *** [build-linux] Error 2
Exited with code 2

Also this is going to need a rebase. Would love some feedback on the approach.

zramsay · 2018-08-14T23:38:35Z

related: tendermint/tendermint#1532
once this PR is merged and is smooth, i'll add the same same to tendermint

jackzampolin · 2018-08-14T23:51:26Z

I need an extra set of 👀 on this from @greg-szabo

jackzampolin · 2018-08-15T00:11:08Z

Alright! ht to @greg-szabo for helping me find that <tab>s in the Makefile was causing the build failure. The test is currently unable to get past the 2nd block because of the nondeterminism issue we debugged yesterday. So this is ready for review!

cwgoes

Agreed with the concept, still a few concerns about implementation, see comments.

cwgoes · 2018-08-15T10:08:11Z

Makefile

-ifndef GCC
-$(error "gcc not installed for ledger support, please install or set LEDGER_ENABLED to false in the Makefile")
-endif
+   	ifeq ($(UNAME_S),OpenBSD)


Is this change intended to be in this PR?

cwgoes · 2018-08-15T10:08:20Z

PENDING.md


 BUG FIXES
+*  \#1988 Make us compile on OpenBSD (disable ledger) [#1988] (https://github.com/cosmos/cosmos-sdk/issues/1988)


Is this change intended to be in this PR?

cwgoes · 2018-08-15T10:08:27Z

PENDING.md

@@ -38,6 +38,7 @@ BREAKING CHANGES
 * [types] sdk.NewCoin now takes sdk.Int, sdk.NewInt64Coin takes int64
 * [cli] #1551: Officially removed `--name` from CLI commands
 * [cli] Genesis/key creation (`init`) now supports user-provided key passwords
+* [cli] unsafe_reset_all, show_validator, and show_node_id have been renamed to unsafe-reset-all, show-validator, and show-node-id


Is this change intended to be in this PR?

No :( this is going to need another PR.

cwgoes · 2018-08-15T10:08:34Z

PENDING.md

@@ -73,8 +74,10 @@ IMPROVEMENTS
 * [x/stake] \#1815 Sped up the processing of `EditValidator` txs.
 * [server] \#1930 Transactions indexer indexes all tags by default.
 * [x/stake] \#2000 Added tests for new staking endpoints
+* [tools] Make get_vendor_deps deletes `.vendor-new` directories, in case scratch files are present.


Is this change intended to be in this PR?

cwgoes · 2018-08-15T10:11:42Z

.circleci/config.yml

+            make get_vendor_deps
+            make build-linux
+            make localnet-start
+            docker run -it --network=host jackzampolin/gaiad-poll:latest ./poll.sh 40 5 10 localhost


I still am not a fan of this. You're right that it's just in CI, which we don't give a trusted role (presently), but it's hard to debug - now if something breaks where do I go to figure out what's in your Docker image?

Tried going to https://hub.docker.com/r/jackzampolin/gaiad-poll/ but that doesn't appear to have a Dockerfile

Can we just run the poll script natively in CI?

jackzampolin · 2018-08-16T01:02:49Z

Closing this in favor of #2057

With Make 4.3+, the empty whitespace does not seem to work as originally intended. This causes build tags to be "netgo ledger," on Ubuntu 22.04 and other systems that include the newer Make version. The build tags were intended as "netgo,ledger" which can be observed on Make 4.2 (shipped with Ubuntu 20.04). This change swaps out the `+=` use in favor of an explicit `:=`. Ref: https://www.gnu.org/software/make/manual/html_node/Appending.html Closes cosmos/gaia#2017. Co-authored-by: Marius Poke <marius.poke@posteo.de> Co-authored-by: Milan Mulji <98309852+mmulji-ic@users.noreply.github.com>

Add circle job to run the localnet to 10 blocks

978c082

jackzampolin requested review from cwgoes, ebuchman and rigelrozanski as code owners August 14, 2018 02:04

add a bit of debug

da3508a

ValarDragon added the wip label Aug 14, 2018

try fixing cache restoration

c48bf4e

ValarDragon force-pushed the jack/localnet-ci branch from 26bd636 to c48bf4e Compare August 14, 2018 02:59

mslipper and others added 2 commits August 14, 2018 10:23

Merge PR #1984: Replace _ with - in command names

8fa2c3b

Closes #1955

Merge PR #2016: dep: Fix error with scratch directory

9997ea2

If dep already sees its scratch directory (.vendor-new), dep ensure fails. This rm -rf's that directory so make get_vendor_deps doesn't fail.

cwgoes suggested changes Aug 14, 2018

View reviewed changes

Merge PR #1989: Openbsd support

0adbd60

jackzampolin added 13 commits August 14, 2018 14:45

Try printing message

c6176a0

Fix targeting

bd13676

Fix issue where binary wasn't in the proper place

a1d1ff2

Try building binary in folder

360ca24

Switch to using docker machine to enable mounts 🙏

4e6fa99

Fix yaml

af5e46f

Remove docker step as it is not required by machine

9e73097

Address issues introduced by new build environment

ca0fe01

Try another way

d256bc2

Add gopath

a9e64c1

Fix gopath issues

cdfacbc

Run localnet on every commit through CI

687e6d1

Enable runing localnet during CI

4b3b748

Enable running localnet on each push with CI

136ed09

jackzampolin requested a review from zramsay as a code owner August 14, 2018 23:07

jackzampolin added 2 commits August 14, 2018 16:26

Try building binary manually

58d2237

Fix syntax error

e743152

Fix makefile issue

dd3222e

jackzampolin added ready-for-review T: CI and removed wip labels Aug 15, 2018

jackzampolin changed the title ~~Add job to circle to test localnet~~ R4R: Test that localnet-start makes at least 10 blocks Aug 15, 2018

jackzampolin mentioned this pull request Aug 15, 2018

Local testnet as a (docker-compose) daemon #2028

Closed

9 tasks

cwgoes suggested changes Aug 15, 2018

View reviewed changes

jackzampolin mentioned this pull request Aug 16, 2018

Run make localnet-start in CI and ensure network reaches at least 10 blocks #2057

Closed

7 tasks

jackzampolin closed this Aug 16, 2018

jackzampolin deleted the jack/localnet-ci branch August 16, 2018 01:02

jackzampolin mentioned this pull request Aug 16, 2018

Run localnet on every commit ensure network reaches at least 10 blocks #2067

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

R4R: Test that localnet-start makes at least 10 blocks #2018

R4R: Test that localnet-start makes at least 10 blocks #2018

jackzampolin commented Aug 14, 2018

codecov bot commented Aug 14, 2018 •

edited

Loading

ValarDragon commented Aug 14, 2018 •

edited

Loading

cwgoes left a comment

jackzampolin commented Aug 14, 2018

alexanderbez commented Aug 14, 2018

ValarDragon commented Aug 14, 2018 •

edited

Loading

jackzampolin commented Aug 14, 2018 •

edited

Loading

zramsay commented Aug 14, 2018

jackzampolin commented Aug 14, 2018

jackzampolin commented Aug 15, 2018

cwgoes left a comment

cwgoes Aug 15, 2018

jackzampolin Aug 15, 2018

cwgoes Aug 15, 2018

jackzampolin Aug 15, 2018

cwgoes Aug 15, 2018

jackzampolin Aug 15, 2018

cwgoes Aug 15, 2018

cwgoes Aug 15, 2018

jackzampolin commented Aug 16, 2018


		BUG FIXES
		* \#1988 Make us compile on OpenBSD (disable ledger) [#1988] (https://github.com/cosmos/cosmos-sdk/issues/1988)

R4R: Test that localnet-start makes at least 10 blocks #2018

R4R: Test that localnet-start makes at least 10 blocks #2018

Conversation

jackzampolin commented Aug 14, 2018

codecov bot commented Aug 14, 2018 • edited Loading

Codecov Report

ValarDragon commented Aug 14, 2018 • edited Loading

cwgoes left a comment

Choose a reason for hiding this comment

jackzampolin commented Aug 14, 2018

alexanderbez commented Aug 14, 2018

ValarDragon commented Aug 14, 2018 • edited Loading

jackzampolin commented Aug 14, 2018 • edited Loading

zramsay commented Aug 14, 2018

jackzampolin commented Aug 14, 2018

jackzampolin commented Aug 15, 2018

cwgoes 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

jackzampolin commented Aug 16, 2018

codecov bot commented Aug 14, 2018 •

edited

Loading

ValarDragon commented Aug 14, 2018 •

edited

Loading

ValarDragon commented Aug 14, 2018 •

edited

Loading

jackzampolin commented Aug 14, 2018 •

edited

Loading