Skip to content

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:
If a resident app is deleted before Marathon has a chance to
release the associated resources, then later attempts to release them
fail because because we ForceExpunge the task-ID when processing our
UnreserveAndDestroyVolumes operation.

This change makes ForceExpunge for a non-existent task respond with
no-op, rather than failure.

Fixes #5142

Backport of f441c57

Test Plan: n/a

Reviewers: jasongilanfarr, meichstedt

Subscribers: marathon-team

Differential Revision: https://phabricator.mesosphere.com/D547
  • Loading branch information
Tim Harper committed Feb 22, 2017
1 parent 1637c31 commit 2b2b0e4
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,13 @@ private[marathon] class InstanceUpdateOpResolver(
createInstance(op.instanceId)(updater.reserve(op, clock.now()))

case op: ForceExpunge =>
updateExistingInstance(op.instanceId)(updater.forceExpunge(_, clock.now()))
directInstanceTracker.instance(op.instanceId).map {
case Some(existingInstance) =>
updater.forceExpunge(existingInstance, clock.now())

case None =>
InstanceUpdateEffect.Noop(op.instanceId)
}

case op: Revert =>
Future.successful(updater.revert(op.instance))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ class InstanceUpdateOpResolverTest extends UnitTest {
f.instanceTracker.instance(f.notExistingInstanceId) returns Future.successful(None)
val stateChange = f.updateOpResolver.resolve(InstanceUpdateOperation.ForceExpunge(f.notExistingInstanceId)).futureValue
"call taskTracker.task" in { verify(f.instanceTracker).instance(f.notExistingInstanceId) }
"result in a Failure" in { stateChange shouldBe a[InstanceUpdateEffect.Failure] }
"result in a Failure" in { stateChange shouldBe a[InstanceUpdateEffect.Noop] }
"invoke no more interactions" in { f.verifyNoMoreInteractions() }
}

Expand Down

0 comments on commit 2b2b0e4

Please sign in to comment.