fix(provider/kubernetes): upsertScalingPolicy will patch existing aut…

…oscaler instead of re-creating (#1893)
spinnaker · Feb 23, 2018 · bebfa2a · bebfa2a
1 parent 8e57059
commit bebfa2a
Show file tree

Hide file tree

Showing 4 changed files with 29 additions and 18 deletions.
diff --git a/...in/groovy/com/netflix/spinnaker/clouddriver/kubernetes/v1/api/KubernetesApiAdaptor.groovy b/...in/groovy/com/netflix/spinnaker/clouddriver/kubernetes/v1/api/KubernetesApiAdaptor.groovy
@@ -22,6 +22,7 @@ import com.netflix.spinnaker.clouddriver.kubernetes.v1.deploy.KubernetesUtil
 import com.netflix.spinnaker.clouddriver.kubernetes.v1.deploy.exception.KubernetesOperationException
 import groovy.util.logging.Slf4j
 import io.fabric8.kubernetes.api.model.ConfigMap
+import io.fabric8.kubernetes.api.model.DoneableHorizontalPodAutoscaler
 import io.fabric8.kubernetes.api.model.DoneableSecret
 import io.fabric8.kubernetes.api.model.Event
 import io.fabric8.kubernetes.api.model.HasMetadata
@@ -461,6 +462,12 @@ class KubernetesApiAdaptor {
     }
   }
 
