Reduce housekeeping pressure on CloudFormation

.java-version

@@ -0,0 +1 @@
+1.8

CHANGELOG.md

@@ -28,8 +28,13 @@ Dropping a requirement of a major version of a dependency is a new contract.
 
 ### Fixed
 - Add missing `iam:GetRole` permission. You have to update the policy manually. Fix [JPERF-1407].
+- Reduce pressure on CloudFormation when cleaning long lists of expired stacks. Help [JPERF-1332].
+- Clean up EC2 security groups before CloudFormation stacks. Fix [JPERF-1208].
+- Fix housekeeping fail logging.
 
 [JPERF-1407]: https://ecosystem.atlassian.net/browse/JPERF-1407
+[JPERF-1332]: https://ecosystem.atlassian.net/browse/JPERF-1332
+[JPERF-1208]: https://ecosystem.atlassian.net/browse/JPERF-1208
 
 ## [1.13.0] - 2023-08-14
 [1.13.0]: https://github.com/atlassian-labs/aws-resources/compare/release-1.12.2...release-1.13.0

src/main/kotlin/com/atlassian/performance/tools/aws/Cloudformation.kt

@@ -7,25 +7,23 @@ import com.atlassian.performance.tools.aws.api.Aws
 import com.atlassian.performance.tools.aws.api.Investment
 import com.atlassian.performance.tools.aws.api.ProvisionedStack
 import com.atlassian.performance.tools.aws.api.ScrollingCloudformation
