feat(perf): Favor a single pipeline config lookup (#3001)

Fetching a single pipeline config _should_ be more efficient than fetching all pipelines (for an application) and filtering in `orca`. This is somewhat nuanced as the "all pipelines for application" is likely being served out of the in-memory cache. We have seen what looks like serialization-related load and this PR aims to offer some relief.
spinnaker · Jun 20, 2019 · 2f0e25f · 2f0e25f
1 parent 02ab2a1
commit 2f0e25f
Show file tree

Hide file tree

Showing 2 changed files with 63 additions and 17 deletions.
diff --git a/...src/main/groovy/com/netflix/spinnaker/orca/front50/pipeline/EnabledPipelineValidator.java b/...src/main/groovy/com/netflix/spinnaker/orca/front50/pipeline/EnabledPipelineValidator.java
@@ -48,6 +48,26 @@ public void checkRunnable(Execution pipeline) {
       throw new UnsupportedOperationException(
           "Front50 not enabled, no way to validate pipeline. Fix this by setting front50.enabled: true");
     }
+
+    if (!isStrategy(pipeline)) {
+      // attempt an optimized lookup via pipeline history vs fetching all pipelines for the
+      // application and filtering
+      List<Map<String, Object>> pipelineConfigHistory =
+          front50Service.getPipelineHistory(pipeline.getPipelineConfigId(), 1);
+
+      if (!pipelineConfigHistory.isEmpty()) {
+        Map<String, Object> pipelineConfig = pipelineConfigHistory.get(0);
+        if ((boolean) pipelineConfig.getOrDefault("disabled", false)) {
+          throw new PipelineIsDisabled(
+              pipelineConfig.get("id").toString(),
+              pipelineConfig.get("application").toString(),
+              pipelineConfig.get("name").toString());
+        }
+
+        return;
+      }
+    }
+
     List<Map<String, Object>> pipelines =
         isStrategy(pipeline)
             ? front50Service.getStrategies(pipeline.getApplication())

diff --git a/...st/groovy/com/netflix/spinnaker/orca/front50/pipeline/EnabledPipelineValidatorSpec.groovy b/...st/groovy/com/netflix/spinnaker/orca/front50/pipeline/EnabledPipelineValidatorSpec.groovy
@@ -26,18 +26,19 @@ import static com.netflix.spinnaker.orca.test.model.ExecutionBuilder.pipeline
 
 class EnabledPipelineValidatorSpec extends Specification {
 
-  def front50Service = Stub(Front50Service)
+  def front50Service = Mock(Front50Service)
+
   @Subject
   def validator = new EnabledPipelineValidator(Optional.of(front50Service))
 
   def "allows one-off pipeline to run"() {
-    given:
-    front50Service.getPipelines(execution.application, false) >> []
-
     when:
     validator.checkRunnable(execution)
 
     then:
+    1 * front50Service.getPipelineHistory(execution.pipelineConfigId, 1) >> []
+    1 * front50Service.getPipelines(execution.application, false) >> []
+
     notThrown(PipelineValidationFailed)
 
     where:
@@ -48,15 +49,28 @@ class EnabledPipelineValidatorSpec extends Specification {
   }
 
   def "allows enabled pipeline to run"() {
-    given:
-    front50Service.getPipelines(execution.application, false) >> [
-      [id: execution.pipelineConfigId, application: execution.application, name: "whatever", disabled: false]
+    when:
+    validator.checkRunnable(execution)
+
+    then:
+    1 * front50Service.getPipelineHistory(execution.pipelineConfigId, 1) >> [
+    ]
+    1 * front50Service.getPipelines(execution.application, false) >> [
+        [id: execution.pipelineConfigId, application: execution.application, name: "whatever", disabled: false]
     ]
+    0 * _
+
+    notThrown(PipelineValidationFailed)
 
     when:
     validator.checkRunnable(execution)
 
     then:
+    1 * front50Service.getPipelineHistory(execution.pipelineConfigId, 1) >> [
+        [id: execution.pipelineConfigId, application: execution.application, name: "whatever", disabled: false]
+    ]
+    0 * _
+
     notThrown(PipelineValidationFailed)
 
     where:
@@ -67,15 +81,28 @@ class EnabledPipelineValidatorSpec extends Specification {
   }
 
   def "prevents disabled pipeline from running"() {
-    given:
-    front50Service.getPipelines(execution.application, false) >> [
-      [id: execution.pipelineConfigId, application: execution.application, name: "whatever", disabled: true]
+    when:
+    validator.checkRunnable(execution)
+
+    then:
+    1 * front50Service.getPipelineHistory(execution.pipelineConfigId, 1) >> [
     ]
+    1 * front50Service.getPipelines(execution.application, false) >> [
+        [id: execution.pipelineConfigId, application: execution.application, name: "whatever", disabled: true]
+    ]
+    0 * _
+
+    thrown(EnabledPipelineValidator.PipelineIsDisabled)
 
     when:
     validator.checkRunnable(execution)
 
     then:
+    1 * front50Service.getPipelineHistory(execution.pipelineConfigId, 1) >> [
+        [id: execution.pipelineConfigId, application: execution.application, name: "whatever", disabled: true]
+    ]
+    0 * _
+
     thrown(EnabledPipelineValidator.PipelineIsDisabled)
 
     where:
@@ -88,7 +115,7 @@ class EnabledPipelineValidatorSpec extends Specification {
   def "allows enabled strategy to run"() {
     given:
     front50Service.getStrategies(execution.application) >> [
-      [id: execution.pipelineConfigId, application: execution.application, name: "whatever", disabled: false]
+        [id: execution.pipelineConfigId, application: execution.application, name: "whatever", disabled: false]
     ]
 
     when:
@@ -109,7 +136,7 @@ class EnabledPipelineValidatorSpec extends Specification {
   def "prevents disabled strategy from running"() {
     given:
     front50Service.getStrategies(execution.application) >> [
-      [id: execution.pipelineConfigId, application: execution.application, name: "whatever", disabled: true]
+        [id: execution.pipelineConfigId, application: execution.application, name: "whatever", disabled: true]
     ]
 
     when:
@@ -128,15 +155,14 @@ class EnabledPipelineValidatorSpec extends Specification {
   }
 
   def "doesn't choke on non-boolean strategy value"() {
-    given:
-    front50Service.getPipelines(execution.application, false) >> [
-      [id: execution.pipelineConfigId, application: execution.application, name: "whatever", disabled: false]
-    ]
-
     when:
     validator.checkRunnable(execution)
 
     then:
+    1 * front50Service.getPipelineHistory(execution.pipelineConfigId, 1) >> [
+        [id: execution.pipelineConfigId, application: execution.application, name: "whatever", disabled: false]
+    ]
+
     notThrown(PipelineValidationFailed)
 
     where: