Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(orca) Place produced artifacts in global context #1441

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

package com.netflix.spinnaker.orca.bakery.api

import com.netflix.spinnaker.kork.artifacts.model.Artifact
import groovy.transform.CompileStatic
import groovy.transform.EqualsAndHashCode
import groovy.transform.ToString
Expand All @@ -35,5 +36,6 @@ class Bake {
String ami
String amiName
String imageName
Artifact artifact
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be a list, right? We can produce multiple images per bake.

// TODO(duftler): Add a cloudProviderType property here? Will be straightforward once rosco is backed by redis.
}
Original file line number Diff line number Diff line change
Expand Up @@ -127,10 +127,12 @@ class BakeStage implements StageDefinitionBuilder, RestartableStage {
it.parentStageId == stage.parentStageId && it.status == ExecutionStatus.RUNNING
}

def relatedBakeStages = stage.execution.stages.findAll {
it.type == PIPELINE_CONFIG_TYPE && bakeInitializationStages*.id.contains(it.parentStageId)
}

def globalContext = [
deploymentDetails: stage.execution.stages.findAll {
it.type == PIPELINE_CONFIG_TYPE && bakeInitializationStages*.id.contains(it.parentStageId) && (it.context.ami || it.context.imageId)
}.collect { Stage bakeStage ->
deploymentDetails: relatedBakeStages.findAll{it.context.ami || it.context.imageId}.collect { Stage bakeStage ->
def deploymentDetails = [:]
["ami", "imageId", "amiSuffix", "baseLabel", "baseOs", "refId", "storeType", "vmType", "region", "package", "cloudProviderType", "cloudProvider"].each {
if (bakeStage.context.containsKey(it)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package com.netflix.spinnaker.orca.bakery.tasks

import com.netflix.spinnaker.kork.artifacts.model.Artifact
import com.netflix.spinnaker.orca.ExecutionStatus
import com.netflix.spinnaker.orca.Task
import com.netflix.spinnaker.orca.TaskResult
Expand All @@ -39,7 +40,7 @@ class CompletedBakeTask implements Task {
def bakeStatus = stage.context.status as BakeStatus
def bake = bakery.lookupBake(region, bakeStatus.resourceId).toBlocking().first()
// This treatment of ami allows both the aws and gce bake results to be propagated.
def results = [ami: bake.ami ?: bake.imageName, imageId: bake.ami ?: bake.imageName]
def results = [ami: bake.ami ?: bake.imageName, imageId: bake.ami ?: bake.imageName, artifact: bake.artifact ?: new Artifact()]
/**
* TODO:
* It would be good to standardize on the key here. "imageId" works for all providers.
Expand All @@ -49,6 +50,7 @@ class CompletedBakeTask implements Task {
if (bake.imageName || bake.amiName) {
results.imageName = bake.imageName ?: bake.amiName
}

new TaskResult(ExecutionStatus.SUCCEEDED, results)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package com.netflix.spinnaker.orca.bakery.tasks

import com.netflix.spinnaker.kork.artifacts.model.Artifact
import com.netflix.spinnaker.orca.ExecutionStatus
import com.netflix.spinnaker.orca.bakery.api.Bake
import com.netflix.spinnaker.orca.bakery.api.BakeStatus
Expand Down Expand Up @@ -44,10 +45,10 @@ class CompletedBakeTaskSpec extends Specification {
null
)

def "finds the AMI created by a bake"() {
def "finds the AMI and artifact created by a bake"() {
given:
task.bakery = Stub(BakeryService) {
lookupBake(region, bakeId) >> Observable.from(new Bake(id: bakeId, ami: ami))
lookupBake(region, bakeId) >> Observable.from(new Bake(id: bakeId, ami: ami, artifact: artifact))
}

and:
Expand All @@ -59,11 +60,13 @@ class CompletedBakeTaskSpec extends Specification {
then:
result.status == ExecutionStatus.SUCCEEDED
result.context.ami == ami
result.context.artifact.reference == ami

where:
region = "us-west-1"
bakeId = "b-5af233wjj78mwt2f420wt8ey3w"
ami = "ami-280c3b6d"
artifact = new Artifact(reference: ami)
}

def "fails if the bake is not found"() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package com.netflix.spinnaker.orca.clouddriver.tasks.cluster
import com.fasterxml.jackson.core.type.TypeReference
import com.fasterxml.jackson.databind.ObjectMapper
import com.netflix.frigga.Names
import com.netflix.spinnaker.kork.artifacts.model.Artifact
import com.netflix.spinnaker.moniker.Moniker
import com.netflix.spinnaker.orca.ExecutionStatus
import com.netflix.spinnaker.orca.RetryableTask
Expand Down Expand Up @@ -229,8 +230,45 @@ class FindImageFromClusterTask extends AbstractCloudProviderAwareTask implements
}
}.flatten()

List<Artifact> artifacts = imageSummaries.collect { placement, summaries ->
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this meant to be an interim solution to make it appear like all stages emit artifacts rather than image summaries?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. The idea is to do this in two takes. This first PR to make artifacts available where we produce/find images. Then the next iteration will be to actually use these artifacts for deployments which will in turn make them even more available.

Artifact artifact = new Artifact()
summaries.findResults { summary ->
if (config.imageNamePattern && !(summary.imageName ==~ config.imageNamePattern)) {
return null
}
def location = "global"

if (placement.type == Location.Type.REGION) {
location = placement.value
} else if (placement.type == Location.Type.ZONE) {
location = placement.value
}

def metadata = [
sourceServerGroup: summary.serverGroupName,
refId: stage.refId
]

try {
metadata.putAll(summary.image ?: [:])
metadata.putAll(summary.buildInfo ?: [:])
} catch (Exception e) {
log.error("Unable to merge server group image/build info (summary: ${summary})", e)
}

artifact.metadata = metadata
artifact.name = summary.imageName
artifact.location = location
artifact.type = "${cloudProvider}/image"
artifact.reference = "${summary.imageId}"
artifact.uuid = UUID.randomUUID().toString()
}
return artifact
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems a little odd - instead of assigning to artifact created in the parent scope, I would create artifact here, and then return that. Then assign the result of findResults {..}.flatten to something like List<Artifact> artifacts

}.flatten()

return new TaskResult(ExecutionStatus.SUCCEEDED, [
amiDetails: deploymentDetails
amiDetails: deploymentDetails,
artifacts: artifacts
], [
deploymentDetails: deploymentDetails
])
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,17 @@

package com.netflix.spinnaker.orca.clouddriver.tasks.image;

import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.UUID;

import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.netflix.spinnaker.kork.artifacts.model.Artifact;
import com.netflix.spinnaker.orca.ExecutionStatus;
import com.netflix.spinnaker.orca.RetryableTask;
import com.netflix.spinnaker.orca.TaskResult;
Expand Down Expand Up @@ -53,13 +58,40 @@ public TaskResult execute(Stage stage) {
throw new IllegalStateException("Could not find tagged image for package: " + stageData.packageName + " and tags: " + stageData.tags);
}

List<Artifact> artifacts = new ArrayList<>();
imageDetails.forEach(imageDetail -> artifacts.add(generateArtifactFrom(imageDetail, cloudProvider)));

Map<String, Object> stageOutputs = new HashMap<>();
stageOutputs.put("amiDetails", imageDetails);
stageOutputs.put("artifacts", artifacts);

return new TaskResult(
ExecutionStatus.SUCCEEDED,
Collections.singletonMap("amiDetails", imageDetails),
stageOutputs,
Collections.singletonMap("deploymentDetails", imageDetails)
);

}

private Artifact generateArtifactFrom(ImageFinder.ImageDetails imageDetails, String cloudProvider) {
Map<String, String> metadata = new HashMap<>();
try {
ImageFinder.JenkinsDetails jenkinsDetails = imageDetails.getJenkins();
metadata.put("build_info_url", jenkinsDetails.get("host"));
metadata.put("build_number", jenkinsDetails.get("number"));
} catch (Exception e) {
// This is either all or nothing
}

Artifact artifact = new Artifact();
artifact.setName(imageDetails.getImageName());
artifact.setReference(imageDetails.getImageId());
artifact.setLocation(imageDetails.getRegion());
artifact.setType(cloudProvider + "/image");
artifact.setMetadata(metadata);
artifact.setUuid(UUID.randomUUID().toString());

return artifact;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ public interface ImageFinder {
interface ImageDetails {
String getImageId();
String getImageName();
String getRegion();
JenkinsDetails getJenkins();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,11 @@ public String getImageName() {
return (String) super.get("imageName");
}

@Override
public String getRegion() {
return (String) super.get("region");
}

@Override
public JenkinsDetails getJenkins() {
return (JenkinsDetails) super.get("jenkins");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,11 @@ public String getImageName() {
return (String) super.get("imageName");
}

@Override
public String getRegion() {
return (String) super.get("region");
}

@Override
public JenkinsDetails getJenkins() {
return (JenkinsDetails) super.get("jenkins");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,13 @@ import spock.lang.Subject
class FindImageFromTagTaskSpec extends Specification {

def imageFinder = Mock(ImageFinder)
def imageDetails = Mock(ImageFinder.ImageDetails)
Stage stage = new Stage<>(new Pipeline("orca"), "", [packageName: 'myPackage', tags: ['foo': 'bar']])

@Subject
def task = new FindImageFromTagsTask(imageFinders: [imageFinder])

def "Not finding an images should throw IllegalStateException"() {
def "Not finding images should throw IllegalStateException"() {
when:
task.execute(stage)

Expand All @@ -25,14 +26,18 @@ class FindImageFromTagTaskSpec extends Specification {
1 * imageFinder.getCloudProvider() >> 'aws'
}

def "Finding an images should set task state to SUCCEEDED"() {
def "Finding images should set task state to SUCCEEDED"() {
when:
def result = task.execute(stage)

then:
result.status == ExecutionStatus.SUCCEEDED

1 * imageFinder.getCloudProvider() >> 'aws'
1 * imageFinder.byTags(stage, stage.context.packageName, stage.context.tags) >> [[:]]
1 * imageFinder.byTags(stage, stage.context.packageName, stage.context.tags) >> [imageDetails]
1 * imageDetails.getImageName() >> "somename"
1 * imageDetails.getImageId() >> "someId"
1 * imageDetails.getJenkins() >> new ImageFinder.JenkinsDetails("somehost", "somename", "42")
}

}