Disable auto IP discovery if not all ports are configured #1275
Conversation
node/node.go
Outdated
| @@ -349,14 +349,35 @@ func (n *Node) Start(ctx context.Context) error { | |||
|
|
|||
| n.CurrentSocket = socket | |||
| // Start the Node IP updater only if the PUBLIC_IP_PROVIDER is greater than 0. | |||
| if n.Config.PubIPCheckInterval > 0 { | |||
| if n.Config.PubIPCheckInterval > 0 && n.allPortsConfigured() { | |||
There was a problem hiding this comment.
It seems unnecessary, configs is validated already https://github.com/Layr-Labs/eigenda/blob/master/node/config.go#L250-L274
There was a problem hiding this comment.
This makes sure the feature is disabled if its only running v1 and missing v2 ports for example
There was a problem hiding this comment.
Then the condition is if v2Enabled {...} ?
There was a problem hiding this comment.
That would cover most of the cases and is probably fine. Checking the ports is the closest condition we can check here imo, for example, this allows the feature if node is only running v1 but does have the v2 ports configured.
There was a problem hiding this comment.
why v1 should disable this feature again?
There was a problem hiding this comment.
Its possible to run the node in v1-only mode without defining v2 ports.
The other concern is chance of split brain problem when running 2 nodes in isolation mode on 2 separate hosts (potentially in different subnets/AZs) the .env port configs could diverge and/or the egress IP could be different - then both nodes start flip flopping with different socket updates.
So the idea is to only enable IP discovery updates when the node is in v1-and-v2 runtime mode
There was a problem hiding this comment.
Then the condition is
if v2Enabled {...}?
if n.Config.v1Enabled && n.Config.v2Enabled {...}
There was a problem hiding this comment.
Its possible to run the node in
v1-onlymode without defining v2 ports.
This sounds different than "node is only running v1 but does have the v2 ports"
The other concern is chance of split brain problem when running 2 nodes in isolation mode on 2 separate hosts (potentially in different subnets/AZs) the
.envport configs could diverge and/or the egress IP could be different - then both nodes start flip flopping with different socket updates.So the idea is to only enable IP discovery updates when the node is in v1-and-v2 runtime mode
Even it's running v1 and v2 together, they can still run two machines that both have v1 and v2 running, v1-and-v2 mode doesn't seem safe as well
Why are these changes needed?
If it auto registers the socket with missing ports, it can render the node unreachable.
Checks