Skip to content

Commit

Permalink
Allow dialer to re-establish terminated peering (#16776)
Browse files Browse the repository at this point in the history
Currently, if an acceptor peer deletes a peering the dialer's peering
will eventually get to a "terminated" state. If the two clusters need to
be re-peered the acceptor will re-generate the token but the dialer will
encounter this error on the call to establish:

"failed to get addresses to dial peer: failed to refresh peer server
addresses, will continue to use initial addresses: there is no active
peering for "<<<ID>>>""

This is because in `exchangeSecret().GetDialAddresses()` we will get an
error if fetching addresses for an inactive peering. The peering shows
up as inactive at this point because of the existing terminated state.

Rather than checking whether a peering is active we can instead check
whether it was deleted. This way users do not need to delete terminated
peerings in the dialing cluster before re-establishing them.
  • Loading branch information
freddygv committed Apr 3, 2023
1 parent 1af1322 commit e121c9e
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 4 deletions.
3 changes: 3 additions & 0 deletions .changelog/16776.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
peering: allow re-establishing terminated peering from new token without deleting existing peering first.
```
24 changes: 24 additions & 0 deletions agent/consul/leader_peering_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -467,6 +467,30 @@ func TestLeader_PeeringSync_Lifecycle_ServerDeletion(t *testing.T) {
require.NoError(r, err)
require.Equal(r, pbpeering.PeeringState_TERMINATED, peering.State)
})

// Re-establishing a peering terminated by the acceptor should be possible
// without needing to delete the terminated peering first.
ctx, cancel = context.WithTimeout(context.Background(), 3*time.Second)
t.Cleanup(cancel)

req = pbpeering.GenerateTokenRequest{
PeerName: "my-peer-dialer",
}
resp, err = peeringClient.GenerateToken(ctx, &req)
require.NoError(t, err)

tokenJSON, err = base64.StdEncoding.DecodeString(resp.PeeringToken)
require.NoError(t, err)

token = structs.PeeringToken{}
require.NoError(t, json.Unmarshal(tokenJSON, &token))

establishReq = pbpeering.EstablishRequest{
PeerName: "my-peer-acceptor",
PeeringToken: resp.PeeringToken,
}
_, err = dialerClient.Establish(ctx, &establishReq)
require.NoError(t, err)
}

func TestLeader_PeeringSync_FailsForTLSError(t *testing.T) {
Expand Down
7 changes: 5 additions & 2 deletions agent/consul/peering_backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,8 +147,11 @@ func (b *PeeringBackend) fetchPeerServerAddresses(ws memdb.WatchSet, peerID stri
if err != nil {
return nil, fmt.Errorf("failed to fetch peer %q: %w", peerID, err)
}
if !peering.IsActive() {
return nil, fmt.Errorf("there is no active peering for %q", peerID)
if peering == nil {
return nil, fmt.Errorf("unknown peering %q", peerID)
}
if peering.DeletedAt != nil && !structs.IsZeroProtoTime(peering.DeletedAt) {
return nil, fmt.Errorf("peering %q was deleted", peerID)
}
return bufferFromAddresses(peering.GetAddressesToDial())
}
Expand Down
23 changes: 21 additions & 2 deletions agent/consul/peering_backend_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ func TestPeeringBackend_GetDialAddresses(t *testing.T) {
},
peerID: acceptorPeerID,
expect: expectation{
err: fmt.Sprintf(`there is no active peering for %q`, acceptorPeerID),
err: fmt.Sprintf(`unknown peering %q`, acceptorPeerID),
},
},
{
Expand Down Expand Up @@ -383,6 +383,25 @@ func TestPeeringBackend_GetDialAddresses(t *testing.T) {
gatewayAddrs: []string{"5.6.7.8:8443", "6.7.8.9:8443"},
},
},
{
name: "addresses are returned if the peering is marked as terminated",
setup: func(store *state.Store) {
require.NoError(t, store.PeeringWrite(5, &pbpeering.PeeringWriteRequest{
Peering: &pbpeering.Peering{
Name: "dialer",
ID: dialerPeerID,
PeerServerAddresses: []string{"1.2.3.4:8502", "2.3.4.5:8503"},
State: pbpeering.PeeringState_TERMINATED,
},
}))
},
peerID: dialerPeerID,
expect: expectation{
// Gateways come first, and we use their LAN addresses since this is for outbound communication.
addrs: []string{"5.6.7.8:8443", "6.7.8.9:8443", "1.2.3.4:8502", "2.3.4.5:8503"},
gatewayAddrs: []string{"5.6.7.8:8443", "6.7.8.9:8443"},
},
},
{
name: "addresses are not returned if the peering is deleted",
setup: func(store *state.Store) {
Expand All @@ -400,7 +419,7 @@ func TestPeeringBackend_GetDialAddresses(t *testing.T) {
},
peerID: dialerPeerID,
expect: expectation{
err: fmt.Sprintf(`there is no active peering for %q`, dialerPeerID),
err: fmt.Sprintf(`peering %q was deleted`, dialerPeerID),
},
},
}
Expand Down

0 comments on commit e121c9e

Please sign in to comment.