+  DoneableHorizontalPodAutoscaler editAutoscaler(String namespace, String name) {
+    exceptionWrapper("horizontalPodAutoscalers.edit", "Edit Autoscaler $name", namespace) {
+      client.autoscaling().horizontalPodAutoscalers().inNamespace(namespace).withName(name).edit()
+    }
+  }
+
   HorizontalPodAutoscaler getAutoscaler(String namespace, String name) {
     exceptionWrapper("horizontalPodAutoscalers.get", "Get Autoscaler $name", namespace) {
       client.autoscaling().horizontalPodAutoscalers().inNamespace(namespace).withName(name).get()

diff --git a/.../groovy/com/netflix/spinnaker/clouddriver/kubernetes/v1/api/KubernetesApiConverter.groovy b/.../groovy/com/netflix/spinnaker/clouddriver/kubernetes/v1/api/KubernetesApiConverter.groovy
@@ -86,6 +86,7 @@ import io.fabric8.kubernetes.api.model.Handler
 import io.fabric8.kubernetes.api.model.HandlerBuilder
 import io.fabric8.kubernetes.api.model.HorizontalPodAutoscaler
 import io.fabric8.kubernetes.api.model.HorizontalPodAutoscalerBuilder
+import io.fabric8.kubernetes.api.model.HorizontalPodAutoscalerFluentImpl
 import io.fabric8.kubernetes.api.model.HostPathVolumeSourceBuilder
 import io.fabric8.kubernetes.api.model.IntOrString
 import io.fabric8.kubernetes.api.model.KeyToPath
@@ -800,10 +801,10 @@ class KubernetesApiConverter {
     description.scalingPolicy = new KubernetesScalingPolicy(cpuUtilization: cpuUtilization)
   }
 
-  static HorizontalPodAutoscaler toAutoscaler(KubernetesAutoscalerDescription description,
-                                              String resourceName,
-                                              String resourceKind) {
-    def autoscalerBuilder = new HorizontalPodAutoscalerBuilder()
+  static HorizontalPodAutoscalerFluentImpl toAutoscaler(HorizontalPodAutoscalerFluentImpl autoscalerBuilder,
+                                                        KubernetesAutoscalerDescription description,
+                                                        String resourceName,
+                                                        String resourceKind) {
     autoscalerBuilder.withNewMetadata()
       .withName(resourceName)
       .withNamespace(description.namespace)
@@ -816,7 +817,7 @@ class KubernetesApiConverter {
       .withKind(resourceKind)
       .withName(resourceName)
       .endScaleTargetRef()
-      .endSpec().build()
+      .endSpec()
   }
 
   static DeployKubernetesAtomicOperationDescription fromReplicationController(ReplicationController replicationController) {

diff --git a/...iver/kubernetes/v1/deploy/ops/autoscaler/UpsertKubernetesAutoscalerAtomicOperation.groovy b/...iver/kubernetes/v1/deploy/ops/autoscaler/UpsertKubernetesAutoscalerAtomicOperation.groovy
@@ -27,6 +27,8 @@ import com.netflix.spinnaker.clouddriver.kubernetes.v1.deploy.description.server
 import com.netflix.spinnaker.clouddriver.kubernetes.v1.deploy.description.servergroup.KubernetesScalingPolicy
 import com.netflix.spinnaker.clouddriver.kubernetes.v1.deploy.exception.KubernetesOperationException
 import com.netflix.spinnaker.clouddriver.orchestration.AtomicOperation
+import io.fabric8.kubernetes.api.model.DoneableHorizontalPodAutoscaler
+import io.fabric8.kubernetes.api.model.HorizontalPodAutoscalerBuilder
 
 class UpsertKubernetesAutoscalerAtomicOperation implements AtomicOperation<Void> {
   KubernetesAutoscalerDescription description
@@ -80,22 +82,22 @@ class UpsertKubernetesAutoscalerAtomicOperation implements AtomicOperation<Void>
         description.scalingPolicy.cpuUtilization.target :
         autoscaler.spec.targetCPUUtilizationPercentage
 
-      task.updateStatus BASE_PHASE, "Deleting old autoscaler..."
-      credentials.apiAdaptor.deleteAutoscaler(namespace, name)
-    }
+      ((DoneableHorizontalPodAutoscaler) KubernetesApiConverter.toAutoscaler(
+        credentials.apiAdaptor.editAutoscaler(namespace, name), description, name, kind
+      )).done()
+    } else {
+      if (!description.scalingPolicy || !description.scalingPolicy.cpuUtilization || description.scalingPolicy.cpuUtilization.target == null) {
+        throw new KubernetesOperationException("Scaling policy must be specified when the target server group has no autoscaler.")
+      }
 
-    if (!description.scalingPolicy || !description.scalingPolicy.cpuUtilization || description.scalingPolicy.cpuUtilization.target == null) {
-      throw new KubernetesOperationException("Scaling policy must be specified when the target server group has no autoscaler.")
-    }
+      if (!description.capacity || description.capacity.min == null || description.capacity.max == null) {
+        throw new KubernetesOperationException("Capacity min and max must be fully specified when the target server group has no autoscaler.")
+      }
 
-    if (!description.capacity || description.capacity.min == null || description.capacity.max == null) {
-      throw new KubernetesOperationException("Capacity min and max must be fully specified when the target server group has no autoscaler.")
+      task.updateStatus BASE_PHASE, "Creating autoscaler..."
+      credentials.apiAdaptor.createAutoscaler(namespace, ((HorizontalPodAutoscalerBuilder) KubernetesApiConverter.toAutoscaler(new HorizontalPodAutoscalerBuilder(), description, name, kind)).build())
     }
 
-
-    task.updateStatus BASE_PHASE, "Creating autoscaler..."
-    credentials.apiAdaptor.createAutoscaler(namespace, KubernetesApiConverter.toAutoscaler(description, name, kind))
-
     return null
   }
 }
diff --git a/...r/clouddriver/kubernetes/v1/deploy/ops/servergroup/DeployKubernetesAtomicOperation.groovy b/...r/clouddriver/kubernetes/v1/deploy/ops/servergroup/DeployKubernetesAtomicOperation.groovy
@@ -31,6 +31,7 @@ import com.netflix.spinnaker.clouddriver.kubernetes.v1.deploy.exception.Kubernet
 import com.netflix.spinnaker.clouddriver.kubernetes.v1.security.KubernetesV1Credentials
 import com.netflix.spinnaker.clouddriver.orchestration.AtomicOperation
 import io.fabric8.kubernetes.api.model.HasMetadata
+import io.fabric8.kubernetes.api.model.HorizontalPodAutoscalerBuilder
 import io.fabric8.kubernetes.api.model.extensions.DeploymentBuilder
 import io.fabric8.kubernetes.api.model.extensions.DeploymentFluentImpl
 import io.fabric8.kubernetes.api.model.extensions.DoneableDeployment
@@ -161,7 +162,7 @@ class DeployKubernetesAtomicOperation implements AtomicOperation<DeploymentResul
 
       def name = hasDeployment ? clusterName : replicaSetName
       def kind = hasDeployment ? KubernetesUtil.DEPLOYMENT_KIND : KubernetesUtil.SERVER_GROUP_KIND
-      def autoscaler = KubernetesApiConverter.toAutoscaler(new KubernetesAutoscalerDescription(replicaSetName, description), name, kind)
+      def autoscaler = ((HorizontalPodAutoscalerBuilder) KubernetesApiConverter.toAutoscaler(new HorizontalPodAutoscalerBuilder(), new KubernetesAutoscalerDescription(replicaSetName, description), name, kind)).build()
 
       if (credentials.apiAdaptor.getAutoscaler(namespace, name)) {
         credentials.apiAdaptor.deleteAutoscaler(namespace, name)