Try to delete services created by CloudDebug integration tests when deinstrument fails (#2411)

- On deinstrument failure, the ecs services were not cleaned up properly. This has lead to multiple zombie services in our integration test accounts
- Also simplify the code using waiters

Author: hunterwerlla

jetbrains-core/it/software/aws/toolkits/jetbrains/services/clouddebug/CloudDebugTestCase.kt

@@ -9,11 +9,13 @@ import com.nhaarman.mockitokotlin2.mock
 import org.junit.After
 import org.junit.Before
 import org.junit.Rule
+import software.amazon.awssdk.core.waiters.WaiterOverrideConfiguration
 import software.amazon.awssdk.http.apache.ApacheHttpClient
 import software.amazon.awssdk.regions.Region
 import software.amazon.awssdk.services.cloudformation.CloudFormationClient
 import software.amazon.awssdk.services.ecs.EcsClient
 import software.amazon.awssdk.services.ecs.model.AssignPublicIp
+import software.amazon.awssdk.services.ecs.model.DescribeServicesRequest
 import software.amazon.awssdk.services.ecs.model.LaunchType
 import software.amazon.awssdk.services.ecs.model.Service
 import software.aws.toolkits.core.region.AwsRegion
@@ -29,8 +31,7 @@ import software.aws.toolkits.jetbrains.services.ecs.EcsUtils
 import software.aws.toolkits.jetbrains.services.ecs.resources.EcsResources
 import software.aws.toolkits.jetbrains.utils.rules.CloudFormationLazyInitRule
 import java.nio.file.Paths
-import java.util.concurrent.CountDownLatch
-import java.util.concurrent.TimeUnit
+import java.time.Duration
 
 abstract class CloudDebugTestCase(private val taskDefName: String) {
     protected lateinit var service: Service
@@ -69,7 +70,7 @@ abstract class CloudDebugTestCase(private val taskDefName: String) {
         runUnderRealCredentials(getProject()) {
             println("Instrumenting service")
             instrumentService()
-            val instrumentedServiceName = "cloud-debug-${EcsUtils.serviceArnToName(service.serviceArn())}"
+            val instrumentedServiceName = instrumentedServiceName()
             println("Waiting for $instrumentedServiceName to stabilize")
             ecsRule.ecsClient.waiter().waitUntilServicesStable {
                 it.cluster(service.clusterArn())
@@ -87,15 +88,30 @@ abstract class CloudDebugTestCase(private val taskDefName: String) {
 
     @After
     open fun tearDown() {
+        try {
+            deinstrumentService()
+        } finally {
+            // If deinstrumenting fails, or initialization doesn't work properly, we still want to try to delete the services, so kick that off
+            runCatching { ecsClient.deleteService { it.cluster(service.clusterArn()).service(service.serviceArn()).force(true) } }
+            runCatching { ecsClient.deleteService { it.cluster(service.clusterArn()).service(instrumentedServiceName()).force(true) } }
+        }
+    }
+
+    private fun deinstrumentService() {
         // TODO: this doesn't wait for the revert command to complete but fulfills our need to cleanup
         if (::instrumentedService.isInitialized) {
             runUnderRealCredentials(getProject()) {
-                deinstrumentService()
+                DeinstrumentResourceFromExplorerAction.performAction(
+                    getProject(),
+                    service.clusterArn(),
+                    EcsUtils.originalServiceName(instrumentedService.serviceName()),
+                    null
+                )
                 println("Waiting for ${instrumentedService.serviceArn()} to be deinstrumented")
-                ecsClient.waiter().waitUntilServicesInactive {
-                    it.cluster(instrumentedService.clusterArn())
-                    it.services(instrumentedService.serviceArn())
-                }
+                ecsClient.waiter().waitUntilServicesInactive(
+                    DescribeServicesRequest.builder().cluster(instrumentedService.clusterArn()).services(instrumentedService.serviceArn()).build(),
+                    WaiterOverrideConfiguration.builder().waitTimeout(Duration.ofMinutes(5)).build()
+                )
             }
             // TODO: verify that no error toasts were created, or similar mechanism
         }
@@ -146,31 +162,17 @@ abstract class CloudDebugTestCase(private val taskDefName: String) {
         }
     }
 
-    private fun awaitCli(latch: CountDownLatch) = { result: Boolean ->
-        latch.countDown()
-        if (!result) {
-            throw RuntimeException("CLI didn't complete successfully!")
-        }
-    }
-
     private fun instrumentService() {
-        val latch = CountDownLatch(1)
-        InstrumentResourceAction.performAction(getProject(), service.clusterArn(), service.serviceArn(), instrumentationRole, null, awaitCli(latch))
-        latch.await(5, TimeUnit.MINUTES)
-    }
-
-    private fun deinstrumentService() {
-        val latch = CountDownLatch(1)
-        DeinstrumentResourceFromExplorerAction.performAction(
-            getProject(),
-            service.clusterArn(),
-            EcsUtils.originalServiceName(instrumentedService.serviceName()),
-            null,
-            awaitCli(latch)
+        InstrumentResourceAction.performAction(getProject(), service.clusterArn(), service.serviceArn(), instrumentationRole, null)
+        println("Waiting for ${service.serviceArn()} to be instrumented")
+        ecsClient.waiter().waitUntilServicesStable(
+            DescribeServicesRequest.builder().cluster(service.clusterArn()).services(instrumentedServiceName()).build(),
+            WaiterOverrideConfiguration.builder().waitTimeout(Duration.ofMinutes(5)).build()
         )
-        latch.await(5, TimeUnit.MINUTES)
     }
 
+    private fun instrumentedServiceName() = "cloud-debug-${EcsUtils.serviceArnToName(service.serviceArn())}"
+
     abstract fun getProject(): Project
 
     companion object {

jetbrains-core/src/software/aws/toolkits/jetbrains/services/clouddebug/actions/DeinstrumentResourceAction.kt

@@ -44,8 +44,7 @@ class DeinstrumentResourceFromExplorerAction : SingleResourceNodeAction<EcsServi
             project: Project,
             clusterArn: String,
             instrumentedResourceName: String,
-            selected: EcsServiceNode?,
-            callback: ((Boolean) -> Unit)? = null
+            selected: EcsServiceNode?
         ) {
             val originalServiceName = EcsUtils.originalServiceName(instrumentedResourceName)
             DeinstrumentAction(
@@ -55,7 +54,7 @@ class DeinstrumentResourceFromExplorerAction : SingleResourceNodeAction<EcsServi
                 message("cloud_debug.instrument_resource.disable"),
                 message("cloud_debug.instrument_resource.disable.success", originalServiceName),
                 message("cloud_debug.instrument_resource.disable.failed", originalServiceName)
-            ).runAction(selected, callback)
+            ).runAction(selected)
         }
     }
 }

jetbrains-core/src/software/aws/toolkits/jetbrains/services/clouddebug/actions/InstrumentResourceAction.kt

@@ -74,8 +74,7 @@ class InstrumentResourceAction(
             clusterArn: String,
             serviceArn: String,
             roleArn: String,
-            selected: EcsServiceNode?,
-            callback: ((Boolean) -> Unit)? = null
+            selected: EcsServiceNode?
         ) {
             InstrumentAction(
                 project,
@@ -85,7 +84,7 @@ class InstrumentResourceAction(
                 message("cloud_debug.instrument_resource.enable"),
                 message("cloud_debug.instrument_resource.enable.success", EcsUtils.serviceArnToName(serviceArn)),
                 message("cloud_debug.instrument_resource.enable.fail", EcsUtils.serviceArnToName(serviceArn))
-            ).runAction(selected, callback)
+            ).runAction(selected)
         }
     }
 }

jetbrains-core/src/software/aws/toolkits/jetbrains/services/clouddebug/actions/PseCliAction.kt

@@ -58,7 +58,7 @@ abstract class PseCliAction(val project: Project, val actionName: String, privat
     abstract fun buildCommandLine(cmd: GeneralCommandLine)
     protected abstract fun produceTelemetry(startTime: Instant, result: Result, version: String?)
 
-    fun runAction(selectedNode: AbstractTreeNode<*>? = null, callback: ((Boolean) -> Unit)? = null) {
+    fun runAction(selectedNode: AbstractTreeNode<*>? = null) {
         ProgressManager.getInstance().run(
             object : Task.Backgroundable(
                 project,
@@ -105,7 +105,6 @@ abstract class PseCliAction(val project: Project, val actionName: String, privat
                             null
                         }
                     }.toCompletableFuture().join() ?: run {
-                        callback?.invoke(false)
                         return
                     }
 
@@ -162,14 +161,12 @@ abstract class PseCliAction(val project: Project, val actionName: String, privat
                         )
                         // reset the cache
                         project.clearResourceForCurrentConnection(CloudDebuggingResources.LIST_INSTRUMENTED_RESOURCES)
-                        callback?.invoke(true)
                     } else {
                         notifyError(
                             actionName,
                             failureMessage,
                             project
                         )
-                        callback?.invoke(false)
                     }
 
                     // Redraw cluster level if the action was taken from a node