Skip to content

Commit

Permalink
fix(disable): skip disabled check for partially disabled server groups (
Browse files Browse the repository at this point in the history
#4000)

* fix(disable): skip disabled check for partially disabled server groups

Rolling red black and monitored deployments create disable server group stages with a desiredPercentage, which only disables some instances and not the whole server group. In that case, we need to skip the check that the server group is disabled.

* fixup! fix(disable): skip disabled check for partially disabled server groups

* fixup! fixup! fix(disable): skip disabled check for partially disabled server groups

* fixup! fixup! fixup! fix(disable): skip disabled check for partially disabled server groups

* fixup! fixup! fixup! fixup! fix(disable): skip disabled check for partially disabled server groups
  • Loading branch information
dreynaud committed Nov 17, 2020
1 parent 2bd25aa commit adc318f
Show file tree
Hide file tree
Showing 2 changed files with 86 additions and 1 deletion.
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package com.netflix.spinnaker.orca.clouddriver.tasks.servergroup;

import com.fasterxml.jackson.core.type.TypeReference;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.netflix.spinnaker.orca.api.pipeline.RetryableTask;
import com.netflix.spinnaker.orca.api.pipeline.TaskResult;
Expand All @@ -13,6 +14,7 @@
import java.io.IOException;
import java.util.Collections;
import java.util.Map;
import javax.annotation.Nullable;
import lombok.extern.slf4j.Slf4j;
import lombok.val;
import org.jetbrains.annotations.NotNull;
Expand Down Expand Up @@ -46,6 +48,19 @@ public long getTimeout() {
@NotNull
@Override
public TaskResult execute(@NotNull StageExecution stage) {
try {
TaskInput input = stage.mapTo(TaskInput.class);
input.validate();
if (!isPartialDisable(input)) {
return TaskResult.builder(ExecutionStatus.SKIPPED).build();
}
} catch (IllegalArgumentException e) {
log.warn("Error mapping task input", e);
return TaskResult.builder(ExecutionStatus.SKIPPED).build();
}

// we have established that this is a full disable, so we need to enforce that the server group
// is actually disabled
val serverGroupDescriptor = getServerGroupDescriptor(stage);
try {
var serverGroup = fetchServerGroup(serverGroupDescriptor);
Expand All @@ -64,6 +79,13 @@ public TaskResult execute(@NotNull StageExecution stage) {
}
}

// RRB and Monitored Deployments do "partial disables", i.e. they run DisableServerGroupTask with
// a `desiredPercentage` which will only disable some instances, not the entire server group (so
// this won't set the `disabled` flag on the server group)
private boolean isPartialDisable(TaskInput input) {
return input.desiredPercentage != null && input.desiredPercentage < 100;
}

private TargetServerGroup fetchServerGroup(ServerGroupDescriptor serverGroupDescriptor)
throws IOException {
val response =
Expand All @@ -72,7 +94,19 @@ private TargetServerGroup fetchServerGroup(ServerGroupDescriptor serverGroupDesc
serverGroupDescriptor.getRegion(),
serverGroupDescriptor.getName());
var serverGroupData =
(Map<String, Object>) objectMapper.readValue(response.getBody().in(), Map.class);
objectMapper.readValue(
response.getBody().in(), new TypeReference<Map<String, Object>>() {});
return new TargetServerGroup(serverGroupData);
}

private static class TaskInput {
@Nullable public Integer desiredPercentage;

void validate() {
if (desiredPercentage != null && (desiredPercentage < 0 || desiredPercentage > 100)) {
throw new IllegalArgumentException(
"desiredPercentage is expected to be in [0, 100] but found " + desiredPercentage);
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
package com.netflix.spinnaker.orca.clouddriver.tasks.servergroup

import com.netflix.spinnaker.orca.api.pipeline.TaskResult
import com.netflix.spinnaker.orca.api.pipeline.models.PipelineExecution
import com.netflix.spinnaker.orca.api.pipeline.models.StageExecution
import com.netflix.spinnaker.orca.pipeline.StageExecutionFactory
import com.netflix.spinnaker.orca.pipeline.model.PipelineExecutionImpl
import com.netflix.spinnaker.orca.pipeline.model.StageExecutionImpl
import spock.lang.Specification
import spock.lang.Subject
import spock.lang.Unroll

import static com.netflix.spinnaker.orca.api.pipeline.models.ExecutionStatus.*

class WaitForDisabledServerGroupTaskSpec extends Specification {
@Subject
WaitForDisabledServerGroupTask task = new WaitForDisabledServerGroupTask(null, null)

@Unroll
def "handles wonky desiredPercentage=#desiredPercentage gracefully"() {
given:
StageExecution stage = StageExecutionFactory.newStage(
PipelineExecutionImpl.newOrchestration("orca"),
"stageType", "stageName",
[desiredPercentage: desiredPercentage],
null, null)

when:
def taskInput = stage.mapTo(WaitForDisabledServerGroupTask.TaskInput)

then:
taskInput.desiredPercentage == mapsTo

when:
def taskResult = task.execute(stage)

then:
stage.context.desiredPercentage == desiredPercentage
taskResult.status == expected

where:
desiredPercentage | mapsTo || expected
null | null || SKIPPED
-1 | -1 || SKIPPED
"" | null || SKIPPED
new Integer(100) | 100 || SKIPPED
"200" | 200 || SKIPPED
102.5 | 102 || SKIPPED
// 1L << 33, Jackson throws "Numeric value (8589934592) out of range of int"
}
}

0 comments on commit adc318f

Please sign in to comment.