Don't stop checking if the `HealthNode` persistent task is present (#105449)

We assumed that once the `HealthNode` persistent task is registered,
we won't need to register it again. However, when, for instance, we
restore from a snapshot (including cluster state) that was created
in version <= 8.4.3, that task doesn't exist yet, which will result
in the task being removed after the restore. By keeping the listener
active, we will re-add the task after such a restore (or any other
potential situation where the task might get deleted).

docs/changelog/105449.yaml

@@ -0,0 +1,6 @@
+pr: 105449
+summary: Don't stop checking if the `HealthNode` persistent task is present
+area: Health
+type: bug
+issues:
+ - 98926

server/src/main/java/org/elasticsearch/health/node/selection/HealthNodeTaskExecutor.java

@@ -159,9 +159,6 @@ public final class HealthNodeTaskExecutor extends PersistentTasksExecutor<Health
         if (event.state().clusterRecovered() && featureService.clusterHasFeature(event.state(), HealthFeatures.SUPPORTS_HEALTH)) {
             boolean healthNodeTaskExists = HealthNode.findTask(event.state()) != null;
             boolean isElectedMaster = event.localNodeMaster();
-            if (isElectedMaster || healthNodeTaskExists) {
-                clusterService.removeListener(taskStarter);
-            }
             if (isElectedMaster && healthNodeTaskExists == false) {
                 persistentTasksService.sendStartRequest(
                     TASK_NAME,

server/src/test/java/org/elasticsearch/health/node/LocalHealthMonitorTests.java

@@ -27,7 +27,6 @@ import org.elasticsearch.features.FeatureService;
 import org.elasticsearch.health.HealthFeatures;
 import org.elasticsearch.health.HealthStatus;
 import org.elasticsearch.health.metadata.HealthMetadata;
-import org.elasticsearch.health.node.selection.HealthNodeExecutorTests;
 import org.elasticsearch.health.node.tracker.HealthTracker;
 import org.elasticsearch.test.ESTestCase;
 import org.elasticsearch.threadpool.TestThreadPool;
@@ -63,7 +62,7 @@ public class LocalHealthMonitorTests extends ESTestCase {
 
     @BeforeClass
     public static void setUpThreadPool() {
-        threadPool = new TestThreadPool(HealthNodeExecutorTests.class.getSimpleName());
+        threadPool = new TestThreadPool(LocalHealthMonitorTests.class.getSimpleName());
     }
 
     @AfterClass

server/src/test/java/org/elasticsearch/health/node/selection/HealthNodeExecutorTests.java → server/src/test/java/org/elasticsearch/health/node/selection/HealthNodeTaskExecutorTests.java

@@ -8,6 +8,7 @@
 
 package org.elasticsearch.health.node.selection;
 
+import org.elasticsearch.action.ActionListener;
 import org.elasticsearch.cluster.ClusterChangedEvent;
 import org.elasticsearch.cluster.ClusterName;
 import org.elasticsearch.cluster.ClusterState;
@@ -46,7 +47,7 @@ import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 
-public class HealthNodeExecutorTests extends ESTestCase {
+public class HealthNodeTaskExecutorTests extends ESTestCase {
 
     /** Needed by {@link ClusterService} **/
     private static ThreadPool threadPool;
@@ -66,7 +67,7 @@ public class HealthNodeExecutorTests extends ESTestCase {
 
     @BeforeClass
     public static void setUpThreadPool() {
-        threadPool = new TestThreadPool(HealthNodeExecutorTests.class.getSimpleName());
+        threadPool = new TestThreadPool(HealthNodeTaskExecutorTests.class.getSimpleName());
     }
 
     @Before
@@ -91,20 +92,18 @@ public class HealthNodeExecutorTests extends ESTestCase {
         clusterService.close();
     }
 
-    public void testTaskCreation() {
-        HealthNodeTaskExecutor executor = HealthNodeTaskExecutor.create(
-            clusterService,
-            persistentTasksService,
-            featureService,
-            settings,
-            clusterSettings
-        );
-        executor.startTask(new ClusterChangedEvent("", initialState(), ClusterState.EMPTY_STATE));
-        verify(persistentTasksService, times(1)).sendStartRequest(
-            eq("health-node"),
-            eq("health-node"),
-            eq(new HealthNodeTaskParams()),
-            any()
+    public void testTaskCreation() throws Exception {
+        HealthNodeTaskExecutor.create(clusterService, persistentTasksService, featureService, settings, clusterSettings);
+        clusterService.getClusterApplierService().onNewClusterState("initialization", this::initialState, ActionListener.noop());
+        // Ensure that if the task is gone, it will be recreated.
+        clusterService.getClusterApplierService().onNewClusterState("initialization", this::initialState, ActionListener.noop());
+        assertBusy(
+            () -> verify(persistentTasksService, times(2)).sendStartRequest(
+                eq("health-node"),
+                eq("health-node"),
+                eq(new HealthNodeTaskParams()),
+                any()
+            )
         );
     }