• Mahmood Ali's avatar
    Simplify Bootstrap logic in tests · e812954b
    Mahmood Ali authored
    This change updates tests to honor `BootstrapExpect` exclusively when
    forming test clusters and removes test only knobs, e.g.
    `config.DevDisableBootstrap`.
    
    Background:
    
    Test cluster creation is fragile.  Test servers don't follow the
    BootstapExpected route like production clusters.  Instead they start as
    single node clusters and then get rejoin and may risk causing brain
    split or other test flakiness.
    
    The test framework expose few knobs to control those (e.g.
    `config.DevDisableBootstrap` and `config.Bootstrap`) that control
    whether a server should bootstrap the cluster.  These flags are
    confusing and it's unclear when to use: their usage in multi-node
    cluster isn't properly documented.  Furthermore, they have some bad
    side-effects as they don't control Raft library: If
    `config.DevDisableBootstrap` is true, the test server may not
    immediately attempt to bootstrap a cluster, but after an election
    timeout (~50ms), Raft may force a leadership election and win it (with
    only one vote) and cause a split brain.
    
    The knobs are also confusing as Bootstrap is an overloaded term.  In
    BootstrapExpect, we refer to bootstrapping the cluster only after N
    servers are connected.  But in tests and the knobs above, it refers to
    whether the server is a single node cluster and shouldn't wait for any
    other server.
    
    Changes:
    
    This commit makes two changes:
    
    First, it relies on `BootstrapExpected` instead of `Bootstrap` and/or
    `DevMode` flags.  This change is relatively trivial.
    
    Introduce a `Bootstrapped` flag to track if the cluster is bootstrapped.
    This allows us to keep `BootstrapExpected` immutable.  Previously, the
    flag was a config value but it gets set to 0 after cluster bootstrap
    completes.
    e812954b