Single-machine multi-node mixed cluster CI prerequisites #4247

deepfire · 2019-10-23T15:26:51Z

This supplies the necessary changes for a mixed-cluster integration test, as per IntersectMBO/cardano-node#255 :

mainnet_ci_full genesis & configuration, starting in OBFT node
fix for an OBFT EBB rollback issue, which was trying to erase EBB even if the chain was started in OBFT mode, leading to Mixed cluster in CI IntersectMBO/cardano-node#255 (comment)
change in network-transport-tcp to be more lenient regarding remote address claims: deepfire/network-transport-tcp@44f84a8. This is necessary to avoid problems when starting multiple nodes on the same machine.
small improvements in genesis generation

Additionally, this resets the protocol version for the shelley_staging_short_full configuration to 0 -- a prerequisite for its respin.

NOTE: perhaps this PR should be split. But then, this repository sees very little activity, so perhaps the separation wouldn't have much benefit. I don't have a strong opinion myself.

intricate

Good work! Just a few concerns.

I don't think this is worth splitting into multiple PRs. It's already quite small and your commits clearly separate the work done for each concern.

We should also request review from someone who also worked on this OBFT stuff like @mhuesch or @erikd.

lib/src/Pos/Worker/Block.hs

stack.yaml

avieth · 2019-10-23T16:14:36Z

change in network-transport-tcp to be more lenient regarding remote address claims: deepfire/network-transport-tcp@44f84a8. This is necessary to avoid problems when starting multiple nodes on the same machine.

What exactly is the problem? That patch to nt-tcp opens up a denial of service attack. The check was put there for good reason.

avieth

network-transport-tcp changes are not acceptable

avieth · 2019-10-23T16:17:49Z

If you really must disable the hostname check, you can set it in TCPParameters http://hackage.haskell.org/package/network-transport-tcp-0.6.0/docs/Network-Transport-TCP.html#t:TCPParameters

tcpCheckPeerHost :: Bool

If True, new connections will be accepted only if the socket's host matches the host that the peer claims in its EndPointAddress. This is useful when operating on untrusted networks, because the peer could otherwise deny service to some victim by claiming the victim's address.

But beware! Your node will be vulnerable.

deepfire · 2019-10-23T16:24:09Z

@avieth, thank you! I've missed the check at https://github.com/avieth/network-transport-tcp/blob/csl-2.0.0/src/Network/Transport/TCP.hs#L1044

…g startup Discusion in: https://iohk.myjetbrains.com/youtrack/issue/CBR-482

deepfire · 2019-10-23T17:14:12Z

@avieth, the example failure:

Sep 09 01:46:48 a-node xadsbs7wlnm94wv14v9w8c0ba1dpjsyw-unit-script-cardano-node-legacy_-start[22055]: [cardano-sl.diffusion.outboundqueue.c-a-2:Warning:ThreadId 150] [2001-09-09 01:46:48.23 UTC] sending MsgAnnounceBlockHeader OriginSender to NodeId 10.1.0.3:3000:0 failed with TransportError ConnectFailed "setupRemoteEndPoint: Host mismatch. Claimed: 10.1.0.2; Numeric: 10.1.0.3; Resolved: c-b-1.cardano" :: SomeException
Sep 09 01:46:48 a-node xadsbs7wlnm94wv14v9w8c0ba1dpjsyw-unit-script-cardano-node-legacy_-start[22055]: [cardano-sl.diffusion.outboundqueue.c-a-2:Warning:ThreadId 149] [2001-09-09 01:46:48.23 UTC] sending MsgAnnounceBlockHeader OriginSender to NodeId 10.1.0.1:3000:0 failed with TransportError ConnectFailed "setupRemoteEndPoint: Host mismatch. Claimed: 10.1.0.2; Numeric: 10.1.0.1; Resolved: c-a-1.cardano" :: SomeException
Sep 09 01:46:48 a-node xadsbs7wlnm94wv14v9w8c0ba1dpjsyw-unit-script-cardano-node-legacy_-start[22055]: [cardano-sl.diffusion.outboundqueue.c-a-2:Warning:ThreadId 155] [2001-09-09 01:46:48.23 UTC] sending MsgAnnounceBlockHeader OriginSender to NodeId 10.1.0.7:3000:0 failed with TransportError ConnectFailed "setupRemoteEndPoint: Host mismatch. Claimed: 10.1.0.2; Numeric: 10.1.0.7; Resolved: c-d-1.cardano" :: SomeException

