Skip to content
This repository has been archived by the owner on Oct 23, 2024. It is now read-only.

Commit

Permalink
ForceExpunge for missing task is a no-op, rather than a failure
Browse files Browse the repository at this point in the history
Summary: Address #5142

Test Plan: add tests

Reviewers: jasongilanfarr, meichstedt

Subscribers: marathon-team

Differential Revision: https://phabricator.mesosphere.com/D498
  • Loading branch information
Tim Harper committed Feb 9, 2017
1 parent 35506a3 commit 5460a74
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -46,28 +46,36 @@ private[marathon] class InstanceUpdateOpResolver(
createInstance(op.instanceId)(updater.reserve(op, clock.now()))

case op: ForceExpunge =>
updateExistingInstance(op.instanceId)(updater.forceExpunge(_, clock.now()))
updateExistingInstance(op.instanceId)(
applyOperation = updater.forceExpunge(_, clock.now() ),
ifNotExist = InstanceUpdateEffect.Noop(_))

case op: Revert =>
Future.successful(updater.revert(op.instance))
}
}

private[this] def failIfNotExist(id: Instance.Id): InstanceUpdateEffect = {
InstanceUpdateEffect.Failure(
new IllegalStateException(s"$id of app [${id.runSpecId}] does not exist"))
}

/**
* Helper method that verifies that an instance already exists. If it does, it will apply the given function and
* return the resulting effect; otherwise this will result in a. [[InstanceUpdateEffect.Failure]].
* @param id ID of the instance that is expected to exist.
* @param applyOperation the operation that shall be applied to the instance
* @return The [[InstanceUpdateEffect]] that results from applying the given operation.
*/
private[this] def updateExistingInstance(id: Instance.Id)(applyOperation: Instance => InstanceUpdateEffect)(implicit ec: ExecutionContext): Future[InstanceUpdateEffect] = {
private[this] def updateExistingInstance(id: Instance.Id)(
applyOperation: Instance => InstanceUpdateEffect,
ifNotExist: Instance.Id => InstanceUpdateEffect = failIfNotExist(_))(implicit ec: ExecutionContext): Future[InstanceUpdateEffect] = {
directInstanceTracker.instance(id).map {
case Some(existingInstance) =>
applyOperation(existingInstance)

case None =>
InstanceUpdateEffect.Failure(
new IllegalStateException(s"$id of app [${id.runSpecId}] does not exist"))
ifNotExist(id)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ class InstanceUpdateOpResolverTest extends UnitTest {
When("call taskTracker.task")
verify(instanceTracker).instance(notExistingInstanceId)

Then("result in a Failure")
stateChange shouldBe a[InstanceUpdateEffect.Failure]
Then("result in a Noop")
stateChange shouldBe a[InstanceUpdateEffect.Noop]

verifyNoMoreInteractions()
}
Expand Down

0 comments on commit 5460a74

Please sign in to comment.