Skip to content

Commit

Permalink
fix(titus): Tag titus server groups with previous image metadata (#1758)
Browse files Browse the repository at this point in the history
  • Loading branch information
@ajordens
ajordens authored Oct 31, 2017
1 parent 5aef52c commit 20b2847
Show file tree
Hide file tree
Showing 2 changed files with 132 additions and 43 deletions.
107 changes: 70 additions & 37 deletions ...pinnaker/orca/clouddriver/tasks/servergroup/SpinnakerMetadataServerGroupTagGenerator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,11 @@ public SpinnakerMetadataServerGroupTagGenerator(OortService oortService, RetrySu
}

@Override
public Collection<Map<String, Object>> generateTags(Stage stage, String serverGroup, String account, String location, String cloudProvider) {
public Collection<Map<String, Object>> generateTags(Stage stage,
String serverGroup,
String account,
String location,
String cloudProvider) {
Execution execution = stage.getExecution();
Map context = stage.getContext();

Expand Down Expand Up @@ -77,12 +81,13 @@ public Collection<Map<String, Object>> generateTags(Stage stage, String serverGr
try {
cluster = Names.parseName(serverGroup).getCluster();

Map<String, Object> previousServerGroup = getPreviousServerGroup(
Map<String, Object> previousServerGroup = getPreviousServerGroupFromCluster(
execution.getApplication(),
account,
cluster,
cloudProvider,
location
location,
serverGroup
);

if (previousServerGroup != null) {
Expand All @@ -100,48 +105,76 @@ public Collection<Map<String, Object>> generateTags(Stage stage, String serverGr
return Collections.singletonList(tag);
}

Map<String, Object> getPreviousServerGroup(String application,
String account,
String cluster,
String cloudProvider,
String location) {
Map<String, Object> getPreviousServerGroupFromCluster(String application,
String account,
String cluster,
String cloudProvider,
String location,
String createdServerGroup) {
if (cloudProvider.equals("titus")) {
// TODO-AJ titus does not force cache refresh so `ANCESTOR` will return inconsistent results
return null;
// titus does not (currently!) force cache refresh to it's possible that `NEWEST` is actually the `ANCESTOR` to
// the just created server group
Map<String, Object> newestServerGroup = retrySupport.retry(() -> {
return getPreviousServerGroupFromClusterByTarget(
application, account, cluster, cloudProvider, location, "NEWEST"
);
}, 12, 5000, false); // retry for up to one minute

if (newestServerGroup == null) {
// cluster has no enabled server groups
return null;
}

if (!newestServerGroup.get("name").equals(createdServerGroup)) {
// if the `NEWEST` server group is _NOT_ what was just created, we've found our `ANCESTOR`
// if not, fall through to an explicit `ANCESTOR` lookup
return newestServerGroup;
}
}

return retrySupport.retry(() -> {
try {
Map<String, Object> targetServerGroup = oortService.getServerGroupSummary(
application,
account,
cluster,
cloudProvider,
location,
"ANCESTOR",
"image",
"true"
);
return getPreviousServerGroupFromClusterByTarget(
application, account, cluster, cloudProvider, location, "ANCESTOR"
);
}, 12, 5000, false); // retry for up to one minute
}

Map<String, Object> previousServerGroup = new HashMap<>();
previousServerGroup.put("name", targetServerGroup.get("serverGroupName"));
previousServerGroup.put("imageName", targetServerGroup.get("imageName"));
previousServerGroup.put("imageId", targetServerGroup.get("imageId"));
previousServerGroup.put("cloudProvider", cloudProvider);
Map<String, Object> getPreviousServerGroupFromClusterByTarget(String application,
String account,
String cluster,
String cloudProvider,
String location,
String target) {
try {
Map<String, Object> targetServerGroup = oortService.getServerGroupSummary(
application,
account,
cluster,
cloudProvider,
location,
target,
"image",
"true"
);

if (targetServerGroup.containsKey("buildInfo")) {
previousServerGroup.put("buildInfo", targetServerGroup.get("buildInfo"));
}
Map<String, Object> previousServerGroup = new HashMap<>();
previousServerGroup.put("name", targetServerGroup.get("serverGroupName"));
previousServerGroup.put("imageName", targetServerGroup.get("imageName"));
previousServerGroup.put("imageId", targetServerGroup.get("imageId"));
previousServerGroup.put("cloudProvider", cloudProvider);

return previousServerGroup;
} catch (RetrofitError e) {
if (e.getKind() == RetrofitError.Kind.HTTP && e.getResponse().getStatus() == 404) {
// it's ok if the previous server group does not exist
return null;
}
if (targetServerGroup.containsKey("buildInfo")) {
previousServerGroup.put("buildInfo", targetServerGroup.get("buildInfo"));
}

throw e;
return previousServerGroup;
} catch (RetrofitError e) {
if (e.getKind() == RetrofitError.Kind.HTTP && e.getResponse().getStatus() == 404) {
// it's ok if the previous server group does not exist
return null;
}
}, 12, 5000, false); // retry for up to one minute

throw e;
}
}
}
68 changes: 62 additions & 6 deletions ...er/orca/clouddriver/tasks/servergroup/SpinnakerMetadataServerGroupTagGeneratorSpec.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ class SpinnakerMetadataServerGroupTagGeneratorSpec extends Specification {
def "should build spinnaker:metadata tag for pipeline"() {
given:
def tagGenerator = Spy(SpinnakerMetadataServerGroupTagGenerator, constructorArgs: [oortService, retrySupport]) {
1 * getPreviousServerGroup(_, _, _, _, _) >> { return previousServerGroup }
1 * getPreviousServerGroupFromClusterByTarget(_, _, _, _, _, "ANCESTOR") >> { return previousServerGroup }
}

def pipeline = pipeline {
Expand Down Expand Up @@ -93,7 +93,7 @@ class SpinnakerMetadataServerGroupTagGeneratorSpec extends Specification {
def "should build spinnaker:metadata tag for orchestration"() {
given:
def tagGenerator = Spy(SpinnakerMetadataServerGroupTagGenerator, constructorArgs: [oortService, retrySupport]) {
1 * getPreviousServerGroup(_, _, _, _, _) >> { return previousServerGroup }
1 * getPreviousServerGroupFromClusterByTarget(_, _, _, _, _, "ANCESTOR") >> { return previousServerGroup }
}

def orchestration = orchestration {
Expand Down Expand Up @@ -133,8 +133,8 @@ class SpinnakerMetadataServerGroupTagGeneratorSpec extends Specification {
def tagGenerator = new SpinnakerMetadataServerGroupTagGenerator(oortService, retrySupport)

when: "previous server does exist"
def previousServerGroupMetadata = tagGenerator.getPreviousServerGroup(
"application", "account", "cluster", "aws", "us-west-2"
def previousServerGroupMetadata = tagGenerator.getPreviousServerGroupFromCluster(
"application", "account", "cluster", "aws", "us-west-2", "application-v002"
)

then: "metadata should be returned"
Expand All @@ -145,6 +145,7 @@ class SpinnakerMetadataServerGroupTagGeneratorSpec extends Specification {
imageName : "my_image"
]
}
0 * oortService._
previousServerGroupMetadata == [
name : "application-v001",
imageId : "ami-1234567",
Expand All @@ -153,14 +154,69 @@ class SpinnakerMetadataServerGroupTagGeneratorSpec extends Specification {
]

when: "previous server group does NOT exist"
previousServerGroupMetadata = tagGenerator.getPreviousServerGroup(
"application", "account", "cluster", "aws", "us-west-2"
previousServerGroupMetadata = tagGenerator.getPreviousServerGroupFromCluster(
"application", "account", "cluster", "aws", "us-west-2", "application-v002"
)

then: "no metadata should be returned"
1 * oortService.getServerGroupSummary("application", "account", "cluster", "aws", "us-west-2", "ANCESTOR", "image", "true") >> {
throw notFoundError
}
0 * oortService._
previousServerGroupMetadata == null
}

def "should check NEWEST and ANCESTOR when constructing previous server group metadata for titus"() {
given:
def tagGenerator = new SpinnakerMetadataServerGroupTagGenerator(oortService, retrySupport)

when: "NEWEST != just created server group"
def previousServerGroupMetadata = tagGenerator.getPreviousServerGroupFromCluster(
"application", "account", "cluster", "titus", "us-west-2", "application-v002"
)

then: "previous server group == NEWEST"
1 * oortService.getServerGroupSummary("application", "account", "cluster", "titus", "us-west-2", "NEWEST", "image", "true") >> {
return [
serverGroupName: "application-v001",
imageId : "1234567",
imageName : "my_image"
]
}
0 * oortService._
previousServerGroupMetadata == [
name : "application-v001",
imageId : "1234567",
imageName : "my_image",
cloudProvider: "titus"
]

when: "NEWEST == just created server group"
previousServerGroupMetadata = tagGenerator.getPreviousServerGroupFromCluster(
"application", "account", "cluster", "titus", "us-west-2", "application-v002"
)

then: "previous server group == ANCESTOR"
1 * oortService.getServerGroupSummary("application", "account", "cluster", "titus", "us-west-2", "NEWEST", "image", "true") >> {
return [
serverGroupName: "application-v002",
imageId : "1234567",
imageName : "my_image"
]
}
1 * oortService.getServerGroupSummary("application", "account", "cluster", "titus", "us-west-2", "ANCESTOR", "image", "true") >> {
return [
serverGroupName: "application-v001",
imageId : "1234567",
imageName : "my_image"
]
}
0 * oortService._
previousServerGroupMetadata == [
name : "application-v001",
imageId : "1234567",
imageName : "my_image",
cloudProvider: "titus"
]
}
}

0 comments on commit 20b2847

Please sign in to comment.