[root@a-node:~]# netstat -pltn | grep cardano
tcp        0      0 10.1.0.2:3000           0.0.0.0:*               LISTEN      22055/cardano-node- 
tcp        0      0 10.1.0.5:3000           0.0.0.0:*               LISTEN      22078/cardano-node- 
tcp        0      0 10.1.0.6:3000           0.0.0.0:*               LISTEN      22075/cardano-node- 
tcp        0      0 10.1.0.7:3000           0.0.0.0:*               LISTEN      22068/cardano-node- 
tcp        0      0 10.1.0.3:3000           0.0.0.0:*               LISTEN      22092/cardano-node- 
tcp        0      0 10.1.0.4:3000           0.0.0.0:*               LISTEN      22083/cardano-node- 
tcp        0      0 10.1.0.1:3000           0.0.0.0:*               LISTEN      22053/cardano-node-

[root@a-node:~]# host 10.1.0.1
1.0.1.10.in-addr.arpa domain name pointer c-a-1.cardano.

[root@a-node:~]# host 10.1.0.2
2.0.1.10.in-addr.arpa domain name pointer c-a-2.cardano.

[root@a-node:~]# host 10.1.0.3
3.0.1.10.in-addr.arpa domain name pointer c-b-1.cardano.

[root@a-node:~]# host 10.1.0.4
4.0.1.10.in-addr.arpa domain name pointer c-b-2.cardano.

deepfire · 2019-10-23T17:19:50Z

@avieth,

dropped the network-transport-tcp change
added a CLI option to drive the host peer address consistency checking -- accompanied with sufficiently scary message during startup & comments

intricate

LGTM. Let's just await a green light from @avieth.

deepfire · 2019-10-28T18:26:46Z

bors r+

4247: Single-machine multi-node mixed cluster CI prerequisites r=deepfire a=deepfire This supplies the necessary changes for a mixed-cluster integration test, as per IntersectMBO/cardano-node#255 : 1. `mainnet_ci_full` genesis & configuration, starting in OBFT node 2. fix for an OBFT EBB rollback issue, which was trying to erase EBB even if the chain was started in OBFT mode, leading to IntersectMBO/cardano-node#255 (comment) 3. change in `network-transport-tcp` to be more lenient regarding remote address claims: deepfire/network-transport-tcp@44f84a8. This is necessary to avoid problems when starting multiple nodes on the same machine. 4. small improvements in genesis generation Additionally, this resets the protocol version for the `shelley_staging_short_full` configuration to 0 -- a prerequisite for its respin. *NOTE*: perhaps this PR should be split. But then, this repository sees very little activity, so perhaps the separation wouldn't have much benefit. I don't have a strong opinion myself. Co-authored-by: Kosyrev Serge <[email protected]>

deepfire · 2019-10-28T18:28:37Z

bors r-

iohk-bors · 2019-10-28T18:28:39Z

Canceled

deepfire · 2019-10-29T13:58:26Z

Waiting on @mhuesch's review of the EBB rollback change.

mhuesch

LGTM - the OBFT change seems reasonable & minimal.

mhuesch · 2019-10-29T16:08:23Z

lib/src/Pos/Worker/Block.hs

+        -- If we're starting up a chain in OBFT mode, we need to ensure
+        -- that we don't rollback the actual genesis block out of existence
+        -- (i.e. the EBB at epoch 0):
+        BlockHeaderGenesis _   -> unless (tipHeader ^. epochIndexL == 0) $ do


Yes, this looks completely reasonable. My memory is fuzzy, but I thought we had tested scenarios where the cluster started in OBFT mode, but perhaps I am misremembering - it seems like we would've hit the same snag that Serge encountered here.

@mhuesch: Yeah, we definitely had tested those scenarios before but I think after implementing your rollback solution we eventually ended up accepting a trade-off here such that we'd not be able to start up a chain directly in OBFT.

deepfire · 2019-10-29T16:27:02Z

bors r+

4247: Single-machine multi-node mixed cluster CI prerequisites r=deepfire a=deepfire This supplies the necessary changes for a mixed-cluster integration test, as per IntersectMBO/cardano-node#255 : 1. `mainnet_ci_full` genesis & configuration, starting in OBFT node 2. fix for an OBFT EBB rollback issue, which was trying to erase EBB even if the chain was started in OBFT mode, leading to IntersectMBO/cardano-node#255 (comment) 3. change in `network-transport-tcp` to be more lenient regarding remote address claims: deepfire/network-transport-tcp@44f84a8. This is necessary to avoid problems when starting multiple nodes on the same machine. 4. small improvements in genesis generation Additionally, this resets the protocol version for the `shelley_staging_short_full` configuration to 0 -- a prerequisite for its respin. *NOTE*: perhaps this PR should be split. But then, this repository sees very little activity, so perhaps the separation wouldn't have much benefit. I don't have a strong opinion myself. Co-authored-by: Kosyrev Serge <[email protected]>

iohk-bors · 2019-10-29T17:03:30Z

Build succeeded