+import java.util.function.Consumer
 
 internal class Cloudformation(
     private val aws: Aws,
     private val cloudformation: AmazonCloudFormation
 ) {
-    fun listExpiredStacks(): List<ProvisionedStack> {
+    fun consumeExpiredStacks(call: Consumer<List<ProvisionedStack>>) {
         val cleanStackStatuses = listOf(
             StackStatus.DELETE_COMPLETE,
             StackStatus.DELETE_IN_PROGRESS
         )
-        val scrollingCloudformation = ScrollingCloudformation(cloudformation)
-        val stacks = mutableListOf<ProvisionedStack>()
-        scrollingCloudformation.scrollThroughStacks { stackBatch ->
-            stackBatch
+        ScrollingCloudformation(cloudformation).scrollThroughStacks { stackBatch ->
+            val expiredStacks = stackBatch
                 .filter { StackStatus.fromValue(it.stackStatus) !in cleanStackStatuses }
-                .forEach { stacks += ProvisionedStack(it, aws) }
-        }
-        return stacks.filter {
-            it.isExpired()
+                .map { ProvisionedStack(it, aws) }
+                .filter { it.isExpired() }
+            call.accept(expiredStacks)
         }
     }
 

src/main/kotlin/com/atlassian/performance/tools/aws/Ec2.kt

@@ -7,26 +7,27 @@ import com.atlassian.performance.tools.aws.ami.AmiImage
 import com.atlassian.performance.tools.aws.api.Resource
 import com.atlassian.performance.tools.aws.api.TerminationBatchingEc2
 import com.atlassian.performance.tools.aws.api.TerminationPollingEc2
+import java.util.function.Consumer
 
 internal class Ec2(
     private val ec2: AmazonEC2
 ) {
-    fun listExpiredInstances(): List<Resource> {
+    fun consumeExpiredInstances(call: Consumer<List<Ec2Instance>>) {
         val scrollingEc2 = TokenScrollingEc2(ec2)
         val terminationPollingEc2 = TerminationPollingEc2(scrollingEc2)
         val terminationBatchingEc2 = TerminationBatchingEc2(ec2, terminationPollingEc2)
 
-        val instances = mutableListOf<Resource>()
         val cleanInstanceStatuses = listOf(
             InstanceStateName.ShuttingDown,
             InstanceStateName.Terminated
         )
         scrollingEc2.scrollThroughInstances { instanceBatch ->
-            instanceBatch
+            val expiredInstances = instanceBatch
                 .filter { InstanceStateName.fromValue(it.state.name) !in cleanInstanceStatuses }
-                .forEach { instances += Ec2Instance(it, terminationBatchingEc2) }
+                .map { Ec2Instance(it, terminationBatchingEc2) }
+                .filter { it.isExpired() }
+            call.accept(expiredInstances)
         }
-        return instances.filter { it.isExpired() }
     }
 
     fun listExpiredAmis(): List<Resource> {
@@ -36,5 +37,4 @@ internal class Ec2(
             .map { AmiImage(image = it, ec2 = ec2) }
             .filter { it.isExpired() }
     }
-
 }

src/main/kotlin/com/atlassian/performance/tools/aws/api/housekeeping/ConcurrentHousekeeping.kt

@@ -8,6 +8,7 @@ import org.apache.logging.log4j.LogManager
 import java.time.Duration
 import java.time.Instant.now
 import java.util.concurrent.CompletableFuture
+import java.util.function.Consumer
 
 class ConcurrentHousekeeping(
     private val stackTimeout: Duration,
@@ -17,25 +18,26 @@ class ConcurrentHousekeeping(
     private val logger = LogManager.getLogger(this::class.java)
 
     override fun cleanLeftovers(aws: Aws) {
-        val stacks = Cloudformation(aws, aws.cloudformation).listExpiredStacks()
-        waitUntilReleased(stacks, stackTimeout)
-
-        val instances = Ec2(aws.ec2).listExpiredInstances()
-        waitUntilReleased(instances, instanceTimeout)
+        Ec2(aws.ec2).consumeExpiredInstances(Consumer { instances ->
+            waitUntilReleased(instances, instanceTimeout)
+        })
 
         val amis = Ec2(aws.ec2).listExpiredAmis()
         waitUntilReleased(amis, amiTimeout)
 
-        val keys = aws.ec2.describeKeyPairs().keyPairs.map { key ->
-            RemoteSshKey(SshKeyName(key.keyName), aws.ec2)
-        }.filter { it.isExpired() }
+        aws.ec2.describeKeyPairs().keyPairs
+            .map { key -> RemoteSshKey(SshKeyName(key.keyName), aws.ec2) }
+            .filter { it.isExpired() }
+            .forEach { it.release().get() }
 
         val securityGroups = aws.ec2.describeSecurityGroups().securityGroups.map { securityGroup ->
             Ec2SecurityGroup(securityGroup, aws.ec2)
         }.filter { it.isExpired() }
-
-        waitUntilReleased(keys)
         waitUntilReleased(securityGroups)
+
+        Cloudformation(aws, aws.cloudformation).consumeExpiredStacks(Consumer { stacks ->
+            waitUntilReleased(stacks, stackTimeout)
+        })
     }
 
     private fun waitUntilReleased(
@@ -54,7 +56,7 @@ class ConcurrentHousekeeping(
         if (!resource.isExpired()) {
             throw Exception("You can't release $resource. It hasn't expired.")
         }
-        return resource.release().handle { throwable, _ ->
+        return resource.release().handle { _, throwable: Throwable? ->
             if (throwable != null) {
                 logger.error("$resource failed to release itself", throwable)
             }

src/test/kotlin/com/atlassian/performance/tools/aws/CloudformationTest.kt

@@ -6,10 +6,12 @@ import com.amazonaws.services.cloudformation.model.DescribeStacksResult
 import com.amazonaws.services.cloudformation.model.Stack
 import com.atlassian.performance.tools.aws.api.Tag
 import org.assertj.core.api.Assertions.assertThat
+import org.assertj.core.api.Assertions.fail
 import org.junit.jupiter.api.Test
 import java.time.Duration.ofMinutes
 import java.time.Instant.now
 import java.util.*
+import java.util.function.Consumer
 
 class CloudformationTest {
     private val awsMock = FakeAws.awsForUnitTests()
@@ -21,9 +23,9 @@ class CloudformationTest {
         )
         val cloudformation = Cloudformation(awsMock, CloudformationMock(stacks))
 
-        val expiredStacks = cloudformation.listExpiredStacks()
-
-        assertThat(expiredStacks).isEmpty()
+        cloudformation.consumeExpiredStacks(Consumer { expiredStacks ->
+            assertThat(expiredStacks).isEmpty()
+        })
     }
 
     @Test
@@ -40,9 +42,9 @@ class CloudformationTest {
         )
         val cloudformation = Cloudformation(awsMock, CloudformationMock(stacks))
 
-        val expiredStacks = cloudformation.listExpiredStacks()
-
-        assertThat(expiredStacks).hasSize(1)
+        cloudformation.consumeExpiredStacks(Consumer { expiredStacks ->
+            assertThat(expiredStacks).hasSize(1)
+        })
     }
 
     @Test
@@ -58,9 +60,9 @@ class CloudformationTest {
         )
         val cloudformation = Cloudformation(awsMock, CloudformationMock(stacks))
 
-        val expiredStacks = cloudformation.listExpiredStacks()
-
-        assertThat(expiredStacks).isEmpty()
+        cloudformation.consumeExpiredStacks(Consumer { expiredStacks ->
+            assertThat(expiredStacks).isEmpty()
+        })
     }
 
     private class CloudformationMock(