Skip to content

Commit

Permalink
fix(orca): fix ami retrieval for diff commits
Browse files Browse the repository at this point in the history 
* fix bug in orca-igor whereby the field searched for the ami needed for
performing a commit diff (amiId) was changed without falling back to the
original value (ami) if the `amiId` was `null`.
  • Loading branch information
@icfantv
icfantv committed Apr 5, 2017
1 parent 8df1ff7 commit b4a150f
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 11 deletions.
2 changes: 1 addition & 1 deletion .../groovy/com/netflix/spinnaker/orca/clouddriver/tasks/providers/aws/AmazonImageFinder.java
View file
Expand Up @@ -126,7 +126,7 @@ private static class AmazonImageDetails extends HashMap<String, Object> implemen
put("imageName", imageName);
put("imageId", imageId);

put("ami", imageName);
put("ami", imageId);
put("amiId", imageId);

put("region", region);
Expand Down
2 changes: 1 addition & 1 deletion orca-igor/src/main/groovy/com/netflix/spinnaker/orca/igor/tasks/GetCommitsTask.groovy
View file
Expand Up @@ -150,7 +150,7 @@ class GetCommitsTask implements DiffTask {
clusterPair?.canary?.availabilityZones?.findResult { key, value -> key == region }
}?.canary?.amiName
} else if (context.deploymentDetails) { // deploy asg stage
return context.deploymentDetails.find { it.region == region }?.amiId
return context.deploymentDetails.find { it.region == region }?.imageId
} else if (context.amiName) { // copyLastAsg stage
return context.amiName
} else {
Expand Down
35 changes: 26 additions & 9 deletions orca-igor/src/test/groovy/com/netflix/spinnaker/orca/igor/tasks/GetCommitsTaskSpec.groovy
View file
Expand Up @@ -90,7 +90,7 @@ class GetCommitsTaskSpec extends Specification {
ObjectMapper mapper = new ObjectMapper()
def katoMap = mapper.readValue(katoTasks, List)
def contextMap = [application: app, account: account,
source : [asgName: serverGroup, region: region, account: account], "deploy.server.groups": ["us-west-1": [targetServerGroup]], deploymentDetails: [[amiId: "ami-foo", ami: "amiFooName", region: "us-east-1"], [amiId: targetImage, ami: targetImageName, region: region]], "kato.tasks" : katoMap]
source : [asgName: serverGroup, region: region, account: account], "deploy.server.groups": ["us-west-1": [targetServerGroup]], deploymentDetails: [[imageId: "ami-foo", ami: "amiFooName", region: "us-east-1"], [imageId: targetImage, ami: targetImageName, region: region]], "kato.tasks" : katoMap]
def stage = setupGetCommits(contextMap, account, app, sourceImage, targetImage, region, cluster, serverGroup)

when:
Expand Down Expand Up @@ -123,7 +123,7 @@ class GetCommitsTaskSpec extends Specification {
ObjectMapper mapper = new ObjectMapper()
def katoMap = mapper.readValue(katoTasks, List)
def contextMap = [application: app, account: account,
source : [asgName: serverGroup, region: region, account: account], amiId: targetImage, amiName : targetImageName, "kato.tasks" : katoMap]
source : [asgName: serverGroup, region: region, account: account], imageId: targetImage, amiName : targetImageName, "kato.tasks" : katoMap]
def stage = setupGetCommits(contextMap, account, app, sourceImage, targetImageName, region, cluster, serverGroup)

when:
Expand All @@ -150,7 +150,7 @@ class GetCommitsTaskSpec extends Specification {
def contextMap = [application: app, account: account,
source : [asgName: serverGroup, region: region, account: account],
clusterPairs :
[[ baseline : [amiName: sourceImage, availabilityZones: [(region) : ["${region}-1c"]]], canary : [amiId: targetImage, amiName: targetImageName, availabilityZones: [(region) : ["${region}-1c"]]]]]
[[ baseline : [amiName: sourceImage, availabilityZones: [(region) : ["${region}-1c"]]], canary : [imageId: targetImage, amiName: targetImageName, availabilityZones: [(region) : ["${region}-1c"]]]]]
]
def stage = setupGetCommits(contextMap, account, app, sourceImage, targetImageName, region, cluster, serverGroup, 0)

Expand Down Expand Up @@ -254,7 +254,7 @@ class GetCommitsTaskSpec extends Specification {
ObjectMapper mapper = new ObjectMapper()
def katoMap = mapper.readValue(katoTasks, List)
def stage = new Stage<>(pipeline, "stash", [application: app, account: account,
source : [asgName: serverGroup, region: region, account: account], "deploy.server.groups": ["us-west-1": [targetServerGroup]], deploymentDetails: [[ami: "ami-foo", region: "us-east-1"], [amiId: targetImage, ami: targetImageName, region: region]], "kato.tasks": katoMap])
source : [asgName: serverGroup, region: region, account: account], "deploy.server.groups": ["us-west-1": [targetServerGroup]], deploymentDetails: [[ami: "ami-foo", region: "us-east-1"], [imageId: targetImage, ami: targetImageName, region: region]], "kato.tasks": katoMap])

and:
task.front50Service = front50Service
Expand Down Expand Up @@ -302,7 +302,7 @@ class GetCommitsTaskSpec extends Specification {
@Unroll
def "returns success if there are no repo details provided"() {
given:
def stage = new Stage<>(pipeline, "stash", [application: app, account: account, source: [asgName: serverGroup, region: region, account: account], "deploy.server.groups": ["us-west-1": [targetServerGroup]], deploymentDetails: [[ami: "ami-foo", region: "us-east-1"], [amiId: targetImage, ami: targetImageName, region: region]]])
def stage = new Stage<>(pipeline, "stash", [application: app, account: account, source: [asgName: serverGroup, region: region, account: account], "deploy.server.groups": ["us-west-1": [targetServerGroup]], deploymentDetails: [[ami: "ami-foo", region: "us-east-1"], [imageId: targetImage, ami: targetImageName, region: region]]])

and:
task.buildService = buildService
Expand Down Expand Up @@ -334,7 +334,7 @@ class GetCommitsTaskSpec extends Specification {
ObjectMapper mapper = new ObjectMapper()
def katoMap = mapper.readValue(katoTasks, List)
def stage = new Stage<>(pipeline, "stash", [application: app, account: account,
source : [asgName: serverGroup, region: region, account: account], "deploy.server.groups": ["us-west-1": [targetServerGroup]], deploymentDetails: [[ami: "ami-foo", region: "us-east-1"], [ami: targetImageName, amiId: targetImage, region: region]], "kato.tasks": katoMap])
source : [asgName: serverGroup, region: region, account: account], "deploy.server.groups": ["us-west-1": [targetServerGroup]], deploymentDetails: [[ami: "ami-foo", region: "us-east-1"], [ami: targetImageName, imageId: targetImage, region: region]], "kato.tasks": katoMap])

and:
task.buildService = Stub(BuildService) {
Expand Down Expand Up @@ -385,7 +385,7 @@ class GetCommitsTaskSpec extends Specification {
ObjectMapper mapper = new ObjectMapper()
def katoMap = mapper.readValue(katoTasks, List)
def stage = new Stage<>(pipeline, "stash", [application: app, account: account,
source : [asgName: serverGroup, region: region, account: account], "deploy.server.groups": ["us-west-1": [targetServerGroup]], deploymentDetails: [[ami: "ami-foo", region: "us-east-1"], [ami: targetImageName, amiId: targetImage, region: region]], "kato.tasks": katoMap])
source : [asgName: serverGroup, region: region, account: account], "deploy.server.groups": ["us-west-1": [targetServerGroup]], deploymentDetails: [[ami: "ami-foo", region: "us-east-1"], [ami: targetImageName, imageId: targetImage, region: region]], "kato.tasks": katoMap])

and:
task.buildService = Stub(BuildService) {
Expand Down Expand Up @@ -454,7 +454,7 @@ class GetCommitsTaskSpec extends Specification {
ObjectMapper mapper = new ObjectMapper()
def katoMap = mapper.readValue(katoTasks, List)
def stage = new Stage<>(pipeline, "stash", [application: app, account: account,
source : [asgName: serverGroup, region: region, account: account], "deploy.server.groups": ["us-west-1": [targetServerGroup]], deploymentDetails: [[ami: "ami-foo", region: "us-east-1"], [ami: targetImageName, amiId: targetImage, region: region]], "kato.tasks": katoMap])
source : [asgName: serverGroup, region: region, account: account], "deploy.server.groups": ["us-west-1": [targetServerGroup]], deploymentDetails: [[ami: "ami-foo", region: "us-east-1"], [ami: targetImageName, imageId: targetImage, region: region]], "kato.tasks": katoMap])

and:
task.buildService = Stub(BuildService) {
Expand Down Expand Up @@ -517,7 +517,7 @@ class GetCommitsTaskSpec extends Specification {
ObjectMapper mapper = new ObjectMapper()
def katoMap = mapper.readValue(katoTasks, List)
def contextMap = [application: app, account: account,
source : [asgName: serverGroup, region: region, account: account], "deploy.server.groups": ["us-west-1": [targetServerGroup]], deploymentDetails: [[ami: "ami-foo", region: "us-east-1"], [ami: targetImageName, amiId: targetImage ,region: region]], "kato.tasks" : katoMap]
source : [asgName: serverGroup, region: region, account: account], "deploy.server.groups": ["us-west-1": [targetServerGroup]], deploymentDetails: [[ami: "ami-foo", region: "us-east-1"], [ami: targetImageName, imageId: targetImage ,region: region]], "kato.tasks" : katoMap]
def stage = setupGetCommits(contextMap, account, app, sourceImage, targetImage, region, cluster, serverGroup, 0, 0)

when:
Expand All @@ -540,4 +540,21 @@ class GetCommitsTaskSpec extends Specification {
"myapp" | null | "myapp-v000"
}

@Unroll
def "return proper value for target ami for a deploy stage"() {

given:
def context = [deploymentDetails: [[imageId: imageId, region: contextRegion]]]

when:
def result = task.getTargetAmi(context, region)

then:
result == expectedAncestorAmi

where:
imageId | contextRegion | region | expectedAncestorAmi
null | "us-west-1" | "us-west-1" | null
"ami-123" | "us-west-1" | "us-west-1" | "ami-123"
}
}

0 comments on commit b4a150f

Please sign in to comment.