Skip to content

Commit

Permalink
feat(entitytags): Include previous server group image details (#1705)
Browse files Browse the repository at this point in the history 
This will ultimately facilitate an orchestrated rollback even if the
previous server group no longer exists.

It relies on the entity tags feature being enabled (dependency on
elastic search and not enabled by default in `clouddriver` / `orca`).

This PR also introduces some common retry handling (see `RetrySupport`).
  • Loading branch information
@ajordens
ajordens authored Oct 18, 2017
1 parent e01058b commit c29ec72
Show file tree
Hide file tree
Showing 9 changed files with 418 additions and 169 deletions.
45 changes: 17 additions & 28 deletions .../spinnaker/orca/clouddriver/pipeline/servergroup/support/TargetServerGroupResolver.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package com.netflix.spinnaker.orca.clouddriver.pipeline.servergroup.support

import com.netflix.spinnaker.orca.RetrySupport
import groovy.util.logging.Slf4j
import com.fasterxml.jackson.databind.ObjectMapper
import com.netflix.spinnaker.orca.clouddriver.OortService
Expand All @@ -37,6 +38,9 @@ class TargetServerGroupResolver {
@Autowired
ObjectMapper mapper

@Autowired
RetrySupport retrySupport

List<TargetServerGroup> resolve(Stage stage) {
resolveByParams(TargetServerGroup.Params.fromStage(stage))
}
Expand Down Expand Up @@ -159,38 +163,23 @@ class TargetServerGroupResolver {
}

private <T> T fetchWithRetries(Class<T> responseType, int maxRetries, long retryBackoff, Closure<Response> fetchClosure) {
def converter = new JacksonConverter(mapper)
return retrySupport.retry({
def converter = new JacksonConverter(mapper)

def lastException = null
for (int i = 1; i <= maxRetries; i++) {
Response response
try {
Response response
try {
response = fetchClosure.call()
} catch (RetrofitError re) {
if (re.kind == RetrofitError.Kind.HTTP && re.response.status == 404) {
return null
}
throw re
}
try {
return (T) converter.fromBody(response.body, responseType)
} catch (ConversionException ce) {
throw RetrofitError.conversionError(response.url, response, converter, responseType, ce)
}
response = fetchClosure.call()
} catch (RetrofitError re) {
lastException = re

if (re.kind == RetrofitError.Kind.NETWORK) {
Thread.sleep(retryBackoff)
} else if (re.kind == RetrofitError.Kind.HTTP && re.response.status == 429) {
Thread.sleep(retryBackoff)
} else {
throw re
if (re.kind == RetrofitError.Kind.HTTP && re.response.status == 404) {
return null
}
throw re
}
}

throw lastException
try {
return (T) converter.fromBody(response.body, responseType)
} catch (ConversionException ce) {
throw RetrofitError.conversionError(response.url, response, converter, responseType, ce)
}
}, maxRetries, retryBackoff, false)
}
}
71 changes: 70 additions & 1 deletion ...pinnaker/orca/clouddriver/tasks/servergroup/SpinnakerMetadataServerGroupTagGenerator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,17 @@

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

import com.netflix.frigga.Names;
import com.netflix.spinnaker.orca.RetrySupport;
import com.netflix.spinnaker.orca.clouddriver.OortService;
import com.netflix.spinnaker.orca.pipeline.model.Execution;
import com.netflix.spinnaker.orca.pipeline.model.Orchestration;
import com.netflix.spinnaker.orca.pipeline.model.Pipeline;
import com.netflix.spinnaker.orca.pipeline.model.Stage;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.stereotype.Component;
import retrofit.RetrofitError;

import java.util.Collection;
import java.util.Collections;
Expand All @@ -29,13 +35,21 @@

@Component
public class SpinnakerMetadataServerGroupTagGenerator implements ServerGroupEntityTagGenerator {
private final Logger log = LoggerFactory.getLogger(this.getClass());
private final OortService oortService;
private final RetrySupport retrySupport;

public SpinnakerMetadataServerGroupTagGenerator(OortService oortService, RetrySupport retrySupport) {
this.oortService = oortService;
this.retrySupport = retrySupport;
}

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

Map<String, String> value = new HashMap<>();
Map<String, Object> value = new HashMap<>();
value.put("stageId", stage.getId());
value.put("executionId", execution.getId());
value.put("executionType", execution.getClass().getSimpleName().toLowerCase());
Expand All @@ -59,11 +73,66 @@ public Collection<Map<String, Object>> generateTags(Stage stage, String serverGr
value.put("comments", (String) context.get("comments"));
}

String cluster = null;
try {
cluster = Names.parseName(serverGroup).getCluster();

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

if (previousServerGroup != null) {
value.put("previousServerGroup", previousServerGroup);
}
} catch (Exception e) {
// failure to populate `previousServerGroup` is not considered a fatal error that would cause this task to fail
log.error("Unable to determine ancestor image details for {}:{}:{}:{}", cloudProvider, account, location, cluster, e);
}

Map<String, Object> tag = new HashMap<>();
tag.put("name", "spinnaker:metadata");
tag.put("value", value);

return Collections.singletonList(tag);
}

Map<String, Object> getPreviousServerGroup(String application,
String account,
String cluster,
String cloudProvider,
String location) {
return retrySupport.retry(() -> {
try {
Map<String, Object> targetServerGroup = oortService.getServerGroupSummary(
application,
account,
cluster,
cloudProvider,
location,
"ANCESTOR",
"image",
"true"
);

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;
}

throw e;
}
}, 12, 5000, false); // retry for up to one minute
}
}
19 changes: 14 additions & 5 deletions ...nnaker/orca/clouddriver/pipeline/servergroup/support/TargetServerGroupResolverSpec.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package com.netflix.spinnaker.orca.clouddriver.pipeline.servergroup.support

