chore(pollers): add configuration properties (#3609)

No functional change here, just move `@Value`s on the two cleanup agents to `ConfigurationProperties` Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
spinnaker · Apr 16, 2020 · 759c1eb · 759c1eb
1 parent 580237b
commit 759c1eb
Show file tree

Hide file tree

Showing 5 changed files with 121 additions and 32 deletions.
diff --git a/...ain/kotlin/com/netflix/spinnaker/config/OldPipelineCleanupAgentConfigurationProperties.kt b/...ain/kotlin/com/netflix/spinnaker/config/OldPipelineCleanupAgentConfigurationProperties.kt
@@ -0,0 +1,48 @@
+/*
+ * Copyright 2020 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.config
+
+import org.springframework.boot.context.properties.ConfigurationProperties
+
+@ConfigurationProperties("pollers.old-pipeline-cleanup")
+class OldPipelineCleanupAgentConfigurationProperties() {
+  /**
+   * How often the agent runs, in millis
+   */
+  var intervalMs: Long = 3600000
+
+  /**
+   * Maximum age of pipelines to keep (if more than minimumPipelineExecutions exist)
+   */
+  var thresholdDays: Long = 30
+
+  /**
+   * Always keep this number of pipelines regardless of age
+   */
+  var minimumPipelineExecutions: Int = 5
+
+  /**
+   * Chunk size for SQL operations
+   */
+  var chunkSize: Int = 1
+
+  constructor(intervalMs: Long, thresholdDays: Long, minimumPipelineExecutions: Int, chunkSize: Int) : this() {
+    this.intervalMs = intervalMs
+    this.thresholdDays = thresholdDays
+    this.minimumPipelineExecutions = minimumPipelineExecutions
+    this.chunkSize = chunkSize
+  }
+}
diff --git a/...om/netflix/spinnaker/config/TopApplicationExecutionCleanupAgentConfigurationProperties.kt b/...om/netflix/spinnaker/config/TopApplicationExecutionCleanupAgentConfigurationProperties.kt
@@ -0,0 +1,36 @@
+/*
+ * Copyright 2020 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.config
+
+import org.springframework.boot.context.properties.ConfigurationProperties
+
+@ConfigurationProperties("pollers.top-application-execution-cleanup")
+class TopApplicationExecutionCleanupAgentConfigurationProperties {
+  /**
+   * How often the agent runs, in millis
+   */
+  var intervalMs: Long = 3600000
+
+  /**
+   * Maximum number of orchestrations to keep
+   */
+  var threshold: Int = 2000
+
+  /**
+   * Chunk size for SQL operations
+   */
+  var chunkSize: Int = 1
+}
diff --git a/...tlin/com/netflix/spinnaker/orca/sql/cleanup/OldPipelineCleanupPollingNotificationAgent.kt b/...tlin/com/netflix/spinnaker/orca/sql/cleanup/OldPipelineCleanupPollingNotificationAgent.kt
@@ -17,6 +17,8 @@
 package com.netflix.spinnaker.orca.sql.cleanup
 
 import com.netflix.spectator.api.Registry
+import com.netflix.spinnaker.config.OldPipelineCleanupAgentConfigurationProperties
+import com.netflix.spinnaker.config.OrcaSqlProperties
 import com.netflix.spinnaker.orca.api.pipeline.models.ExecutionType
 import com.netflix.spinnaker.orca.notifications.NotificationClusterLock
 import com.netflix.spinnaker.orca.pipeline.persistence.ExecutionRepository
@@ -28,36 +30,34 @@ import org.jooq.DSLContext
 import org.jooq.impl.DSL.count
 import org.jooq.impl.DSL.field
 import org.jooq.impl.DSL.table
-import org.springframework.beans.factory.annotation.Value
 import org.springframework.boot.autoconfigure.condition.ConditionalOnExpression
+import org.springframework.boot.context.properties.EnableConfigurationProperties
 import org.springframework.stereotype.Component
 
 @Component
 @ConditionalOnExpression("\${pollers.old-pipeline-cleanup.enabled:false} && \${execution-repository.sql.enabled:false}")
