From a576fabab8cf545fb1b20e32acad6f3d75e09f09 Mon Sep 17 00:00:00 2001 From: Emily Burns Date: Tue, 16 Oct 2018 09:13:15 -0700 Subject: [PATCH] feat(rules): track when we saw an image in use last (#116) --- build.gradle | 2 +- .../swabbie/aws/images/AmazonImageHandler.kt | 71 ++++++++++-- .../spinnaker/swabbie/aws/images/Rules.kt | 36 +++--- .../swabbie/aws/instances/AmazonInstance.kt | 9 +- .../LaunchConfiguration.kt | 5 +- .../aws/images/AmazonImageHandlerTest.kt | 81 +++++++++++++- swabbie-aws/swabbie-aws.gradle | 2 + .../spinnaker/config/SwabbieProperties.kt | 1 + .../ResourceUseTrackingRepository.kt | 46 ++++++++ .../redis/RedisResourceTrackingRepository.kt | 2 +- .../RedisResourceUseTrackingRepository.kt | 104 ++++++++++++++++++ .../redis/RedisTaskTrackingRepository.kt | 5 +- .../RedisResourceUseTrackingRepositoryTest.kt | 92 ++++++++++++++++ swabbie-redis/swabbie-redis.gradle | 1 + 14 files changed, 422 insertions(+), 35 deletions(-) create mode 100644 swabbie-core/src/main/kotlin/com/netflix/spinnaker/swabbie/repository/ResourceUseTrackingRepository.kt create mode 100644 swabbie-redis/src/main/kotlin/com/netflix/spinnaker/swabbie/redis/RedisResourceUseTrackingRepository.kt create mode 100644 swabbie-redis/src/test/kotlin/com/netflix/spinnaker/swabbie/redis/RedisResourceUseTrackingRepositoryTest.kt diff --git a/build.gradle b/build.gradle index 754160de..1c94e202 100644 --- a/build.gradle +++ b/build.gradle @@ -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] diff --git a/swabbie-aws/src/main/kotlin/com/netflix/spinnaker/swabbie/aws/images/AmazonImageHandler.kt b/swabbie-aws/src/main/kotlin/com/netflix/spinnaker/swabbie/aws/images/AmazonImageHandler.kt index 7e501076..2053007e 100644 --- a/swabbie-aws/src/main/kotlin/com/netflix/spinnaker/swabbie/aws/images/AmazonImageHandler.kt +++ b/swabbie-aws/src/main/kotlin/com/netflix/spinnaker/swabbie/aws/images/AmazonImageHandler.kt @@ -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.* @@ -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 @@ -60,7 +59,9 @@ class AmazonImageHandler( private val orcaService: OrcaService, private val applicationsCaches: List>, private val taggingService: TaggingService, - private val taskTrackingRepository: TaskTrackingRepository + private val taskTrackingRepository: TaskTrackingRepository, + private val resourceUseTrackingRepository: ResourceUseTrackingRepository, + private val swabbieProperties: SwabbieProperties ) : AbstractResourceTypeHandler( registry, clock, @@ -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) @@ -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"]) } } @@ -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) } } @@ -323,6 +340,34 @@ class AmazonImageHandler( } } + /** + * Checks if an image has been seen in use recently. + */ + private fun setSeenWithinUnusedThreshold(images: List) { + log.info("Checking for images that haven't been seen in more than ${swabbieProperties.outOfUseThresholdDays} days") + val unseenImages: Map = 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. */ @@ -346,10 +391,10 @@ class AmazonImageHandler( return result } - private fun onMatchedImages(ids: List, image: AmazonImage, onFound: () -> Unit) { + private fun onMatchedImages(ids: List, image: AmazonImage, onFound: (String) -> Unit) { ids.forEach { - if (image.imageId == it) { - onFound.invoke() + if (image.imageId == it.imageId) { + onFound.invoke(it.usedByResourceName) } } } @@ -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 +) diff --git a/swabbie-aws/src/main/kotlin/com/netflix/spinnaker/swabbie/aws/images/Rules.kt b/swabbie-aws/src/main/kotlin/com/netflix/spinnaker/swabbie/aws/images/Rules.kt index ae0142b1..4cdad66b 100644 --- a/swabbie-aws/src/main/kotlin/com/netflix/spinnaker/swabbie/aws/images/Rules.kt +++ b/swabbie-aws/src/main/kotlin/com/netflix/spinnaker/swabbie/aws/images/Rules.kt @@ -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 { 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" diff --git a/swabbie-aws/src/main/kotlin/com/netflix/spinnaker/swabbie/aws/instances/AmazonInstance.kt b/swabbie-aws/src/main/kotlin/com/netflix/spinnaker/swabbie/aws/instances/AmazonInstance.kt index 98464ccd..cbfcbad2 100644 --- a/swabbie-aws/src/main/kotlin/com/netflix/spinnaker/swabbie/aws/instances/AmazonInstance.kt +++ b/swabbie-aws/src/main/kotlin/com/netflix/spinnaker/swabbie/aws/instances/AmazonInstance.kt @@ -28,6 +28,7 @@ import java.time.ZoneId data class AmazonInstance( private val instanceId: String, val imageId: String, + val tags: List>, private val launchTime: Long, override val resourceId: String = instanceId, override val resourceType: String = INSTANCE, @@ -35,4 +36,10 @@ data class AmazonInstance( 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") + } +} diff --git a/swabbie-aws/src/main/kotlin/com/netflix/spinnaker/swabbie/aws/launchconfigurations/LaunchConfiguration.kt b/swabbie-aws/src/main/kotlin/com/netflix/spinnaker/swabbie/aws/launchconfigurations/LaunchConfiguration.kt index a223f9a3..413c41f8 100644 --- a/swabbie-aws/src/main/kotlin/com/netflix/spinnaker/swabbie/aws/launchconfigurations/LaunchConfiguration.kt +++ b/swabbie-aws/src/main/kotlin/com/netflix/spinnaker/swabbie/aws/launchconfigurations/LaunchConfiguration.kt @@ -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("-") +} diff --git a/swabbie-aws/src/test/java/com/netflix/spinnaker/swabbie/aws/images/AmazonImageHandlerTest.kt b/swabbie-aws/src/test/java/com/netflix/spinnaker/swabbie/aws/images/AmazonImageHandlerTest.kt index 21829649..d99da225 100644 --- a/swabbie-aws/src/test/java/com/netflix/spinnaker/swabbie/aws/images/AmazonImageHandlerTest.kt +++ b/swabbie-aws/src/test/java/com/netflix/spinnaker/swabbie/aws/images/AmazonImageHandlerTest.kt @@ -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 @@ -66,6 +64,8 @@ object AmazonImageHandlerTest { private val applicationsCache = mock>() private val taggingService = mock() private val taskTrackingRepository = mock() + private val resourceUseTrackingRepository = mock() + private val swabbieProperties = mock() private val subject = AmazonImageHandler( clock = clock, @@ -90,7 +90,9 @@ object AmazonImageHandlerTest { accountProvider = accountProvider, applicationsCaches = listOf(applicationsCache), taggingService = taggingService, - taskTrackingRepository = taskTrackingRepository + taskTrackingRepository = taskTrackingRepository, + resourceUseTrackingRepository = resourceUseTrackingRepository, + swabbieProperties = swabbieProperties ) @BeforeEach @@ -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 @@ -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" + )) ) ) @@ -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() + + 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 = "test@netflix.com", + projectedDeletionStamp = fifteenDaysAgo, + projectedSoftDeletionStamp = thirteenDaysAgo, + notificationInfo = NotificationInfo( + recipient = "test@netflix.com", + 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() diff --git a/swabbie-aws/swabbie-aws.gradle b/swabbie-aws/swabbie-aws.gradle index de5c7e47..ef2e9a77 100644 --- a/swabbie-aws/swabbie-aws.gradle +++ b/swabbie-aws/swabbie-aws.gradle @@ -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") } diff --git a/swabbie-core/src/main/kotlin/com/netflix/spinnaker/config/SwabbieProperties.kt b/swabbie-core/src/main/kotlin/com/netflix/spinnaker/config/SwabbieProperties.kt index 9ae95ab4..266c1ee1 100644 --- a/swabbie-core/src/main/kotlin/com/netflix/spinnaker/config/SwabbieProperties.kt +++ b/swabbie-core/src/main/kotlin/com/netflix/spinnaker/config/SwabbieProperties.kt @@ -26,6 +26,7 @@ import java.time.ZoneId @ConfigurationProperties("swabbie") open class SwabbieProperties { var dryRun: Boolean = true + var outOfUseThresholdDays: Int = 30 var providers: List = mutableListOf() var schedule: Schedule = Schedule() var testing: Testing = Testing() diff --git a/swabbie-core/src/main/kotlin/com/netflix/spinnaker/swabbie/repository/ResourceUseTrackingRepository.kt b/swabbie-core/src/main/kotlin/com/netflix/spinnaker/swabbie/repository/ResourceUseTrackingRepository.kt new file mode 100644 index 00000000..961da001 --- /dev/null +++ b/swabbie-core/src/main/kotlin/com/netflix/spinnaker/swabbie/repository/ResourceUseTrackingRepository.kt @@ -0,0 +1,46 @@ +/* + * + * * Copyright 2018 Netflix, Inc. + * * + * * Licensed under the Apache License, Version 2.0 (the "License") + * * you may not use this file except in compliance with the License. + * * You may obtain a copy of the License at + * * + * * http://www.apache.org/licenses/LICENSE-2.0 + * * + * * Unless required by applicable law or agreed to in writing, software + * * distributed under the License is distributed on an "AS IS" BASIS, + * * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * * See the License for the specific language governing permissions and + * * limitations under the License. + * + */ + +package com.netflix.spinnaker.swabbie.repository + +interface ResourceUseTrackingRepository { + /** + * record that resource is used by another resource at this exact time + */ + fun recordUse(resourceIdentifier: String, usedByResourceIdentifier: String) + + /** + * gets resources that haven't been seen in use for X days + */ + fun getUnused(): List + + fun isUnused(resourceIdentifier: String): Boolean + + /** + * Returns true if data is present for the whole outOfUseThreshold period. + * Returns false if data is only present for part of the period, + * i.e. if the outOfUseThreshold is 10 days but we only have data for 5. + */ + fun hasCompleteData(): Boolean +} + +data class LastSeenInfo( + val resourceIdentifier: String, + val usedByResourceIdentifier: String, + val timeSeen: Long +) diff --git a/swabbie-redis/src/main/kotlin/com/netflix/spinnaker/swabbie/redis/RedisResourceTrackingRepository.kt b/swabbie-redis/src/main/kotlin/com/netflix/spinnaker/swabbie/redis/RedisResourceTrackingRepository.kt index 722f6dc1..549569a2 100644 --- a/swabbie-redis/src/main/kotlin/com/netflix/spinnaker/swabbie/redis/RedisResourceTrackingRepository.kt +++ b/swabbie-redis/src/main/kotlin/com/netflix/spinnaker/swabbie/redis/RedisResourceTrackingRepository.kt @@ -30,7 +30,7 @@ import java.time.Instant @Component class RedisResourceTrackingRepository( - @Qualifier("redisClientSelector") redisClientSelector: RedisClientSelector, + redisClientSelector: RedisClientSelector, private val objectMapper: ObjectMapper, private val clock: Clock ) : ResourceTrackingRepository { diff --git a/swabbie-redis/src/main/kotlin/com/netflix/spinnaker/swabbie/redis/RedisResourceUseTrackingRepository.kt b/swabbie-redis/src/main/kotlin/com/netflix/spinnaker/swabbie/redis/RedisResourceUseTrackingRepository.kt new file mode 100644 index 00000000..d5a50695 --- /dev/null +++ b/swabbie-redis/src/main/kotlin/com/netflix/spinnaker/swabbie/redis/RedisResourceUseTrackingRepository.kt @@ -0,0 +1,104 @@ +/* + * + * * Copyright 2018 Netflix, Inc. + * * + * * Licensed under the Apache License, Version 2.0 (the "License") + * * you may not use this file except in compliance with the License. + * * You may obtain a copy of the License at + * * + * * http://www.apache.org/licenses/LICENSE-2.0 + * * + * * Unless required by applicable law or agreed to in writing, software + * * distributed under the License is distributed on an "AS IS" BASIS, + * * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * * See the License for the specific language governing permissions and + * * limitations under the License. + * + */ + +package com.netflix.spinnaker.swabbie.redis + +import com.fasterxml.jackson.databind.ObjectMapper +import com.fasterxml.jackson.module.kotlin.readValue +import com.netflix.spinnaker.config.SwabbieProperties +import com.netflix.spinnaker.kork.jedis.RedisClientDelegate +import com.netflix.spinnaker.kork.jedis.RedisClientSelector +import com.netflix.spinnaker.swabbie.repository.LastSeenInfo +import com.netflix.spinnaker.swabbie.repository.ResourceUseTrackingRepository +import org.slf4j.LoggerFactory +import org.springframework.stereotype.Component +import java.time.Clock +import java.time.Duration +import java.time.temporal.ChronoUnit + +@Component +class RedisResourceUseTrackingRepository( + redisClientSelector: RedisClientSelector, + private val objectMapper: ObjectMapper, + private val clock: Clock, + swabbieProperties: SwabbieProperties +) : ResourceUseTrackingRepository { + + private val log = LoggerFactory.getLogger(javaClass) + private val LAST_SEEN = "{swabbie:resourceUseTracking}:lastseen" + private val READY_FOR_CLEANING = "{swabbie:resourceUseTracking}:readyforcleaning" + private val INIT_TIME = "swabbie:inittime" + private val outOfUseThresholdDays = swabbieProperties.outOfUseThresholdDays + + private val redisClientDelegate: RedisClientDelegate = redisClientSelector.primary("default") + + init { + log.info("Using ${javaClass.simpleName}") + + redisClientDelegate.withCommandsClient { client -> + client.setnx(INIT_TIME, clock.instant().toEpochMilli().toString()) + } + } + + override fun hasCompleteData(): Boolean { + return redisClientDelegate.withCommandsClient { client -> + client.get(INIT_TIME) + }.toLong() < clock.instant().minus(Duration.ofDays(outOfUseThresholdDays.toLong())).toEpochMilli() + } + + override fun recordUse(resourceIdentifier: String, usedByResourceIdentifier: String) { + redisClientDelegate.withCommandsClient { client -> + client.hset( + LAST_SEEN, + resourceIdentifier, + objectMapper.writeValueAsString( + LastSeenInfo(resourceIdentifier, usedByResourceIdentifier, clock.instant().toEpochMilli())) + ) + // add to sorted set by score + // score = day when resource will be ready to delete + // if resource is seen in next X days, resource will be updated + client.zadd(READY_FOR_CLEANING, plusXdays(outOfUseThresholdDays).toDouble(), resourceIdentifier) + } + } + + override fun getUnused(): List { + val keys = redisClientDelegate.withCommandsClient> { client -> + client.zrangeByScore(READY_FOR_CLEANING, 0.0, clock.instant().toEpochMilli().toDouble()) + } + return hydrateLastSeen(keys) + } + + private fun hydrateLastSeen(keys: Set): List { + if (keys.isEmpty()) return emptyList() + return redisClientDelegate.withCommandsClient> { client -> + client.hmget(LAST_SEEN, *keys.toTypedArray()).toSet() + }.map { json -> + objectMapper.readValue(json) + } + } + + override fun isUnused(resourceIdentifier: String): Boolean { + return redisClientDelegate.withCommandsClient { client -> + client.zscore(READY_FOR_CLEANING, resourceIdentifier) + }.toLong() < clock.instant().toEpochMilli() + } + + fun plusXdays(days: Int): Long { + return clock.instant().plus(days.toLong(), ChronoUnit.DAYS).toEpochMilli() + } +} diff --git a/swabbie-redis/src/main/kotlin/com/netflix/spinnaker/swabbie/redis/RedisTaskTrackingRepository.kt b/swabbie-redis/src/main/kotlin/com/netflix/spinnaker/swabbie/redis/RedisTaskTrackingRepository.kt index 90f141ff..82dcdfe5 100644 --- a/swabbie-redis/src/main/kotlin/com/netflix/spinnaker/swabbie/redis/RedisTaskTrackingRepository.kt +++ b/swabbie-redis/src/main/kotlin/com/netflix/spinnaker/swabbie/redis/RedisTaskTrackingRepository.kt @@ -24,7 +24,6 @@ import com.netflix.spinnaker.kork.jedis.RedisClientSelector import com.netflix.spinnaker.swabbie.repository.TaskCompleteEventInfo import com.netflix.spinnaker.swabbie.repository.TaskState import com.netflix.spinnaker.swabbie.repository.TaskTrackingRepository -import net.logstash.logback.argument.StructuredArguments.kv import org.slf4j.Logger import org.slf4j.LoggerFactory import org.springframework.beans.factory.annotation.Qualifier @@ -34,7 +33,7 @@ import java.util.concurrent.TimeUnit @Component class RedisTaskTrackingRepository( - @Qualifier("redisClientSelector") redisClientSelector: RedisClientSelector, + redisClientSelector: RedisClientSelector, private val objectMapper: ObjectMapper, private val clock: Clock ) : TaskTrackingRepository { @@ -141,6 +140,8 @@ class RedisTaskTrackingRepository( //todo eb: this actually cleans up running tasks as well override fun cleanUpFinishedTasks(daysToLeave: Int) { val allBefore: Set = getAllBefore(daysToLeave) + if (allBefore.isEmpty()) return + log.debug("Cleaning ${allBefore.size} tasks") redisClientDelegate.withCommandsClient { client -> client.hdel(TASK_STATUS_KEY, *allBefore.toTypedArray()) diff --git a/swabbie-redis/src/test/kotlin/com/netflix/spinnaker/swabbie/redis/RedisResourceUseTrackingRepositoryTest.kt b/swabbie-redis/src/test/kotlin/com/netflix/spinnaker/swabbie/redis/RedisResourceUseTrackingRepositoryTest.kt new file mode 100644 index 00000000..0e18b3cf --- /dev/null +++ b/swabbie-redis/src/test/kotlin/com/netflix/spinnaker/swabbie/redis/RedisResourceUseTrackingRepositoryTest.kt @@ -0,0 +1,92 @@ +/* + * + * * Copyright 2018 Netflix, Inc. + * * + * * Licensed under the Apache License, Version 2.0 (the "License") + * * you may not use this file except in compliance with the License. + * * You may obtain a copy of the License at + * * + * * http://www.apache.org/licenses/LICENSE-2.0 + * * + * * Unless required by applicable law or agreed to in writing, software + * * distributed under the License is distributed on an "AS IS" BASIS, + * * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * * See the License for the specific language governing permissions and + * * limitations under the License. + * + */ + +package com.netflix.spinnaker.swabbie.redis + +import com.fasterxml.jackson.databind.DeserializationFeature +import com.fasterxml.jackson.databind.ObjectMapper +import com.fasterxml.jackson.module.kotlin.KotlinModule +import com.netflix.spinnaker.config.SwabbieProperties +import com.netflix.spinnaker.config.resourceDeserializerModule +import com.netflix.spinnaker.kork.jedis.EmbeddedRedis +import com.netflix.spinnaker.kork.jedis.JedisClientDelegate +import com.netflix.spinnaker.kork.jedis.RedisClientSelector +import com.netflix.spinnaker.swabbie.test.TestResource +import main.java.com.netflix.spinnaker.kork.test.time.MutableClock +import org.junit.jupiter.api.AfterAll +import org.junit.jupiter.api.BeforeEach +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.TestInstance +import org.springframework.util.Assert +import redis.clients.jedis.JedisPool +import java.time.Duration + +@TestInstance(TestInstance.Lifecycle.PER_CLASS) +object RedisResourceUseTrackingRepositoryTest { + + private val embeddedRedis = EmbeddedRedis.embed() + private val jedisPool = embeddedRedis.pool as JedisPool + private val objectMapper = ObjectMapper().apply { + registerSubtypes(TestResource::class.java) + registerModule(KotlinModule()) + registerModule(resourceDeserializerModule()) + disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES) + } + + private val clock = MutableClock() + + private val resourceUseTrackingRepository = RedisResourceUseTrackingRepository( + RedisClientSelector(listOf(JedisClientDelegate("primaryDefault", jedisPool))), + objectMapper, + clock, + SwabbieProperties().apply { outOfUseThresholdDays = 3 } + ) + + @BeforeEach + fun setup() { + jedisPool.resource.use { + it.flushDB() + } + } + + @AfterAll + fun cleanup() { + embeddedRedis.destroy() + } + + @Test + fun `adds a resource correctly`() { + resourceUseTrackingRepository.recordUse("ami-111", "servergroup-v001") + Assert.isTrue(!resourceUseTrackingRepository.isUnused("ami-111"), "ami was just seen, it shouldn't be unused") + } + + @Test + fun `correctly returns unused resource after 5 days`() { + resourceUseTrackingRepository.recordUse("ami-111", "servergroup-v001") + clock.incrementBy(Duration.ofDays(2)) + resourceUseTrackingRepository.recordUse("ami-222", "anothersg-v004") + clock.incrementBy(Duration.ofDays(2)) + + val unused = resourceUseTrackingRepository.getUnused() + Assert.notEmpty(unused, "ami should be unused after 5 days") + Assert.isTrue( + unused.first().usedByResourceIdentifier == "servergroup-v001", + "should be the correct server group" + ) + } +} diff --git a/swabbie-redis/swabbie-redis.gradle b/swabbie-redis/swabbie-redis.gradle index 829da4b9..4c480bb8 100644 --- a/swabbie-redis/swabbie-redis.gradle +++ b/swabbie-redis/swabbie-redis.gradle @@ -20,6 +20,7 @@ dependencies { compile spinnaker.dependency('jedis') compile spinnaker.dependency('korkJedis') compile spinnaker.dependency('korkDynomite') + compile spinnaker.dependency('korkTest') testCompile spinnaker.dependency('korkJedisTest') testCompile project(":swabbie-test")