Skip to content

Commit

Permalink
fix: remove conflicting etcd member on rejoin with empty data directory
Browse files Browse the repository at this point in the history
This fixes a scenario when control plane node loses contents of `/var`
without leaving etcd first: on reboot etcd data directory is empty, but
member is already present in the etcd member list, so etcd won't be able
to join because of raft log being empty.

The fix is to remove a member with matching hostname if found in the
etcd member list followed by new member add.

The risk here is removing another member which has same hostname as the
joining node, but having duplicate hostnames for control plane node is a
problem anyways.

Signed-off-by: Andrey Smirnov <smirnov.andrey@gmail.com>
  • Loading branch information
smira authored and talos-bot committed Jun 3, 2021
1 parent ff62a59 commit 62c702c
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 7 deletions.
8 changes: 7 additions & 1 deletion internal/app/machined/pkg/system/services/etcd.go
Original file line number Diff line number Diff line change
Expand Up @@ -225,8 +225,14 @@ func addMember(ctx context.Context, r runtime.Runtime, addrs []string, name stri
}

for _, member := range list.Members {
// addMember only gets called when the etcd data directory is empty, so the node is about to join the etcd cluster
// if there's already a member with same hostname, it should be removed, as there will be a conflict between the existing
// member and a new joining member.
// here we assume that control plane nodes have unique hostnames (if that's not the case, it will be a problem anyways)
if member.Name == name {
return list, member.ID, nil
if _, err = client.MemberRemove(ctx, member.ID); err != nil {
return nil, 0, fmt.Errorf("error removing self from the member list: %w", err)
}
}
}

Expand Down
21 changes: 15 additions & 6 deletions internal/integration/api/reset.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,10 +101,7 @@ func (suite *ResetSuite) TestResetNodeByNode() {
}
}

// TestResetNoGraceful resets a worker in !graceful to test the flow.
//
// We can't reset control plane node in !graceful mode as it won't be able to join back the cluster.
func (suite *ResetSuite) TestResetNoGraceful() {
func (suite *ResetSuite) testResetNoGraceful(nodeType machine.Type) {
if !suite.Capabilities().SupportsReboot {
suite.T().Skip("cluster doesn't support reboot (and reset)")
}
Expand All @@ -113,9 +110,9 @@ func (suite *ResetSuite) TestResetNoGraceful() {
suite.T().Skip("without full cluster state reset test is not reliable (can't wait for cluster readiness in between resets)")
}

node := suite.RandomDiscoveredNode(machine.TypeJoin)
node := suite.RandomDiscoveredNode(nodeType)

suite.T().Log("Resetting node !graceful", node)
suite.T().Logf("Resetting %s node !graceful %s", nodeType, node)

preReset, err := suite.HashKubeletCert(suite.ctx, node)
suite.Require().NoError(err)
Expand All @@ -133,6 +130,18 @@ func (suite *ResetSuite) TestResetNoGraceful() {
suite.Assert().NotEqual(preReset, postReset, "reset should lead to new kubelet cert being generated")
}

// TestResetNoGracefulWorker resets a worker in !graceful mode.
func (suite *ResetSuite) TestResetNoGracefulWorker() {
suite.testResetNoGraceful(machine.TypeJoin)
}

// TestResetNoGracefulControlplane resets a control plane node in !graceful mode.
//
// As the node doesn't leave etcd, it relies on Talos to fix the problem on rejoin.
func (suite *ResetSuite) TestResetNoGracefulControlplane() {
suite.testResetNoGraceful(machine.TypeControlPlane)
}

// TestResetWithSpecEphemeral resets only ephemeral partition on the node.
//
//nolint:dupl
Expand Down

0 comments on commit 62c702c

Please sign in to comment.