59: Bump deps and update to changes in them r=deepfire a=deepfire This: 1. Bumps dependencies to the version used in `cardano-node`, 2. Uses a bumped `cardano-sl` with input-output-hk/cardano-sl#4247 3. Updates to changes in `cardano-ledger` and `ouroboros-network` 4. Adds new dependencies to facilitate the above. Co-authored-by: Kosyrev Serge <[email protected]>

deepfire added 3 commits October 23, 2019 18:14

configuration.yaml | mainnet_ci_full: new config

2877ace

scripts/prepare-genesis: fix

eab1e16

default.nix: expose the legacy genesis generator script generator

3adb120

deepfire requested a review from intricate October 23, 2019 15:26

deepfire mentioned this pull request Oct 23, 2019

Mixed cluster in CI IntersectMBO/cardano-node#255

Closed

intricate suggested changes Oct 23, 2019

View reviewed changes

lib/src/Pos/Worker/Block.hs Outdated Show resolved Hide resolved

stack.yaml Outdated Show resolved Hide resolved

intricate requested review from erikd and mhuesch October 23, 2019 15:57

deepfire force-pushed the serge/obft-mainnet-ci branch from 6743bc7 to 5530860 Compare October 23, 2019 16:03

deepfire requested a review from avieth October 23, 2019 16:07

avieth suggested changes Oct 23, 2019

View reviewed changes

deepfire added 3 commits October 23, 2019 19:24

shelley-staging-short: debump block version

c4e0296

mainnet-ci-full: debump block version

1bf8447

[CBR-482] dropObftEbb: don't roll back genesis out of existence durin…

03c458b

…g startup Discusion in: https://iohk.myjetbrains.com/youtrack/issue/CBR-482

deepfire force-pushed the serge/obft-mainnet-ci branch from 5530860 to 03c458b Compare October 23, 2019 16:25

Nix: add shells

ce49dba

deepfire force-pushed the serge/obft-mainnet-ci branch from 8485432 to 0e8cb4c Compare October 25, 2019 12:01

deepfire mentioned this pull request Oct 25, 2019

Bump deps and update to changes in them input-output-hk/cardano-byron-proxy#59

Merged

network-transport-tcp: conditionalie the peer host address check

f32a634

deepfire force-pushed the serge/obft-mainnet-ci branch from 0e8cb4c to f32a634 Compare October 28, 2019 17:44

avieth approved these changes Oct 28, 2019

View reviewed changes

intricate approved these changes Oct 28, 2019

View reviewed changes

mhuesch approved these changes Oct 29, 2019

View reviewed changes

iohk-bors bot merged commit f32a634 into develop Oct 29, 2019

iohk-bors bot deleted the serge/obft-mainnet-ci branch October 29, 2019 17:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Single-machine multi-node mixed cluster CI prerequisites #4247

Single-machine multi-node mixed cluster CI prerequisites #4247

deepfire commented Oct 23, 2019 •

edited

Loading

intricate left a comment

avieth commented Oct 23, 2019

avieth left a comment

avieth commented Oct 23, 2019 •

edited

Loading

deepfire commented Oct 23, 2019

deepfire commented Oct 23, 2019

deepfire commented Oct 23, 2019

intricate left a comment

deepfire commented Oct 28, 2019

deepfire commented Oct 28, 2019

iohk-bors bot commented Oct 28, 2019

deepfire commented Oct 29, 2019

mhuesch left a comment

mhuesch Oct 29, 2019

intricate Oct 29, 2019

deepfire commented Oct 29, 2019

iohk-bors bot commented Oct 29, 2019

Single-machine multi-node mixed cluster CI prerequisites #4247

Single-machine multi-node mixed cluster CI prerequisites #4247

Conversation

deepfire commented Oct 23, 2019 • edited Loading

intricate left a comment

Choose a reason for hiding this comment

avieth commented Oct 23, 2019

avieth left a comment

Choose a reason for hiding this comment

avieth commented Oct 23, 2019 • edited Loading

deepfire commented Oct 23, 2019

deepfire commented Oct 23, 2019

deepfire commented Oct 23, 2019

intricate left a comment

Choose a reason for hiding this comment

deepfire commented Oct 28, 2019

deepfire commented Oct 28, 2019

iohk-bors bot commented Oct 28, 2019

Canceled

deepfire commented Oct 29, 2019

mhuesch left a comment

Choose a reason for hiding this comment

mhuesch Oct 29, 2019

Choose a reason for hiding this comment

intricate Oct 29, 2019

Choose a reason for hiding this comment

deepfire commented Oct 29, 2019

iohk-bors bot commented Oct 29, 2019

Build succeeded

deepfire commented Oct 23, 2019 •

edited

Loading

avieth commented Oct 23, 2019 •

edited

Loading