fix(RRB): disable traffic on the source server group only #3037
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dreynaud
approved these changes
Jul 17, 2019
| @Slf4j | ||
| @Component | ||
| class RollingRedBlackStrategy implements Strategy, ApplicationContextAware { | ||
| final String name = "rollingredblack" | ||
|
|
||
| @Autowired | ||
| DisableClusterStage disableClusterStage | ||
| DisableServerGroupStage disableServerGroupStage |
There was a problem hiding this comment.
very elegant change, my mouth is agape in amazement
There was a problem hiding this comment.
i thought it was clever :)
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
marchello2000
force-pushed
the
mark/spin-4593
branch
from
9c8ce49
to
c9449c9
Compare
marchello2000
added a commit
to marchello2000/orca
that referenced
this pull request
Jul 22, 2019
As a result of spinnaker#3037, the RRB flow now no longer bypasses traffic guards (it used to do that before, here: https://github.com/spinnaker/orca/blob/master/orca-clouddriver/src/main/groovy/com/netflix/spinnaker/orca/clouddriver/tasks/cluster/AbstractClusterWideClouddriverTask.groovy#L145 `shouldSkipTrafficGuards`.) Instead, now when we run the first disable step in RRB (e.g. disable 5% of source), the traffic guards think the whole ASG is going away as it is unaware of the percentage disable flag and thus it terminates the operation since "disabling entire ASG while new is still not fully up" is most certainly unsafe. For now, add back the logic we had in the `AbstractClusterWideTask` to `DisableServerGroupTask` to skip check on RRB deploys. In the future, we should not have artisanal checks like this and instead have traffic guards understand that a percentage of an ASG is going away and validate against that percentage.
marchello2000
added a commit
that referenced
this pull request
Jul 22, 2019
As a result of #3037, the RRB flow now no longer bypasses traffic guards (it used to do that before, here: https://github.com/spinnaker/orca/blob/master/orca-clouddriver/src/main/groovy/com/netflix/spinnaker/orca/clouddriver/tasks/cluster/AbstractClusterWideClouddriverTask.groovy#L145 `shouldSkipTrafficGuards`.) Instead, now when we run the first disable step in RRB (e.g. disable 5% of source), the traffic guards think the whole ASG is going away as it is unaware of the percentage disable flag and thus it terminates the operation since "disabling entire ASG while new is still not fully up" is most certainly unsafe. For now, add back the logic we had in the `AbstractClusterWideTask` to `DisableServerGroupTask` to skip check on RRB deploys. In the future, we should not have artisanal checks like this and instead have traffic guards understand that a percentage of an ASG is going away and validate against that percentage.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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:
we are deploying v003, with the following steps: 50%, 100%
Currently, RRB will do the following:
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