From e4a8d3f8e8b4b420f5d25684f2decb5d77946a77 Mon Sep 17 00:00:00 2001 From: Mark Vulfson Date: Mon, 1 Jul 2019 22:17:13 -0700 Subject: [PATCH] fix(bake): Lookup artifact details from all upstream stages (#3011) * fix(bake): Lookup artifact details from all upstream stages Imagine following setup: ``` Pipeline A -> Jenkins job (produces foo.*.deb) -> Run Pipeline B -> Jenkins job (produces bar.*.deb) -> Bake package foo ``` In this case, the package lookup for bake stage will fail to lookup artifact details for `foo.deb` to pass to the bakery. Thus the bakery is free to pick the latest artifact matching the name which can be the wrong artifact. This change allows package look up to traverse up parent pipeline stages, similar to `FindImage` tasks --- .../ecs/EcsServerGroupCreator.groovy | 2 +- .../kato/tasks/DeploymentDetailsAware.groovy | 53 +------ .../spinnaker/orca/pipeline/model/Stage.java | 118 +++++++++++++++ .../orca/pipeline/util/PackageInfo.java | 29 ++-- .../orca/pipeline/util/PackageInfoSpec.groovy | 135 ++++++++++++++++++ 5 files changed, 272 insertions(+), 65 deletions(-) diff --git a/orca-clouddriver/src/main/groovy/com/netflix/spinnaker/orca/clouddriver/tasks/providers/ecs/EcsServerGroupCreator.groovy b/orca-clouddriver/src/main/groovy/com/netflix/spinnaker/orca/clouddriver/tasks/providers/ecs/EcsServerGroupCreator.groovy index 6e1fa12a03..695be0cae6 100644 --- a/orca-clouddriver/src/main/groovy/com/netflix/spinnaker/orca/clouddriver/tasks/providers/ecs/EcsServerGroupCreator.groovy +++ b/orca-clouddriver/src/main/groovy/com/netflix/spinnaker/orca/clouddriver/tasks/providers/ecs/EcsServerGroupCreator.groovy @@ -49,7 +49,7 @@ class EcsServerGroupCreator implements ServerGroupCreator, DeploymentDetailsAwar if (imageDescription.fromContext) { if (stage.execution.type == ExecutionType.ORCHESTRATION) { // Use image from specific "find image from tags" stage - def imageStage = getAncestors(stage, stage.execution).find { + def imageStage = stage.ancestorsWithParentPipelines().find { it.refId == imageDescription.stageId && it.context.containsKey("amiDetails") } diff --git a/orca-clouddriver/src/main/groovy/com/netflix/spinnaker/orca/kato/tasks/DeploymentDetailsAware.groovy b/orca-clouddriver/src/main/groovy/com/netflix/spinnaker/orca/kato/tasks/DeploymentDetailsAware.groovy index 9827ab95a8..06f9ae2c2b 100644 --- a/orca-clouddriver/src/main/groovy/com/netflix/spinnaker/orca/kato/tasks/DeploymentDetailsAware.groovy +++ b/orca-clouddriver/src/main/groovy/com/netflix/spinnaker/orca/kato/tasks/DeploymentDetailsAware.groovy @@ -62,7 +62,7 @@ trait DeploymentDetailsAware { return null } - return getAncestors(stage, stage.execution).find { + Stage ancestorWithImage = stage.findAncestor({ def regions = (it.context.region ? [it.context.region] : it.context.regions) as Set def cloudProviderFromContext = it.context.cloudProvider ?: it.context.cloudProviderType boolean hasTargetCloudProvider = !cloudProviderFromContext || targetCloudProvider == cloudProviderFromContext @@ -70,7 +70,9 @@ trait DeploymentDetailsAware { boolean hasImage = it.context.containsKey("ami") || it.context.containsKey("amiDetails") return hasImage && hasTargetRegion && hasTargetCloudProvider - } + }) + + return ancestorWithImage } List getPipelineExecutions(Execution execution) { @@ -88,53 +90,6 @@ trait DeploymentDetailsAware { return true } - List getAncestors(Stage stage, Execution execution) { - if (stage?.requisiteStageRefIds) { - def previousStages = execution.stages.findAll { - it.refId in stage.requisiteStageRefIds - - } - def syntheticStages = execution.stages.findAll { - it.parentStageId in previousStages*.id - } - return (previousStages + syntheticStages) + previousStages.collect { getAncestors(it, execution ) }.flatten() - } else if (stage?.parentStageId) { - def parent = execution.stages.find { it.id == stage.parentStageId } - return ([parent] + getAncestors(parent, execution)).flatten() - } else if (execution.type == PIPELINE) { - def parentPipelineExecution = getParentPipelineExecution(execution) - - if (parentPipelineExecution) { - String parentPipelineStageId = (execution.trigger as PipelineTrigger).parentPipelineStageId - Stage parentPipelineStage = parentPipelineExecution.stages?.find { - it.type == "pipeline" && it.id == parentPipelineStageId - } - - if (parentPipelineStage) { - return getAncestors(parentPipelineStage, parentPipelineExecution) - } else { - List parentPipelineStages = parentPipelineExecution.stages?.collect()?.sort { - a, b -> b.endTime <=> a.endTime - } - - if (parentPipelineStages) { - // The list is sorted in reverse order by endTime. - Stage firstStage = parentPipelineStages.last() - - return parentPipelineStages + getAncestors(firstStage, parentPipelineExecution) - } else { - // Parent pipeline has no stages. - return getAncestors(null, parentPipelineExecution) - } - } - } - - return [] - } else { - return [] - } - } - private Execution getParentPipelineExecution(Execution execution) { // The initial stage execution is a Pipeline, and the ancestor executions are Maps. if (execution.type == PIPELINE && execution.trigger instanceof PipelineTrigger) { diff --git a/orca-core/src/main/java/com/netflix/spinnaker/orca/pipeline/model/Stage.java b/orca-core/src/main/java/com/netflix/spinnaker/orca/pipeline/model/Stage.java index 973ed6c8d6..e4982b4c46 100644 --- a/orca-core/src/main/java/com/netflix/spinnaker/orca/pipeline/model/Stage.java +++ b/orca-core/src/main/java/com/netflix/spinnaker/orca/pipeline/model/Stage.java @@ -30,6 +30,7 @@ import com.fasterxml.jackson.databind.annotation.JsonDeserialize; import com.fasterxml.jackson.databind.node.ObjectNode; import com.fasterxml.jackson.databind.node.TreeTraversingParser; +import com.google.common.base.Strings; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Sets; @@ -40,6 +41,7 @@ import java.io.IOException; import java.io.Serializable; import java.util.*; +import java.util.function.Predicate; import java.util.stream.Stream; import javax.annotation.Nonnull; import javax.annotation.Nullable; @@ -402,6 +404,122 @@ private List ancestorsOnly(Set visited) { } } + /** + * Computes all ancestor stages, including those of parent pipelines + * + * @return list of ancestor stages + */ + public Stage findAncestor(Predicate predicate) { + return Stage.findAncestor(this, this.execution, predicate); + } + + private static Stage findAncestor(Stage stage, Execution execution, Predicate predicate) { + Stage matchingStage = null; + + if (stage != null && !stage.getRequisiteStageRefIds().isEmpty()) { + List previousStages = + execution.getStages().stream() + .filter(s -> stage.getRequisiteStageRefIds().contains(s.getRefId())) + .collect(toList()); + + Set previousStageIds = + new HashSet<>(previousStages.stream().map(Stage::getId).collect(toList())); + List syntheticStages = + execution.getStages().stream() + .filter(s -> previousStageIds.contains(s.getParentStageId())) + .collect(toList()); + + List priorStages = new ArrayList<>(); + priorStages.addAll(previousStages); + priorStages.addAll(syntheticStages); + + matchingStage = priorStages.stream().filter(predicate).findFirst().orElse(null); + + if (matchingStage == null) { + for (Stage s : previousStages) { + matchingStage = findAncestor(s, execution, predicate); + + if (matchingStage != null) { + break; + } + } + } + } else if ((stage != null) && !Strings.isNullOrEmpty(stage.getParentStageId())) { + Optional parent = + execution.getStages().stream() + .filter(s -> s.getId().equals(stage.getParentStageId())) + .findFirst(); + + if (!parent.isPresent()) { + throw new IllegalStateException( + "Couldn't find parent of stage " + + stage.getId() + + " with parent " + + stage.getParentStageId()); + } + + if (predicate.test(parent.get())) { + matchingStage = parent.get(); + } else { + matchingStage = findAncestor(parent.get(), execution, predicate); + } + } else if ((execution.getType() == PIPELINE) + && (execution.getTrigger() instanceof PipelineTrigger)) { + PipelineTrigger parentTrigger = (PipelineTrigger) execution.getTrigger(); + + Execution parentPipelineExecution = parentTrigger.getParentExecution(); + String parentPipelineStageId = parentTrigger.getParentPipelineStageId(); + + Optional parentPipelineStage = + parentPipelineExecution.getStages().stream() + .filter( + s -> s.getType().equals("pipeline") && s.getId().equals(parentPipelineStageId)) + .findFirst(); + + if (parentPipelineStage.isPresent()) { + matchingStage = findAncestor(parentPipelineStage.get(), parentPipelineExecution, predicate); + } else { + List parentPipelineStages = new ArrayList<>(parentPipelineExecution.getStages()); + parentPipelineStages.sort( + (s1, s2) -> { + if ((s1.endTime == null) && (s2.endTime == null)) { + return 0; + } + + if (s1.endTime == null) { + return 1; + } + + if (s2.endTime == null) { + return -1; + } + + return s1.endTime.compareTo(s2.endTime); + }); + + if (parentPipelineStages.size() > 0) { + // The list is sorted in reverse order by endTime. + matchingStage = parentPipelineStages.stream().filter(predicate).findFirst().orElse(null); + + if (matchingStage == null) { + Stage firstStage = parentPipelineStages.get(0); + + if (predicate.test(firstStage)) { + matchingStage = firstStage; + } else { + matchingStage = findAncestor(firstStage, parentPipelineExecution, predicate); + } + } + } else { + // Parent pipeline has no stages. + matchingStage = findAncestor(null, parentPipelineExecution, predicate); + } + } + } + + return matchingStage; + } + /** Recursively get all stages that are children of the current one */ public List allDownstreamStages() { List children = new ArrayList<>(); diff --git a/orca-core/src/main/java/com/netflix/spinnaker/orca/pipeline/util/PackageInfo.java b/orca-core/src/main/java/com/netflix/spinnaker/orca/pipeline/util/PackageInfo.java index 2b6cc2b5e9..0d294cfb9f 100644 --- a/orca-core/src/main/java/com/netflix/spinnaker/orca/pipeline/util/PackageInfo.java +++ b/orca-core/src/main/java/com/netflix/spinnaker/orca/pipeline/util/PackageInfo.java @@ -31,9 +31,9 @@ import java.util.regex.Pattern; /** - * This class inspects the context of a stage, preceding stages, the trigger, and possibly the - * parent pipeline in order to see if an artifact matching the name(s) specified in the bake stage - * was produced. If so, that version will be used in the bake request. If nothing is found after all + * This class inspects the context of a stage, preceding stages, the trigger, and the parent + * pipeline in order to see if an artifact matching the name(s) specified in the bake stage was + * produced. If so, that version will be used in the bake request. If nothing is found after all * this searching it is up to the bakery to pull the latest package version. * *

Artifact information comes from Jenkins on the pipeline trigger in the field @@ -385,19 +385,17 @@ private Map filterRPMArtifacts( private static Map findBuildInfoInUpstreamStage( Stage currentStage, List packageFilePatterns) { + Stage upstreamStage = - currentStage.ancestors().stream() - .filter( - it -> { - Map buildInfo = - (Map) it.getOutputs().get("buildInfo"); - return buildInfo != null - && artifactMatch( - (List>) buildInfo.get("artifacts"), - packageFilePatterns); - }) - .findFirst() - .orElse(null); + currentStage.findAncestor( + it -> { + Map buildInfo = + (Map) it.getOutputs().get("buildInfo"); + return buildInfo != null + && artifactMatch( + (List>) buildInfo.get("artifacts"), packageFilePatterns); + }); + return upstreamStage != null ? (Map) upstreamStage.getOutputs().get("buildInfo") : emptyMap(); @@ -405,6 +403,7 @@ && artifactMatch( private static boolean artifactMatch( List> artifacts, List patterns) { + return artifacts != null && artifacts.stream() .anyMatch( diff --git a/orca-core/src/test/groovy/com/netflix/spinnaker/orca/pipeline/util/PackageInfoSpec.groovy b/orca-core/src/test/groovy/com/netflix/spinnaker/orca/pipeline/util/PackageInfoSpec.groovy index dae64ad8a1..7a80e110ba 100644 --- a/orca-core/src/test/groovy/com/netflix/spinnaker/orca/pipeline/util/PackageInfoSpec.groovy +++ b/orca-core/src/test/groovy/com/netflix/spinnaker/orca/pipeline/util/PackageInfoSpec.groovy @@ -778,4 +778,139 @@ class PackageInfoSpec extends Specification { triggerFilename | buildFilename | requestPackage | result [["fileName": "test-package-1539516142-1.x86_64.rpm"]] | [["fileName": "test-package-1539516142-1.x86_64.rpm"]] | "test-package" | "test-package-1539516142-1.x86_64" } + + def "locates correct artifact from prior Jenkins stages (including parent executions)"() { + given: + def grandParentPipeline = pipeline { + stage { + refId = "1-gp" + type = "jenkins" + outputs["buildInfo"] = [ + url : "http://jenkins", + master : "master", + name : "grandParentStage", + number : 1, + artifacts: [[fileName: "artifact1_1.1.1-h01.grandParent.deb", relativePath: "."], + [fileName: "artifact2_1.1.1-h01.grandParent.deb", relativePath: "."]], + scm : [] + ] + } + stage { + refId = "2-gp" + type = "pipeline" + requisiteStageRefIds = ["1-gp"] + } + } + + def parentPipeline = pipeline { + trigger = new PipelineTrigger( + "pipeline", + null, + "example@example.com", + [:], + [], + [], + false, + false, + false, + grandParentPipeline, + grandParentPipeline.stageByRef("2-gp").id + ) + stage { + refId = "1-p" + type = "jenkins" + outputs["buildInfo"] = [ + url : "http://jenkins", + master : "master", + name : "parentStage", + number : 1, + artifacts: [[fileName: "artifact1_1.1.2-h02-parent.deb", relativePath: "."]], + scm : [] + ] + } + stage { + refId = "2-p" + type = "pipeline" + requisiteStageRefIds = ["1-p"] + } + } + def pipeline = pipeline { + trigger = new PipelineTrigger( + "pipeline", + null, + "example@example.com", + [:], + [], + [], + false, + false, + false, + parentPipeline, + parentPipeline.stageByRef("2-p").id + ) + stage { + refId = "1-child" + type = "jenkins" + outputs["buildInfo"] = [ + url : "http://jenkins", + master : "master", + name : "childStage", + number : 1, + artifacts: [[fileName: "blah_1.1.1-h01.child.deb", relativePath: "."]], + scm : [] + ] + } + stage { + type = "bake-child" + refId = "child-bake-bakes-blah" + requisiteStageRefIds = ["1-child"] + context["package"] = "blah" + } + stage { + type = "bake-parent" + refId = "child-bake-bakes-artifact1" + requisiteStageRefIds = ["1-child"] + context["package"] = "artifact1" + } + stage { + type = "bake-grandparent" + refId = "child-bake-bakes-artifact2" + requisiteStageRefIds = ["1-child"] + context["package"] = "artifact2" + } + } + + PackageType packageType = DEB + ObjectMapper objectMapper = new ObjectMapper() + + when: + PackageInfo packageInfo1 = new PackageInfo( + pipeline.stageByRef("child-bake-bakes-blah"), [], packageType.packageType, packageType.versionDelimiter, true, true, objectMapper) + def buildInfo1 = packageInfo1.findTargetPackage(false) + + then: + noExceptionThrown() + buildInfo1 == [packageVersion:"1.1.1-h01.child", package:"blah_1.1.1-h01.child", + buildInfoUrl:"http://jenkins", job:"childStage", buildNumber:1] + + when: + PackageInfo packageInfo2 = new PackageInfo( + pipeline.stageByRef("child-bake-bakes-artifact1"), [], packageType.packageType, packageType.versionDelimiter, true, true, objectMapper) + def buildInfo2 = packageInfo2.findTargetPackage(false) + + then: + noExceptionThrown() + buildInfo2 == [packageVersion:"1.1.2-h02-parent", package:"artifact1_1.1.2-h02-parent", + buildInfoUrl:"http://jenkins", job:"parentStage", buildNumber:1] + + when: + PackageInfo packageInfo3 = new PackageInfo( + pipeline.stageByRef("child-bake-bakes-artifact2"), [], packageType.packageType, packageType.versionDelimiter, true, true, objectMapper) + def buildInfo3 = packageInfo3.findTargetPackage(false) + + then: + noExceptionThrown() + buildInfo3 == [packageVersion:"1.1.1-h01.grandParent", package:"artifact2_1.1.1-h01.grandParent", + buildInfoUrl:"http://jenkins", job:"grandParentStage", buildNumber:1] + } }