From c2a4d72bf2e4325990be7233f6d00d705dfbfffb Mon Sep 17 00:00:00 2001 From: Rob Zienert Date: Wed, 6 Sep 2017 09:50:55 -1000 Subject: [PATCH] fix(pipeline_template): Respect UI-configured concurrency options (#1599) --- .../PipelineTemplatePipelinePreprocessor.java | 54 +---------- .../TemplatedPipelineRequest.java | 92 +++++++++++++++++++ .../generator/ExecutionGenerator.java | 3 +- .../v1schema/V1SchemaExecutionGenerator.java | 25 +++-- ...ineTemplatePipelinePreprocessorSpec.groovy | 29 +++++- .../V1SchemaExecutionGeneratorSpec.groovy | 7 +- 6 files changed, 142 insertions(+), 68 deletions(-) create mode 100644 orca-pipelinetemplate/src/main/java/com/netflix/spinnaker/orca/pipelinetemplate/TemplatedPipelineRequest.java diff --git a/orca-pipelinetemplate/src/main/java/com/netflix/spinnaker/orca/pipelinetemplate/PipelineTemplatePipelinePreprocessor.java b/orca-pipelinetemplate/src/main/java/com/netflix/spinnaker/orca/pipelinetemplate/PipelineTemplatePipelinePreprocessor.java index b44488e193..5c3cf8489d 100644 --- a/orca-pipelinetemplate/src/main/java/com/netflix/spinnaker/orca/pipelinetemplate/PipelineTemplatePipelinePreprocessor.java +++ b/orca-pipelinetemplate/src/main/java/com/netflix/spinnaker/orca/pipelinetemplate/PipelineTemplatePipelinePreprocessor.java @@ -117,7 +117,7 @@ private Map processInternal(Map pipeline) { graphMutator.mutate(template); ExecutionGenerator executionGenerator = new V1SchemaExecutionGenerator(); - Map generatedPipeline = executionGenerator.generate(template, templateConfiguration, (String) pipeline.get("id")); + Map generatedPipeline = executionGenerator.generate(template, templateConfiguration, request); return generatedPipeline; } @@ -163,56 +163,4 @@ private void setTemplateSourceWithJinja(TemplatedPipelineRequest request) { RenderContext context = new DefaultRenderContext(request.getConfig().getPipeline().getApplication(), null, request.getTrigger()); request.getConfig().getPipeline().getTemplate().setSource(renderer.render(request.getConfig().getPipeline().getTemplate().getSource(), context )); } - - private static class TemplatedPipelineRequest { - String type; - Map trigger; - TemplateConfiguration config; - PipelineTemplate template; - Boolean plan = false; - - public boolean isTemplatedPipelineRequest() { - return "templatedPipeline".equals(type); - } - - public String getType() { - return type; - } - - public void setType(String type) { - this.type = type; - } - - public TemplateConfiguration getConfig() { - return config; - } - - public void setConfig(TemplateConfiguration config) { - this.config = config; - } - - public Map getTrigger() { - return trigger; - } - - public void setTrigger(Map trigger) { - this.trigger = trigger; - } - - public PipelineTemplate getTemplate() { - return template; - } - - public void setTemplate(PipelineTemplate template) { - this.template = template; - } - - public Boolean getPlan() { - return plan; - } - - public void setPlan(Boolean plan) { - this.plan = plan; - } - } } diff --git a/orca-pipelinetemplate/src/main/java/com/netflix/spinnaker/orca/pipelinetemplate/TemplatedPipelineRequest.java b/orca-pipelinetemplate/src/main/java/com/netflix/spinnaker/orca/pipelinetemplate/TemplatedPipelineRequest.java new file mode 100644 index 0000000000..1c7e4b9a87 --- /dev/null +++ b/orca-pipelinetemplate/src/main/java/com/netflix/spinnaker/orca/pipelinetemplate/TemplatedPipelineRequest.java @@ -0,0 +1,92 @@ +/* + * Copyright 2017 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.orca.pipelinetemplate; + +import com.netflix.spinnaker.orca.pipelinetemplate.v1schema.model.PipelineTemplate; +import com.netflix.spinnaker.orca.pipelinetemplate.v1schema.model.TemplateConfiguration; + +import java.util.Map; + +public class TemplatedPipelineRequest { + String id; + String type; + Map trigger; + TemplateConfiguration config; + PipelineTemplate template; + Boolean plan = false; + boolean limitConcurrent = true; + boolean keepWaitingPipelines = false; + + public String getId() { + return id; + } + + public void setId(String id) { + this.id = id; + } + + public boolean isTemplatedPipelineRequest() { + return "templatedPipeline".equals(type); + } + + public String getType() { + return type; + } + + public void setType(String type) { + this.type = type; + } + + public TemplateConfiguration getConfig() { + return config; + } + + public void setConfig(TemplateConfiguration config) { + this.config = config; + } + + public Map getTrigger() { + return trigger; + } + + public void setTrigger(Map trigger) { + this.trigger = trigger; + } + + public PipelineTemplate getTemplate() { + return template; + } + + public void setTemplate(PipelineTemplate template) { + this.template = template; + } + + public Boolean getPlan() { + return plan; + } + + public void setPlan(Boolean plan) { + this.plan = plan; + } + + public boolean isLimitConcurrent() { + return limitConcurrent; + } + + public boolean isKeepWaitingPipelines() { + return keepWaitingPipelines; + } +} diff --git a/orca-pipelinetemplate/src/main/java/com/netflix/spinnaker/orca/pipelinetemplate/generator/ExecutionGenerator.java b/orca-pipelinetemplate/src/main/java/com/netflix/spinnaker/orca/pipelinetemplate/generator/ExecutionGenerator.java index 7a70259d91..b2a53f85d0 100644 --- a/orca-pipelinetemplate/src/main/java/com/netflix/spinnaker/orca/pipelinetemplate/generator/ExecutionGenerator.java +++ b/orca-pipelinetemplate/src/main/java/com/netflix/spinnaker/orca/pipelinetemplate/generator/ExecutionGenerator.java @@ -15,6 +15,7 @@ */ package com.netflix.spinnaker.orca.pipelinetemplate.generator; +import com.netflix.spinnaker.orca.pipelinetemplate.TemplatedPipelineRequest; import com.netflix.spinnaker.orca.pipelinetemplate.v1schema.model.PipelineTemplate; import com.netflix.spinnaker.orca.pipelinetemplate.v1schema.model.TemplateConfiguration; @@ -22,5 +23,5 @@ public interface ExecutionGenerator { - Map generate(PipelineTemplate template, TemplateConfiguration configuration, String id); + Map generate(PipelineTemplate template, TemplateConfiguration configuration, TemplatedPipelineRequest request); } diff --git a/orca-pipelinetemplate/src/main/java/com/netflix/spinnaker/orca/pipelinetemplate/v1schema/V1SchemaExecutionGenerator.java b/orca-pipelinetemplate/src/main/java/com/netflix/spinnaker/orca/pipelinetemplate/v1schema/V1SchemaExecutionGenerator.java index 038f1df911..47f0155ce7 100644 --- a/orca-pipelinetemplate/src/main/java/com/netflix/spinnaker/orca/pipelinetemplate/v1schema/V1SchemaExecutionGenerator.java +++ b/orca-pipelinetemplate/src/main/java/com/netflix/spinnaker/orca/pipelinetemplate/v1schema/V1SchemaExecutionGenerator.java @@ -15,19 +15,25 @@ */ package com.netflix.spinnaker.orca.pipelinetemplate.v1schema; -import java.util.*; -import java.util.stream.Collectors; +import com.netflix.spinnaker.orca.pipelinetemplate.TemplatedPipelineRequest; import com.netflix.spinnaker.orca.pipelinetemplate.generator.ExecutionGenerator; import com.netflix.spinnaker.orca.pipelinetemplate.v1schema.model.PipelineTemplate; import com.netflix.spinnaker.orca.pipelinetemplate.v1schema.model.PipelineTemplate.Configuration; import com.netflix.spinnaker.orca.pipelinetemplate.v1schema.model.TemplateConfiguration; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; +import java.util.Optional; +import java.util.UUID; +import java.util.stream.Collectors; + public class V1SchemaExecutionGenerator implements ExecutionGenerator { @Override - public Map generate(PipelineTemplate template, TemplateConfiguration configuration, String id) { + public Map generate(PipelineTemplate template, TemplateConfiguration configuration, TemplatedPipelineRequest request) { Map pipeline = new HashMap<>(); - pipeline.put("id", Optional.ofNullable(id).orElse(Optional.ofNullable(configuration.getPipeline().getPipelineConfigId()).orElse("unknown"))); + pipeline.put("id", Optional.ofNullable(request.getId()).orElse(Optional.ofNullable(configuration.getPipeline().getPipelineConfigId()).orElse("unknown"))); pipeline.put("application", configuration.getPipeline().getApplication()); pipeline.put("name", Optional.ofNullable(configuration.getPipeline().getName()).orElse("Unnamed Execution")); @@ -35,14 +41,13 @@ public Map generate(PipelineTemplate template, TemplateConfigura pipeline.put("executionEngine", configuration.getPipeline().getExecutionEngine()); } - // TODO rz - Ehhhh Configuration c = template.getConfiguration(); - if (template.getConfiguration() == null) { - pipeline.put("limitConcurrent", true); - pipeline.put("keepWaitingPipelines", false); + if (c.getConcurrentExecutions().isEmpty()) { + pipeline.put("limitConcurrent", request.isLimitConcurrent()); + pipeline.put("keepWaitingPipelines", request.isKeepWaitingPipelines()); } else { - pipeline.put("limitConcurrent", c.getConcurrentExecutions().getOrDefault("limitConcurrent", true)); - pipeline.put("keepWaitingPipelines", c.getConcurrentExecutions().getOrDefault("keepWaitingPipelines", false)); + pipeline.put("limitConcurrent", c.getConcurrentExecutions().getOrDefault("limitConcurrent", request.isLimitConcurrent())); + pipeline.put("keepWaitingPipelines", c.getConcurrentExecutions().getOrDefault("keepWaitingPipelines", request.isKeepWaitingPipelines())); } addNotifications(pipeline, template, configuration); diff --git a/orca-pipelinetemplate/src/test/groovy/com/netflix/spinnaker/orca/pipelinetemplate/PipelineTemplatePipelinePreprocessorSpec.groovy b/orca-pipelinetemplate/src/test/groovy/com/netflix/spinnaker/orca/pipelinetemplate/PipelineTemplatePipelinePreprocessorSpec.groovy index b5bc63d409..71e3989dca 100644 --- a/orca-pipelinetemplate/src/test/groovy/com/netflix/spinnaker/orca/pipelinetemplate/PipelineTemplatePipelinePreprocessorSpec.groovy +++ b/orca-pipelinetemplate/src/test/groovy/com/netflix/spinnaker/orca/pipelinetemplate/PipelineTemplatePipelinePreprocessorSpec.groovy @@ -25,7 +25,6 @@ import com.netflix.spinnaker.orca.pipelinetemplate.loader.TemplateLoader import com.netflix.spinnaker.orca.pipelinetemplate.v1schema.model.PipelineTemplate import com.netflix.spinnaker.orca.pipelinetemplate.v1schema.model.StageDefinition import com.netflix.spinnaker.orca.pipelinetemplate.v1schema.model.TemplateConfiguration -import com.netflix.spinnaker.orca.pipelinetemplate.v1schema.render.DefaultRenderContext import com.netflix.spinnaker.orca.pipelinetemplate.v1schema.render.JinjaRenderer import com.netflix.spinnaker.orca.pipelinetemplate.v1schema.render.RenderUtil import com.netflix.spinnaker.orca.pipelinetemplate.v1schema.render.Renderer @@ -35,6 +34,7 @@ import org.yaml.snakeyaml.Yaml import spock.lang.Specification import spock.lang.Subject import spock.lang.Unroll + import static org.unitils.reflectionassert.ReflectionAssert.assertReflectionEquals class PipelineTemplatePipelinePreprocessorSpec extends Specification { @@ -319,6 +319,33 @@ class PipelineTemplatePipelinePreprocessorSpec extends Specification { result.stages*.group == ['my group of stages: wowow waiting', 'my group of stages: wowow waiting'] } + def "should respect request-defined concurrency options if configuration does not define them"() { + given: + def pipeline = [ + type: 'templatedPipeline', + config: [ + schema: '1', + pipeline: [ + application: 'myapp' + ] + ], + template: [ + schema: '1', + id: 'myTemplate', + configuration: [:], + stages: [] + ], + plan: true, + limitConcurrent: false + ] + + when: + def result = subject.process(pipeline) + + then: + result.limitConcurrent == false + } + Map createTemplateRequest(String templatePath, Map variables = [:], List> stages = [], boolean plan = false) { return [ type: 'templatedPipeline', diff --git a/orca-pipelinetemplate/src/test/groovy/com/netflix/spinnaker/orca/pipelinetemplate/v1schema/V1SchemaExecutionGeneratorSpec.groovy b/orca-pipelinetemplate/src/test/groovy/com/netflix/spinnaker/orca/pipelinetemplate/v1schema/V1SchemaExecutionGeneratorSpec.groovy index 9a23efff08..44f8aa156a 100644 --- a/orca-pipelinetemplate/src/test/groovy/com/netflix/spinnaker/orca/pipelinetemplate/v1schema/V1SchemaExecutionGeneratorSpec.groovy +++ b/orca-pipelinetemplate/src/test/groovy/com/netflix/spinnaker/orca/pipelinetemplate/v1schema/V1SchemaExecutionGeneratorSpec.groovy @@ -15,6 +15,7 @@ */ package com.netflix.spinnaker.orca.pipelinetemplate.v1schema +import com.netflix.spinnaker.orca.pipelinetemplate.TemplatedPipelineRequest import com.netflix.spinnaker.orca.pipelinetemplate.generator.ExecutionGenerator import com.netflix.spinnaker.orca.pipelinetemplate.v1schema.model.NamedHashMap import com.netflix.spinnaker.orca.pipelinetemplate.v1schema.model.PipelineTemplate @@ -44,7 +45,7 @@ class V1SchemaExecutionGeneratorSpec extends Specification { ) when: - def result = subject.generate(template, configuration, "124") + def result = subject.generate(template, configuration, new TemplatedPipelineRequest(id: "124")) then: noExceptionThrown() @@ -69,7 +70,7 @@ class V1SchemaExecutionGeneratorSpec extends Specification { ) when: - def result = subject.generate(template, configuration, pipelineId) + def result = subject.generate(template, configuration, new TemplatedPipelineRequest(id: pipelineId)) then: result.id == expectedId @@ -102,7 +103,7 @@ class V1SchemaExecutionGeneratorSpec extends Specification { ) when: - def result = subject.generate(template, configuration, "pipelineConfigId") + def result = subject.generate(template, configuration, new TemplatedPipelineRequest(id: "pipelineConfigId")) then: result.notifications*.address == addresses