Skip to content

Commit

Permalink
feat(rules): track when we saw an image in use last (#116)
Browse files Browse the repository at this point in the history
  • Loading branch information
emjburns authored Oct 16, 2018
1 parent b355789 commit a576fab
Show file tree
Hide file tree
Showing 14 changed files with 422 additions and 35 deletions.
2 changes: 1 addition & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ allprojects {
group = "com.netflix.spinnaker.swabbie"

ext {
spinnakerDependenciesVersion = project.hasProperty('spinnakerDependenciesVersion') ? project.property('spinnakerDependenciesVersion') : '1.0.18'
spinnakerDependenciesVersion = project.hasProperty('spinnakerDependenciesVersion') ? project.property('spinnakerDependenciesVersion') : '1.2.5'
}

def checkLocalVersions = [spinnakerDependenciesVersion: spinnakerDependenciesVersion]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package com.netflix.spinnaker.swabbie.aws.images

import com.netflix.spectator.api.Registry
import com.netflix.spinnaker.config.SwabbieProperties
import com.netflix.spinnaker.kork.core.RetrySupport
import com.netflix.spinnaker.moniker.frigga.FriggaReflectiveNamer
import com.netflix.spinnaker.swabbie.*
Expand All @@ -27,12 +28,10 @@ import com.netflix.spinnaker.swabbie.exclusions.ResourceExclusionPolicy
import com.netflix.spinnaker.swabbie.model.*
import com.netflix.spinnaker.swabbie.notifications.Notifier
import com.netflix.spinnaker.swabbie.orca.*
import com.netflix.spinnaker.swabbie.repository.ResourceStateRepository
import com.netflix.spinnaker.swabbie.repository.ResourceTrackingRepository
import com.netflix.spinnaker.swabbie.repository.TaskCompleteEventInfo
import com.netflix.spinnaker.swabbie.repository.TaskTrackingRepository
import com.netflix.spinnaker.swabbie.repository.*
import com.netflix.spinnaker.swabbie.tagging.TaggingService
import com.netflix.spinnaker.swabbie.tagging.UpsertImageTagsRequest
import net.logstash.logback.argument.StructuredArguments.kv
import org.springframework.context.ApplicationEventPublisher
import org.springframework.stereotype.Component
import java.time.Clock
Expand Down Expand Up @@ -60,7 +59,9 @@ class AmazonImageHandler(
private val orcaService: OrcaService,
private val applicationsCaches: List<InMemoryCache<Application>>,
private val taggingService: TaggingService,
private val taskTrackingRepository: TaskTrackingRepository
private val taskTrackingRepository: TaskTrackingRepository,
private val resourceUseTrackingRepository: ResourceUseTrackingRepository,
private val swabbieProperties: SwabbieProperties
) : AbstractResourceTypeHandler<AmazonImage>(
registry,
clock,
Expand Down Expand Up @@ -224,6 +225,7 @@ class AmazonImageHandler(
setUsedByInstances(images, params)
setUsedByLaunchConfigurations(images, params)
setHasSiblings(images, params)
setSeenWithinUnusedThreshold(images)
} catch (e: Exception) {
log.error("Failed to check image references. Params: {}", params, e)
throw IllegalStateException("Unable to process ${images.size} images. Params: $params", e)
Expand All @@ -249,8 +251,13 @@ class AmazonImageHandler(
images.filter {
NAIVE_EXCLUSION !in it.details && USED_BY_INSTANCES !in it.details
}.forEach { image ->
onMatchedImages(instances.map { it.imageId }, image) {
onMatchedImages(
instances.map { amazonInstance ->
SlimAwsResource(amazonInstance.name, amazonInstance.imageId, amazonInstance.getAutoscalingGroup().orEmpty())},
image
) { usedByResource ->
image.set(USED_BY_INSTANCES, true)
resourceUseTrackingRepository.recordUse(image.resourceId, usedByResource)
log.debug("Image {} ({}) in {} is USED_BY_INSTANCES", image.imageId, image.name, params["region"])
}
}
Expand All @@ -276,8 +283,18 @@ class AmazonImageHandler(
USED_BY_INSTANCES !in it.details &&
USED_BY_LAUNCH_CONFIGURATIONS !in it.details
}.forEach { image ->
onMatchedImages(launchConfigurations.map { it.imageId }, image) {
onMatchedImages(
launchConfigurations.map { launchConfiguration ->
SlimAwsResource(
launchConfiguration.resourceId,
launchConfiguration.imageId,
launchConfiguration.getAutoscalingGroupName()
)
},
image
) { usedByResource ->
log.debug("Image {} ({}) in {} is USED_BY_LAUNCH_CONFIGURATIONS", image.imageId, image.name, params["region"])
resourceUseTrackingRepository.recordUse(image.resourceId, usedByResource)
image.set(USED_BY_LAUNCH_CONFIGURATIONS, true)
}
}
Expand Down Expand Up @@ -323,6 +340,34 @@ class AmazonImageHandler(
}
}

/**
* Checks if an image has been seen in use recently.
*/
private fun setSeenWithinUnusedThreshold(images: List<AmazonImage>) {
log.info("Checking for images that haven't been seen in more than ${swabbieProperties.outOfUseThresholdDays} days")
val unseenImages: Map<String, String> = resourceUseTrackingRepository
.getUnused()
.map {
it.resourceIdentifier to it.usedByResourceIdentifier
}.toMap()

images.filter {
NAIVE_EXCLUSION !in it.details &&
USED_BY_INSTANCES !in it.details &&
USED_BY_LAUNCH_CONFIGURATIONS !in it.details &&
HAS_SIBLINGS_IN_OTHER_ACCOUNTS !in it.details &&
IS_BASE_OR_ANCESTOR !in it.details
}.forEach { image ->
if (!unseenImages.containsKey(image.imageId)) {
// Image is not in list of unseen, so we've seen it recently
// set that on image
// If there are no unseen images, this will set the key on every image.
image.set(SEEN_IN_USE_RECENTLY, true)
log.debug("Image {} ({}) has been SEEN_IN_USE_RECENTLY", kv("imageId", image.imageId), image.name)
}
}
}

/**
* Get images in accounts. Used to check for siblings in those accounts.
*/
Expand All @@ -346,10 +391,10 @@ class AmazonImageHandler(
return result
}

private fun onMatchedImages(ids: List<String>, image: AmazonImage, onFound: () -> Unit) {
private fun onMatchedImages(ids: List<SlimAwsResource>, image: AmazonImage, onFound: (String) -> Unit) {
ids.forEach {
if (image.imageId == it) {
onFound.invoke()
if (image.imageId == it.imageId) {
onFound.invoke(it.usedByResourceName)
}
}
}
Expand All @@ -374,3 +419,9 @@ private fun AmazonImage.isAncestorOf(image: AmazonImage): Boolean {
private fun AmazonImage.matches(image: AmazonImage): Boolean {
return name == image.name || imageId == image.imageId
}

private data class SlimAwsResource(
val id: String,
val imageId: String,
val usedByResourceName: String
)
Original file line number Diff line number Diff line change
Expand Up @@ -21,36 +21,44 @@ import com.netflix.spinnaker.swabbie.model.Rule
import com.netflix.spinnaker.swabbie.model.Summary
import org.springframework.stereotype.Component

/**
* Images are marked when they are orphaned.
*
* The `outOfUseThresholdDays` property controls the amount of time
* we let a resource be unseen (out of use) before it is marked and deleted.
* For example, if `outOfUseThresholdDays = 10`, then an image is allowed to sit in
* orphaned state (defined by the rules below) for 10 days before it will be marked.
*/
@Component
class OrphanedImageRule : Rule<AmazonImage> {
override fun apply(resource: AmazonImage): Result {
val isReferencedByInstances = resource.details.containsKey(USED_BY_INSTANCES) &&
resource.details[USED_BY_INSTANCES] as Boolean

val isReferencedByLaunchConfigs = resource.details.containsKey(USED_BY_LAUNCH_CONFIGURATIONS) &&
resource.details[USED_BY_LAUNCH_CONFIGURATIONS] as Boolean

val hasSiblings = resource.details.containsKey(HAS_SIBLINGS_IN_OTHER_ACCOUNTS) &&
resource.details[HAS_SIBLINGS_IN_OTHER_ACCOUNTS] as Boolean

val isBaseOrAncestorImage = resource.details.containsKey(IS_BASE_OR_ANCESTOR) &&
resource.details[IS_BASE_OR_ANCESTOR] as Boolean

if (isReferencedByInstances || isReferencedByLaunchConfigs || hasSiblings || isBaseOrAncestorImage) {
if (resource.matchesAnyRule(
USED_BY_INSTANCES,
USED_BY_LAUNCH_CONFIGURATIONS,
HAS_SIBLINGS_IN_OTHER_ACCOUNTS,
IS_BASE_OR_ANCESTOR,
SEEN_IN_USE_RECENTLY
)){
return Result(null)
}

return Result(
Summary(
description = "Image is not referenced by an Instance, Launch Configuration, " +
"and has no siblings in other accounts",
"and has no siblings in other accounts, " +
"and has been that way for over the outOfUseThreshold days",
ruleName = name()
)
)
}

private fun AmazonImage.matchesAnyRule(vararg ruleName: String): Boolean {
return ruleName.any { details.containsKey(it) && details[it] as Boolean }
}
}

const val USED_BY_INSTANCES = "usedByInstances"
const val USED_BY_LAUNCH_CONFIGURATIONS = "usedByLaunchConfigurations"
const val HAS_SIBLINGS_IN_OTHER_ACCOUNTS = "hasSiblingsInOtherAccounts"
const val IS_BASE_OR_ANCESTOR = "isBaseOrAncestor"
const val SEEN_IN_USE_RECENTLY = "seenInUseRecently"
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,18 @@ import java.time.ZoneId
data class AmazonInstance(
private val instanceId: String,
val imageId: String,
val tags: List<Map<String, String>>,
private val launchTime: Long,
override val resourceId: String = instanceId,
override val resourceType: String = INSTANCE,
override val cloudProvider: String = AWS,
override val name: String = instanceId,
private val creationDate: String? =
LocalDateTime.ofInstant(Instant.ofEpochMilli(launchTime), ZoneId.systemDefault()).toString()
) : AmazonResource(creationDate)
) : AmazonResource(creationDate) {
fun getAutoscalingGroup(): String? {
return tags
.find { it.containsKey("aws:autoscaling:groupName") }
?.get("aws:autoscaling:groupName")
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -35,4 +35,7 @@ data class AmazonLaunchConfiguration(
override val name: String = launchConfigurationName,
private val creationDate: String? =
LocalDateTime.ofInstant(Instant.ofEpochMilli(createdTime), ZoneId.systemDefault()).toString()
) : AmazonResource(creationDate)
) : AmazonResource(creationDate) {
fun getAutoscalingGroupName() =
launchConfigurationName.substringBeforeLast("-")
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,7 @@ import com.netflix.spinnaker.swabbie.exclusions.LiteralExclusionPolicy
import com.netflix.spinnaker.swabbie.model.*
import com.netflix.spinnaker.swabbie.orca.OrcaService
import com.netflix.spinnaker.swabbie.orca.TaskResponse
import com.netflix.spinnaker.swabbie.repository.ResourceStateRepository
import com.netflix.spinnaker.swabbie.repository.ResourceTrackingRepository
import com.netflix.spinnaker.swabbie.repository.TaskTrackingRepository
import com.netflix.spinnaker.swabbie.repository.*
import com.netflix.spinnaker.swabbie.tagging.TaggingService
import com.nhaarman.mockito_kotlin.*
import org.junit.jupiter.api.AfterEach
Expand Down Expand Up @@ -66,6 +64,8 @@ object AmazonImageHandlerTest {
private val applicationsCache = mock<InMemoryCache<Application>>()
private val taggingService = mock<TaggingService>()
private val taskTrackingRepository = mock<TaskTrackingRepository>()
private val resourceUseTrackingRepository = mock<ResourceUseTrackingRepository>()
private val swabbieProperties = mock<SwabbieProperties>()

private val subject = AmazonImageHandler(
clock = clock,
Expand All @@ -90,7 +90,9 @@ object AmazonImageHandlerTest {
accountProvider = accountProvider,
applicationsCaches = listOf(applicationsCache),
taggingService = taggingService,
taskTrackingRepository = taskTrackingRepository
taskTrackingRepository = taskTrackingRepository,
resourceUseTrackingRepository = resourceUseTrackingRepository,
swabbieProperties = swabbieProperties
)

@BeforeEach
Expand Down Expand Up @@ -148,6 +150,20 @@ object AmazonImageHandlerTest {
creationDate = LocalDateTime.now().minusDays(3).toString()
)
)

whenever(resourceUseTrackingRepository.getUnused()) doReturn
listOf(
LastSeenInfo(
"ami-123",
"sg-123-v001",
clock.instant().minus(Duration.ofDays(32)).toEpochMilli()
),
LastSeenInfo(
"ami-132",
"sg-132-v001",
clock.instant().minus(Duration.ofDays(32)).toEpochMilli()
)
)
}

@AfterEach
Expand Down Expand Up @@ -283,7 +299,13 @@ object AmazonImageHandlerTest {
instanceId = "i-123",
cloudProvider = AWS,
imageId = "ami-123", // reference to ami-123
launchTime = clock.instant().toEpochMilli()
launchTime = clock.instant().toEpochMilli(),
tags = listOf(mapOf(
"class" to "com.amazonaws.services.ec2.model.Tag",
"value" to "test-servergroup-v111",
"key" to "aws:autoscaling:groupName",
"aws:autoscaling:groupName" to "test-servergroup-v111"
))
)
)

Expand Down Expand Up @@ -440,6 +462,55 @@ object AmazonImageHandlerTest {
verify(taskTrackingRepository, times(1)).add(any(), any())
}


@Test
fun `should not soft delete if the images have been seen recently`() {
whenever(resourceUseTrackingRepository.getUnused()) doReturn
emptyList<LastSeenInfo>()

val fifteenDaysAgo = clock.instant().minusSeconds(15 * 24 * 60 * 60).toEpochMilli()
val thirteenDaysAgo = clock.instant().minusSeconds(13 * 24 * 60 * 60).toEpochMilli()
val workConfiguration = getWorkConfiguration(maxAgeDays = 2)
val image = AmazonImage(
imageId = "ami-123",
resourceId = "ami-123",
description = "ancestor_id=ami-122",
ownerId = null,
state = "available",
resourceType = IMAGE,
cloudProvider = AWS,
name = "123-xenial-hvm-sriov-ebs",
creationDate = LocalDateTime.now().minusDays(3).toString()
)

whenever(resourceRepository.getMarkedResourcesToSoftDelete()) doReturn
listOf(
MarkedResource(
resource = image,
summaries = listOf(Summary("Image is unused", "testRule 1")),
namespace = workConfiguration.namespace,
resourceOwner = "[email protected]",
projectedDeletionStamp = fifteenDaysAgo,
projectedSoftDeletionStamp = thirteenDaysAgo,
notificationInfo = NotificationInfo(
recipient = "[email protected]",
notificationType = "Email",
notificationStamp = clock.instant().toEpochMilli()
)
)
)
val params = Parameters(
mapOf("account" to "1234", "region" to "us-east-1", "imageId" to "ami-123")
)
whenever(taggingService.upsertImageTag(any())) doReturn "1234"
whenever(imageProvider.getAll(params)) doReturn listOf(image)

subject.softDelete(workConfiguration) {}

verify(taggingService, times(0)).upsertImageTag(any())
verify(taskTrackingRepository, times(0)).add(any(), any())
}

private fun Long.inDays(): Int
= Duration.between(Instant.ofEpochMilli(clock.millis()), Instant.ofEpochMilli(this)).toDays().toInt()

Expand Down
2 changes: 2 additions & 0 deletions swabbie-aws/swabbie-aws.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ dependencies {
compile project(":swabbie-core")
compile project(":swabbie-orca")
compile project(":swabbie-retrofit")
compile spinnaker.dependency("aws")


testCompile project(":swabbie-test")
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import java.time.ZoneId
@ConfigurationProperties("swabbie")
open class SwabbieProperties {
var dryRun: Boolean = true
var outOfUseThresholdDays: Int = 30
var providers: List<CloudProviderConfiguration> = mutableListOf()
var schedule: Schedule = Schedule()
var testing: Testing = Testing()
Expand Down
Loading

0 comments on commit a576fab

Please sign in to comment.