FIX: Prevent job stuck at Starting status when AgentWatchdog replaces failed agents

agent/apiharness/src/main/java/com/intuit/tank/harness/APIMonitor.java

@@ -83,7 +83,10 @@ private void updateInstanceStatus() {
                 newStatus.setTotalTps(tpsInfo.getTotalTps());
                 sendTps(tpsInfo);
             }
-            if (!isLocal) setInstanceStatus(newStatus.getInstanceId(), newStatus);
+
+            if (!isLocal) {
+                setInstanceStatus(newStatus.getInstanceId(), newStatus);
+            }
             APITestHarness.getInstance().checkAgentThreads();
         } catch (Exception t) {
             LOG.error(LogUtil.getLogMessage("Unable to send status metrics | " + t.getMessage()), t);
@@ -120,13 +123,20 @@ private CloudVmStatus createStatus(WatsAgentStatusResponse agentStatus) {
      */
     private JobStatus calculateJobStatus(WatsAgentStatusResponse agentStatus, JobStatus currentStatus) {
         AgentCommand cmd = APITestHarness.getInstance().getCmd();
-        return cmd == AgentCommand.pause ? JobStatus.Paused
+        JobStatus newStatus = cmd == AgentCommand.pause ? JobStatus.Paused
                 : cmd == AgentCommand.stop ? JobStatus.Stopped
                 : cmd == AgentCommand.pause_ramp ? JobStatus.RampPaused
                 : currentStatus == JobStatus.Unknown
                     || currentStatus == JobStatus.Starting
                     && agentStatus.getCurrentNumberUsers() > 0 ? JobStatus.Running
                 : currentStatus;
+
+        if (newStatus != currentStatus) {
+            LOG.info(LogUtil.getLogMessage("Agent JobStatus transition: " + currentStatus + " -> " + newStatus +
+                " (cmd=" + cmd + ", currentUsers=" + agentStatus.getCurrentNumberUsers() + ")"));
+        }
+
+        return newStatus;
     }
 
     public static void setDoMonitor(boolean monitor) {

agent/apiharness/src/main/java/com/intuit/tank/harness/CommandListener.java

@@ -78,6 +78,8 @@ private static void handleRequest(HttpExchange exchange) {
             String path = exchange.getRequestURI().getPath();
             if (path.equals(AgentCommand.start.getPath()) || path.equals(AgentCommand.run.getPath())) {
                 response = "Received command " + path + ", Starting Test JobId=" + APITestHarness.getInstance().getAgentRunData().getJobId();
+                LOG.info(LogUtil.getLogMessage("Received START command - launching test threads for job " + 
+                    APITestHarness.getInstance().getAgentRunData().getJobId()));
                 startTest();
             } else if (path.startsWith(AgentCommand.stop.getPath())) {
                 response = "Received command " + path + ", Stopping Test JobId=" + APITestHarness.getInstance().getAgentRunData().getJobId();

api/src/main/java/com/intuit/tank/agent/models/VMStatus.java

@@ -22,13 +22,18 @@ public enum VMStatus implements Serializable {
     terminated;
 
     public static final VMStatus fromString(String value) {
-        VMStatus ret = null;
+        if (value == null || value.isEmpty()) {
+            return VMStatus.unknown;
+        }
         if ("shutting-down".equals(value)) {
-            ret = VMStatus.shutting_down;
-        } else {
-            ret = VMStatus.valueOf(value);
+            return VMStatus.shutting_down;
+        }
+        try {
+            return VMStatus.valueOf(value);
+        } catch (IllegalArgumentException e) {
+            // Gracefully handle unknown values (e.g., 'replaced' from controller which agent doesn't need)
+            return VMStatus.unknown;
         }
-        return ret != null ? ret : VMStatus.unknown;
     }
 
 }

api/src/main/java/com/intuit/tank/vm/vmManager/VMInformation.java

@@ -96,6 +96,44 @@ public String getPrivateDNS() {
         return (String) this.items.get("privateDns");
     }
 
+    /**
+     * Set the virtual machine private IP address
+     * 
+     * @param data
+     *            The virtual machine's private IP address
+     */
+    public void setPrivateIp(String data) {
+        this.items.put("privateIp", data);
+    }
+
+    /**
+     * Get the virtual machine private IP address
+     * 
+     * @return The virtual machine's private IP address
+     */
+    public String getPrivateIp() {
+        return (String) this.items.get("privateIp");
+    }
+
+    /**
+     * Set the virtual machine public IP address
+     * 
+     * @param data
+     *            The virtual machine's public IP address
+     */
+    public void setPublicIp(String data) {
+        this.items.put("publicIp", data);
+    }
+
+    /**
+     * Get the virtual machine public IP address
+     * 
+     * @return The virtual machine's public IP address
+     */
+    public String getPublicIp() {
+        return (String) this.items.get("publicIp");
+    }
+
     public void setLaunchTime(Calendar data) {
         this.items.put("launchTime", data);
     }

api/src/main/java/com/intuit/tank/vm/vmManager/models/VMStatus.java

@@ -26,16 +26,22 @@ public enum VMStatus implements Serializable {
     stopping,
     stopped,
     shutting_down,
-    terminated;
+    terminated,
+    replaced;  // replaced by AgentWatchdog due to failure to report back
 
     public static final VMStatus fromString(String value) {
-        VMStatus ret = null;
+        if (value == null || value.isEmpty()) {
+            return VMStatus.unknown;
+        }
         if ("shutting-down".equals(value)) {
-            ret = VMStatus.shutting_down;
-        } else {
-            ret = VMStatus.valueOf(value);
+            return VMStatus.shutting_down;
+        }
+        try {
+            return VMStatus.valueOf(value);
+        } catch (IllegalArgumentException e) {
+            // Gracefully handle unknown values (e.g., from older/newer clients with different enum versions)
+            return VMStatus.unknown;
         }
-        return ret != null ? ret : VMStatus.unknown;
     }
 
 }

api/src/test/java/com/intuit/tank/agent/models/VMStatusTest.java

@@ -0,0 +1,75 @@
+package com.intuit.tank.agent.models;
+
+import org.junit.jupiter.api.*;
+
+import static org.junit.jupiter.api.Assertions.*;
+
+/**
+ * Tests for the agent-side VMStatus enum.
+ * This enum is used by agents and doesn't include the 'replaced' status
+ * (which is only meaningful on the controller side).
+ */
+public class VMStatusTest {
+
+    @Test
+    @DisplayName("fromString handles shutting-down special case")
+    public void testFromString_shuttingDown() {
+        VMStatus result = VMStatus.fromString("shutting-down");
+
+        assertNotNull(result);
+        assertEquals(VMStatus.shutting_down, result);
+    }
+
+    @Test
+    @DisplayName("fromString returns correct enum for valid values")
+    public void testFromString_validValues() {
+        assertEquals(VMStatus.running, VMStatus.fromString("running"));
+        assertEquals(VMStatus.pending, VMStatus.fromString("pending"));
+        assertEquals(VMStatus.starting, VMStatus.fromString("starting"));
+        assertEquals(VMStatus.rebooting, VMStatus.fromString("rebooting"));
+        assertEquals(VMStatus.terminated, VMStatus.fromString("terminated"));
+        assertEquals(VMStatus.stopped, VMStatus.fromString("stopped"));
+        assertEquals(VMStatus.stopping, VMStatus.fromString("stopping"));
+        assertEquals(VMStatus.rampPaused, VMStatus.fromString("rampPaused"));
+    }
+
+    @Test
+    @DisplayName("fromString returns unknown for null input")
+    public void testFromString_nullReturnsUnknown() {
+        VMStatus result = VMStatus.fromString(null);
+
+        assertNotNull(result);
+        assertEquals(VMStatus.unknown, result);
+    }
+
+    @Test
+    @DisplayName("fromString returns unknown for empty string")
+    public void testFromString_emptyReturnsUnknown() {
+        VMStatus result = VMStatus.fromString("");
+
+        assertNotNull(result);
+        assertEquals(VMStatus.unknown, result);
+    }
+
+    @Test
+    @DisplayName("fromString returns unknown for unrecognized values")
+    public void testFromString_unknownValueReturnsUnknown() {
+        // Should not throw IllegalArgumentException - gracefully returns unknown
+        VMStatus result = VMStatus.fromString("garbage-value");
+
+        assertNotNull(result);
+        assertEquals(VMStatus.unknown, result);
+    }
+
+    @Test
+    @DisplayName("fromString handles 'replaced' from controller gracefully (returns unknown)")
+    public void testFromString_replacedFromControllerReturnsUnknown() {
+        // The agent-side VMStatus doesn't have a 'replaced' enum value
+        // When the controller sends 'replaced', the agent should handle it gracefully
+        VMStatus result = VMStatus.fromString("replaced");
+
+        assertNotNull(result);
+        assertEquals(VMStatus.unknown, result);
+    }
+}
+

api/src/test/java/com/intuit/tank/vm/vmManager/models/VMStatusTest.java

@@ -19,50 +19,85 @@
 
 /**
  * The class <code>VMStatusTest</code> contains tests for the class <code>{@link VMStatus}</code>.
- *
- * @generatedBy CodePro at 12/15/14 2:57 PM
  */
 public class VMStatusTest {
-    /**
-     * Run the VMStatus fromString(String) method test.
-     *
-     * @throws Exception
-     *
-     * @generatedBy CodePro at 12/15/14 2:57 PM
-     */
+
     @Test
-    @Disabled
-    public void testFromString_1()
-        throws Exception {
-        String value = "shutting-down";
+    @DisplayName("fromString handles shutting-down special case")
+    public void testFromString_shuttingDown() {
+        VMStatus result = VMStatus.fromString("shutting-down");
+
+        assertNotNull(result);
+        assertEquals(VMStatus.shutting_down, result);
+    }
 
-        VMStatus result = VMStatus.fromString(value);
+    @Test
+    @DisplayName("fromString returns correct enum for valid values")
+    public void testFromString_validValues() {
+        assertEquals(VMStatus.running, VMStatus.fromString("running"));
+        assertEquals(VMStatus.pending, VMStatus.fromString("pending"));
+        assertEquals(VMStatus.starting, VMStatus.fromString("starting"));
+        assertEquals(VMStatus.ready, VMStatus.fromString("ready"));
+        assertEquals(VMStatus.rebooting, VMStatus.fromString("rebooting"));
+        assertEquals(VMStatus.terminated, VMStatus.fromString("terminated"));
+        assertEquals(VMStatus.stopped, VMStatus.fromString("stopped"));
+        assertEquals(VMStatus.stopping, VMStatus.fromString("stopping"));
+        assertEquals(VMStatus.rampPaused, VMStatus.fromString("rampPaused"));
+    }
 
+    @Test
+    @DisplayName("fromString returns replaced for 'replaced' value")
+    public void testFromString_replaced() {
+        VMStatus result = VMStatus.fromString("replaced");
+
         assertNotNull(result);
-        assertEquals("shutting_down", result.name());
-        assertEquals("shutting_down", result.toString());
-        assertEquals(6, result.ordinal());
+        assertEquals(VMStatus.replaced, result);
     }
 
-    /**
-     * Run the VMStatus fromString(String) method test.
-     *
-     * @throws Exception
-     *
-     * @generatedBy CodePro at 12/15/14 2:57 PM
-     */
     @Test
-    public void testFromString_2()
-        throws Exception {
-        String value = VMStatus.rebooting.name();
+    @DisplayName("fromString returns unknown for null input")
+    public void testFromString_nullReturnsUnknown() {
+        VMStatus result = VMStatus.fromString(null);
+
+        assertNotNull(result);
+        assertEquals(VMStatus.unknown, result);
+    }
 
-        VMStatus result = VMStatus.fromString(value);
+    @Test
+    @DisplayName("fromString returns unknown for empty string")
+    public void testFromString_emptyReturnsUnknown() {
+        VMStatus result = VMStatus.fromString("");
+
+        assertNotNull(result);
+        assertEquals(VMStatus.unknown, result);
+    }
 
-        // An unexpected exception was thrown in user code while executing this test:
-        //    java.lang.IllegalArgumentException: No enum constant com.intuit.tank.vm.vmManager.models.VMStatus.
-        //       at java.lang.Enum.valueOf(Enum.java:238)
-        //       at com.intuit.tank.vm.vmManager.models.VMStatus.valueOf(VMStatus.java:5)
-        //       at com.intuit.tank.vm.vmManager.models.VMStatus.fromString(VMStatus.java:20)
+    @Test
+    @DisplayName("fromString returns unknown for unrecognized values (graceful handling)")
+    public void testFromString_unknownValueReturnsUnknown() {
+        // Should not throw IllegalArgumentException - gracefully returns unknown
+        VMStatus result = VMStatus.fromString("garbage-value");
+
         assertNotNull(result);
+        assertEquals(VMStatus.unknown, result);
+    }
+
+    @Test
+    @DisplayName("fromString handles future/unknown enum values gracefully")
+    public void testFromString_futureCompatibility() {
+        // Simulates receiving a value from a newer version of the API
+        VMStatus result = VMStatus.fromString("some_new_status_from_future");
+
+        assertNotNull(result);
+        assertEquals(VMStatus.unknown, result);
+    }
+
+    @Test
+    @DisplayName("replaced is a terminal state (for documentation)")
+    public void testReplaced_isTerminalState() {
+        // This test documents that 'replaced' is intended as a terminal state
+        // like 'terminated', used by AgentWatchdog when replacing failed agents
+        assertNotNull(VMStatus.replaced);
+        assertEquals("replaced", VMStatus.replaced.name());
     }
 }

rest-mvc/impl/src/main/java/com/intuit/tank/rest/mvc/rest/cloud/JobEventSender.java

@@ -247,7 +247,18 @@ private List<String> getInstancesForJob(String jobId) {
         List<String> instanceIds = new ArrayList<String>();
         CloudVmStatusContainer statuses = vmTracker.getVmStatusForJob(jobId);
         if (statuses != null) {
-            instanceIds = statuses.getStatuses().stream().map(CloudVmStatus::getInstanceId).collect(Collectors.toList());
+            instanceIds = statuses.getStatuses().stream()
+                .filter(s -> {
+                    VMStatus vmStatus = s.getVmStatus();
+                    // skip unreachable instances - they can't receive commands
+                    return vmStatus != VMStatus.terminated &&
+                           vmStatus != VMStatus.replaced &&
+                           vmStatus != VMStatus.stopped &&
+                           vmStatus != VMStatus.shutting_down &&
+                           vmStatus != VMStatus.stopping;
+                })
+                .map(CloudVmStatus::getInstanceId)
+                .collect(Collectors.toList());
         }
         return instanceIds;
     }
@@ -260,8 +271,10 @@ public CloudVmStatus getVmStatus(String instanceId) {
 
     public void setVmStatus(final String instanceId, final CloudVmStatus status) {
         vmTracker.setStatus(status);
-        if (status.getJobStatus() == JobStatus.Completed || status.getVmStatus() == VMStatus.terminated) {
-            // will terrminate instance after waiting for some cleanup time
+        if (status.getJobStatus() == JobStatus.Completed
+                || status.getVmStatus() == VMStatus.terminated
+                || status.getVmStatus() == VMStatus.replaced) {
+            // will terminate instance after waiting for some cleanup time
             terminator.terminate(status.getInstanceId());
             // check job status and kill off instances appropriately
             checkJobStatus(status.getJobId());

rest-mvc/impl/src/main/java/com/intuit/tank/rest/mvc/rest/services/agent/AgentServiceV2Impl.java

@@ -220,6 +220,11 @@ public void setInstanceStatus(String instanceId, CloudVmStatus status) {
         segment.putAnnotation("currentUsers", status.getCurrentUsers());
         segment.putAnnotation("TotalUsers", status.getTotalUsers());
         segment.putAnnotation("totalTps", status.getTotalTps());
+
+        LOGGER.debug("Agent " + instanceId + " reporting status - VMStatus: " + status.getVmStatus() + 
+            ", JobStatus: " + status.getJobStatus() + ", Users: " + status.getCurrentUsers() + 
+            "/" + status.getTotalUsers() + ", Job: " + status.getJobId());
+
         try {
             JobEventSender controller = new ServletInjector<JobEventSender>().getManagedBean(
                     servletContext, JobEventSender.class);