diff --git a/keel-core/src/main/kotlin/com/netflix/spinnaker/config/KeelConfiguration.kt b/keel-core/src/main/kotlin/com/netflix/spinnaker/config/KeelConfiguration.kt index e996536710..d74d092420 100644 --- a/keel-core/src/main/kotlin/com/netflix/spinnaker/config/KeelConfiguration.kt +++ b/keel-core/src/main/kotlin/com/netflix/spinnaker/config/KeelConfiguration.kt @@ -60,7 +60,7 @@ open class KeelConfiguration { @Bean @ConditionalOnMissingBean(IntentActivityRepository::class) - open fun memoryIntentActivityRepository(): IntentActivityRepository = MemoryIntentActivityRepository(properties) + open fun memoryIntentActivityRepository(): IntentActivityRepository = MemoryIntentActivityRepository() @Bean open fun clock(): Clock = Clock.systemDefaultZone() diff --git a/keel-core/src/main/kotlin/com/netflix/spinnaker/config/KeelProperties.kt b/keel-core/src/main/kotlin/com/netflix/spinnaker/config/KeelProperties.kt index 57fb22dc24..d25867e45a 100644 --- a/keel-core/src/main/kotlin/com/netflix/spinnaker/config/KeelProperties.kt +++ b/keel-core/src/main/kotlin/com/netflix/spinnaker/config/KeelProperties.kt @@ -21,10 +21,9 @@ import org.springframework.boot.context.properties.ConfigurationProperties class KeelProperties { var prettyPrintJson: Boolean = false var immediatelyRunIntents: Boolean = true - var maxConvergenceLogEntriesPerIntent: Int = 720 // one entry every 30 s, this will keep 6 hours of logs + var maxConvergenceLogEntriesPerIntent: Int = 1000 var intentPackages: List = listOf("com.netflix.spinnaker.keel.intent") var intentSpecPackages: List = listOf("com.netflix.spinnaker.keel.intent") - var policyPackages: List = listOf("com.netflix.spinnaker.keel.policy") var attributePackages: List = listOf("com.netflix.spinnaker.keel.attribute") } diff --git a/keel-core/src/main/kotlin/com/netflix/spinnaker/keel/IntentActivityRepository.kt b/keel-core/src/main/kotlin/com/netflix/spinnaker/keel/IntentActivityRepository.kt index 08e5e65733..c7742e60ec 100644 --- a/keel-core/src/main/kotlin/com/netflix/spinnaker/keel/IntentActivityRepository.kt +++ b/keel-core/src/main/kotlin/com/netflix/spinnaker/keel/IntentActivityRepository.kt @@ -15,51 +15,70 @@ */ package com.netflix.spinnaker.keel -import com.netflix.spinnaker.keel.dryrun.ChangeType -import com.netflix.spinnaker.keel.state.FieldState +import com.fasterxml.jackson.annotation.JsonSubTypes +import com.fasterxml.jackson.annotation.JsonSubTypes.Type +import com.fasterxml.jackson.annotation.JsonTypeInfo +import com.fasterxml.jackson.annotation.JsonTypeInfo.As.PROPERTY +import com.fasterxml.jackson.annotation.JsonTypeInfo.Id.NAME +import com.fasterxml.jackson.annotation.JsonTypeName +import com.netflix.spinnaker.keel.model.ListCriteria +import java.time.Instant -/** - * @param intentId The ID of the intent that is being evaluated. - * @param changeType The type of change that took place this cycle. - * @param orchestrations A resultant (if any) list of orchestration IDs from the intent. - * @param messages Human-friendly messages about the change. - * @param diff The diff between current and desired state. - * @param actor Who (or what) initiated the operation. - * @param timestampMillis The timestamp in millis the record was created. - */ -data class IntentConvergenceRecord( - val intentId: String, - val changeType: ChangeType, - val orchestrations: List?, - val messages: List?, - val diff: Set, - val actor: String, - val timestampMillis: Long +@JsonTypeInfo(use = NAME, include = PROPERTY, property = "kind") +@JsonSubTypes( + Type(IntentChangeRecord::class), + Type(IntentConvergenceRecord::class) ) +sealed class ActivityRecord { + abstract val intentId: String + abstract val actor: String + val timestamp: Instant = Instant.now() +} -interface IntentActivityRepository { +enum class IntentChangeAction { UPSERT, DELETE } - fun addOrchestration(intentId: String, orchestrationId: String) +/** + * An activity record for whenever a change is made to [intentId] + */ +@JsonTypeName("IntentChange") +data class IntentChangeRecord( + override val intentId: String, + override val actor: String, + val action: IntentChangeAction, + val value: Intent +) : ActivityRecord() - fun addOrchestrations(intentId: String, orchestrations: List) +/** + * Activity record whenever an intent is converged. + */ +@JsonTypeName("IntentConvergence") +data class IntentConvergenceRecord( + override val intentId: String, + override val actor: String, + val result: ConvergeResult +) : ActivityRecord() - fun getHistory(intentId: String): List +/** + * Find the [ActivityRecord] class for [name]. Everyone loves reflection. + */ +fun activityRecordClassForName(name: String): Class? { + return ActivityRecord::class.annotations + .filterIsInstance() + .firstOrNull() + ?.let { subTypes -> + @Suppress("UNCHECKED_CAST") + subTypes.value.find { it.name == name }?.value as Class? + } +} - fun logConvergence(intentConvergenceRecord: IntentConvergenceRecord) +/** + * Responsible for recording all activity related to intents. This is primarily meant for diagnostics and auditing. + */ +interface IntentActivityRepository { - fun getLog(intentId: String): List + fun record(activity: ActivityRecord) - /** - * Permalink to a specific log message, identified by timestampMillis - * @param intentId The ID of the intent that is being evaluated. - * @param timestampMillis The timestamp of the log message, - * used as the unique identifier of the message, in milliseconds. - */ - fun getLogEntry(intentId: String, timestampMillis: Long): IntentConvergenceRecord? + fun getHistory(intentId: String, criteria: ListCriteria): List - /** - * If orchestrationId is passed in as a link to the task in orca, strip the leading path - * off so that we're storing the actual orchestrationId - */ - fun parseOrchestrationId(orchestrationId: String) = orchestrationId.removePrefix("/tasks/") + fun getHistory(intentId: String, kind: Class, criteria: ListCriteria): List } diff --git a/keel-core/src/main/kotlin/com/netflix/spinnaker/keel/event/IntentActivityListener.kt b/keel-core/src/main/kotlin/com/netflix/spinnaker/keel/event/IntentActivityListener.kt new file mode 100644 index 0000000000..85c96e8c24 --- /dev/null +++ b/keel-core/src/main/kotlin/com/netflix/spinnaker/keel/event/IntentActivityListener.kt @@ -0,0 +1,40 @@ +/* + * 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.keel.event + +import com.netflix.spinnaker.keel.IntentActivityRepository +import com.netflix.spinnaker.keel.IntentChangeAction.DELETE +import com.netflix.spinnaker.keel.IntentChangeAction.UPSERT +import com.netflix.spinnaker.keel.IntentChangeRecord +import org.springframework.context.event.EventListener +import org.springframework.stereotype.Component + +@Component +class IntentActivityListener( + private val intentActivityRepository: IntentActivityRepository +) { + + @EventListener(AfterIntentUpsertEvent::class, AfterIntentDeleteEvent::class) + fun recordUpsert(event: IntentAwareEvent) { + // TODO rz - actor needs to be pulled out of AuthenticatedRequest + intentActivityRepository.record(IntentChangeRecord( + event.intent.id(), + action = if (event is AfterIntentUpsertEvent) UPSERT else DELETE, + actor = "TODO: unknown", + value = event.intent + )) + } +} diff --git a/keel-core/src/main/kotlin/com/netflix/spinnaker/keel/memory/MemoryIntentActivityRepository.kt b/keel-core/src/main/kotlin/com/netflix/spinnaker/keel/memory/MemoryIntentActivityRepository.kt index cb8f764d0e..9d909384d9 100644 --- a/keel-core/src/main/kotlin/com/netflix/spinnaker/keel/memory/MemoryIntentActivityRepository.kt +++ b/keel-core/src/main/kotlin/com/netflix/spinnaker/keel/memory/MemoryIntentActivityRepository.kt @@ -15,73 +15,58 @@ */ package com.netflix.spinnaker.keel.memory -import com.netflix.spinnaker.config.KeelProperties +import com.netflix.spinnaker.keel.ActivityRecord import com.netflix.spinnaker.keel.IntentActivityRepository -import com.netflix.spinnaker.keel.IntentConvergenceRecord +import com.netflix.spinnaker.keel.model.ListCriteria import org.slf4j.LoggerFactory -import org.springframework.beans.factory.annotation.Autowired import java.util.concurrent.ConcurrentHashMap import javax.annotation.PostConstruct -class MemoryIntentActivityRepository -@Autowired constructor( - private val keelProperties: KeelProperties -) : IntentActivityRepository { +class MemoryIntentActivityRepository : IntentActivityRepository { private val log = LoggerFactory.getLogger(javaClass) - private val orchestrations: ConcurrentHashMap> = ConcurrentHashMap() - - private val convergenceLog: ConcurrentHashMap> = ConcurrentHashMap() + private val activities: ConcurrentHashMap> = ConcurrentHashMap() @PostConstruct fun init() { log.info("Using ${javaClass.simpleName}") } - override fun addOrchestration(intentId: String, orchestrationId: String) { - val orchestrationUUID = parseOrchestrationId(orchestrationId) - if (!orchestrations.containsKey(intentId)) { - orchestrations[intentId] = mutableSetOf() - } - if (orchestrations[intentId]?.contains(orchestrationUUID) == false) { - orchestrations[intentId]?.add(orchestrationUUID) - } + override fun record(activity: ActivityRecord) { + ensureIntentLog(activity.intentId) + activities[activity.intentId]!!.add(activity) + } + + override fun getHistory(intentId: String, criteria: ListCriteria): List { + ensureIntentLog(intentId) + return activities[intentId]!!.let { limitOffset(it, criteria) } } - override fun addOrchestrations(intentId: String, orchestrations: List) { - orchestrations.forEach { addOrchestration(intentId, it) } + override fun getHistory(intentId: String, kind: Class, criteria: ListCriteria): List { + ensureIntentLog(intentId) + return activities[intentId]!! + .filterIsInstance(kind) + .let { limitOffset(it, criteria) } } - override fun getHistory(intentId: String) = orchestrations.getOrDefault(intentId, mutableSetOf()).toList() + private fun ensureIntentLog(intentId: String) { + if (!activities.containsKey(intentId)) { + activities[intentId] = mutableListOf() + } + } - override fun logConvergence(intentConvergenceRecord: IntentConvergenceRecord) { - val intentId = intentConvergenceRecord.intentId - if (!convergenceLog.containsKey(intentId)){ - convergenceLog[intentId] = mutableListOf(intentConvergenceRecord) - } else { - if (convergenceLog[intentId] == null) { - convergenceLog[intentId] = mutableListOf(intentConvergenceRecord) + private fun limitOffset(list: List, criteria: ListCriteria): List = + list.let { + val size = it.size + if (size <= criteria.offset) { + listOf() } else { - convergenceLog[intentId]?.let { l -> - l.add(intentConvergenceRecord) - // Drop oldest entries if we're over the message limit - val numMsgsLeft = keelProperties.maxConvergenceLogEntriesPerIntent - l.count() - if (numMsgsLeft < 0){ - convergenceLog[intentId] = l.drop(-1*numMsgsLeft).toMutableList() - } + var lastIndex = criteria.offset + criteria.limit + if (lastIndex >= size) { + lastIndex = size } + it.subList(criteria.offset, lastIndex).toList() } } - } - - override fun getLog(intentId: String): List - = convergenceLog[intentId] ?: emptyList() - - // if there are multiple messages with the same timestamp, return the first - override fun getLogEntry(intentId: String, timestampMillis: Long) - = convergenceLog[intentId]?.filter { it.timestampMillis == timestampMillis }?.toList()?.also { - // The same intent shouldn't be processed more than once at the exact same time. - if (it.size > 1) log.warn("Two messages with the same timestampMillis. This shouldn't happen.") - }?.first() } diff --git a/keel-core/src/main/kotlin/com/netflix/spinnaker/keel/model/ListCriteria.kt b/keel-core/src/main/kotlin/com/netflix/spinnaker/keel/model/ListCriteria.kt new file mode 100644 index 0000000000..dd15fd980d --- /dev/null +++ b/keel-core/src/main/kotlin/com/netflix/spinnaker/keel/model/ListCriteria.kt @@ -0,0 +1,27 @@ +/* + * 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.keel.model + +// TODO validation +interface ListCriteria { + val limit: Int + val offset: Int +} + +data class PagingListCriteria( + override val limit: Int = 10, + override val offset: Int = 0 +) : ListCriteria diff --git a/keel-orca/src/main/kotlin/com/netflix/spinnaker/keel/orca/OrcaIntentLauncher.kt b/keel-orca/src/main/kotlin/com/netflix/spinnaker/keel/orca/OrcaIntentLauncher.kt index 9fa2e89af6..a828300fc9 100644 --- a/keel-orca/src/main/kotlin/com/netflix/spinnaker/keel/orca/OrcaIntentLauncher.kt +++ b/keel-orca/src/main/kotlin/com/netflix/spinnaker/keel/orca/OrcaIntentLauncher.kt @@ -21,21 +21,17 @@ import com.netflix.spinnaker.keel.* import com.netflix.spinnaker.keel.dryrun.ChangeSummary import net.logstash.logback.argument.StructuredArguments.value import org.slf4j.LoggerFactory -import org.springframework.beans.factory.annotation.Autowired import org.springframework.context.ApplicationEventPublisher import org.springframework.stereotype.Component -import java.time.Clock import java.util.concurrent.TimeUnit @Component(value = "orcaIntentLauncher") -open class OrcaIntentLauncher -@Autowired constructor( +open class OrcaIntentLauncher( private val intentProcessors: List>, private val orcaService: OrcaService, private val registry: Registry, private val applicationEventPublisher: ApplicationEventPublisher, - private val intentActivityRepository: IntentActivityRepository, - private val clock: Clock + private val intentActivityRepository: IntentActivityRepository ) : IntentLauncher { private val log = LoggerFactory.getLogger(javaClass) @@ -61,14 +57,10 @@ open class OrcaIntentLauncher orcaService.orchestrate(it).ref } - intentActivityRepository.logConvergence(IntentConvergenceRecord( + intentActivityRepository.record(IntentConvergenceRecord( intentId = intent.id(), - changeType = result.changeSummary.type, - orchestrations = orchestrationIds, - messages = result.changeSummary.message, - diff = result.changeSummary.diff, - actor = "keel:scheduledConvergence", - timestampMillis = clock.millis() + actor = "keel:scheduledConverge", + result = result )) log.info( diff --git a/keel-redis/src/main/kotlin/com/netflix/spinnaker/keel/redis/RedisIntentActivityRepository.kt b/keel-redis/src/main/kotlin/com/netflix/spinnaker/keel/redis/RedisIntentActivityRepository.kt index 204e4ec1d8..dbdd09c9fc 100644 --- a/keel-redis/src/main/kotlin/com/netflix/spinnaker/keel/redis/RedisIntentActivityRepository.kt +++ b/keel-redis/src/main/kotlin/com/netflix/spinnaker/keel/redis/RedisIntentActivityRepository.kt @@ -16,23 +16,21 @@ package com.netflix.spinnaker.keel.redis import com.fasterxml.jackson.databind.ObjectMapper +import com.fasterxml.jackson.module.kotlin.readValue import com.netflix.spinnaker.config.KeelProperties +import com.netflix.spinnaker.keel.ActivityRecord import com.netflix.spinnaker.keel.IntentActivityRepository -import com.netflix.spinnaker.keel.IntentConvergenceRecord +import com.netflix.spinnaker.keel.model.ListCriteria import com.netflix.spinnaker.kork.jedis.RedisClientSelector import org.slf4j.LoggerFactory -import org.springframework.beans.factory.annotation.Autowired import org.springframework.stereotype.Component -import java.time.Clock import javax.annotation.PostConstruct @Component -class RedisIntentActivityRepository -@Autowired constructor( +class RedisIntentActivityRepository( redisClientSelector: RedisClientSelector, private val keelProperties: KeelProperties, - private val objectMapper: ObjectMapper, - private val clock: Clock + private val objectMapper: ObjectMapper ) : AbstractRedisRepository(redisClientSelector), IntentActivityRepository { private val log = LoggerFactory.getLogger(javaClass) @@ -43,77 +41,47 @@ class RedisIntentActivityRepository log.info("Using ${javaClass.simpleName}") } - override fun addOrchestration(intentId: String, orchestrationId: String) - = addOrchestrations(intentId, listOf(orchestrationId)) - - override fun addOrchestrations(intentId: String, orchestrations: List) { - val score = clock.millis() - historyKey(intentId).let { key -> + override fun record(activity: ActivityRecord) { + logKey(activity.intentId).let { key -> getClientForId(key).withCommandsClient { c -> - c.zadd(key, orchestrations.map { parseOrchestrationId(it) to score.toDouble() }.toMap().toMutableMap()) - } - } + c.rpush(key, objectMapper.writeValueAsString(activity)) - currentKey(intentId).let { key -> - getClientForId(key).withCommandsClient { c -> - c.zadd(key, orchestrations.map { parseOrchestrationId(it) to score.toDouble() }.toMap().toMutableMap()) + // Prune old log entries + c.ltrim(key, 0, keelProperties.maxConvergenceLogEntriesPerIntent - 1L) } } } - override fun getHistory(intentId: String) - = historyKey(intentId).let { key -> - getClientForId(key).withCommandsClient> { c -> - c.zrangeByScore(key, "-inf", "+inf") - } - }.toList() - - override fun logConvergence(intentConvergenceRecord: IntentConvergenceRecord) { - val score = intentConvergenceRecord.timestampMillis // This timestampMillis is recorded when the record is created. - logKey(intentConvergenceRecord.intentId). let { key -> - getClientForId(key).withCommandsClient { c -> - c.zadd(key, mapOf(objectMapper.writeValueAsString(intentConvergenceRecord) to score.toDouble()) ) + override fun getHistory(intentId: String, criteria: ListCriteria): List = + logKey(intentId).let { key -> + getClientForId(key).withCommandsClient> { c -> + c.lrange(key, criteria.offset.toLong(), criteria.offset - 1L) + .map { objectMapper.readValue(it) } } } - // If we're over the message limit, we need to drop log entries. - logKey(intentConvergenceRecord.intentId). let { key -> - getClientForId(key).withCommandsClient { c -> - val count = c.zcount(key, "-inf", "+inf") - val numMsgsLeft = keelProperties.maxConvergenceLogEntriesPerIntent - count - if (numMsgsLeft < 0){ - // Set is sorted from lowest score to highest. - // Set is scored by timestampMillis, which only grows as time increases. - // Drop the lowest score messages. - c.zremrangeByRank(key, 0, (-1*numMsgsLeft) - 1) - } + override fun getHistory(intentId: String, kind: Class, criteria: ListCriteria): List = + logKey(intentId).let { key -> + getClientForId(key).withCommandsClient> { c -> + c.lrange(key, 0, c.llen(key) - 1) + .map { objectMapper.readValue(it) } + .filterIsInstance(kind) + .let { limitOffset(it, criteria) } } } - } - - override fun getLog(intentId: String): List - = logKey(intentId). let { key -> - getClientForId(key).withCommandsClient> { c -> - c.zrangeByScore(key, "-inf", "+inf") - } - }.map { objectMapper.readValue(it, IntentConvergenceRecord::class.java) }.toList() - override fun getLogEntry(intentId: String, timestampMillis: Long) - = logKey(intentId). let { key -> - getClientForId(key).withCommandsClient> { c -> - c.zrangeByScore(key, timestampMillis.toDouble(), timestampMillis.toDouble()) - } - }.map { objectMapper.readValue(it, IntentConvergenceRecord::class.java) } - .toList() - .also { - // The same intent shouldn't be processed more than once at the exact same time. - if (it.size > 1) log.warn("Two messages with the same timestampMillis. This shouldn't happen.") + private fun limitOffset(list: Collection, criteria: ListCriteria): List { + val size = list.size + return if (size <= criteria.offset) { + listOf() + } else { + var lastIndex = criteria.offset + criteria.limit + if (lastIndex >= size) { + lastIndex = size } - .firstOrNull() + list.toList().subList(criteria.offset, lastIndex) + } + } } -internal fun historyKey(intentId: String) = "history:$intentId" - -internal fun currentKey(intentId: String) = "current:$intentId" - internal fun logKey(intentId: String) = "log:$intentId" diff --git a/keel-redis/src/test/kotlin/com/netflix/spinnaker/keel/redis/RedisIntentActivityRepositoryTest.kt b/keel-redis/src/test/kotlin/com/netflix/spinnaker/keel/redis/RedisIntentActivityRepositoryTest.kt index d14e8cb9eb..17f9ee6949 100644 --- a/keel-redis/src/test/kotlin/com/netflix/spinnaker/keel/redis/RedisIntentActivityRepositoryTest.kt +++ b/keel-redis/src/test/kotlin/com/netflix/spinnaker/keel/redis/RedisIntentActivityRepositoryTest.kt @@ -21,8 +21,12 @@ import com.natpryce.hamkrest.should.shouldMatch import com.netflix.spinnaker.config.KeelProperties import com.netflix.spinnaker.config.configureObjectMapper import com.netflix.spinnaker.hamkrest.shouldEqual -import com.netflix.spinnaker.keel.IntentConvergenceRecord -import com.netflix.spinnaker.keel.dryrun.ChangeType +import com.netflix.spinnaker.keel.* +import com.netflix.spinnaker.keel.dryrun.ChangeSummary +import com.netflix.spinnaker.keel.model.PagingListCriteria +import com.netflix.spinnaker.keel.test.GenericTestIntentSpec +import com.netflix.spinnaker.keel.test.TestIntent +import com.netflix.spinnaker.kork.jackson.ObjectMapperSubtypeConfigurer import com.netflix.spinnaker.kork.jedis.EmbeddedRedis import com.netflix.spinnaker.kork.jedis.JedisClientDelegate import com.netflix.spinnaker.kork.jedis.RedisClientSelector @@ -32,7 +36,6 @@ import org.junit.jupiter.api.Test import org.junit.jupiter.api.TestInstance import org.junit.jupiter.api.TestInstance.Lifecycle import redis.clients.jedis.JedisPool -import java.time.Clock @TestInstance(Lifecycle.PER_CLASS) object RedisIntentActivityRepositoryTest { @@ -42,14 +45,14 @@ object RedisIntentActivityRepositoryTest { val keelProperties = KeelProperties().apply { maxConvergenceLogEntriesPerIntent = 5 } - val clock = Clock.systemDefaultZone() - val mapper = configureObjectMapper(ObjectMapper(), keelProperties, listOf()) + val mapper = configureObjectMapper(ObjectMapper(), keelProperties, listOf( + ObjectMapperSubtypeConfigurer.ClassSubtypeLocator(Intent::class.java, listOf("com.netflix.spinnaker.keel")) + )) val subject = RedisIntentActivityRepository( redisClientSelector = RedisClientSelector(listOf(JedisClientDelegate("primaryDefault", jedisPool))), - keelProperties = keelProperties , - objectMapper = mapper, - clock = clock + keelProperties = keelProperties, + objectMapper = mapper ) @BeforeEach @@ -65,41 +68,39 @@ object RedisIntentActivityRepositoryTest { } @Test - fun `listing history for an intent returns ordered orchestrations`() { - subject.addOrchestration("hello", "abcd") - subject.addOrchestrations("world", listOf("1234", "5678", "lol")) - subject.addOrchestration("hello", "covfefe") + fun `listing log for an intent returns ordered records`() { + val aUpsertRecord = IntentChangeRecord("a", "rob", IntentChangeAction.UPSERT, TestIntent(GenericTestIntentSpec("a"))) + val aConvergeRecord = IntentConvergenceRecord("a", "keel", ConvergeResult(listOf(), ChangeSummary("a"))) + val bConvergeRecord = IntentConvergenceRecord("b", "keel", ConvergeResult(listOf(), ChangeSummary("b"))) + val aUpsertRecord2 = IntentChangeRecord("a", "rob", IntentChangeAction.UPSERT, TestIntent(GenericTestIntentSpec("a"))) + subject.record(aUpsertRecord) + subject.record(aConvergeRecord) + subject.record(bConvergeRecord) + subject.record(aUpsertRecord2) - subject.getHistory("world").let { - it shouldMatch equalTo(listOf("1234", "5678", "lol")) - } - - subject.getHistory("hello").let { - it shouldMatch equalTo(listOf("abcd", "covfefe")) - } + subject.getHistory("a", PagingListCriteria()).size shouldMatch equalTo(3) + subject.getHistory("a", IntentChangeRecord::class.java, PagingListCriteria()).size shouldMatch equalTo(2) } @Test fun `only the specified number of convergence log messages should be kept`() { val intentId = "Application:emilykeeltest" - val record = IntentConvergenceRecord( - intentId = intentId, - changeType = ChangeType.NO_CHANGE, - orchestrations = emptyList(), - messages = listOf("System state matches desired state"), - diff = emptySet(), - actor = "keel:scheduledConvergence", - timestampMillis = 1516214128706 - ) - subject.logConvergence(record) - subject.logConvergence(record.copy(timestampMillis = 1516214135581)) - subject.logConvergence(record.copy(timestampMillis = 1516214157620)) - subject.logConvergence(record.copy(timestampMillis = 1516214167665)) - subject.logConvergence(record.copy(timestampMillis = 1516214192806)) - subject.logConvergence(record.copy(timestampMillis = 1516214217947)) - subject.logConvergence(record.copy(timestampMillis = 1516214243088)) + val recordFactory = { + IntentConvergenceRecord( + intentId = intentId, + actor = "keel:scheduledConvergence", + result = ConvergeResult(listOf(), ChangeSummary("a")) + ) + } + subject.record(recordFactory()) + subject.record(recordFactory()) + subject.record(recordFactory()) + subject.record(recordFactory()) + subject.record(recordFactory()) + subject.record(recordFactory()) + subject.record(recordFactory()) - subject.getLog(intentId).size shouldEqual keelProperties.maxConvergenceLogEntriesPerIntent + subject.getHistory(intentId, PagingListCriteria()).size shouldEqual keelProperties.maxConvergenceLogEntriesPerIntent } } diff --git a/keel-scheduler/src/main/kotlin/com/netflix/spinnaker/keel/scheduler/handler/ConvergeIntentHandler.kt b/keel-scheduler/src/main/kotlin/com/netflix/spinnaker/keel/scheduler/handler/ConvergeIntentHandler.kt index a4e1e0c986..f564ee1801 100644 --- a/keel-scheduler/src/main/kotlin/com/netflix/spinnaker/keel/scheduler/handler/ConvergeIntentHandler.kt +++ b/keel-scheduler/src/main/kotlin/com/netflix/spinnaker/keel/scheduler/handler/ConvergeIntentHandler.kt @@ -16,7 +16,10 @@ package com.netflix.spinnaker.keel.scheduler.handler import com.netflix.spectator.api.Registry -import com.netflix.spinnaker.keel.* +import com.netflix.spinnaker.keel.Intent +import com.netflix.spinnaker.keel.IntentRepository +import com.netflix.spinnaker.keel.IntentSpec +import com.netflix.spinnaker.keel.IntentStatus import com.netflix.spinnaker.keel.event.* import com.netflix.spinnaker.keel.orca.OrcaIntentLauncher import com.netflix.spinnaker.keel.scheduler.ConvergeIntent @@ -37,7 +40,6 @@ class ConvergeIntentHandler @Autowired constructor( override val queue: Queue, private val intentRepository: IntentRepository, - private val intentActivityRepository: IntentActivityRepository, private val orcaIntentLauncher: OrcaIntentLauncher, private val clock: Clock, private val registry: Registry, @@ -74,7 +76,6 @@ class ConvergeIntentHandler orcaIntentLauncher.launch(intent) .takeIf { it.orchestrationIds.isNotEmpty() } ?.also { result -> - intentActivityRepository.addOrchestrations(intent.id(), result.orchestrationIds) applicationEventPublisher.publishEvent(IntentConvergeSuccessEvent(intent, result.orchestrationIds)) if (intent.status.shouldIsolate()) { diff --git a/keel-scheduler/src/test/kotlin/com/netflix/spinnaker/keel/scheduler/handler/ConvergeIntentHandlerTest.kt b/keel-scheduler/src/test/kotlin/com/netflix/spinnaker/keel/scheduler/handler/ConvergeIntentHandlerTest.kt index af10660b5a..fadcd6aebf 100644 --- a/keel-scheduler/src/test/kotlin/com/netflix/spinnaker/keel/scheduler/handler/ConvergeIntentHandlerTest.kt +++ b/keel-scheduler/src/test/kotlin/com/netflix/spinnaker/keel/scheduler/handler/ConvergeIntentHandlerTest.kt @@ -16,22 +16,15 @@ package com.netflix.spinnaker.keel.scheduler.handler import com.netflix.spectator.api.NoopRegistry -import com.netflix.spinnaker.keel.IntentActivityRepository import com.netflix.spinnaker.keel.IntentRepository import com.netflix.spinnaker.keel.dryrun.ChangeSummary import com.netflix.spinnaker.keel.orca.OrcaIntentLauncher import com.netflix.spinnaker.keel.orca.OrcaLaunchedIntentResult import com.netflix.spinnaker.keel.scheduler.ConvergeIntent -import com.netflix.spinnaker.keel.test.TestIntent import com.netflix.spinnaker.keel.test.GenericTestIntentSpec +import com.netflix.spinnaker.keel.test.TestIntent import com.netflix.spinnaker.q.Queue -import com.nhaarman.mockito_kotlin.doReturn -import com.nhaarman.mockito_kotlin.mock -import com.nhaarman.mockito_kotlin.reset -import com.nhaarman.mockito_kotlin.verify -import com.nhaarman.mockito_kotlin.verifyNoMoreInteractions -import com.nhaarman.mockito_kotlin.verifyZeroInteractions -import com.nhaarman.mockito_kotlin.whenever +import com.nhaarman.mockito_kotlin.* import org.junit.jupiter.api.AfterEach import org.junit.jupiter.api.Test import org.springframework.context.ApplicationEventPublisher @@ -43,17 +36,16 @@ object ConvergeIntentHandlerTest { val queue = mock() val intentRepository = mock() - val intentActivityRepository = mock() val orcaIntentLauncher = mock() val clock = Clock.fixed(Instant.ofEpochSecond(500), ZoneId.systemDefault()) val registry = NoopRegistry() val applicationEventPublisher = mock() - val subject = ConvergeIntentHandler(queue, intentRepository, intentActivityRepository, orcaIntentLauncher, clock, registry, applicationEventPublisher) + val subject = ConvergeIntentHandler(queue, intentRepository, orcaIntentLauncher, clock, registry, applicationEventPublisher) @AfterEach fun cleanup() { - reset(queue, intentRepository, intentActivityRepository, orcaIntentLauncher, applicationEventPublisher) + reset(queue, intentRepository, orcaIntentLauncher, applicationEventPublisher) } @Test @@ -96,7 +88,6 @@ object ConvergeIntentHandlerTest { subject.handle(message) verify(orcaIntentLauncher).launch(refreshedIntent) - verify(intentActivityRepository).addOrchestrations("test:1", listOf("one")) - verifyNoMoreInteractions(orcaIntentLauncher, intentActivityRepository) + verifyNoMoreInteractions(orcaIntentLauncher) } } diff --git a/keel-web/src/main/kotlin/com/netflix/spinnaker/keel/controllers/v1/IntentController.kt b/keel-web/src/main/kotlin/com/netflix/spinnaker/keel/controllers/v1/IntentController.kt index f1326a90f8..d1ff491fc2 100644 --- a/keel-web/src/main/kotlin/com/netflix/spinnaker/keel/controllers/v1/IntentController.kt +++ b/keel-web/src/main/kotlin/com/netflix/spinnaker/keel/controllers/v1/IntentController.kt @@ -18,6 +18,7 @@ package com.netflix.spinnaker.keel.controllers.v1 import com.netflix.spinnaker.config.KeelProperties import com.netflix.spinnaker.keel.* import com.netflix.spinnaker.keel.dryrun.DryRunIntentLauncher +import com.netflix.spinnaker.keel.model.PagingListCriteria import com.netflix.spinnaker.keel.model.UpsertIntentRequest import com.netflix.spinnaker.keel.scheduler.ScheduleService import com.netflix.spinnaker.security.AuthenticatedRequest @@ -98,15 +99,19 @@ class IntentController } } - @RequestMapping(value = ["/{id}/history"], method = [(RequestMethod.GET)]) - fun getIntentHistory(@PathVariable("id") id: String) = intentActivityRepository.getHistory(id) - @RequestMapping(value = ["/{id}/log"], method = [(RequestMethod.GET)]) - fun getLog(@PathVariable("id") id: String) = intentActivityRepository.getLog(id) + fun getLog(@PathVariable("id") id: String, + @RequestParam("limit", defaultValue = "10", required = false) limit: Int, + @RequestParam("offset", defaultValue = "10", required = false) offset: Int, + @RequestParam("kind", required = false) kind: String?): List { + if (kind == null) { + return intentActivityRepository.getHistory(id, PagingListCriteria(limit, offset)) + } - @RequestMapping(value = ["/{id}/log/{timestampMillis}"], method = [(RequestMethod.GET)]) - fun getLogEntry(@PathVariable("id") id: String, @PathVariable("timestampMillis") timestampMillis: Long) - = intentActivityRepository.getLogEntry(id, timestampMillis) + val clazz = activityRecordClassForName(kind) ?: throw IllegalArgumentException("Unknown activity record kind: $kind") + + return intentActivityRepository.getHistory(id, clazz, PagingListCriteria(limit, offset)) + } } data class UpsertIntentResponse(