Merge pull request #6497 from vespa-engine/mpolden/release-lock-in-test

Ensure test releases lock

2 files changed, 29 insertions, 23 deletions

diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockDeployer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockDeployer.java
index 4111a3aee1c..368f1fef776 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockDeployer.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/testutils/MockDeployer.java
@@ -67,8 +67,8 @@ public class MockDeployer implements Deployer {
 
     @Override
     public Optional<Deployment> deployFromLocalActive(ApplicationId id, Duration timeout) {
+        lock.lock();
         try {
-            lock.lock();
             lastDeployTimes.put(id, clock.instant());
             return Optional.of(new MockDeployment(provisioner, applications.get(id)));
         } finally {
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/PeriodicApplicationMaintainerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/PeriodicApplicationMaintainerTest.java
index 34e2bd633f3..22ce879cac6 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/PeriodicApplicationMaintainerTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/PeriodicApplicationMaintainerTest.java
@@ -180,28 +180,34 @@ public class PeriodicApplicationMaintainerTest {
         // Lock deployer to simulate slow deployments
         fixture.deployer.lock().lock();
 
-        // Queues all eligible applications
-        assertEquals(2, fixture.maintainer.applicationsNeedingMaintenance().size());
-        fixture.runApplicationMaintainer(false);
-        assertEquals(2, fixture.maintainer.pendingDeployments());
-
-        // Enough time passes to make applications eligible for another periodic deployment
-        clock.advance(Duration.ofMinutes(30).plus(Duration.ofSeconds(1)));
-        fixture.runApplicationMaintainer(false);
-
-        // Deployments are not re-queued as previous deployments are still pending
-        assertEquals(2, fixture.maintainer.pendingDeployments());
-
-        // Slow deployments complete
-        fixture.deployer.lock().unlock();
-        fixture.runApplicationMaintainer();
-        Instant deployTime = clock.instant();
-        assertEquals(deployTime, fixture.deployer.lastDeployTime(fixture.app1).get());
-        assertEquals(deployTime, fixture.deployer.lastDeployTime(fixture.app2).get());
-
-        // Too soon: Already deployed recently
-        clock.advance(Duration.ofMinutes(5));
-        assertEquals(0, fixture.maintainer.applicationsNeedingMaintenance().size());
+        try {
+            // Queues all eligible applications
+            assertEquals(2, fixture.maintainer.applicationsNeedingMaintenance().size());
+            fixture.runApplicationMaintainer(false);
+            assertEquals(2, fixture.maintainer.pendingDeployments());
+
+            // Enough time passes to make applications eligible for another periodic deployment
+            clock.advance(Duration.ofMinutes(30).plus(Duration.ofSeconds(1)));
+            fixture.runApplicationMaintainer(false);
+
+            // Deployments are not re-queued as previous deployments are still pending
+            assertEquals(2, fixture.maintainer.pendingDeployments());
+
+            // Slow deployments complete
+            fixture.deployer.lock().unlock();
+            fixture.runApplicationMaintainer();
+            Instant deployTime = clock.instant();
+            assertEquals(deployTime, fixture.deployer.lastDeployTime(fixture.app1).get());
+            assertEquals(deployTime, fixture.deployer.lastDeployTime(fixture.app2).get());
+
+            // Too soon: Already deployed recently
+            clock.advance(Duration.ofMinutes(5));
+            assertEquals(0, fixture.maintainer.applicationsNeedingMaintenance().size());
+        } finally {
+            if (fixture.deployer.lock().isHeldByCurrentThread()) {
+                fixture.deployer.lock().unlock();
+            }
+        }
     }
 
     private void createReadyNodes(int count, NodeRepository nodeRepository, NodeFlavors nodeFlavors) {