feat(*): Removing intent trace system in favor of just activity (#81)

spinnaker · Apr 5, 2018 · 7ef8333 · 7ef8333
1 parent 0bd20a6
commit 7ef8333
Show file tree

Hide file tree

Showing 14 changed files with 7 additions and 284 deletions.
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
@@ -20,9 +20,7 @@ import com.netflix.spinnaker.keel.*
 import com.netflix.spinnaker.keel.attribute.Attribute
 import com.netflix.spinnaker.keel.memory.MemoryIntentActivityRepository
 import com.netflix.spinnaker.keel.memory.MemoryIntentRepository
-import com.netflix.spinnaker.keel.memory.MemoryTraceRepository
 import com.netflix.spinnaker.keel.policy.Policy
-import com.netflix.spinnaker.keel.tracing.TraceRepository
 import com.netflix.spinnaker.kork.jackson.ObjectMapperSubtypeConfigurer.ClassSubtypeLocator
 import org.springframework.beans.factory.annotation.Autowired
 import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean
@@ -68,10 +66,6 @@ open class KeelConfiguration {
   @ConditionalOnMissingBean(IntentActivityRepository::class)
   open fun memoryIntentActivityRepository(): IntentActivityRepository = MemoryIntentActivityRepository(properties)
 
-  @Bean
-  @ConditionalOnMissingBean(TraceRepository::class)
-  open fun memoryTraceRepository(): TraceRepository = MemoryTraceRepository()
-
   @Bean open fun clock(): Clock = Clock.systemDefaultZone()
 
   @Bean open fun applicationIntentGuard(registry: Registry, properties: ApplicationIntentGuardProperties) =

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
@@ -18,21 +18,6 @@ package com.netflix.spinnaker.keel
 import com.netflix.spinnaker.keel.dryrun.ChangeType
 import com.netflix.spinnaker.keel.state.FieldState
 
-/**
- * TODO rz - refactor IntentActivityRepository to use this instead
- *
- * @param intentId The ID of the intent that is being evaluated.
- * @param orchestrations A resultant (if any) list of orchestration IDs from the intent.
- * @param reason A human-friendly description of why this operation occurred.
- * @param actor Who (or what) initiated the operation.
- */
-data class IntentActivity(
-  val intentId: String,
-  val orchestrations: List<String>,
-  val reason: String,
-  val actor: String
-)
-
 /**
  * @param intentId The ID of the intent that is being evaluated.
  * @param changeType The type of change that took place this cycle.

diff --git a/keel-core/src/main/kotlin/com/netflix/spinnaker/keel/memory/MemoryTraceRepository.kt b/keel-core/src/main/kotlin/com/netflix/spinnaker/keel/memory/MemoryTraceRepository.kt
diff --git a/.../com/netflix/spinnaker/keel/intent/aws/loadbalancer/ClassicLoadBalancerIntentProcessor.kt b/.../com/netflix/spinnaker/keel/intent/aws/loadbalancer/ClassicLoadBalancerIntentProcessor.kt
@@ -30,7 +30,6 @@ import com.netflix.spinnaker.keel.intent.LoadBalancerIntent
 import com.netflix.spinnaker.keel.model.OrchestrationRequest
 import com.netflix.spinnaker.keel.model.OrchestrationTrigger
 import com.netflix.spinnaker.keel.state.StateInspector
-import com.netflix.spinnaker.keel.tracing.TraceRepository
 import net.logstash.logback.argument.StructuredArguments.value
 import org.slf4j.LoggerFactory
 import org.springframework.beans.factory.annotation.Autowired
@@ -39,7 +38,6 @@ import org.springframework.stereotype.Component
 @Component
 class ClassicLoadBalancerIntentProcessor
 @Autowired constructor(
-  private val traceRepository: TraceRepository,
   private val cloudDriverService: CloudDriverService,
   private val clouddriverCache: CloudDriverCache,
   objectMapper: ObjectMapper,

diff --git a/...com/netflix/spinnaker/keel/intent/aws/securitygroup/AmazonSecurityGroupIntentProcessor.kt b/...com/netflix/spinnaker/keel/intent/aws/securitygroup/AmazonSecurityGroupIntentProcessor.kt
@@ -21,15 +21,12 @@ import com.netflix.spinnaker.keel.clouddriver.model.SecurityGroup
 import com.netflix.spinnaker.keel.dryrun.ChangeSummary
 import com.netflix.spinnaker.keel.dryrun.ChangeType
 import com.netflix.spinnaker.keel.exceptions.DeclarativeException
-import com.netflix.spinnaker.keel.intent.ANY_MAP_TYPE
 import com.netflix.spinnaker.keel.intent.DefaultConvertToJobCommand
 import com.netflix.spinnaker.keel.intent.NamedReferenceSupport
 import com.netflix.spinnaker.keel.intent.SecurityGroupSpec
 import com.netflix.spinnaker.keel.model.OrchestrationRequest
 import com.netflix.spinnaker.keel.model.OrchestrationTrigger
 import com.netflix.spinnaker.keel.state.StateInspector
-import com.netflix.spinnaker.keel.tracing.Trace
-import com.netflix.spinnaker.keel.tracing.TraceRepository
 import org.springframework.stereotype.Component
 
 /**
@@ -47,7 +44,6 @@ import org.springframework.stereotype.Component
  */
 @Component
 class AmazonSecurityGroupIntentProcessor(
-  private val traceRepository: TraceRepository,
   private val intentRepository: IntentRepository,
   private val objectMapper: ObjectMapper,
   private val converter: AmazonSecurityGroupConverter,
@@ -60,11 +56,6 @@ class AmazonSecurityGroupIntentProcessor(
     val changeSummary = ChangeSummary(intent.id())
     val currentState = loader.load(intent.spec)
 
-    traceRepository.record(Trace(
-      startingState = objectMapper.convertValue(mapOf("state" to currentState), ANY_MAP_TYPE),
-      intent = intent
-    ))
-
     // This processor handles both root security groups, as well as individual rules. If a rule is passed in, we
     // need to source the root intent (or fake it out if it doesn't exist).
     var rootIntent = getRootIntent(intent)

diff --git a/...netflix/spinnaker/keel/intent/aws/securitygroup/AmazonSecurityGroupIntentProcessorTest.kt b/...netflix/spinnaker/keel/intent/aws/securitygroup/AmazonSecurityGroupIntentProcessorTest.kt
@@ -30,7 +30,6 @@ import com.netflix.spinnaker.keel.dryrun.ChangeSummary
 import com.netflix.spinnaker.keel.dryrun.ChangeType
 import com.netflix.spinnaker.keel.intent.ApplicationIntent
 import com.netflix.spinnaker.keel.intent.ReferenceSecurityGroupRule
-import com.netflix.spinnaker.keel.tracing.TraceRepository
 import com.nhaarman.mockito_kotlin.any
 import com.nhaarman.mockito_kotlin.doReturn
 import com.nhaarman.mockito_kotlin.doThrow
@@ -45,7 +44,6 @@ import retrofit.client.Response
 
 object AmazonSecurityGroupIntentProcessorTest {
 
-  val traceRepository = mock<TraceRepository>()
   val clouddriverCache = mock<CloudDriverCache>()
   val clouddriverService = mock<CloudDriverService>().apply {
     whenever(listNetworks()) doReturn mapOf(
@@ -63,11 +61,11 @@ object AmazonSecurityGroupIntentProcessorTest {
   val converter = AmazonSecurityGroupConverter(clouddriverCache, objectMapper)
   val loader = AmazonSecurityGroupLoader(clouddriverService, clouddriverCache)
 
-  val subject = AmazonSecurityGroupIntentProcessor(traceRepository, intentRepository, objectMapper, converter, loader)
+  val subject = AmazonSecurityGroupIntentProcessor(intentRepository, objectMapper, converter, loader)
 
   @AfterEach
   fun cleanup() {
-    reset(traceRepository, clouddriverService, clouddriverCache)
+    reset(clouddriverService, clouddriverCache)
   }
 
   @Test

diff --git a/...ent/src/main/kotlin/com/netflix/spinnaker/keel/intent/pipeline/PipelineIntentProcessor.kt b/...ent/src/main/kotlin/com/netflix/spinnaker/keel/intent/pipeline/PipelineIntentProcessor.kt
@@ -21,19 +21,15 @@ import com.netflix.spinnaker.keel.dryrun.ChangeSummary
 import com.netflix.spinnaker.keel.dryrun.ChangeType
 import com.netflix.spinnaker.keel.front50.Front50Service
 import com.netflix.spinnaker.keel.front50.model.PipelineConfig
-import com.netflix.spinnaker.keel.intent.ANY_MAP_TYPE
 import com.netflix.spinnaker.keel.intent.notFound
 import com.netflix.spinnaker.keel.model.OrchestrationRequest
 import com.netflix.spinnaker.keel.model.OrchestrationTrigger
 import com.netflix.spinnaker.keel.state.StateInspector
-import com.netflix.spinnaker.keel.tracing.Trace
-import com.netflix.spinnaker.keel.tracing.TraceRepository
 import org.springframework.stereotype.Component
 import retrofit.RetrofitError
 
 @Component
 class PipelineIntentProcessor(
-  private val traceRepository: TraceRepository,
   private val front50Service: Front50Service,
   private val objectMapper: ObjectMapper,
   private val pipelineConverter: PipelineConverter
@@ -59,11 +55,6 @@ class PipelineIntentProcessor(
 
     changeSummary.type = if (currentState == null) ChangeType.CREATE else ChangeType.UPDATE
 
-    traceRepository.record(Trace(
-      startingState = if (currentState == null) mapOf() else objectMapper.convertValue(currentState, ANY_MAP_TYPE),
-      intent = intent
-    ))
-
     return ConvergeResult(listOf(
       OrchestrationRequest(
         name = "Upsert pipeline",

diff --git a/...in/kotlin/com/netflix/spinnaker/keel/intent/processor/ApplicationUpsertIntentProcessor.kt b/...in/kotlin/com/netflix/spinnaker/keel/intent/processor/ApplicationUpsertIntentProcessor.kt
@@ -30,16 +30,13 @@ import com.netflix.spinnaker.keel.model.OrchestrationRequest
 import com.netflix.spinnaker.keel.model.OrchestrationTrigger
 import com.netflix.spinnaker.keel.state.FieldMutator
 import com.netflix.spinnaker.keel.state.StateInspector
-import com.netflix.spinnaker.keel.tracing.Trace
-import com.netflix.spinnaker.keel.tracing.TraceRepository
 import org.springframework.beans.factory.annotation.Autowired
 import org.springframework.stereotype.Component
 import retrofit.RetrofitError
 
 @Component
 class ApplicationUpsertIntentProcessor
 @Autowired constructor(
-  private val traceRepository: TraceRepository,
   private val front50Service: Front50Service,
   private val objectMapper: ObjectMapper
 ): IntentProcessor<ApplicationIntent> {
@@ -59,11 +56,6 @@ class ApplicationUpsertIntentProcessor
 
     changeSummary.type = if (currentState == null) ChangeType.CREATE else ChangeType.UPDATE
 
-    traceRepository.record(Trace(
-      startingState = if (currentState == null) mapOf() else objectMapper.convertValue(currentState, ANY_MAP_TYPE),
-      intent = intent
-    ))
-
     return ConvergeResult(listOf(
       OrchestrationRequest(
         name = if (currentState == null) "Create application" else "Update application",

diff --git a/...tent/src/main/kotlin/com/netflix/spinnaker/keel/intent/processor/ParrotIntentProcessor.kt b/...tent/src/main/kotlin/com/netflix/spinnaker/keel/intent/processor/ParrotIntentProcessor.kt
@@ -24,23 +24,16 @@ import com.netflix.spinnaker.keel.intent.ParrotIntent
 import com.netflix.spinnaker.keel.model.Job
 import com.netflix.spinnaker.keel.model.OrchestrationRequest
 import com.netflix.spinnaker.keel.model.OrchestrationTrigger
-import com.netflix.spinnaker.keel.tracing.Trace
-import com.netflix.spinnaker.keel.tracing.TraceRepository
-import org.springframework.beans.factory.annotation.Autowired
 import org.springframework.stereotype.Component
 
 @Component
-class ParrotIntentProcessor
-@Autowired constructor(
-  private val traceRepository: TraceRepository
-) : IntentProcessor<ParrotIntent> {
+class ParrotIntentProcessor : IntentProcessor<ParrotIntent> {
 
   override fun supports(intent: Intent<IntentSpec>) = intent is ParrotIntent
 
   override fun converge(intent: ParrotIntent): ConvergeResult {
     val changeSummary = ChangeSummary(intent.id())
     changeSummary.addMessage("Squawk!")
-    traceRepository.record(Trace(mapOf(), intent))
     return ConvergeResult(listOf(
       OrchestrationRequest(
         name = "Squawk!",

diff --git a/...src/test/kotlin/com/netflix/spinnaker/keel/intent/pipeline/PipelineIntentProcessorTest.kt b/...src/test/kotlin/com/netflix/spinnaker/keel/intent/pipeline/PipelineIntentProcessorTest.kt
@@ -25,7 +25,6 @@ import com.netflix.spinnaker.keel.front50.Front50Service
 import com.netflix.spinnaker.keel.front50.model.Application
 import com.netflix.spinnaker.keel.front50.model.PipelineConfig
 import com.netflix.spinnaker.keel.intent.ApplicationIntent
-import com.netflix.spinnaker.keel.tracing.TraceRepository
 import com.nhaarman.mockito_kotlin.any
 import com.nhaarman.mockito_kotlin.doReturn
 import com.nhaarman.mockito_kotlin.doThrow
@@ -39,16 +38,15 @@ import retrofit.client.Response
 
 object PipelineIntentProcessorTest {
 
-  val traceRepository = mock<TraceRepository>()
   val front50Service = mock<Front50Service>()
   val objectMapper = ObjectMapper()
   val converter = PipelineConverter(objectMapper)
 
-  val subject = PipelineIntentProcessor(traceRepository, front50Service, objectMapper, converter)
+  val subject = PipelineIntentProcessor(front50Service, objectMapper, converter)
 
   @AfterEach
   fun cleanup() {
-    reset(traceRepository, front50Service)
+    reset(front50Service)
   }
 
   @Test

diff --git a/...otlin/com/netflix/spinnaker/keel/intent/processor/ApplicationUpsertIntentProcessorTest.kt b/...otlin/com/netflix/spinnaker/keel/intent/processor/ApplicationUpsertIntentProcessorTest.kt
@@ -24,7 +24,6 @@ import com.natpryce.hamkrest.should.shouldMatch
 import com.netflix.spinnaker.keel.front50.Front50Service
 import com.netflix.spinnaker.keel.front50.model.Application
 import com.netflix.spinnaker.keel.intent.*
-import com.netflix.spinnaker.keel.tracing.TraceRepository
 import com.nhaarman.mockito_kotlin.*
 import org.junit.jupiter.api.AfterEach
 import org.junit.jupiter.api.Test
@@ -33,15 +32,14 @@ import retrofit.client.Response
 
 object ApplicationUpsertIntentProcessorTest {
 
-  val traceRepository = mock<TraceRepository>()
   val front50Service = mock<Front50Service>()
   val objectMapper = ObjectMapper()
 
-  val subject = ApplicationUpsertIntentProcessor(traceRepository, front50Service, objectMapper)
+  val subject = ApplicationUpsertIntentProcessor(front50Service, objectMapper)
 
   @AfterEach
   fun cleanup() {
-    reset(traceRepository, front50Service)
+    reset(front50Service)
   }
 
   @Test
@@ -60,8 +58,6 @@ object ApplicationUpsertIntentProcessorTest {
 
     result.orchestrations.size shouldMatch equalTo(1)
     result.orchestrations[0].name shouldMatch equalTo("Create application")
-
-    verify(traceRepository).record(any())
   }
 
   @Test
@@ -77,8 +73,6 @@ object ApplicationUpsertIntentProcessorTest {
     result.orchestrations.size shouldMatch equalTo(1)
     result.orchestrations[0].name shouldMatch equalTo("Update application")
     result.orchestrations[0].job[0]["application"] as Map<Any, Any> shouldMatch hasEntry("description", equalTo("my updated description"))
-
-    verify(traceRepository).record(any())
   }
 
   @Test