import com.fasterxml.jackson.databind.ObjectMapper
import com.netflix.spinnaker.orca.RetrySupport
import com.netflix.spinnaker.orca.clouddriver.OortService
import com.netflix.spinnaker.orca.pipeline.model.Stage
import retrofit.RetrofitError
Expand All @@ -32,8 +33,16 @@ class TargetServerGroupResolverSpec extends Specification {

OortService oort = Mock(OortService)
ObjectMapper mapper = new ObjectMapper()
RetrySupport retrySupport = Spy(RetrySupport) {
_ * sleep(_) >> { /* do nothing */ }
}

@Subject
TargetServerGroupResolver subject = new TargetServerGroupResolver(oortService: oort, mapper: mapper)
TargetServerGroupResolver subject = new TargetServerGroupResolver(
oortService: oort,
mapper: mapper,
retrySupport: retrySupport
)

def "should resolve to target server groups"() {
when:
Expand Down Expand Up @@ -131,7 +140,7 @@ class TargetServerGroupResolverSpec extends Specification {
}

@Unroll
def "should retry on network + 429 errors"() {
def "should retry on non 404 errors"() {
given:
def invocationCount = 0
def capturedResult
Expand All @@ -152,9 +161,9 @@ class TargetServerGroupResolverSpec extends Specification {

where:
exception || expectNull || expectedInvocationCount
new IllegalStateException("should not retry") || false || 1
retrofitError(RetrofitError.Kind.UNEXPECTED, 400) || false || 1
retrofitError(RetrofitError.Kind.HTTP, 500) || false || 1
new IllegalStateException("should retry") || false || 10
retrofitError(RetrofitError.Kind.UNEXPECTED, 400) || false || 10
retrofitError(RetrofitError.Kind.HTTP, 500) || false || 10
retrofitError(RetrofitError.Kind.HTTP, 404) || true || 1 // a 404 should short-circuit and return null
retrofitError(RetrofitError.Kind.NETWORK, 0) || false || 10
retrofitError(RetrofitError.Kind.HTTP, 429) || false || 10
Expand Down
Loading

0 comments on commit c29ec72

Please sign in to comment.