Skip to content

Commit 2adac4e

Browse files
authored
Merge pull request #2266 from HubSpot/allow_empty_instance_requests
Mutate request instances based on existing request
2 parents 6324a26 + b34d852 commit 2adac4e

3 files changed

Lines changed: 51 additions & 0 deletions

File tree

SingularityService/src/main/java/com/hubspot/singularity/config/SingularityConfiguration.java

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -455,6 +455,8 @@ public class SingularityConfiguration extends Configuration {
455455

456456
private boolean skipPersistingTooLongTaskIds = false;
457457

458+
private boolean allowEmptyRequestInstances = false;
459+
458460
public long getAskDriverToKillTasksAgainAfterMillis() {
459461
return askDriverToKillTasksAgainAfterMillis;
460462
}
@@ -2129,4 +2131,12 @@ public boolean skipPersistingTooLongTaskIds() {
21292131
public void setSkipPersistingTooLongTaskIds(boolean skipPersistingTooLongTaskIds) {
21302132
this.skipPersistingTooLongTaskIds = skipPersistingTooLongTaskIds;
21312133
}
2134+
2135+
public boolean allowEmptyRequestInstances() {
2136+
return allowEmptyRequestInstances;
2137+
}
2138+
2139+
public void setAllowEmptyRequestInstances(boolean allowEmptyRequestInstances) {
2140+
this.allowEmptyRequestInstances = allowEmptyRequestInstances;
2141+
}
21322142
}

SingularityService/src/main/java/com/hubspot/singularity/data/SingularityValidator.java

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,11 +230,26 @@ public SingularityRequest checkSingularityRequest(
230230
request.getId().length(),
231231
request.getId()
232232
);
233+
233234
checkBadRequest(
234235
!request.getInstances().isPresent() || request.getInstances().get() > 0,
235236
"Instances must be greater than 0"
236237
);
237238

239+
if (singularityConfiguration.allowEmptyRequestInstances()) {
240+
if (
241+
(
242+
request.getRequestType().equals(RequestType.SERVICE) ||
243+
request.getRequestType().equals(RequestType.WORKER)
244+
) &&
245+
existingRequest.flatMap(SingularityRequest::getInstances).isPresent() &&
246+
!request.getInstances().isPresent()
247+
) {
248+
request =
249+
request.toBuilder().setInstances(existingRequest.get().getInstances()).build();
250+
}
251+
}
252+
238253
checkBadRequest(
239254
request.getInstancesSafe() <= maxInstancesPerRequest,
240255
"Instances (%s) be greater than %s (maxInstancesPerRequest in mesos configuration)",

SingularityService/src/test/java/com/hubspot/singularity/data/ValidatorTest.java

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -562,4 +562,30 @@ public void itDoesNotAllowOtherRequestTypesToChange() {
562562
)
563563
);
564564
}
565+
566+
@Test
567+
public void itKeepPreviousInstancesCountWhenEmpty() {
568+
singularityConfiguration.setAllowEmptyRequestInstances(true);
569+
SingularityRequest request = new SingularityRequestBuilder("test", RequestType.WORKER)
570+
.setInstances(Optional.of(5))
571+
.build();
572+
SingularityRequest newRequest = new SingularityRequestBuilder(
573+
"test",
574+
RequestType.WORKER
575+
)
576+
.build();
577+
578+
SingularityRequest returnedRequest = validator.checkSingularityRequest(
579+
newRequest,
580+
Optional.of(request),
581+
Optional.empty(),
582+
Optional.empty()
583+
);
584+
585+
Assertions.assertTrue(returnedRequest.getInstances().isPresent());
586+
Assertions.assertSame(
587+
returnedRequest.getInstances().get(),
588+
request.getInstances().get()
589+
);
590+
}
565591
}

0 commit comments

Comments
 (0)