Skip to content

Commit

Permalink
fix(RRB): disable traffic on the source server group only (#3037)
Browse files Browse the repository at this point in the history
Make sure RRB disable step disabled the same number of instances as it brought online,
and only in the source server group.

Imagine there are multiple active server groups for RRB:

- v001 (capacity: 6, active)
- v002 (capacity: 10, active)

we are deploying v003, with the following steps: 50%, 100%

Currently, RRB will do the following:
1. Spin up 5 instances in v003 (50% of the source which is v002)
2. Disable 3 instances in v001 and 5 in v002 for a total of 8!
3. Spin up another 5 in v003 (100%)
4. Disable 3 and 5 respectively

Effectively we went from capacity 16 to 10 and are now under provisioned.
This is an edge case, but we want it to have predictable behavior that doesn't leave the cluster under provisioned
  • Loading branch information
marchello2000 authored Jul 17, 2019
1 parent 51347a8 commit 75bcafe
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@
*/
package com.netflix.spinnaker.orca.clouddriver.pipeline.servergroup.strategies

import com.netflix.spinnaker.orca.clouddriver.pipeline.cluster.DisableClusterStage
import com.netflix.spinnaker.orca.clouddriver.pipeline.cluster.ScaleDownClusterStage
import com.netflix.spinnaker.orca.clouddriver.pipeline.servergroup.CreateServerGroupStage
import com.netflix.spinnaker.orca.clouddriver.pipeline.servergroup.DisableServerGroupStage
import com.netflix.spinnaker.orca.clouddriver.pipeline.servergroup.ResizeServerGroupStage
import com.netflix.spinnaker.orca.clouddriver.pipeline.servergroup.support.DetermineTargetServerGroupStage
import com.netflix.spinnaker.orca.clouddriver.pipeline.servergroup.support.TargetServerGroup
Expand All @@ -40,7 +40,7 @@ class RollingRedBlackStrategy implements Strategy, ApplicationContextAware {
final String name = "rollingredblack"

@Autowired
DisableClusterStage disableClusterStage
DisableServerGroupStage disableServerGroupStage

@Autowired
ResizeServerGroupStage resizeServerGroupStage
Expand Down Expand Up @@ -185,17 +185,16 @@ class RollingRedBlackStrategy implements Strategy, ApplicationContextAware {
// only generate the "disable p% of traffic" stages if we have something to disable
if (source) {
def disableContext = baseContext + [
desiredPercentage : p,
remainingEnabledServerGroups: 1,
preferLargerOverNewer : false
desiredPercentage : p,
serverGroupName : source.serverGroupName
]

log.info("Adding `Disable $p% of Desired Size` stage with context $disableContext [executionId=${stage.execution.id}]")

stages << newStage(
stage.execution,
disableClusterStage.type,
"Disable $p% of Traffic",
disableServerGroupStage.type,
"Disable $p% of Traffic on ${source.serverGroupName}",
disableContext,
stage,
SyntheticStageOwner.STAGE_AFTER
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ import com.netflix.spinnaker.orca.clouddriver.utils.OortHelper
import com.netflix.spinnaker.orca.clouddriver.utils.TrafficGuard
import com.netflix.spinnaker.orca.kato.pipeline.CopyLastAsgStage
import com.netflix.spinnaker.orca.kato.pipeline.DeployStage
import com.netflix.spinnaker.orca.locks.LockingConfigurationProperties
import com.netflix.spinnaker.orca.pipeline.model.Stage
import groovy.util.logging.Slf4j
import org.springframework.beans.factory.annotation.Autowired
Expand Down Expand Up @@ -82,7 +81,7 @@ abstract class AbstractClusterWideClouddriverTask extends AbstractCloudProviderA
clusterSelection.cloudProvider)
if (!cluster.isPresent()) {
if (stage.context.continueIfClusterNotFound) {
return TaskResult.SUCCEEDED;
return TaskResult.SUCCEEDED
}
return missingClusterResult(stage, clusterSelection)
}
Expand All @@ -92,7 +91,7 @@ abstract class AbstractClusterWideClouddriverTask extends AbstractCloudProviderA

if (!serverGroups) {
if (stage.context.continueIfClusterNotFound) {
return TaskResult.SUCCEEDED;
return TaskResult.SUCCEEDED
}
return emptyClusterResult(stage, clusterSelection, cluster.get())
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ package com.netflix.spinnaker.orca.clouddriver.pipeline.servergroup.strategies
import com.netflix.spinnaker.kork.dynamicconfig.DynamicConfigService
import com.netflix.spinnaker.kork.dynamicconfig.SpringDynamicConfigService
import com.netflix.spinnaker.moniker.Moniker
import com.netflix.spinnaker.orca.clouddriver.pipeline.cluster.DisableClusterStage
import com.netflix.spinnaker.orca.clouddriver.pipeline.cluster.ScaleDownClusterStage
import com.netflix.spinnaker.orca.clouddriver.pipeline.servergroup.DisableServerGroupStage
import com.netflix.spinnaker.orca.clouddriver.pipeline.servergroup.ResizeServerGroupStage
import com.netflix.spinnaker.orca.clouddriver.pipeline.servergroup.support.DetermineTargetServerGroupStage
import com.netflix.spinnaker.orca.clouddriver.utils.TrafficGuard
Expand All @@ -24,7 +24,7 @@ class RollingRedBlackStrategySpec extends Specification {
def config = new LockingConfigurationProperties(new SpringDynamicConfigService(environment: env))
def dynamicConfigService = Mock(DynamicConfigService)
def trafficGuard = Mock(TrafficGuard)
def disableClusterStage = new DisableClusterStage(trafficGuard, config, dynamicConfigService)
def disableServerGroupStage = new DisableServerGroupStage(dynamicConfigService)
def scaleDownClusterStage = new ScaleDownClusterStage(trafficGuard, config, dynamicConfigService)
def resizeServerGroupStage = new ResizeServerGroupStage()
def waitStage = new WaitStage()
Expand Down Expand Up @@ -86,7 +86,7 @@ class RollingRedBlackStrategySpec extends Specification {

def strat = new RollingRedBlackStrategy(
scaleDownClusterStage: scaleDownClusterStage,
disableClusterStage: disableClusterStage,
disableServerGroupStage: disableServerGroupStage,
resizeServerGroupStage: resizeServerGroupStage,
waitStage: waitStage,
pipelineStage: pipelineStage,
Expand Down

0 comments on commit 75bcafe

Please sign in to comment.