Skip to content
This repository has been archived by the owner on Nov 30, 2021. It is now read-only.

fix errors and make testnet work #403

Merged
merged 8 commits into from
Jul 29, 2020
Merged

fix errors and make testnet work #403

merged 8 commits into from
Jul 29, 2020

Conversation

araskachoi
Copy link
Contributor

changed alpine -> ubuntu (to make binaries work)
changed a few lines for docker image names in make file

Closes: #XXX

Description


For contributor use:

  • Targeted PR against correct branch (see CONTRIBUTING.md)
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Code follows the module structure standards.
  • Wrote unit and integration tests
  • Updated relevant documentation (docs/) or specification (x/<module>/spec/)
  • Added relevant godoc comments.
  • Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
  • Re-reviewed Files changed in the Github PR explorer

For admin use:

  • Added appropriate labels to PR (ex. WIP, R4R, docs, etc)
  • Reviewers assigned
  • Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)

Dockerfile Outdated Show resolved Hide resolved
@fedekunze
Copy link
Contributor

fedekunze commented Jul 22, 2020

@araskachoi can you pull the latest changes from the testnet branch? 🙏 Works for me at least but it'd be great if we could get an ACK from @J-Thompson12 and @noot too 👍

@noot
Copy link
Contributor

noot commented Jul 22, 2020

looks good to me!

Creating network "ethermint_localnet" with driver "bridge"
Creating emintdnode3 ... done
Creating emintdnode2 ... done
Creating emintdnode1 ... done
Creating emintdnode0 ... done

at this point, are the nodes up/ am I able to query them?

Comment on lines -271 to -272
docker cp $${container_id}:/usr/bin/emintd ./build/ ; \
docker cp $${container_id}:/usr/bin/emintcli ./build/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so this copyies the binaries to the build folder, which allows the ./build:/emintd:Z volumes to be mounted.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it necessary for the binary to be copied over to the directory? Why don't we have the binary just be built into the image directly?

This is how i do that:
https://github.com/ChainSafe/ethermint/blob/fix-testnet/Dockerfile#L25-L26

Makefile Outdated
if ! [ -f build/node0/$(ETHERMINT_DAEMON_BINARY)/config/genesis.json ]; then docker run --rm -v $(CURDIR)/build:/$(ETHERMINT_DAEMON_BINARY):Z emintd/node testnet --v 4 -o . --starting-ip-address 192.168.10.2 --keyring-backend=test ; fi
docker container start $${container_id} ;

if ! [ -f build/node0/$(ETHERMINT_DAEMON_BINARY)/config/genesis.json ]; then docker run --rm -v $(CURDIR)/build:/ethermint:Z ethermint-build-linux "emintd testnet --v 4 -o ethermint --starting-ip-address 192.168.10.2 --keyring-backend=test"; fi
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$(CURDIR)/build:/ethermint:Z

I don't think the ethermint directory exists

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the directory is mounted to the volume on your local machine ($CURDIR/build) and the ethermint dir is created when mounted

@@ -3,63 +3,67 @@ version: "3"
services:
emintdnode0:
container_name: emintdnode0
image: "emintd/node"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd suggest keeping the image names as they are now to isolate the changes

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure, but we would need to change it everywhere on the makefile as well, that can be an easy change

@@ -28,4 +28,4 @@ COPY --from=build-env /go/src/github.com/ChainSafe/ethermint/build/emintcli /usr
EXPOSE 26656 26657 1317

# Run emintd by default, omit entrypoint to ease using container with emintcli
CMD ["emintd"]
ENTRYPOINT ["/bin/bash", "-c"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand the diff here with CMD ["emintd"] can you add a code comment?

Copy link
Contributor Author

@araskachoi araskachoi Jul 23, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CMD is used when there are no args passed the the docker run command (i.e. docker run <image>). We pass args like testnets [flags] ...` so we would need entrypoint.

this might be a better explanation
https://stackoverflow.com/a/44201230

Comment on lines +18 to +22
FROM golang:1.14 as final

# Install ca-certificates
RUN apk add --update ca-certificates
WORKDIR /root
WORKDIR /

RUN apt-get update
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

correct me if I'm wrong but this change should be the only one made in order to make this work for all the OS?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well all this does is allow the binaries created from the previous build step (build-env) to be executable on the final image

@araskachoi
Copy link
Contributor Author

@fedekunze done, spins up emintd and emintcli in bg and logs to respective files (emintd.log, emintcli.log)

@J-Thompson12 @noot can you guys try again to get the local net up?

@noot
Copy link
Contributor

noot commented Jul 28, 2020

@araskachoi i see this:

Creating network "ethermint_localnet" with driver "bridge"
Creating emintdnode1 ... done
Creating emintdnode3 ... done
Creating emintdnode0 ... done
Creating emintdnode2 ... done

there's no logs, is that expected?

@araskachoi
Copy link
Contributor Author

@araskachoi i see this:

Creating network "ethermint_localnet" with driver "bridge"
Creating emintdnode1 ... done
Creating emintdnode3 ... done
Creating emintdnode0 ... done
Creating emintdnode2 ... done

there's no logs, is that expected?

Yep, that should be expected. had to redirect output to make the daemon into a background process (to run rcp server too). If you want to check and see the logs of the nodes, you can do this:
docker exec emintdnode0 tail emintd.log

@fedekunze
Copy link
Contributor

@araskachoi I still get the address book message when initializing the nodes. Is that expected?

If you want to check and see the logs of the nodes, you can do this:
docker exec emintdnode0 tail emintd.log

Can you update the docs (docs/quickstart/testnet.md) and add an instruction for that? 🙏

@araskachoi
Copy link
Contributor Author

@araskachoi I still get the address book message when initializing the nodes. Is that expected?

yikes.. i forgot to check that. if it is still showing, it should be an issue that isnt related to the rpc. however, now the rpc servers are spun up and can be queriable! Should be fine as long as the blocks are synchronized and no errors regarding dialing the peers. We should maybe make this as a todo to change that to some sort of WARN level or something?

Can you update the docs (docs/quickstart/testnet.md) and add an instruction for that? 🙏

👍 will do!

@fedekunze fedekunze merged commit a9983c3 into testnet Jul 29, 2020
@fedekunze fedekunze deleted the fix-testnet branch July 29, 2020 17:35
fedekunze added a commit that referenced this pull request Jul 31, 2020
* evm: fix non-determinism

* fixes

* typo

* fix tests

* local testnet command

* fix testnet cmd (#383)

fix export-eth-key

generate eth type account in genesis.json file

* fixes

* update docker

* minor changes

* fix build-docker-local-ethermint

* fix dockerfile

* update Makefile

* update denoms

* update genesis file

* update makefile

* fix docker-compose.yml images

* fix localnet execution (#398)

* finish documentation

* changelog and comment rpc tests workflow

* update codecov

* update testnet docs

* fix docker-compose execution

* update docs

* fix errors and make testnet work (#403)

* fix errors and make testnet work

* Update Dockerfile

Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com>

* wip - fix db

* fixes emintd nodes and syncs nodes

* starts daemon and rpc server in bg

Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com>
Co-authored-by: Federico Kunze <federico.kunze94@gmail.com>

* update entrypoint and docs

* update logs

* try fix rpc

* build docker image

* Update Dockerfile

Co-authored-by: Holechain <nrgh@foxmail.com>
Co-authored-by: Alessio Treglia <quadrispro@ubuntu.com>
Co-authored-by: Daniel Choi <choidanielw@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants