Skip to content

Commit f8a9c8f

Browse files
Merge branch 'release-1.28.x' into mergify/bp/release-1.28.x/pr-4618
2 parents bb18a96 + 6a37596 commit f8a9c8f

File tree

3 files changed

+137
-20
lines changed

3 files changed

+137
-20
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
/*
2+
* Copyright 2024 Harness, Inc.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package com.netflix.spinnaker.orca.clouddriver.tasks.servergroup;
18+
19+
import org.springframework.boot.context.properties.ConfigurationProperties;
20+
import org.springframework.stereotype.Component;
21+
22+
@Component
23+
@ConfigurationProperties(prefix = "server-group")
24+
public class ServerGroupProperties {
25+
26+
private Resize resize = new Resize();
27+
28+
public static class Resize {
29+
private boolean matchInstancesSize;
30+
31+
public void setMatchInstancesSize(boolean matchInstancesSize) {
32+
this.matchInstancesSize = matchInstancesSize;
33+
}
34+
35+
public boolean isMatchInstancesSize() {
36+
return matchInstancesSize;
37+
}
38+
}
39+
40+
public void setResize(Resize resize) {
41+
this.resize = resize;
42+
}
43+
44+
public Resize getResize() {
45+
return resize;
46+
}
47+
}
Lines changed: 35 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
/*
2-
* Copyright 2014 Netflix, Inc.
2+
* Copyright 2024 Netflix, Inc.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
66
* You may obtain a copy of the License at
77
*
8-
* http://www.apache.org/licenses/LICENSE-2.0
8+
* http://www.apache.org/licenses/LICENSE-2.0
99
*
1010
* Unless required by applicable law or agreed to in writing, software
1111
* distributed under the License is distributed on an "AS IS" BASIS,
@@ -27,6 +27,12 @@
2727
@Component
2828
public class WaitForCapacityMatchTask extends AbstractInstancesCheckTask {
2929

30+
private final ServerGroupProperties serverGroupProperties;
31+
32+
public WaitForCapacityMatchTask(ServerGroupProperties serverGroupProperties) {
33+
this.serverGroupProperties = serverGroupProperties;
34+
}
35+
3036
@Override
3137
protected Map<String, List<String>> getServerGroups(StageExecution stage) {
3238
return (Map<String, List<String>>) stage.getContext().get("deploy.server.groups");
@@ -88,27 +94,37 @@ protected boolean hasSucceeded(
8894
desired = capacity.getDesired();
8995
}
9096

91-
Integer targetDesiredSize =
92-
Optional.ofNullable((Number) context.get("targetDesiredSize"))
93-
.map(Number::intValue)
94-
.orElse(null);
95-
96-
splainer.add(
97-
String.format(
98-
"checking if capacity matches (desired=%s, target=%s current=%s)",
99-
desired, targetDesiredSize == null ? "none" : targetDesiredSize, instances.size()));
100-
if (targetDesiredSize != null && targetDesiredSize != 0) {
101-
// `targetDesiredSize` is derived from `targetHealthyDeployPercentage` and if present,
102-
// then scaling has succeeded if the number of instances is greater than this value.
103-
if (instances.size() < targetDesiredSize) {
97+
if (serverGroupProperties.getResize().isMatchInstancesSize()) {
98+
splainer.add(
99+
"checking if capacity matches (desired=${desired}, instances.size()=${instances.size()}) ");
100+
if (desired == null || desired != instances.size()) {
104101
splainer.add(
105-
"short-circuiting out of WaitForCapacityMatchTask because targetDesired and current capacity don't match");
102+
"short-circuiting out of WaitForCapacityMatchTask because expected and current capacity don't match}");
106103
return false;
107104
}
108-
} else if (desired == null || desired != instances.size()) {
105+
} else {
106+
Integer targetDesiredSize =
107+
Optional.ofNullable((Number) context.get("targetDesiredSize"))
108+
.map(Number::intValue)
109+
.orElse(null);
110+
109111
splainer.add(
110-
"short-circuiting out of WaitForCapacityMatchTask because expected and current capacity don't match");
111-
return false;
112+
String.format(
113+
"checking if capacity matches (desired=%s, target=%s current=%s)",
114+
desired, targetDesiredSize == null ? "none" : targetDesiredSize, instances.size()));
115+
if (targetDesiredSize != null && targetDesiredSize != 0) {
116+
// `targetDesiredSize` is derived from `targetHealthyDeployPercentage` and if present,
117+
// then scaling has succeeded if the number of instances is greater than this value.
118+
if (instances.size() < targetDesiredSize) {
119+
splainer.add(
120+
"short-circuiting out of WaitForCapacityMatchTask because targetDesired and current capacity don't match");
121+
return false;
122+
}
123+
} else if (desired == null || desired != instances.size()) {
124+
splainer.add(
125+
"short-circuiting out of WaitForCapacityMatchTask because expected and current capacity don't match");
126+
return false;
127+
}
112128
}
113129

114130
boolean disabled = Boolean.TRUE.equals(serverGroup.getDisabled());

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

Lines changed: 55 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ import spock.lang.Unroll
3030
class WaitForCapacityMatchTaskSpec extends Specification {
3131

3232
CloudDriverService cloudDriverService = Mock()
33-
@Subject WaitForCapacityMatchTask task = new WaitForCapacityMatchTask() {
33+
@Subject WaitForCapacityMatchTask task = new WaitForCapacityMatchTask(new ServerGroupProperties()) {
3434
@Override
3535
void verifyServerGroupsExist(StageExecution stage) {
3636
// do nothing
@@ -264,6 +264,60 @@ class WaitForCapacityMatchTaskSpec extends Specification {
264264
true || 4 | [min: 3, max: 10, desired: 4] | [min: "1", max: "50", desired: "5"]
265265
}
266266
267+
@Unroll
268+
void 'should use number of instances when determining if scaling has succeeded even if targetHealthyDeployPercentage is defined'() {
269+
def serverGroupProperties = new ServerGroupProperties()
270+
def resize = new ServerGroupProperties.Resize()
271+
resize.setMatchInstancesSize(true)
272+
serverGroupProperties.setResize(resize)
273+
WaitForCapacityMatchTask task = new WaitForCapacityMatchTask(serverGroupProperties) {
274+
@Override
275+
void verifyServerGroupsExist(StageExecution stage) {
276+
// do nothing
277+
}
278+
}
279+
when:
280+
def context = [
281+
capacity: [
282+
min: configured.min,
283+
max: configured.max,
284+
desired: configured.desired
285+
],
286+
targetHealthyDeployPercentage: targetHealthyDeployPercentage,
287+
targetDesiredSize: targetHealthyDeployPercentage
288+
? Math.round(targetHealthyDeployPercentage * configured.desired / 100) : null
289+
]
290+
291+
def serverGroup = ModelUtils.serverGroup([
292+
asg: [
293+
desiredCapacity: asg.desired
294+
],
295+
capacity: [
296+
min: asg.min,
297+
max: asg.max,
298+
desired: asg.desired
299+
]
300+
])
301+
302+
def instances = []
303+
(1..healthy).each {
304+
instances << ModelUtils.instance([health: [[state: 'Up']]])
305+
}
306+
307+
then:
308+
result == task.hasSucceeded(
309+
new StageExecutionImpl(PipelineExecutionImpl.newPipeline("orca"), "", "", context),
310+
serverGroup, instances, null
311+
)
312+
313+
where:
314+
result || healthy | asg | configured | targetHealthyDeployPercentage
315+
false || 5 | [min: 10, max: 15, desired: 15] | [min: 10, max: 15, desired: 15] | 85
316+
false || 12 | [min: 10, max: 15, desired: 15] | [min: 10, max: 15, desired: 15] | 85
317+
false || 13 | [min: 10, max: 15, desired: 15] | [min: 10, max: 15, desired: 15] | 85
318+
true || 15 | [min: 10, max: 15, desired: 15] | [min: 10, max: 15, desired: 15] | 100
319+
}
320+
267321
private static Instance makeInstance(id, healthState = 'Up') {
268322
ModelUtils.instance([instanceId: id, health: [ [ state: healthState ] ]])
269323
}

0 commit comments

Comments
 (0)