Skip to content

Commit

Permalink
fix: discovery refactoring
Browse files Browse the repository at this point in the history
  • Loading branch information
rosstimothy committed Oct 23, 2024
1 parent 7a7b91a commit c26f411
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 11 deletions.
23 changes: 16 additions & 7 deletions lib/srv/discovery/discovery.go
Original file line number Diff line number Diff line change
Expand Up @@ -854,7 +854,9 @@ func (s *Server) handleEC2Instances(instances *server.EC2Instances) error {
// EICE Nodes must never be filtered, so that we can extend their expiration and sync labels.
totalInstancesFound := len(instances.Instances)
if !instances.Rotation && instances.EnrollMode != types.InstallParamEnrollMode_INSTALL_PARAM_ENROLL_MODE_EICE {
s.filterExistingEC2Nodes(instances)
if err := s.filterExistingEC2Nodes(instances); err != nil {
return trace.Wrap(err)
}
}

instancesAlreadyEnrolled := totalInstancesFound - len(instances.Instances)
Expand Down Expand Up @@ -916,13 +918,16 @@ func (s *Server) heartbeatEICEInstance(instances *server.EC2Instances) {
continue
}

if len(existingNodes) != 1 {
s.Log.Warnf("Found multiple matching nodes with name :q", eiceNode.GetName())
var existingNode types.Server
switch len(existingNodes) {
case 0:
case 1:
existingNode = existingNodes[0]
default:
s.Log.Warnf("Found multiple matching nodes with name %q", eiceNode.GetName())
continue
}

existingNode := existingNodes[0]

// EICE Node's Name are deterministic (based on the Account and Instance ID).
//
// To reduce load, nodes are skipped if
Expand Down Expand Up @@ -1173,7 +1178,9 @@ func (s *Server) handleAzureInstances(instances *server.AzureInstances) error {
if err != nil {
return trace.Wrap(err)
}
s.filterExistingAzureNodes(instances)
if err := s.filterExistingAzureNodes(instances); err != nil {
return trace.Wrap(err)
}
if len(instances.Instances) == 0 {
return trace.Wrap(errNoInstances)
}
Expand Down Expand Up @@ -1265,7 +1272,9 @@ func (s *Server) handleGCPInstances(instances *server.GCPInstances) error {
if err != nil {
return trace.Wrap(err)
}
s.filterExistingGCPNodes(instances)
if err := s.filterExistingGCPNodes(instances); err != nil {
return trace.Wrap(err)
}
if len(instances.Instances) == 0 {
return trace.Wrap(errNoInstances)
}
Expand Down
7 changes: 3 additions & 4 deletions lib/srv/discovery/discovery_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -852,11 +852,10 @@ func TestDiscoveryServerConcurrency(t *testing.T) {

// We must get only one EC2 EICE Node.
// Even when two servers are discovering the same EC2 Instance, they will use the same name when converting to EICE Node.
require.Eventually(t, func() bool {
require.EventuallyWithT(t, func(t *assert.CollectT) {
allNodes, err := tlsServer.Auth().GetNodes(ctx, "default")
require.NoError(t, err)

return len(allNodes) == 1
assert.NoError(t, err)
assert.Len(t, allNodes, 1)
}, 1*time.Second, 50*time.Millisecond)

// We should never get a duplicate instance.
Expand Down

0 comments on commit c26f411

Please sign in to comment.