op-conductor,op-node: allow system to select port, make op-node wait …

…for conductor endpoint (#12863)

* op-conductor,op-node: allow system to select port, make op-node wait for conductor endpoint

* op-conductor,op-node: debugging conductor test

* op-conductor: more debugging

* op-e2e: increase conductor timeout

op-conductor/conductor/config.go

@@ -19,12 +19,19 @@ import (
 )
 
 type Config struct {
-	// ConsensusAddr is the address to listen for consensus connections.
+	// ConsensusAddr is the address, excluding port, to listen on for consensus connections.
+	// E.g. 0.0.0.0 to bind to the external-facing network interface.
 	ConsensusAddr string
 
-	// ConsensusPort is the port to listen for consensus connections.
+	// ConsensusPort is the port to listen on for consensus connections.
+	// If 0, the server binds to a port selected by the system.
 	ConsensusPort int
 
+	// ConsensusAdvertisedAddr is the network address, including port, to advertise to other peers.
+	// This is optional: if empty, the address that the server network transport binds to is used instead.
+	// E.g. local tests may use temporary addresses, rather than preset known addresses.
+	ConsensusAdvertisedAddr string
+
 	// RaftServerID is the unique ID for this server used by raft consensus.
 	RaftServerID string
 
@@ -117,8 +124,11 @@ func NewConfig(ctx *cli.Context, log log.Logger) (*Config, error) {
 	}
 
 	return &Config{
-		ConsensusAddr:         ctx.String(flags.ConsensusAddr.Name),
-		ConsensusPort:         ctx.Int(flags.ConsensusPort.Name),
+		ConsensusAddr: ctx.String(flags.ConsensusAddr.Name),
+		ConsensusPort: ctx.Int(flags.ConsensusPort.Name),
+		// The consensus server will advertise the address it binds to if this is empty/unspecified.
+		ConsensusAdvertisedAddr: ctx.String(flags.AdvertisedFullAddr.Name),
+
 		RaftBootstrap:         ctx.Bool(flags.RaftBootstrap.Name),
 		RaftServerID:          ctx.String(flags.RaftServerID.Name),
 		RaftStorageDir:        ctx.String(flags.RaftStorageDir.Name),

op-conductor/conductor/service.go

@@ -169,10 +169,12 @@ func (c *OpConductor) initConsensus(ctx context.Context) error {
 		return nil
 	}
 
-	serverAddr := fmt.Sprintf("%s:%d", c.cfg.ConsensusAddr, c.cfg.ConsensusPort)
 	raftConsensusConfig := &consensus.RaftConsensusConfig{
-		ServerID:          c.cfg.RaftServerID,
-		ServerAddr:        serverAddr,
+		ServerID: c.cfg.RaftServerID,
+		// AdvertisedAddr may be empty: the server will then default to what it binds to.
+		AdvertisedAddr:    raft.ServerAddress(c.cfg.ConsensusAdvertisedAddr),
+		ListenAddr:        c.cfg.ConsensusAddr,
+		ListenPort:        c.cfg.ConsensusPort,
 		StorageDir:        c.cfg.RaftStorageDir,
 		Bootstrap:         c.cfg.RaftBootstrap,
 		RollupCfg:         &c.cfg.RollupCfg,
@@ -472,6 +474,12 @@ func (oc *OpConductor) Paused() bool {
 	return oc.paused.Load()
 }
 
+// ConsensusEndpoint returns the raft consensus server address to connect to.
+func (oc *OpConductor) ConsensusEndpoint() string {
+	return oc.cons.Addr()
+}
+
+// HTTPEndpoint returns the HTTP RPC endpoint
 func (oc *OpConductor) HTTPEndpoint() string {
 	if oc.rpcServer == nil {
 		return ""
@@ -613,7 +621,8 @@ func (oc *OpConductor) handleHealthUpdate(hcerr error) {
 		oc.queueAction()
 	}
 
-	if oc.healthy.Swap(healthy) != healthy {
+	if old := oc.healthy.Swap(healthy); old != healthy {
+		oc.log.Info("Health state changed", "old", old, "new", healthy)
 		// queue an action if health status changed.
 		oc.queueAction()
 	}

op-conductor/conductor/service_test.go

@@ -30,7 +30,7 @@ func mockConfig(t *testing.T) Config {
 	now := uint64(time.Now().Unix())
 	return Config{
 		ConsensusAddr:  "127.0.0.1",
-		ConsensusPort:  50050,
+		ConsensusPort:  0,
 		RaftServerID:   "SequencerA",
 		RaftStorageDir: "/tmp/raft",
 		RaftBootstrap:  false,

op-conductor/consensus/iface.go

@@ -42,6 +42,9 @@ type ServerInfo struct {
 //
 //go:generate mockery --name Consensus --output mocks/ --with-expecter=true
 type Consensus interface {
+	// Addr returns the address of this consensus server.
+	// Internally the server may override what is advertised, or fall back to the address it listens to.
+	Addr() string
 	// AddVoter adds a voting member into the cluster, voter is eligible to become leader.
 	// If version is non-zero, this will only be applied if the current cluster version matches the expected version.
 	AddVoter(id, addr string, version uint64) error

op-conductor/consensus/raft.go

@@ -29,12 +29,30 @@ type RaftConsensus struct {
 	serverID raft.ServerID
 	r        *raft.Raft
 
+	transport *raft.NetworkTransport
+	// advertisedAddr is the host & port to contact this server.
+	// If empty, the address of the transport should be used instead.
+	advertisedAddr string
+
 	unsafeTracker *unsafeHeadTracker
 }
 
 type RaftConsensusConfig struct {
-	ServerID          string
-	ServerAddr        string
+	ServerID string
+
+	// AdvertisedAddr is the address to advertise,
+	// i.e. the address external raft peers use to contact us.
+	// If left empty, it defaults to the resulting
+	// local address that we bind the underlying transport to.
+	AdvertisedAddr raft.ServerAddress
+
+	// ListenPort is the port to bind the server to.
+	// This may be 0, an available port will then be selected by the system.
+	ListenPort int
+	// ListenAddr is the address to bind the server to.
+	// E.g. use 0.0.0.0 to bind to an external-facing network.
+	ListenAddr string
+
 	StorageDir        string
 	Bootstrap         bool
 	RollupCfg         *rollup.Config
@@ -86,18 +104,31 @@ func NewRaftConsensus(log log.Logger, cfg *RaftConsensusConfig) (*RaftConsensus,
 		return nil, fmt.Errorf(`raft.NewFileSnapshotStore(%q): %w`, baseDir, err)
 	}
 
-	addr, err := net.ResolveTCPAddr("tcp", cfg.ServerAddr)
-	if err != nil {
-		return nil, errors.Wrap(err, "failed to resolve tcp address")
+	var advertiseAddr net.Addr
+	if cfg.AdvertisedAddr == "" {
+		log.Warn("No advertised address specified. Advertising local address.")
+	} else {
+		x, err := net.ResolveTCPAddr("tcp", string(cfg.AdvertisedAddr))
+		if err != nil {
+			return nil, fmt.Errorf("failed to resolve advertised TCP address %q: %w", string(cfg.AdvertisedAddr), err)
+		}
+		advertiseAddr = x
+		log.Info("Resolved advertising address", "adAddr", cfg.AdvertisedAddr,
+			"adIP", x.IP, "adPort", x.Port, "adZone", x.Zone)
 	}
 
+	bindAddr := fmt.Sprintf("%s:%d", cfg.ListenAddr, cfg.ListenPort)
+	log.Info("Binding raft server to network transport", "listenAddr", bindAddr)
+
 	maxConnPool := 10
 	timeout := 5 * time.Second
-	bindAddr := fmt.Sprintf("0.0.0.0:%d", addr.Port)
-	transport, err := raft.NewTCPTransportWithLogger(bindAddr, addr, maxConnPool, timeout, rc.Logger)
+
+	// When advertiseAddr == nil, the transport will use the local address that it is bound to.
+	transport, err := raft.NewTCPTransportWithLogger(bindAddr, advertiseAddr, maxConnPool, timeout, rc.Logger)
 	if err != nil {
 		return nil, errors.Wrap(err, "failed to create raft tcp transport")
 	}
+	log.Info("Raft server network transport is up", "addr", transport.LocalAddr())
 
 	fsm := NewUnsafeHeadTracker(log)
 
@@ -110,11 +141,19 @@ func NewRaftConsensus(log log.Logger, cfg *RaftConsensusConfig) (*RaftConsensus,
 	// If bootstrap = true, start raft in bootstrap mode, this will allow the current node to elect itself as leader when there's no other participants
 	// and allow other nodes to join the cluster.
 	if cfg.Bootstrap {
+		var advertisedAddr raft.ServerAddress
+		if cfg.AdvertisedAddr == "" {
+			advertisedAddr = transport.LocalAddr()
+		} else {
+			advertisedAddr = cfg.AdvertisedAddr
+		}
+		log.Info("Bootstrapping raft consensus cluster with self", "addr", advertisedAddr)
+
 		raftCfg := raft.Configuration{
 			Servers: []raft.Server{
 				{
 					ID:       rc.LocalID,
-					Address:  raft.ServerAddress(cfg.ServerAddr),
+					Address:  advertisedAddr,
 					Suffrage: raft.Voter,
 				},
 			},
@@ -132,9 +171,20 @@ func NewRaftConsensus(log log.Logger, cfg *RaftConsensusConfig) (*RaftConsensus,
 		serverID:      raft.ServerID(cfg.ServerID),
 		unsafeTracker: fsm,
 		rollupCfg:     cfg.RollupCfg,
+		transport:     transport,
 	}, nil
 }
 
+// Addr returns the address to contact this raft consensus server.
+// If no explicit address to advertise was configured,
+// the local network address that the raft-consensus server is listening on will be used.
+func (rc *RaftConsensus) Addr() string {
+	if rc.advertisedAddr != "" {
+		return rc.advertisedAddr
+	}
+	return string(rc.transport.LocalAddr())
+}
+
 // AddNonVoter implements Consensus, it tries to add a non-voting member into the cluster.
 func (rc *RaftConsensus) AddNonVoter(id string, addr string, version uint64) error {
 	if err := checkTCPPortOpen(addr); err != nil {

op-conductor/consensus/raft_test.go

@@ -28,7 +28,9 @@ func TestCommitAndRead(t *testing.T) {
 	}
 	raftConsensusConfig := &RaftConsensusConfig{
 		ServerID:          "SequencerA",
-		ServerAddr:        "127.0.0.1:0",
+		ListenPort:        0,
+		ListenAddr:        "127.0.0.1", // local test, don't bind to external interface
+		AdvertisedAddr:    "",          // use local address that the server binds to
 		StorageDir:        storageDir,
 		Bootstrap:         true,
 		RollupCfg:         rollupCfg,

op-conductor/flags/flags.go

@@ -19,16 +19,22 @@ const EnvVarPrefix = "OP_CONDUCTOR"
 var (
 	ConsensusAddr = &cli.StringFlag{
 		Name:    "consensus.addr",
-		Usage:   "Address to listen for consensus connections",
+		Usage:   "Address (excluding port) to listen for consensus connections.",
 		EnvVars: opservice.PrefixEnvVar(EnvVarPrefix, "CONSENSUS_ADDR"),
 		Value:   "127.0.0.1",
 	}
 	ConsensusPort = &cli.IntFlag{
 		Name:    "consensus.port",
-		Usage:   "Port to listen for consensus connections",
+		Usage:   "Port to listen for consensus connections. May be 0 to let the system select a port.",
 		EnvVars: opservice.PrefixEnvVar(EnvVarPrefix, "CONSENSUS_PORT"),
 		Value:   50050,
 	}
+	AdvertisedFullAddr = &cli.StringFlag{
+		Name:    "consensus.advertised",
+		Usage:   "Full address (host and port) for other peers to contact the consensus server. Optional: if left empty, the local address is advertised.",
+		EnvVars: opservice.PrefixEnvVar(EnvVarPrefix, "CONSENSUS_ADVERTISED"),
+		Value:   "",
+	}
 	RaftBootstrap = &cli.BoolFlag{
 		Name:    "raft.bootstrap",
 		Usage:   "If this node should bootstrap a new raft cluster",
@@ -127,6 +133,7 @@ var requiredFlags = []cli.Flag{
 }
 
 var optionalFlags = []cli.Flag{
+	AdvertisedFullAddr,
 	Paused,
 	RPCEnableProxy,
 	RaftBootstrap,