Skip to content
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

fix: use net_enode to retrieve Enode addresses #9

Merged
merged 4 commits into from
Jun 15, 2021
Merged

fix: use net_enode to retrieve Enode addresses #9

merged 4 commits into from
Jun 15, 2021

Conversation

fllaca
Copy link
Contributor

@fllaca fllaca commented Jun 14, 2021

No description provided.

@fllaca fllaca force-pushed the net_enode branch 3 times, most recently from 269a72e to c75078b Compare June 14, 2021 14:12
@fllaca fllaca marked this pull request as draft June 14, 2021 14:15
@fllaca fllaca force-pushed the net_enode branch 2 times, most recently from c356917 to 0a6e134 Compare June 14, 2021 16:56
pkg/rpc/net.go Outdated Show resolved Hide resolved
Copy link
Contributor

@marcdk marcdk left a comment

Choose a reason for hiding this comment

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

tested locally with expected output (connecting 4 validators)

connecting with both validators and permissioners with the fix here https://github.com/AdharaProjects/cluster-config-apps/pull/1371/files#diff-2c65a4818e188bc2f8f7e4db189b00dedc70f6123efcad4fba6ff035f68d3a0dR25

@fllaca fllaca force-pushed the net_enode branch 2 times, most recently from 6274e0c to 4bb0e82 Compare June 15, 2021 09:30
Dockerfile Outdated
@@ -2,6 +2,11 @@ FROM golang:1.14.0-buster as builder

WORKDIR /workdir

COPY ./go.mod /workdir/go.mod
COPY ./go.sum /workdir/go.suml
Copy link
Contributor

Choose a reason for hiding this comment

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

typo I think COPY ./go.sum /workdir/go.sum

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed, thanks! 🦅

@fllaca fllaca marked this pull request as ready for review June 15, 2021 09:51
kind create cluster
```

2. Apply example manifests
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe these two points as well (just suggestion)

Copy link
Contributor Author

@fllaca fllaca Jun 15, 2021

Choose a reason for hiding this comment

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

default namespace is created by default in kubernetes, it doesn't need to be created.

The second point about loading the docker-image won't be needed once v0.0.1-rc5 is published, but it's true that it can be handy having these instructions while developing. If you agree, I will think on a nice way to document this and open another PR with that extended documentation (like adding a DEVELOPING.md doc or something 🤔 )

Copy link
Contributor

@marcdk marcdk left a comment

Choose a reason for hiding this comment

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

just the one comment

locally tested, looks correct:

0: 10.244.0.67	client-besu-0	ce7edc292d7b747fab2f23584bbafaffde5c8ff17cf689969614441e0527b90015ea9fee96aed6d9c0fc2fbe0bd1883dee223b3200246ff1e21976bdbc9a0fc8
1: 10.244.0.68	validator-besu-0	cc88932af38c9c8444930d52932df9ea220d520e0d32dba76f6c142d458ae5b6e76891a7c3bb5585e25d12e17600e704b388862dcfe97932a9ca45261cb53033

Checking node [client-besu-0]
add:  validator-besu-0

Checking node [validator-besu-0]
add:  client-besu-0

@marcdk marcdk merged commit 1a0ac0b into main Jun 15, 2021
@marcdk marcdk deleted the net_enode branch June 15, 2021 10:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants