Skip to content

Commit

Permalink
rename task issues variable name
Browse files Browse the repository at this point in the history
  • Loading branch information
marcoandredinis committed Oct 17, 2024
1 parent 143341c commit b5579b8
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 46 deletions.
30 changes: 15 additions & 15 deletions api/types/usertasks/object.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,40 +90,40 @@ const (
// This value is used to populate the UserTasks.Spec.IssueType for Discover EC2 tasks.
// The Web UI will then use those identifiers to show detailed instructions on how to fix the issue.
const (
// AutoDiscoverEC2IssueScriptInstanceNotRegistered is used to identify instances that failed to auto-enroll
// AutoDiscoverEC2IssueSSMInstanceNotRegistered is used to identify instances that failed to auto-enroll
// because they are not present in Amazon Systems Manager.
// This usually means that the Instance does not have the SSM Agent running,
// or that the instance's IAM Profile does not allow have the managed IAM Policy AmazonSSMManagedInstanceCore assigned to it.
AutoDiscoverEC2IssueScriptInstanceNotRegistered = "ec2-ssm-agent-not-registered"
AutoDiscoverEC2IssueSSMInstanceNotRegistered = "ec2-ssm-agent-not-registered"

// AutoDiscoverEC2IssueScriptInstanceConnectionLost is used to identify instances that failed to auto-enroll
// AutoDiscoverEC2IssueSSMInstanceConnectionLost is used to identify instances that failed to auto-enroll
// because the agent lost connection to Amazon Systems Manager.
// This can happen if the user changed some setting in the instance's network or IAM profile.
AutoDiscoverEC2IssueScriptInstanceConnectionLost = "ec2-ssm-agent-connection-lost"
AutoDiscoverEC2IssueSSMInstanceConnectionLost = "ec2-ssm-agent-connection-lost"

// AutoDiscoverEC2IssueScriptInstanceUnsupportedOS is used to identify instances that failed to auto-enroll
// AutoDiscoverEC2IssueSSMInstanceUnsupportedOS is used to identify instances that failed to auto-enroll
// because its OS is not supported by teleport.
// This can happen if the instance is running Windows.
AutoDiscoverEC2IssueScriptInstanceUnsupportedOS = "ec2-ssm-unsupported-os"
AutoDiscoverEC2IssueSSMInstanceUnsupportedOS = "ec2-ssm-unsupported-os"

// AutoDiscoverEC2IssueScriptFailure is used to identify instances that failed to auto-enroll
// AutoDiscoverEC2IssueSSMScriptFailure is used to identify instances that failed to auto-enroll
// because the installation script failed.
// The invocation url must be included in the report, so that users can see what was wrong.
AutoDiscoverEC2IssueScriptFailure = "ec2-ssm-script-failure"
AutoDiscoverEC2IssueSSMScriptFailure = "ec2-ssm-script-failure"

// AutoDiscoverEC2IssueInvocationFailure is used to identify instances that failed to auto-enroll
// AutoDiscoverEC2IssueSSMInvocationFailure is used to identify instances that failed to auto-enroll
// because the SSM Script Run (also known as Invocation) failed.
// This happens when there's a failure with permissions or an invalid configuration (eg, invalid document name).
AutoDiscoverEC2IssueInvocationFailure = "ec2-ssm-invocation-failure"
AutoDiscoverEC2IssueSSMInvocationFailure = "ec2-ssm-invocation-failure"
)

// discoverEC2IssueTypes is a list of issue types that can occur when trying to auto enroll EC2 instances.
var discoverEC2IssueTypes = []string{
AutoDiscoverEC2IssueScriptInstanceNotRegistered,
AutoDiscoverEC2IssueScriptInstanceConnectionLost,
AutoDiscoverEC2IssueScriptInstanceUnsupportedOS,
AutoDiscoverEC2IssueScriptFailure,
AutoDiscoverEC2IssueInvocationFailure,
AutoDiscoverEC2IssueSSMInstanceNotRegistered,
AutoDiscoverEC2IssueSSMInstanceConnectionLost,
AutoDiscoverEC2IssueSSMInstanceUnsupportedOS,
AutoDiscoverEC2IssueSSMScriptFailure,
AutoDiscoverEC2IssueSSMInvocationFailure,
}

// ValidateUserTask validates the UserTask object without modifying it.
Expand Down
4 changes: 2 additions & 2 deletions lib/srv/discovery/discovery.go
Original file line number Diff line number Diff line change
Expand Up @@ -980,10 +980,10 @@ func (s *Server) handleEC2RemoteInstallation(instances *server.EC2Instances) err

for _, instance := range req.Instances {
s.awsEC2Tasks.addFailedEnrollment(
awsEC2FailedEnrollmentTaskKey{
awsEC2TaskKey{
accountID: instances.AccountID,
integration: instances.Integration,
issueType: usertasks.AutoDiscoverEC2IssueInvocationFailure,
issueType: usertasks.AutoDiscoverEC2IssueSSMInvocationFailure,
region: instances.Region,
},
&usertasksv1.DiscoverEC2Instance{
Expand Down
38 changes: 19 additions & 19 deletions lib/srv/discovery/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ func (s *Server) ReportEC2SSMInstallationResult(ctx context.Context, result *ser
s.updateDiscoveryConfigStatus(result.DiscoveryConfig)

s.awsEC2Tasks.addFailedEnrollment(
awsEC2FailedEnrollmentTaskKey{
awsEC2TaskKey{
integration: result.IntegrationName,
issueType: result.IssueType,
accountID: result.SSMRunEvent.AccountID,
Expand All @@ -324,15 +324,15 @@ func (s *Server) ReportEC2SSMInstallationResult(ctx context.Context, result *ser
type awsEC2Tasks struct {
mu sync.RWMutex
// instancesIssues maps the Discover EC2 User Task grouping parts to a set of instances metadata.
instancesIssues map[awsEC2FailedEnrollmentTaskKey]map[string]*usertasksv1.DiscoverEC2Instance
// groupsToSync is used to register which groups were changed in memory but were not yet sent to the cluster.
// When upserting User Tasks, if the group is not in groupsToSync,
instancesIssues map[awsEC2TaskKey]map[string]*usertasksv1.DiscoverEC2Instance
// issuesSyncQueue is used to register which groups were changed in memory but were not yet sent to the cluster.
// When upserting User Tasks, if the group is not in issuesSyncQueue,
// then the cluster already has the latest version of this particular group.
taskKeysToSync map[awsEC2FailedEnrollmentTaskKey]struct{}
issuesSyncQueue map[awsEC2TaskKey]struct{}
}

// awsEC2FailedEnrollmentTaskKey identifies a UserTask group.
type awsEC2FailedEnrollmentTaskKey struct {
// awsEC2TaskKey identifies a UserTask group.
type awsEC2TaskKey struct {
integration string
issueType string
accountID string
Expand All @@ -345,12 +345,12 @@ func (d *awsEC2Tasks) reset() {
d.mu.Lock()
defer d.mu.Unlock()

d.instancesIssues = make(map[awsEC2FailedEnrollmentTaskKey]map[string]*usertasksv1.DiscoverEC2Instance)
d.taskKeysToSync = make(map[awsEC2FailedEnrollmentTaskKey]struct{})
d.instancesIssues = make(map[awsEC2TaskKey]map[string]*usertasksv1.DiscoverEC2Instance)
d.issuesSyncQueue = make(map[awsEC2TaskKey]struct{})
}

// addFailedEnrollment adds an enrollment failure of a given instance.
func (d *awsEC2Tasks) addFailedEnrollment(g awsEC2FailedEnrollmentTaskKey, instance *usertasksv1.DiscoverEC2Instance) {
func (d *awsEC2Tasks) addFailedEnrollment(g awsEC2TaskKey, instance *usertasksv1.DiscoverEC2Instance) {
// Only failures associated with an Integration are reported.
// There's no major blocking for showing non-integration User Tasks, but this keeps scope smaller.
if g.integration == "" {
Expand All @@ -360,23 +360,23 @@ func (d *awsEC2Tasks) addFailedEnrollment(g awsEC2FailedEnrollmentTaskKey, insta
d.mu.Lock()
defer d.mu.Unlock()
if d.instancesIssues == nil {
d.instancesIssues = make(map[awsEC2FailedEnrollmentTaskKey]map[string]*usertasksv1.DiscoverEC2Instance)
d.instancesIssues = make(map[awsEC2TaskKey]map[string]*usertasksv1.DiscoverEC2Instance)
}
if _, ok := d.instancesIssues[g]; !ok {
d.instancesIssues[g] = make(map[string]*usertasksv1.DiscoverEC2Instance)
}
d.instancesIssues[g][instance.InstanceId] = instance

if d.taskKeysToSync == nil {
d.taskKeysToSync = make(map[awsEC2FailedEnrollmentTaskKey]struct{})
if d.issuesSyncQueue == nil {
d.issuesSyncQueue = make(map[awsEC2TaskKey]struct{})
}
d.taskKeysToSync[g] = struct{}{}
d.issuesSyncQueue[g] = struct{}{}
}

// acquireSemaphoreForUserTask tries to acquire a semaphore lock for this user task.
// It returns a func which must be called to release the lock.
// It also returns a context which is tied to the lease and will be canceled if the lease ends.
func (s *Server) acquireSemaphoreForUserTask(userTaskName string) (func(), context.Context, error) {
func (s *Server) acquireSemaphoreForUserTask(userTaskName string) (releaseFn func(), ctx context.Context, err error) {
// Use the deterministic task name as semaphore name.
semaphoreName := userTaskName
semaphoreExpiration := 5 * time.Second
Expand Down Expand Up @@ -415,7 +415,7 @@ func (s *Server) acquireSemaphoreForUserTask(userTaskName string) (func(), conte
ctxWithLease, cancel := context.WithCancel(lease)
defer cancel()

releaseFn := func() {
releaseFn = func() {
lease.Stop()
if err := lease.Wait(); err != nil {
s.Log.WithError(err).WithField("semaphore", userTaskName).Warn("error cleaning up UserTask semaphore")
Expand All @@ -429,7 +429,7 @@ func (s *Server) acquireSemaphoreForUserTask(userTaskName string) (func(), conte
// merges them against the ones that exist in the cluster.
//
// All of this flow is protected by a lock to ensure there's no race between this and other DiscoveryServices.
func (s *Server) mergeUpsertDiscoverEC2Task(taskGroup awsEC2FailedEnrollmentTaskKey, failedInstances map[string]*usertasksv1.DiscoverEC2Instance) error {
func (s *Server) mergeUpsertDiscoverEC2Task(taskGroup awsEC2TaskKey, failedInstances map[string]*usertasksv1.DiscoverEC2Instance) error {
userTaskName := usertasks.TaskNameForDiscoverEC2(usertasks.TaskNameForDiscoverEC2Parts{
Integration: taskGroup.integration,
IssueType: taskGroup.issueType,
Expand Down Expand Up @@ -509,7 +509,7 @@ func (s *Server) discoverEC2UserTaskAddExistingInstances(currentUserTask *userta
func (s *Server) upsertTasksForAWSEC2FailedEnrollments() {
s.awsEC2Tasks.mu.Lock()
defer s.awsEC2Tasks.mu.Unlock()
for g := range s.awsEC2Tasks.taskKeysToSync {
for g := range s.awsEC2Tasks.issuesSyncQueue {
instancesIssueByID := s.awsEC2Tasks.instancesIssues[g]
if len(instancesIssueByID) == 0 {
continue
Expand All @@ -526,6 +526,6 @@ func (s *Server) upsertTasksForAWSEC2FailedEnrollments() {
continue
}

delete(s.awsEC2Tasks.taskKeysToSync, g)
delete(s.awsEC2Tasks.issuesSyncQueue, g)
}
}
10 changes: 5 additions & 5 deletions lib/srv/server/ssm_install.go
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ func (si *SSMInstaller) emitInvalidInstanceEvents(ctx context.Context, req SSMRu
for _, instanceID := range instanceIDsState.missing {
installationResult := invalidSSMInstanceInstallationResult(req, instanceID,
"EC2 Instance is not registered in SSM. Make sure that the instance has AmazonSSMManagedInstanceCore policy assigned.",
usertasks.AutoDiscoverEC2IssueScriptInstanceNotRegistered,
usertasks.AutoDiscoverEC2IssueSSMInstanceNotRegistered,
)
if err := si.ReportSSMInstallationResultFunc(ctx, installationResult); err != nil {
errs = append(errs, trace.Wrap(err))
Expand All @@ -230,7 +230,7 @@ func (si *SSMInstaller) emitInvalidInstanceEvents(ctx context.Context, req SSMRu
for _, instanceID := range instanceIDsState.connectionLost {
installationResult := invalidSSMInstanceInstallationResult(req, instanceID,
"SSM Agent in EC2 Instance is not connecting to SSM Service. Restart or reinstall the SSM service. See https://docs.aws.amazon.com/systems-manager/latest/userguide/ami-preinstalled-agent.html#verify-ssm-agent-status for more details.",
usertasks.AutoDiscoverEC2IssueScriptInstanceConnectionLost,
usertasks.AutoDiscoverEC2IssueSSMInstanceConnectionLost,
)
if err := si.ReportSSMInstallationResultFunc(ctx, installationResult); err != nil {
errs = append(errs, trace.Wrap(err))
Expand All @@ -240,7 +240,7 @@ func (si *SSMInstaller) emitInvalidInstanceEvents(ctx context.Context, req SSMRu
for _, instanceID := range instanceIDsState.unsupportedOS {
installationResult := invalidSSMInstanceInstallationResult(req, instanceID,
"EC2 instance is running an unsupported Operating System. Only Linux is supported.",
usertasks.AutoDiscoverEC2IssueScriptInstanceUnsupportedOS,
usertasks.AutoDiscoverEC2IssueSSMInstanceUnsupportedOS,
)
if err := si.ReportSSMInstallationResultFunc(ctx, installationResult); err != nil {
errs = append(errs, trace.Wrap(err))
Expand Down Expand Up @@ -358,7 +358,7 @@ func (si *SSMInstaller) checkCommand(ctx context.Context, req SSMRunRequest, com
SSMRunEvent: invocationResultEvent,
IntegrationName: req.IntegrationName,
DiscoveryConfig: req.DiscoveryConfig,
IssueType: usertasks.AutoDiscoverEC2IssueScriptFailure,
IssueType: usertasks.AutoDiscoverEC2IssueSSMScriptFailure,
}))
}

Expand All @@ -372,7 +372,7 @@ func (si *SSMInstaller) checkCommand(ctx context.Context, req SSMRunRequest, com
SSMRunEvent: stepResultEvent,
IntegrationName: req.IntegrationName,
DiscoveryConfig: req.DiscoveryConfig,
IssueType: usertasks.AutoDiscoverEC2IssueScriptFailure,
IssueType: usertasks.AutoDiscoverEC2IssueSSMScriptFailure,
}))
}
}
Expand Down
10 changes: 5 additions & 5 deletions lib/web/usertasks_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,11 @@ func TestUserTask(t *testing.T) {
}

issueTypes := []string{
usertasks.AutoDiscoverEC2IssueInvocationFailure,
usertasks.AutoDiscoverEC2IssueScriptFailure,
usertasks.AutoDiscoverEC2IssueScriptInstanceConnectionLost,
usertasks.AutoDiscoverEC2IssueScriptInstanceNotRegistered,
usertasks.AutoDiscoverEC2IssueScriptInstanceUnsupportedOS,
usertasks.AutoDiscoverEC2IssueSSMInvocationFailure,
usertasks.AutoDiscoverEC2IssueSSMScriptFailure,
usertasks.AutoDiscoverEC2IssueSSMInstanceConnectionLost,
usertasks.AutoDiscoverEC2IssueSSMInstanceNotRegistered,
usertasks.AutoDiscoverEC2IssueSSMInstanceUnsupportedOS,
}
var userTaskForTest *usertasksv1.UserTask
for _, issueType := range issueTypes {
Expand Down

0 comments on commit b5579b8

Please sign in to comment.