feat(clouddriver): make restoreMin only restore the min size (#2059)

* feat(clouddriver): make restoreMin only restore the min size Only perform partial server group resize on aws, as inheriting the max and desired is a hazard that could lead to incidents.
spinnaker · Apr 3, 2018 · 4f3613f · 4f3613f
1 parent ab45ea6
commit 4f3613f
Show file tree

Hide file tree

Showing 3 changed files with 35 additions and 28 deletions.
diff --git a/...innaker/orca/clouddriver/pipeline/providers/aws/ApplySourceServerGroupCapacityTask.groovy b/...innaker/orca/clouddriver/pipeline/providers/aws/ApplySourceServerGroupCapacityTask.groovy
@@ -67,10 +67,16 @@ class ApplySourceServerGroupCapacityTask extends AbstractServerGroupTask {
         targetServerGroup.capacity.min as Long
       )
 
-      // only update the min capacity, desired + max should be inherited/unchanged from the target server group
-      context.capacity = targetServerGroup.capacity + [
-        min: minCapacity
-      ]
+      if (context.cloudProvider == "aws") {
+        // aws is the only cloud provider supporting partial resizes
+        // updating anything other than 'min' could result in instances being
+        // unnecessarily destroyed or created if autoscaling has occurred
+        context.capacity = [ min: minCapacity ]
+      } else {
+        context.capacity = targetServerGroup.capacity + [
+          min: minCapacity
+        ]
+      }
 
       log.info("Restoring min capacity of ${context.region}/${targetServerGroup.name} to ${minCapacity} (currentMin: ${targetServerGroup.capacity.min}, snapshotMin: ${sourceServerGroupCapacitySnapshot.min})")
 

diff --git a/...ker/orca/clouddriver/pipeline/providers/aws/ApplySourceServerGroupSnapshotTaskSpec.groovy b/...ker/orca/clouddriver/pipeline/providers/aws/ApplySourceServerGroupSnapshotTaskSpec.groovy
@@ -121,17 +121,19 @@ class ApplySourceServerGroupSnapshotTaskSpec extends Specification {
       max    : 20
     ]
 
+    def context = [
+      credentials    : "test",
+      region         : "us-east-1",
+      asgName        : "application-stack-v001",
+      serverGroupName: "application-stack-v001",
+      cloudProvider  : cloudProvider
+    ]
+
     task = new ApplySourceServerGroupCapacityTask() {
       @Override
       ApplySourceServerGroupCapacityTask.TargetServerGroupContext getTargetServerGroupContext(Stage stage) {
         return new ApplySourceServerGroupCapacityTask.TargetServerGroupContext(
-          context: [
-            credentials    : "test",
-            region         : "us-east-1",
-            asgName        : "application-stack-v001",
-            serverGroupName: "application-stack-v001",
-            cloudProvider  : "aws"
-          ],
+          context: context,
           sourceServerGroupCapacitySnapshot: sourceServerGroupCapacitySnapshot
         )
       }
@@ -156,26 +158,24 @@ class ApplySourceServerGroupSnapshotTaskSpec extends Specification {
       "test",
       "application-stack-v001",
       "us-east-1",
-      "aws"
+      cloudProvider
     ) >> Optional.of(targetServerGroup)
 
-    result == [
-      cloudProvider  : "aws",
-      credentials    : "test",
-      asgName        : "application-stack-v001",
-      serverGroupName: "application-stack-v001",
-      region         : "us-east-1",
-      capacity       : [
-        min    : expectedMinCapacity,
-        desired: 5,
-        max    : 10
-      ]
-    ]
+    result.cloudProvider == cloudProvider
+    result.credentials == "test"
+    result.asgName == "application-stack-v001"
+    result.serverGroupName == "application-stack-v001"
+    result.region == "us-east-1"
+    result.capacity == (cloudProvider == "aws"
+      ? [ min: expectedMinCapacity ]
+      : [ min: expectedMinCapacity, desired: 5, max : 10 ])
 
     where:
-    originalMinCapacity || expectedMinCapacity
-    0                   || 0            // min(currentMin, snapshotMin) == 0
-    10                  || 5            // min(currentMin, snapshotMin) == 5
+    cloudProvider || originalMinCapacity || expectedMinCapacity
+    "aws"         || 0                   || 0            // min(currentMin, snapshotMin) == 0
+    "aws"         || 10                  || 5            // min(currentMin, snapshotMin) == 5
+    "notaws"      || 0                   || 0            // min(currentMin, snapshotMin) == 0
+    "notaws"      || 10                  || 5            // min(currentMin, snapshotMin) == 5
   }
 
   void "should get TargetServerGroupContext with explicitly provided coordinates"() {

diff --git a/orca-core/src/main/java/com/netflix/spinnaker/orca/pipeline/model/Stage.java b/orca-core/src/main/java/com/netflix/spinnaker/orca/pipeline/model/Stage.java
@@ -40,6 +40,7 @@
 import static com.netflix.spinnaker.orca.pipeline.model.Execution.ExecutionType.PIPELINE;
 import static java.lang.String.format;
 import static java.lang.String.join;
+import static java.nio.charset.StandardCharsets.UTF_8;
 import static java.util.Collections.*;
 import static java.util.stream.Collectors.toList;
 
@@ -451,7 +452,7 @@ public <O> O decodeBase64(String pointer, Class<O> type, ObjectMapper objectMapp
     try {
       return objectMapper.readValue(data, type);
     } catch (IOException e) {
-      throw new RuntimeException("Could not convert " + new String(data) + " to " + type.getSimpleName());
+      throw new RuntimeException("Could not convert " + new String(data, UTF_8) + " to " + type.getSimpleName(), e);
     }
   }