Skip to content

Commit

Permalink
feat(gce): support autoscaler scale-down controls (#4129)
Browse files Browse the repository at this point in the history
* refactor(gce): don't serialize AutoscalingPolicy

Serializing Google API types doesn't work right with Jackson. These
types are actually all `Map` objects, so Jackson does weird things with
them. You can add ``@JsonTypeInfo` and it kinda works, until it doesn't.

In particular, it's okay to for them to reference another Google
API object, but if _that_ object references a third Google API type
(e.g. AutoscalingPolicy has a ScaleDownControl field, which has a
FixedOrPercent field) then exceptions will be thrown when
deserializing that third-tier field.

Fortunately (and maybe for this exact reason??) we already have a class
GoogleAutoscalingPolicy which mirrors exactly the fields in
AutoscalingPolicy, so let's use that one instead.

* refactor(gce): Move the AutoscalingPolicy conversion code into the cache

GCEUtil used to have the function for converting from AutoscalingPolicy
to GoogleAutoscalingPolicy, but now
AbstractGoogleServerGroupCachingAgent is the only place that uses it, so
move it in there.

* feat(gce): support autoscaler scale-down controls

* fix(gce): copy scale-down policy to the cache and back to deck

* fix(gce): allow removing a scaleDownControl by setting it to {}

* fix(gce): address Eric's PR comments

* test(gce): fix the test to handle a null value properly
  • Loading branch information
plumpy authored and mergify[bot] committed Oct 29, 2019
1 parent 5bb06aa commit bc5aeb4
Show file tree
Hide file tree
Showing 12 changed files with 281 additions and 90 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import com.netflix.spinnaker.cats.cache.RelationshipCacheFilter
import com.netflix.spinnaker.clouddriver.artifacts.ArtifactUtils
import com.netflix.spinnaker.clouddriver.data.task.Task
import com.netflix.spinnaker.clouddriver.google.GoogleExecutorTraits
import com.netflix.spinnaker.clouddriver.google.batch.GoogleBatchRequest
import com.netflix.spinnaker.clouddriver.google.cache.Keys
import com.netflix.spinnaker.clouddriver.google.deploy.description.BaseGoogleInstanceDescription
import com.netflix.spinnaker.clouddriver.google.deploy.description.BasicGoogleDeployDescription
Expand All @@ -43,7 +44,6 @@ import com.netflix.spinnaker.clouddriver.google.model.*
import com.netflix.spinnaker.clouddriver.google.model.callbacks.Utils
import com.netflix.spinnaker.clouddriver.google.model.health.GoogleLoadBalancerHealth
import com.netflix.spinnaker.clouddriver.google.model.loadbalancing.*
import com.netflix.spinnaker.clouddriver.google.batch.GoogleBatchRequest
import com.netflix.spinnaker.clouddriver.google.provider.view.GoogleClusterProvider
import com.netflix.spinnaker.clouddriver.google.provider.view.GoogleLoadBalancerProvider
import com.netflix.spinnaker.clouddriver.google.provider.view.GoogleNetworkProvider
Expand Down Expand Up @@ -652,53 +652,6 @@ class GCEUtil {
)
}

static GoogleAutoscalingPolicy buildAutoscalingPolicyDescriptionFromAutoscalingPolicy(
AutoscalingPolicy autoscalingPolicy) {
if (!autoscalingPolicy) {
return null
}

autoscalingPolicy.with {
def autoscalingPolicyDescription =
new GoogleAutoscalingPolicy(
coolDownPeriodSec: coolDownPeriodSec,
minNumReplicas: minNumReplicas,
maxNumReplicas: maxNumReplicas
)

if (cpuUtilization) {
autoscalingPolicyDescription.cpuUtilization =
new GoogleAutoscalingPolicy.CpuUtilization(
utilizationTarget: cpuUtilization.utilizationTarget
)
}

if (loadBalancingUtilization) {
autoscalingPolicyDescription.loadBalancingUtilization =
new GoogleAutoscalingPolicy.LoadBalancingUtilization(
utilizationTarget: loadBalancingUtilization.utilizationTarget
)
}

if (customMetricUtilizations) {
autoscalingPolicyDescription.customMetricUtilizations =
customMetricUtilizations.collect {
new GoogleAutoscalingPolicy.CustomMetricUtilization(
metric: it.metric,
utilizationTarget: it.utilizationTarget,
utilizationTargetType: it.utilizationTargetType
)
}
}

if (mode) {
autoscalingPolicyDescription.mode = GoogleAutoscalingPolicy.AutoscalingMode.valueOf(mode)
}

return autoscalingPolicyDescription
}
}

static GoogleAutoHealingPolicy buildAutoHealingPolicyDescriptionFromAutoHealingPolicy(
InstanceGroupManagerAutoHealingPolicy autoHealingPolicy) {
if (!autoHealingPolicy) {
Expand Down Expand Up @@ -894,6 +847,20 @@ class GCEUtil {
}
}

if (scaleDownControl) {
def scaledDownReplicasInput = scaleDownControl.maxScaledDownReplicas
def maxScaledDownReplicas = null
if (scaledDownReplicasInput != null) {
maxScaledDownReplicas = FixedOrPercent.newInstance()
.setFixed(scaledDownReplicasInput.fixed)
.setPercent(scaledDownReplicasInput.percent)
}
gceAutoscalingPolicy.scaleDownControl =
new AutoscalingPolicyScaleDownControl(
maxScaledDownReplicas: maxScaledDownReplicas,
timeWindowSec: scaleDownControl.timeWindowSec)
}

new Autoscaler(name: serverGroupName,
target: targetLink,
autoscalingPolicy: gceAutoscalingPolicy)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ class ResizeGoogleServerGroupAtomicOperationConverter extends AbstractAtomicOper
AtomicOperation convertOperation(Map input) {
// If the target server group has an Autoscaler configured we need to modify that policy as opposed to the
// target size of the managed instance group itself.
AutoscalingPolicy autoscalingPolicy = resolveServerGroup(input)?.autoscalingPolicy
GoogleAutoscalingPolicy autoscalingPolicy = resolveServerGroup(input)?.autoscalingPolicy
def convertedDescription = convertDescription(input, autoscalingPolicy)

if (autoscalingPolicy) {
Expand All @@ -53,16 +53,13 @@ class ResizeGoogleServerGroupAtomicOperationConverter extends AbstractAtomicOper
}
}

def convertDescription(Map input, AutoscalingPolicy autoscalingPolicy) {
def convertDescription(Map input, GoogleAutoscalingPolicy autoscalingPolicy) {
if (autoscalingPolicy) {
UpsertGoogleAutoscalingPolicyDescription upsertGoogleAutoscalingPolicyDescription =
GoogleAtomicOperationConverterHelper.convertDescription(input, this, UpsertGoogleAutoscalingPolicyDescription)

// Retrieve the existing autoscaling policy and overwrite the min/max settings.
GoogleAutoscalingPolicy googleAutoscalingPolicy =
GCEUtil.buildAutoscalingPolicyDescriptionFromAutoscalingPolicy(autoscalingPolicy)

upsertGoogleAutoscalingPolicyDescription.autoscalingPolicy = googleAutoscalingPolicy
upsertGoogleAutoscalingPolicyDescription.autoscalingPolicy = autoscalingPolicy
upsertGoogleAutoscalingPolicyDescription.autoscalingPolicy.minNumReplicas = input.capacity?.min
upsertGoogleAutoscalingPolicyDescription.autoscalingPolicy.maxNumReplicas = input.capacity?.max
if (input?.writeMetadata != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -458,7 +458,7 @@ class BasicGoogleDeployHandler implements DeployHandler<BasicGoogleDeployDescrip
description.source.serverGroupName)

description.targetSize = ancestorServerGroup.capacity.desired
description.autoscalingPolicy = GCEUtil.buildAutoscalingPolicyDescriptionFromAutoscalingPolicy(ancestorServerGroup.autoscalingPolicy)
description.autoscalingPolicy = ancestorServerGroup.autoscalingPolicy
}

def autoHealingHealthCheck = null
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -491,23 +491,19 @@ abstract class AbstractEnableDisableAtomicOperation extends GoogleAtomicOperatio
String region = serverGroup.region
String zone = serverGroup.zone
if (serverGroup.autoscalingPolicy) {
def policyDescription =
GCEUtil.buildAutoscalingPolicyDescriptionFromAutoscalingPolicy(serverGroup.autoscalingPolicy)
if (policyDescription) {
def autoscaler = GCEUtil.buildAutoscaler(serverGroupName, serverGroup.selfLink, policyDescription)
autoscaler.getAutoscalingPolicy().setMode(mode.toString())

if (serverGroup.regional) {
timeExecute(
compute.regionAutoscalers().update(project, region, autoscaler),
"compute.regionAutoscalers.update",
TAG_SCOPE, SCOPE_REGIONAL, TAG_REGION, region)
} else {
timeExecute(
compute.autoscalers().update(project, zone, autoscaler),
"compute.autoscalers.update",
TAG_SCOPE, SCOPE_ZONAL, TAG_ZONE, zone)
}
def autoscaler = GCEUtil.buildAutoscaler(serverGroupName, serverGroup.selfLink, serverGroup.autoscalingPolicy)
autoscaler.getAutoscalingPolicy().setMode(mode.toString())

if (serverGroup.regional) {
timeExecute(
compute.regionAutoscalers().update(project, region, autoscaler),
"compute.regionAutoscalers.update",
TAG_SCOPE, SCOPE_REGIONAL, TAG_REGION, region)
} else {
timeExecute(
compute.autoscalers().update(project, zone, autoscaler),
"compute.autoscalers.update",
TAG_SCOPE, SCOPE_ZONAL, TAG_ZONE, zone)
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -266,9 +266,7 @@ class CopyLastGoogleServerGroupAtomicOperation extends GoogleAtomicOperation<Dep
}
}

AutoscalingPolicy ancestorAutoscalingPolicy = ancestorServerGroup.autoscalingPolicy
GoogleAutoscalingPolicy ancestorAutoscalingPolicyDescription =
GCEUtil.buildAutoscalingPolicyDescriptionFromAutoscalingPolicy(ancestorAutoscalingPolicy)
GoogleAutoscalingPolicy ancestorAutoscalingPolicyDescription = ancestorServerGroup.autoscalingPolicy

newDescription.autoscalingPolicy = description.autoscalingPolicy ?: ancestorAutoscalingPolicyDescription

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,7 @@ class UpsertGoogleAutoscalingPolicyAtomicOperation extends GoogleAtomicOperation

def autoscaler = null
if (description.autoscalingPolicy) {
def ancestorAutoscalingPolicyDescription =
GCEUtil.buildAutoscalingPolicyDescriptionFromAutoscalingPolicy(serverGroup.autoscalingPolicy)
def ancestorAutoscalingPolicyDescription = serverGroup.autoscalingPolicy
if (ancestorAutoscalingPolicyDescription) {
task.updateStatus BASE_PHASE, "Updating autoscaler for $serverGroupName..."

Expand Down Expand Up @@ -230,6 +229,17 @@ class UpsertGoogleAutoscalingPolicyAtomicOperation extends GoogleAtomicOperation
}
}

// If scaleDownControl is completely absent, we leave the previous value.
// To remove it, set it to an empty object.
if (update.scaleDownControl != null) {
def scaleDownControl = update.scaleDownControl
if (scaleDownControl.timeWindowSec != null && scaleDownControl.maxScaledDownReplicas != null) {
newDescription.scaleDownControl = scaleDownControl
} else {
newDescription.scaleDownControl = null
}
}

// Deletes existing cpuUtilization or loadBalancingUtilization if passed an empty object.
["cpuUtilization", "loadBalancingUtilization"].each {
if (update[it] != null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import com.netflix.spinnaker.clouddriver.data.task.Task
import com.netflix.spinnaker.clouddriver.data.task.TaskRepository
import com.netflix.spinnaker.clouddriver.google.deploy.description.snapshot.SaveSnapshotDescription
import com.netflix.spinnaker.clouddriver.google.deploy.exception.GoogleResourceIllegalStateException
import com.netflix.spinnaker.clouddriver.google.model.GoogleAutoscalingPolicy
import com.netflix.spinnaker.clouddriver.google.model.GoogleCluster
import com.netflix.spinnaker.clouddriver.google.model.GoogleHealthCheck
import com.netflix.spinnaker.clouddriver.google.model.GoogleSecurityGroup
Expand Down Expand Up @@ -378,7 +379,7 @@ class SaveSnapshotAtomicOperation implements AtomicOperation<Void> {
return numDisks > 0
}

private Void addAutoscalerToResourceMap(String targetName, String targetZone, AutoscalingPolicy autoscalingPolicy, Map resourceMap) {
private Void addAutoscalerToResourceMap(String targetName, String targetZone, GoogleAutoscalingPolicy autoscalingPolicy, Map resourceMap) {
def autoscalerMap = [:]

autoscalerMap.name = targetName
Expand Down Expand Up @@ -668,7 +669,7 @@ class SaveSnapshotAtomicOperation implements AtomicOperation<Void> {
}

private ShieldedVmConfig convertMapToShieldedVmConfig(Map shieldedVmConfigMap) {

ShieldedVmConfig shieldedVmConfig = new ShieldedVmConfig()

shieldedVmConfig.enableSecureBoot = shieldedVmConfigMap.enableSecureBoot as Boolean
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ class GoogleAutoscalingPolicy {
CpuUtilization cpuUtilization
LoadBalancingUtilization loadBalancingUtilization
List<CustomMetricUtilization> customMetricUtilizations
ScaleDownControl scaleDownControl
AutoscalingMode mode

@ToString(includeNames = true)
Expand All @@ -56,6 +57,15 @@ class GoogleAutoscalingPolicy {
}
}

static class ScaleDownControl {
FixedOrPercent maxScaledDownReplicas
Integer timeWindowSec
}

static class FixedOrPercent {
Integer fixed
Integer percent
}

static enum AutoscalingMode {
ON,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ package com.netflix.spinnaker.clouddriver.google.model
import com.fasterxml.jackson.annotation.JsonIgnore
import com.fasterxml.jackson.annotation.JsonInclude
import com.fasterxml.jackson.annotation.JsonTypeInfo
import com.google.api.services.compute.model.AutoscalingPolicy
import com.google.api.services.compute.model.InstanceGroupManagerActionsSummary
import com.google.api.services.compute.model.InstanceGroupManagerAutoHealingPolicy
import com.google.api.services.compute.model.ServiceAccount
Expand Down Expand Up @@ -72,8 +71,7 @@ class GoogleServerGroup implements GoogleLabeledResource {
GoogleDistributionPolicy distributionPolicy
Boolean selectZones

@JsonTypeInfo(use=JsonTypeInfo.Id.CLASS, include=JsonTypeInfo.As.PROPERTY, property="class")
AutoscalingPolicy autoscalingPolicy
GoogleAutoscalingPolicy autoscalingPolicy

@JsonTypeInfo(use=JsonTypeInfo.Id.CLASS, include=JsonTypeInfo.As.PROPERTY, property="class")
StatefulPolicy statefulPolicy
Expand Down Expand Up @@ -127,7 +125,7 @@ class GoogleServerGroup implements GoogleLabeledResource {
String selfLink = GoogleServerGroup.this.selfLink
Boolean discovery = GoogleServerGroup.this.discovery
InstanceGroupManagerActionsSummary currentActions = GoogleServerGroup.this.currentActions
AutoscalingPolicy autoscalingPolicy = GoogleServerGroup.this.autoscalingPolicy
GoogleAutoscalingPolicy autoscalingPolicy = GoogleServerGroup.this.autoscalingPolicy
StatefulPolicy statefulPolicy = GoogleServerGroup.this.statefulPolicy
List<String> autoscalingMessages = GoogleServerGroup.this.autoscalingMessages
InstanceGroupManagerAutoHealingPolicy autoHealingPolicy = GoogleServerGroup.this.autoHealingPolicy
Expand Down
Loading

0 comments on commit bc5aeb4

Please sign in to comment.