Merge branch 'release-1.31.x' into mergify/bp/release-1.31.x/pr-4618

orca-clouddriver/src/main/java/com/netflix/spinnaker/orca/clouddriver/tasks/servergroup/ServerGroupProperties.java

@@ -0,0 +1,47 @@
+/*
+ * Copyright 2024 Harness, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.netflix.spinnaker.orca.clouddriver.tasks.servergroup;
+
+import org.springframework.boot.context.properties.ConfigurationProperties;
+import org.springframework.stereotype.Component;
+
+@Component
+@ConfigurationProperties(prefix = "server-group")
+public class ServerGroupProperties {
+
+  private Resize resize = new Resize();
+
+  public static class Resize {
+    private boolean matchInstancesSize;
+
+    public void setMatchInstancesSize(boolean matchInstancesSize) {
+      this.matchInstancesSize = matchInstancesSize;
+    }
+
+    public boolean isMatchInstancesSize() {
+      return matchInstancesSize;
+    }
+  }
+
+  public void setResize(Resize resize) {
+    this.resize = resize;
+  }
+
+  public Resize getResize() {
+    return resize;
+  }
+}

orca-clouddriver/src/main/groovy/com/netflix/spinnaker/orca/clouddriver/tasks/servergroup/WaitForCapacityMatchTask.java → orca-clouddriver/src/main/java/com/netflix/spinnaker/orca/clouddriver/tasks/servergroup/WaitForCapacityMatchTask.java

@@ -1,11 +1,11 @@
 /*
- * Copyright 2014 Netflix, Inc.
+ * Copyright 2024 Netflix, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
  * You may obtain a copy of the License at
  *
- *    http://www.apache.org/licenses/LICENSE-2.0
+ *   http://www.apache.org/licenses/LICENSE-2.0
  *
  * Unless required by applicable law or agreed to in writing, software
  * distributed under the License is distributed on an "AS IS" BASIS,
@@ -27,6 +27,12 @@
 @Component
 public class WaitForCapacityMatchTask extends AbstractInstancesCheckTask {
 
+  private final ServerGroupProperties serverGroupProperties;
+
+  public WaitForCapacityMatchTask(ServerGroupProperties serverGroupProperties) {
+    this.serverGroupProperties = serverGroupProperties;
+  }
+
   @Override
   protected Map<String, List<String>> getServerGroups(StageExecution stage) {
     return (Map<String, List<String>>) stage.getContext().get("deploy.server.groups");
@@ -88,27 +94,37 @@ protected boolean hasSucceeded(
         desired = capacity.getDesired();
       }
 
-      Integer targetDesiredSize =
-          Optional.ofNullable((Number) context.get("targetDesiredSize"))
-              .map(Number::intValue)
-              .orElse(null);
-
-      splainer.add(
-          String.format(
-              "checking if capacity matches (desired=%s, target=%s current=%s)",
-              desired, targetDesiredSize == null ? "none" : targetDesiredSize, instances.size()));
-      if (targetDesiredSize != null && targetDesiredSize != 0) {
-        // `targetDesiredSize` is derived from `targetHealthyDeployPercentage` and if present,
-        // then scaling has succeeded if the number of instances is greater than this value.
-        if (instances.size() < targetDesiredSize) {
+      if (serverGroupProperties.getResize().isMatchInstancesSize()) {
+        splainer.add(
+            "checking if capacity matches (desired=${desired}, instances.size()=${instances.size()}) ");
+        if (desired == null || desired != instances.size()) {
           splainer.add(
-              "short-circuiting out of WaitForCapacityMatchTask because targetDesired and current capacity don't match");
+              "short-circuiting out of WaitForCapacityMatchTask because expected and current capacity don't match}");
           return false;
         }
-      } else if (desired == null || desired != instances.size()) {
+      } else {
+        Integer targetDesiredSize =
+            Optional.ofNullable((Number) context.get("targetDesiredSize"))
+                .map(Number::intValue)
+                .orElse(null);
+
         splainer.add(
-            "short-circuiting out of WaitForCapacityMatchTask because expected and current capacity don't match");
-        return false;
+            String.format(
+                "checking if capacity matches (desired=%s, target=%s current=%s)",
+                desired, targetDesiredSize == null ? "none" : targetDesiredSize, instances.size()));
+        if (targetDesiredSize != null && targetDesiredSize != 0) {
+          // `targetDesiredSize` is derived from `targetHealthyDeployPercentage` and if present,
+          // then scaling has succeeded if the number of instances is greater than this value.
+          if (instances.size() < targetDesiredSize) {
+            splainer.add(
+                "short-circuiting out of WaitForCapacityMatchTask because targetDesired and current capacity don't match");
+            return false;
+          }
+        } else if (desired == null || desired != instances.size()) {
+          splainer.add(
+              "short-circuiting out of WaitForCapacityMatchTask because expected and current capacity don't match");
+          return false;
+        }
       }
 
       boolean disabled = Boolean.TRUE.equals(serverGroup.getDisabled());

orca-clouddriver/src/test/groovy/com/netflix/spinnaker/orca/clouddriver/tasks/servergroup/WaitForCapacityMatchTaskSpec.groovy

@@ -30,7 +30,7 @@ import spock.lang.Unroll
 class WaitForCapacityMatchTaskSpec extends Specification {
 
   CloudDriverService cloudDriverService = Mock()
-  @Subject WaitForCapacityMatchTask task = new WaitForCapacityMatchTask() {
+  @Subject WaitForCapacityMatchTask task = new WaitForCapacityMatchTask(new ServerGroupProperties()) {
     @Override
     void verifyServerGroupsExist(StageExecution stage) {
       // do nothing
@@ -264,6 +264,60 @@ class WaitForCapacityMatchTaskSpec extends Specification {
     true   || 4       | [min: 3, max: 10, desired: 4]   | [min: "1", max: "50", desired: "5"]
   }
 
+  @Unroll
+  void 'should use number of instances when determining if scaling has succeeded even if targetHealthyDeployPercentage is defined'() {
+    def serverGroupProperties = new ServerGroupProperties()
+    def resize = new ServerGroupProperties.Resize()
+    resize.setMatchInstancesSize(true)
+    serverGroupProperties.setResize(resize)
+    WaitForCapacityMatchTask task = new WaitForCapacityMatchTask(serverGroupProperties) {
+      @Override
+      void verifyServerGroupsExist(StageExecution stage) {
+        // do nothing
+      }
+    }
+    when:
+    def context  = [
+        capacity: [
+            min: configured.min,
+            max: configured.max,
+            desired: configured.desired
+        ],
+        targetHealthyDeployPercentage: targetHealthyDeployPercentage,
+        targetDesiredSize: targetHealthyDeployPercentage
+            ? Math.round(targetHealthyDeployPercentage * configured.desired / 100) : null
+    ]
+
+    def serverGroup = ModelUtils.serverGroup([
+        asg: [
+            desiredCapacity: asg.desired
+        ],
+        capacity: [
+            min: asg.min,
+            max: asg.max,
+            desired: asg.desired
+        ]
+    ])
+
+    def instances = []
+    (1..healthy).each {
+      instances << ModelUtils.instance([health: [[state: 'Up']]])
+    }
+
+    then:
+    result == task.hasSucceeded(
+        new StageExecutionImpl(PipelineExecutionImpl.newPipeline("orca"), "", "", context),
+        serverGroup, instances, null
+    )
+
+    where:
+    result || healthy | asg                             | configured                      | targetHealthyDeployPercentage
+    false  || 5       | [min: 10, max: 15, desired: 15] | [min: 10, max: 15, desired: 15] | 85
+    false  || 12      | [min: 10, max: 15, desired: 15] | [min: 10, max: 15, desired: 15] | 85
+    false  || 13      | [min: 10, max: 15, desired: 15] | [min: 10, max: 15, desired: 15] | 85
+    true   || 15      | [min: 10, max: 15, desired: 15] | [min: 10, max: 15, desired: 15] | 100
+  }
+
   private static Instance makeInstance(id, healthState = 'Up') {
     ModelUtils.instance([instanceId: id, health: [ [ state: healthState ] ]])
   }