From c596458695793eddc83c53aaf6a6bf2be27a1f3e Mon Sep 17 00:00:00 2001 From: Joseph Rajewski <83741749+paladin-devops@users.noreply.github.com> Date: Thu, 8 Jun 2023 16:52:14 -0400 Subject: [PATCH 1/3] fix: Delete the file system associated with a runner. The runner uninstall for AWS will now loop correctly through all file systems' paginated results using a marker. Some other updates are made in this commit to skip deleting the file system if certain conditions are met (if there are no file systems that exist, or none with the right tag). Step groups were also updated. --- .changelog/4792.txt | 3 + internal/runnerinstall/ecs.go | 124 +++++++++++++++++++--------------- 2 files changed, 74 insertions(+), 53 deletions(-) create mode 100644 .changelog/4792.txt diff --git a/.changelog/4792.txt b/.changelog/4792.txt new file mode 100644 index 00000000000..7ba54ae0c26 --- /dev/null +++ b/.changelog/4792.txt @@ -0,0 +1,3 @@ +```release-note:bug +runneruninstall/aws-ecs: Fix deletion of file system for AWS ECS runner. +``` \ No newline at end of file diff --git a/internal/runnerinstall/ecs.go b/internal/runnerinstall/ecs.go index 14bbcb52155..4f560dc35e9 100644 --- a/internal/runnerinstall/ecs.go +++ b/internal/runnerinstall/ecs.go @@ -1,6 +1,3 @@ -// Copyright (c) HashiCorp, Inc. -// SPDX-License-Identifier: MPL-2.0 - package runnerinstall import ( @@ -359,89 +356,110 @@ func (i *ECSRunnerInstaller) Uninstall(ctx context.Context, opts *InstallOpts) e return err } - s.Update("Runner uninstalled") + s.Update("Waypoint runner AWS ECS service deleted") } s.Done() // TODO: Still attempt to delete the EFS volume if the ECS service // uninstall fails + s = sg.Add("Deleting runner file system") efsSvc := efs.New(sess) marker := "" done := false var fileSystems []*efs.FileSystemDescription for !done { - fileSystemsResp, err := efsSvc.DescribeFileSystems(&efs.DescribeFileSystemsInput{ - Marker: aws.String(marker), + req := &efs.DescribeFileSystemsInput{ MaxItems: aws.Int64(100), - }) + } + if marker != "" { + req.Marker = aws.String(marker) + } + fileSystemsResp, err := efsSvc.DescribeFileSystems(req) if err != nil { return err } - if marker == *fileSystemsResp.Marker { + + if fileSystemsResp.NextMarker == nil { done = true + } else { + marker = *fileSystemsResp.NextMarker + time.Sleep(5 * time.Second) } fileSystems = append(fileSystems, fileSystemsResp.FileSystems...) } - var fileSystemId *string - for _, fileSystem := range fileSystems { - // Check if tags match ID, if so then delete things - for _, tag := range fileSystem.Tags { - if *tag.Key == "runner-id" && *tag.Value == opts.Id { - fileSystemId = fileSystem.FileSystemId - goto DeleteFileSystem + if len(fileSystems) == 0 { + s.Update("No file systems detected, skipping deletion") + } else { + var fileSystemId *string + for _, fileSystem := range fileSystems { + // Check if tags match ID, if so then delete things + for _, tag := range fileSystem.Tags { + if *tag.Key == "runner-id" && *tag.Value == opts.Id { + fileSystemId = fileSystem.FileSystemId + // This goto skips to the logic for deleting the file system - + // we know which one we need to delete now, so there's no need + // to iterate through any additional fileSystems + goto DeleteFileSystem + } } } - } -DeleteFileSystem: - describeAccessPointsResp, err := efsSvc.DescribeAccessPoints(&efs.DescribeAccessPointsInput{ - FileSystemId: fileSystemId, - }) - if err != nil { - return err - } - for _, accessPoint := range describeAccessPointsResp.AccessPoints { - _, err = efsSvc.DeleteAccessPoint(&efs.DeleteAccessPointInput{AccessPointId: accessPoint.AccessPointId}) - if err != nil { - return err + if *fileSystemId == "" { + s.Update("File system with tag key `runner-id` and value " + opts.Id + " not detected, skipping deletion") + s.Done() + return nil } - } - describeMountTargetsResp, err := efsSvc.DescribeMountTargets(&efs.DescribeMountTargetsInput{ - FileSystemId: fileSystemId, - }) - if err != nil { - return err - } - for _, mountTarget := range describeMountTargetsResp.MountTargets { - _, err = efsSvc.DeleteMountTarget(&efs.DeleteMountTargetInput{MountTargetId: mountTarget.MountTargetId}) + DeleteFileSystem: + describeAccessPointsResp, err := efsSvc.DescribeAccessPoints(&efs.DescribeAccessPointsInput{ + FileSystemId: fileSystemId, + }) if err != nil { return err } - } + for _, accessPoint := range describeAccessPointsResp.AccessPoints { + _, err = efsSvc.DeleteAccessPoint(&efs.DeleteAccessPointInput{AccessPointId: accessPoint.AccessPointId}) + if err != nil { + return err + } + } + + describeMountTargetsResp, err := efsSvc.DescribeMountTargets(&efs.DescribeMountTargetsInput{ + FileSystemId: fileSystemId, + }) + for _, mountTarget := range describeMountTargetsResp.MountTargets { + _, err = efsSvc.DeleteMountTarget(&efs.DeleteMountTargetInput{MountTargetId: mountTarget.MountTargetId}) + if err != nil { + return err + } + } - for { ctx, cancel := context.WithTimeout(ctx, 5*time.Minute) defer cancel() - select { - case <-ctx.Done(): - return errors.New("after 5 minutes, the file system could" + - "not be deleted, because the mount targets weren't deleted") - default: - _, err = efsSvc.DeleteFileSystem(&efs.DeleteFileSystemInput{FileSystemId: fileSystemId}) - if err != nil { - if strings.Contains(err.Error(), "because it has mount targets") { - // sleep here for 5 seconds to avoid slamming the API - time.Sleep(5 * time.Second) - continue + for { + select { + case <-ctx.Done(): + return errors.New("after 5 minutes, the file system could" + + "not be deleted, because the mount targets weren't deleted") + default: + _, err = efsSvc.DeleteFileSystem(&efs.DeleteFileSystemInput{FileSystemId: fileSystemId}) + if err != nil { + if strings.Contains(err.Error(), "because it has mount targets") { + // sleep here for 5 seconds to avoid slamming the API + time.Sleep(5 * time.Second) + continue + } + return err } - return err + // if we reach this point, we're done + s.Update("Runner file system deleted") + s.Done() + return nil } - // if we reach this point, we're done - return nil } } + return nil } func (i *ECSRunnerInstaller) UninstallFlags(set *flag.Set) { @@ -547,7 +565,7 @@ func launchRunner( ExecutionRoleArn: aws.String(executionRoleArn), Cpu: aws.String(cpu), Memory: aws.String(memory), - Family: aws.String(defaultRunnerTagName), + Family: aws.String(installutil.DefaultRunnerName(id)), TaskRoleArn: &taskRoleArn, NetworkMode: aws.String("awsvpc"), RequiresCompatibilities: []*string{aws.String(defaultTaskRuntime)}, From 9329438a03cba611dc21813c2c5d02a58f56d71d Mon Sep 17 00:00:00 2001 From: Joseph Rajewski <83741749+paladin-devops@users.noreply.github.com> Date: Fri, 9 Jun 2023 11:20:31 -0400 Subject: [PATCH 2/3] imp: Use pagination func for searching EFS file systems. Simplify runner uninstall func for ECS by removing an `if`. Add the copyright headers back in. --- internal/runnerinstall/ecs.go | 146 ++++++++++++++++------------------ 1 file changed, 68 insertions(+), 78 deletions(-) diff --git a/internal/runnerinstall/ecs.go b/internal/runnerinstall/ecs.go index 4f560dc35e9..875e529754c 100644 --- a/internal/runnerinstall/ecs.go +++ b/internal/runnerinstall/ecs.go @@ -1,3 +1,6 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + package runnerinstall import ( @@ -364,102 +367,89 @@ func (i *ECSRunnerInstaller) Uninstall(ctx context.Context, opts *InstallOpts) e // uninstall fails s = sg.Add("Deleting runner file system") efsSvc := efs.New(sess) - marker := "" - done := false var fileSystems []*efs.FileSystemDescription - for !done { - req := &efs.DescribeFileSystemsInput{ - MaxItems: aws.Int64(100), - } - if marker != "" { - req.Marker = aws.String(marker) - } - fileSystemsResp, err := efsSvc.DescribeFileSystems(req) - if err != nil { - return err - } - - if fileSystemsResp.NextMarker == nil { - done = true - } else { - marker = *fileSystemsResp.NextMarker - time.Sleep(5 * time.Second) - } - fileSystems = append(fileSystems, fileSystemsResp.FileSystems...) + err = efsSvc.DescribeFileSystemsPages(&efs.DescribeFileSystemsInput{}, + func(page *efs.DescribeFileSystemsOutput, lastPage bool) bool { + fileSystems = append(fileSystems, page.FileSystems...) + return !lastPage + }) + if err != nil { + return err } if len(fileSystems) == 0 { s.Update("No file systems detected, skipping deletion") - } else { - var fileSystemId *string - for _, fileSystem := range fileSystems { - // Check if tags match ID, if so then delete things - for _, tag := range fileSystem.Tags { - if *tag.Key == "runner-id" && *tag.Value == opts.Id { - fileSystemId = fileSystem.FileSystemId - // This goto skips to the logic for deleting the file system - - // we know which one we need to delete now, so there's no need - // to iterate through any additional fileSystems - goto DeleteFileSystem - } + s.Done() + return nil + } + + var fileSystemId *string + for _, fileSystem := range fileSystems { + // Check if tags match ID, if so then delete things + for _, tag := range fileSystem.Tags { + if *tag.Key == "runner-id" && *tag.Value == opts.Id { + fileSystemId = fileSystem.FileSystemId + // This goto skips to the logic for deleting the file system - + // we know which one we need to delete now, so there's no need + // to iterate through any additional fileSystems + goto DeleteFileSystem } } + } - if *fileSystemId == "" { - s.Update("File system with tag key `runner-id` and value " + opts.Id + " not detected, skipping deletion") - s.Done() - return nil - } + if *fileSystemId == "" { + s.Update("File system with tag key `runner-id` and value " + opts.Id + " not detected, skipping deletion") + s.Done() + return nil + } - DeleteFileSystem: - describeAccessPointsResp, err := efsSvc.DescribeAccessPoints(&efs.DescribeAccessPointsInput{ - FileSystemId: fileSystemId, - }) +DeleteFileSystem: + describeAccessPointsResp, err := efsSvc.DescribeAccessPoints(&efs.DescribeAccessPointsInput{ + FileSystemId: fileSystemId, + }) + if err != nil { + return err + } + for _, accessPoint := range describeAccessPointsResp.AccessPoints { + _, err = efsSvc.DeleteAccessPoint(&efs.DeleteAccessPointInput{AccessPointId: accessPoint.AccessPointId}) if err != nil { return err } - for _, accessPoint := range describeAccessPointsResp.AccessPoints { - _, err = efsSvc.DeleteAccessPoint(&efs.DeleteAccessPointInput{AccessPointId: accessPoint.AccessPointId}) - if err != nil { - return err - } - } + } - describeMountTargetsResp, err := efsSvc.DescribeMountTargets(&efs.DescribeMountTargetsInput{ - FileSystemId: fileSystemId, - }) - for _, mountTarget := range describeMountTargetsResp.MountTargets { - _, err = efsSvc.DeleteMountTarget(&efs.DeleteMountTargetInput{MountTargetId: mountTarget.MountTargetId}) - if err != nil { - return err - } + describeMountTargetsResp, err := efsSvc.DescribeMountTargets(&efs.DescribeMountTargetsInput{ + FileSystemId: fileSystemId, + }) + for _, mountTarget := range describeMountTargetsResp.MountTargets { + _, err = efsSvc.DeleteMountTarget(&efs.DeleteMountTargetInput{MountTargetId: mountTarget.MountTargetId}) + if err != nil { + return err } + } - ctx, cancel := context.WithTimeout(ctx, 5*time.Minute) - defer cancel() - for { - select { - case <-ctx.Done(): - return errors.New("after 5 minutes, the file system could" + - "not be deleted, because the mount targets weren't deleted") - default: - _, err = efsSvc.DeleteFileSystem(&efs.DeleteFileSystemInput{FileSystemId: fileSystemId}) - if err != nil { - if strings.Contains(err.Error(), "because it has mount targets") { - // sleep here for 5 seconds to avoid slamming the API - time.Sleep(5 * time.Second) - continue - } - return err + ctx, cancel := context.WithTimeout(ctx, 5*time.Minute) + defer cancel() + for { + select { + case <-ctx.Done(): + return errors.New("after 5 minutes, the file system could" + + "not be deleted, because the mount targets weren't deleted") + default: + _, err = efsSvc.DeleteFileSystem(&efs.DeleteFileSystemInput{FileSystemId: fileSystemId}) + if err != nil { + if strings.Contains(err.Error(), "because it has mount targets") { + // sleep here for 5 seconds to avoid slamming the API + time.Sleep(5 * time.Second) + continue } - // if we reach this point, we're done - s.Update("Runner file system deleted") - s.Done() - return nil + return err } + // if we reach this point, we're done + s.Update("Runner file system deleted") + s.Done() + return nil } } - return nil } func (i *ECSRunnerInstaller) UninstallFlags(set *flag.Set) { @@ -565,7 +555,7 @@ func launchRunner( ExecutionRoleArn: aws.String(executionRoleArn), Cpu: aws.String(cpu), Memory: aws.String(memory), - Family: aws.String(installutil.DefaultRunnerName(id)), + Family: aws.String(defaultRunnerTagName), TaskRoleArn: &taskRoleArn, NetworkMode: aws.String("awsvpc"), RequiresCompatibilities: []*string{aws.String(defaultTaskRuntime)}, From fc462d231ec955bce714173f9664196493b71ab0 Mon Sep 17 00:00:00 2001 From: Joseph Rajewski <83741749+paladin-devops@users.noreply.github.com> Date: Mon, 12 Jun 2023 10:28:15 -0400 Subject: [PATCH 3/3] fix: Return err on DescribeMountTargets failure, during ECS runner uninstall. --- internal/runnerinstall/ecs.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/internal/runnerinstall/ecs.go b/internal/runnerinstall/ecs.go index 875e529754c..01f6356fd1a 100644 --- a/internal/runnerinstall/ecs.go +++ b/internal/runnerinstall/ecs.go @@ -420,6 +420,9 @@ DeleteFileSystem: describeMountTargetsResp, err := efsSvc.DescribeMountTargets(&efs.DescribeMountTargetsInput{ FileSystemId: fileSystemId, }) + if err != nil { + return err + } for _, mountTarget := range describeMountTargetsResp.MountTargets { _, err = efsSvc.DeleteMountTarget(&efs.DeleteMountTargetInput{MountTargetId: mountTarget.MountTargetId}) if err != nil {