HDDS-16387. ContainerBalancer status --verbose throws IllegalStateException after a rejected start.

hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/balancer/ContainerBalancer.java

@@ -250,7 +250,6 @@ private boolean canBalancerStop() {
   @Override
   public void start() throws IllegalContainerBalancerStateException,
       InvalidContainerBalancerConfigurationException {
-    startedAt = OffsetDateTime.now();
     lock.lock();
     try {
       // should be leader-ready, out of safe mode, and not running already
@@ -277,6 +276,9 @@ public void start() throws IllegalContainerBalancerStateException,
               ozoneConfiguration);
       validateConfiguration(configuration);
       this.config = configuration;
+      // Set startedAt only after validation has passed, so a rejected start
+      // does not overwrite the recorded start time of the previous run.
+      startedAt = OffsetDateTime.now();
       startBalancingThread(proto.getNextIterationIndex(), true);
     } finally {
       lock.unlock();
@@ -298,7 +300,6 @@ public void start() throws IllegalContainerBalancerStateException,
   public void startBalancer(ContainerBalancerConfiguration configuration)
       throws IllegalContainerBalancerStateException,
       InvalidContainerBalancerConfigurationException, IOException {
-    startedAt = OffsetDateTime.now();
     lock.lock();
     try {
       // validates state, config, and then saves config
@@ -307,6 +308,9 @@ public void startBalancer(ContainerBalancerConfiguration configuration)
       saveConfiguration(configuration, true, 0);
       this.config = configuration;
 
+      // Set startedAt only after validation has passed, so a rejected start
+      // does not overwrite the recorded start time of the previous run.
+      startedAt = OffsetDateTime.now();
       //start balancing task
       startBalancingThread(0, false);
     } finally {

hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/container/balancer/TestContainerBalancer.java

@@ -38,6 +38,7 @@
 
 import com.google.protobuf.ByteString;
 import java.io.IOException;
+import java.time.OffsetDateTime;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
@@ -459,6 +460,55 @@ public void testRejectInvalidStartupConfiguration() throws Exception {
     assertSame(ContainerBalancerTask.Status.STOPPED, containerBalancer.getBalancerStatus());
   }
 
+  /**
+   * Verifies that a start request rejected during validation does not update startedAt of last run.
+   * Prevents startedAt from advancing past stoppedAt, which previously caused a negative duration and CLI status crash.
+   */
+  @Test
+  public void testRejectedStartDoesNotModifyStartedAt() throws Exception {
+    // The test first starts and immediately stops the balancer normally. This creates a realistic "previous state"
+    // in system memory and later trigger a bad request and verify that startedAt remains identical to startedAtBefore,
+    // rather than being wrongly overwritten with "now", which would make startedAt come after stoppedAt which resulted
+    // in a negative balancing duration.
+    startBalancer(balancerConfiguration);
+    assertSame(ContainerBalancerTask.Status.RUNNING, containerBalancer.getBalancerStatus());
+    stopBalancer();
+    assertSame(ContainerBalancerTask.Status.STOPPED, containerBalancer.getBalancerStatus());
+
+    ContainerBalancerStatusInfo before = containerBalancer.getBalancerStatusInfo();
+    assertNotNull(before);
+    OffsetDateTime startedAtBefore = before.getStartedAt();
+    OffsetDateTime stoppedAt = before.getStoppedAt();
+    assertNotNull(startedAtBefore);
+    assertNotNull(stoppedAt);
+
+    // Pause so that system time advances before the second start attempt so a timestamp overwrite becomes measurable.
+    Thread.sleep(20);
+
+    // A start that is rejected during validation. Here we used an invalid config, this is just one of many rejection
+    // paths (safe mode, non-leader SCM, already running, etc.).
+
+    balancerConfiguration.setMoveReplicationTimeout(TimeUnit.MINUTES.toMillis(60));
+    balancerConfiguration.setMoveTimeout(TimeUnit.MINUTES.toMillis(59));
+    assertThrows(InvalidContainerBalancerConfigurationException.class,
+        () -> containerBalancer.startBalancer(balancerConfiguration));
+
+    // The rejected start must leave the balancer STOPPED.
+    assertSame(ContainerBalancerTask.Status.STOPPED, containerBalancer.getBalancerStatus());
+
+    ContainerBalancerStatusInfo after = containerBalancer.getBalancerStatusInfo();
+    assertNotNull(after);
+
+    // startedAt must still equal the previous run's start time, a rejected start must not reset it.
+    assertEquals(startedAtBefore, after.getStartedAt(),
+        "Rejected start reset startedAt to 'now' (set before validation), replacing the previous run's start time");
+
+    // startedAt must never be after stoppedAt. Violating this is
+    // what produced the negative duration and the status --verbose crash.
+    assertFalse(after.getStartedAt().isAfter(stoppedAt),
+        "startedAt ended up after stoppedAt -> negative balancing duration that crashes 'status --verbose'");
+  }
+
   private static List<DatanodeInfo> createEligibleDatanodes(int count) {
     List<DatanodeInfo> datanodes = new ArrayList<>(count);
     for (int i = 0; i < count; i++) {