+@EnableConfigurationProperties(OldPipelineCleanupAgentConfigurationProperties::class, OrcaSqlProperties::class)
 class OldPipelineCleanupPollingNotificationAgent(
   clusterLock: NotificationClusterLock,
   private val jooq: DSLContext,
   private val clock: Clock,
   registry: Registry,
   private val executionRepository: ExecutionRepository,
-  @Value("\${pollers.old-pipeline-cleanup.interval-ms:3600000}") private val pollingIntervalMs: Long,
-  @Value("\${pollers.old-pipeline-cleanup.threshold-days:30}") private val thresholdDays: Long,
-  @Value("\${pollers.old-pipeline-cleanup.minimum-pipeline-executions:5}") private val minimumPipelineExecutions: Int,
-  @Value("\${pollers.old-pipeline-cleanup.chunk-size:1}") private val chunkSize: Int,
-  @Value("\${sql.partition-name:#{null}}") private val partitionName: String?
+  private val configurationProperties: OldPipelineCleanupAgentConfigurationProperties,
+  private val orcaSqlProperties: OrcaSqlProperties
 ) : AbstractCleanupPollingAgent(
   clusterLock,
-  pollingIntervalMs,
+  configurationProperties.intervalMs,
   registry) {
 
   override fun performCleanup() {
-    val thresholdMillis = Instant.ofEpochMilli(clock.millis()).minus(thresholdDays, ChronoUnit.DAYS).toEpochMilli()
+    val thresholdMillis = Instant.ofEpochMilli(clock.millis()).minus(configurationProperties.thresholdDays, ChronoUnit.DAYS).toEpochMilli()
 
     val candidateApplications = jooq
       .select(field("application"))
       .from(table("pipelines"))
       .groupBy(field("application"))
-      .having(count(field("id")).gt(minimumPipelineExecutions))
+      .having(count(field("id")).gt(configurationProperties.minimumPipelineExecutions))
       .fetch(field("application"), String::class.java)
 
     for (chunk in candidateApplications.chunked(5)) {
@@ -71,15 +71,15 @@ class OldPipelineCleanupPollingNotificationAgent(
         .and(
           field("status").`in`(*completedStatuses.toTypedArray()))
 
-        if (partitionName != null) {
+        if (orcaSqlProperties.partitionName != null) {
           queryBuilder = queryBuilder
             .and(
-              field("`partition`").eq(partitionName))
+              field("`partition`").eq(orcaSqlProperties.partitionName))
         }
 
       val pipelineConfigsWithOldExecutions = queryBuilder
         .groupBy(field("application"), field("config_id"))
-        .having(count(field("id")).gt(minimumPipelineExecutions))
+        .having(count(field("id")).gt(configurationProperties.minimumPipelineExecutions))
         .fetch()
 
       pipelineConfigsWithOldExecutions.forEach {
@@ -131,18 +131,18 @@ class OldPipelineCleanupPollingNotificationAgent(
         .and(field("config_id").eq(pipelineConfigId))
       )
 
-    if (partitionName != null) {
+    if (orcaSqlProperties.partitionName != null) {
       queryBuilder = queryBuilder
         .and(
-          field("`partition`").eq(partitionName))
+          field("`partition`").eq(orcaSqlProperties.partitionName))
     }
 
     val executionsToRemove = queryBuilder
       .orderBy(field("build_time").desc())
-      .limit(minimumPipelineExecutions, Int.MAX_VALUE)
+      .limit(configurationProperties.minimumPipelineExecutions, Int.MAX_VALUE)
       .fetch(field("id"), String::class.java)
 
-    executionsToRemove.chunked(chunkSize).forEach { ids ->
+    executionsToRemove.chunked(configurationProperties.chunkSize).forEach { ids ->
       deletedExecutionCount.addAndGet(ids.size)
       executionRepository.delete(ExecutionType.PIPELINE, ids)
 

diff --git a/...flix/spinnaker/orca/sql/cleanup/TopApplicationExecutionCleanupPollingNotificationAgent.kt b/...flix/spinnaker/orca/sql/cleanup/TopApplicationExecutionCleanupPollingNotificationAgent.kt
@@ -17,30 +17,31 @@
 package com.netflix.spinnaker.orca.sql.cleanup
 
 import com.netflix.spectator.api.Registry
+import com.netflix.spinnaker.config.OrcaSqlProperties
+import com.netflix.spinnaker.config.TopApplicationExecutionCleanupAgentConfigurationProperties
 import com.netflix.spinnaker.orca.api.pipeline.models.ExecutionType
 import com.netflix.spinnaker.orca.notifications.NotificationClusterLock
 import com.netflix.spinnaker.orca.pipeline.persistence.ExecutionRepository
 import java.util.concurrent.atomic.AtomicInteger
 import org.jooq.DSLContext
 import org.jooq.impl.DSL
-import org.springframework.beans.factory.annotation.Value
 import org.springframework.boot.autoconfigure.condition.ConditionalOnExpression
+import org.springframework.boot.context.properties.EnableConfigurationProperties
 import org.springframework.stereotype.Component
 
 @Component
 @ConditionalOnExpression("\${pollers.top-application-execution-cleanup.enabled:false} && \${execution-repository.sql.enabled:false}")
+@EnableConfigurationProperties(TopApplicationExecutionCleanupAgentConfigurationProperties::class, OrcaSqlProperties::class)
 class TopApplicationExecutionCleanupPollingNotificationAgent(
   clusterLock: NotificationClusterLock,
   private val jooq: DSLContext,
   registry: Registry,
   private val executionRepository: ExecutionRepository,
-  @Value("\${pollers.top-application-execution-cleanup.interval-ms:3600000}") private val pollingIntervalMs: Long,
-  @Value("\${pollers.top-application-execution-cleanup.threshold:2000}") private val threshold: Int,
-  @Value("\${pollers.top-application-execution-cleanup.chunk-size:1}") private val chunkSize: Int,
-  @Value("\${sql.partition-name:#{null}}") private val partitionName: String?
+  private val configurationProperties: TopApplicationExecutionCleanupAgentConfigurationProperties,
+  private val orcaSqlProperties: OrcaSqlProperties
 ) : AbstractCleanupPollingAgent(
   clusterLock,
-  pollingIntervalMs,
+  configurationProperties.intervalMs,
   registry) {
 
   override fun performCleanup() {
@@ -49,15 +50,15 @@ class TopApplicationExecutionCleanupPollingNotificationAgent(
       .from(DSL.table("orchestrations"))
       .where(DSL.noCondition())
 
-    if (partitionName != null) {
+    if (orcaSqlProperties.partitionName != null) {
       queryBuilder = queryBuilder
         .and(
-          DSL.field("`partition`").eq(partitionName))
+          DSL.field("`partition`").eq(orcaSqlProperties.partitionName))
     }
 
     val applicationsWithOldOrchestrations = queryBuilder
       .groupBy(DSL.field("application"))
-      .having(DSL.count(DSL.field("id")).gt(threshold))
+      .having(DSL.count(DSL.field("id")).gt(configurationProperties.threshold))
       .fetch(DSL.field("application"), String::class.java)
 
     applicationsWithOldOrchestrations
@@ -95,12 +96,12 @@ class TopApplicationExecutionCleanupPollingNotificationAgent(
           .and(DSL.field("status").`in`(*completedStatuses.toTypedArray()))
       )
       .orderBy(DSL.field("build_time").desc())
-      .limit(threshold, Int.MAX_VALUE)
+      .limit(configurationProperties.threshold, Int.MAX_VALUE)
       .fetch(DSL.field("id"), String::class.java)
 
     log.debug("Found {} old orchestrations for {}", executionsToRemove.size, application)
 
-    executionsToRemove.chunked(chunkSize).forEach { ids ->
+    executionsToRemove.chunked(configurationProperties.chunkSize).forEach { ids ->
       deletedExecutionCount.addAndGet(ids.size)
 
       executionRepository.delete(ExecutionType.ORCHESTRATION, ids)

diff --git a/.../netflix/spinnaker/orca/sql/cleanup/OldPipelineCleanupPollingNotificationAgentSpec.groovy b/.../netflix/spinnaker/orca/sql/cleanup/OldPipelineCleanupPollingNotificationAgentSpec.groovy
@@ -16,6 +16,8 @@
 
 package com.netflix.spinnaker.orca.sql.cleanup
 
+import com.netflix.spinnaker.config.OldPipelineCleanupAgentConfigurationProperties
+import com.netflix.spinnaker.config.OrcaSqlProperties
 import com.netflix.spinnaker.kork.sql.config.RetryProperties
 import com.netflix.spinnaker.kork.sql.test.SqlTestUtil
 
@@ -58,11 +60,13 @@ class OldPipelineCleanupPollingNotificationAgentSpec extends Specification {
     Clock.systemDefaultZone(),
     new NoopRegistry(),
     executionRepository,
-    0,
-    10, // threshold days
-    5,  // minimum pipeline executions
-    1,
-    null
+    new OldPipelineCleanupAgentConfigurationProperties(
+        0L,
+        10L, // threshold days
+        5,  // minimum pipeline executions
+        1
+    ),
+    new OrcaSqlProperties()
   )
 
   def setupSpec() {