Using sc for service start/stop in tests, plus busy waiting (#137451)

This PR attempts to fix (definitely) Windows services tests.

We used procrun as a service control tool; however, procrun (as a service) suffers from a race condition and procrun as a tool seems to suffer from it.
Furthermore, the implementation of stop in procrun is (at the very least), "strange": it seems to wait for he service to stop, but it's not guaranteed to wait till the end, returning an error (potentially)
In general, this is true for all (most) of the service control tools: the return when the service is still in a STOP_PENDING state. We actually have to "busy wait" for the service to be really STOPPED.

This PR adds the busy wait and adjusts how we stop the service slightly. It also moves to the standard Windows sc.exe tool for service control operations (start, stop and delete) in tests.

Fixes: #113177
Fixes: #113160
Fixes: #113219
Fixes: #113313

Author: ldematte

muted-tests.yml

@@ -29,18 +29,6 @@ tests:
 - class: org.elasticsearch.xpack.sql.qa.security.JdbcSqlSpecIT
   method: test {case-functions.testSelectInsertWithLcaseAndLengthWithOrderBy}
   issue: https://github.com/elastic/elasticsearch/issues/112642
-- class: org.elasticsearch.packaging.test.WindowsServiceTests
-  method: test30StartStop
-  issue: https://github.com/elastic/elasticsearch/issues/113160
-- class: org.elasticsearch.packaging.test.WindowsServiceTests
-  method: test33JavaChanged
-  issue: https://github.com/elastic/elasticsearch/issues/113177
-- class: org.elasticsearch.packaging.test.WindowsServiceTests
-  method: test80JavaOptsInEnvVar
-  issue: https://github.com/elastic/elasticsearch/issues/113219
-- class: org.elasticsearch.packaging.test.WindowsServiceTests
-  method: test81JavaOptsInJvmOptions
-  issue: https://github.com/elastic/elasticsearch/issues/113313
 - class: org.elasticsearch.xpack.transform.integration.TransformIT
   method: testStopWaitForCheckpoint
   issue: https://github.com/elastic/elasticsearch/issues/106113

qa/packaging/src/test/java/org/elasticsearch/packaging/test/WindowsServiceTests.java

@@ -20,6 +20,7 @@
 import java.io.UncheckedIOException;
 import java.nio.file.Files;
 import java.nio.file.Path;
+import java.time.Duration;
 
 import static com.carrotsearch.randomizedtesting.RandomizedTest.assumeTrue;
 import static org.elasticsearch.packaging.util.Archives.installArchive;
@@ -44,7 +45,19 @@ public static void ensureWindows() {
 
     @After
     public void uninstallService() {
-        sh.runIgnoreExitCode(serviceScript + " remove");
+        sh.runIgnoreExitCode(deleteCommand(DEFAULT_ID));
+    }
+
+    private static String startCommand(String serviceId) {
+        return "\"sc.exe start " + serviceId + "\"";
+    }
+
+    private static String stopCommand(String serviceId) {
+        return "\"sc.exe stop " + serviceId + "\"";
+    }
+
+    private static String deleteCommand(String serviceId) {
+        return "\"sc.exe delete " + serviceId + "\"";
     }
 
     private void assertService(String id, String status) {
@@ -53,6 +66,29 @@ private void assertService(String id, String status) {
         assertThat(result.stdout(), containsString("Status      : " + status));
     }
 
+    private void waitForStop(String id, Duration timeout) {
+        var stopped = false;
+        var start = System.currentTimeMillis();
+        while (stopped == false) {
+            Result result = sh.run("(Get-Service  " + id + " ).\"Status\"");
+            if (result.exitCode() != 0) {
+                logger.warn("Cannot get status for {}: stdout:[{}], stderr:[{}]", id, result.stdout(), result.stderr());
+                break;
+            }
+            stopped = "Stopped".equalsIgnoreCase(result.stdout());
+            Duration elapsed = Duration.ofMillis(System.currentTimeMillis() - start);
+            if (elapsed.compareTo(timeout) > 0) {
+                logger.warn("Timeout waiting for stop {}: stdout:[{}], stderr:[{}]", id, result.stdout(), result.stderr());
+                break;
+            }
+            try {
+                Thread.sleep(2000);
+            } catch (InterruptedException e) {
+                break;
+            }
+        }
+    }
+
     // runs the service command, dumping all log files on failure
     private Result assertCommand(String script) {
         Result result = sh.runIgnoreExitCode(script);
@@ -108,12 +144,12 @@ public void test10InstallArchive() throws Exception {
     public void test12InstallService() {
         sh.run(serviceScript + " install");
         assertService(DEFAULT_ID, "Stopped");
-        sh.run(serviceScript + " remove");
+        sh.run(deleteCommand(DEFAULT_ID));
     }
 
     public void test15RemoveNotInstalled() {
-        Result result = assertFailure(serviceScript + " remove", 1);
-        assertThat(result.stderr(), containsString("Failed removing '" + DEFAULT_ID + "' service"));
+        Result result = assertFailure(deleteCommand(DEFAULT_ID), 1);
+        assertThat(result.stdout(), containsString("The specified service does not exist as an installed service"));
     }
 
     public void test16InstallSpecialCharactersInJdkPath() throws IOException {
@@ -126,33 +162,37 @@ public void test16InstallSpecialCharactersInJdkPath() throws IOException {
             Result result = sh.run(serviceScript + " install");
             assertThat(result.stdout(), containsString("The service 'elasticsearch-service-x64' has been installed"));
         } finally {
-            sh.runIgnoreExitCode(serviceScript + " remove");
+            sh.runIgnoreExitCode(deleteCommand(DEFAULT_ID));
             mv(relocatedJdk, installation.bundledJdk);
         }
     }
 
     public void test20CustomizeServiceId() {
         String serviceId = "my-es-service";
-        sh.getEnv().put("SERVICE_ID", serviceId);
-        sh.run(serviceScript + " install");
-        assertService(serviceId, "Stopped");
-        sh.run(serviceScript + " remove");
+        try {
+            sh.getEnv().put("SERVICE_ID", serviceId);
+            sh.run(serviceScript + " install");
+            assertService(serviceId, "Stopped");
+        } finally {
+            sh.run(deleteCommand(serviceId));
+        }
     }
 
     public void test21CustomizeServiceDisplayName() {
         String displayName = "my es service display name";
         sh.getEnv().put("SERVICE_DISPLAY_NAME", displayName);
         sh.run(serviceScript + " install");
         assertService(DEFAULT_ID, "Stopped");
-        sh.run(serviceScript + " remove");
+        sh.run(deleteCommand(DEFAULT_ID));
     }
 
     // NOTE: service description is not attainable through any powershell api, so checking it is not possible...
     public void assertStartedAndStop() throws Exception {
         ServerUtils.waitForElasticsearch(installation);
         runElasticsearchTests();
 
-        assertCommand(serviceScript + " stop");
+        assertCommand(stopCommand(DEFAULT_ID));
+        waitForStop(DEFAULT_ID, Duration.ofMinutes(1));
         assertService(DEFAULT_ID, "Stopped");
         // the process is stopped async, and can become a zombie process, so we poll for the process actually being gone
         assertCommand(
@@ -170,36 +210,25 @@ public void assertStartedAndStop() throws Exception {
                 + "} while ($i -lt 300);"
                 + "exit 9;"
         );
-
-        assertCommand(serviceScript + " remove");
-        assertCommand(
-            "$p = Get-Service -Name \"elasticsearch-service-x64\" -ErrorAction SilentlyContinue;"
-                + "echo \"$p\";"
-                + "if ($p -eq $Null) {"
-                + "  exit 0;"
-                + "} else {"
-                + "  exit 1;"
-                + "}"
-        );
     }
 
     public void test30StartStop() throws Exception {
         sh.run(serviceScript + " install");
-        assertCommand(serviceScript + " start");
+        assertCommand(startCommand(DEFAULT_ID));
         assertStartedAndStop();
     }
 
     public void test31StartNotInstalled() throws IOException {
-        Result result = sh.runIgnoreExitCode(serviceScript + " start");
+        Result result = sh.runIgnoreExitCode(startCommand(DEFAULT_ID));
         assertThat(result.stderr(), result.exitCode(), equalTo(1));
         dumpServiceLogs();
-        assertThat(result.stderr(), containsString("Failed starting '" + DEFAULT_ID + "' service"));
+        assertThat(result.stdout(), containsString("The specified service does not exist as an installed service"));
     }
 
     public void test32StopNotStarted() throws IOException {
         sh.run(serviceScript + " install");
-        Result result = sh.run(serviceScript + " stop"); // stop is ok when not started
-        assertThat(result.stdout(), containsString("The service '" + DEFAULT_ID + "' has been stopped"));
+        Result result = sh.runIgnoreExitCode(stopCommand(DEFAULT_ID));
+        assertThat(result.stdout(), containsString("The service has not been started"));
     }
 
     public void test33JavaChanged() throws Exception {
@@ -210,7 +239,7 @@ public void test33JavaChanged() throws Exception {
             sh.getEnv().put("ES_JAVA_HOME", alternateJdk.toString());
             assertCommand(serviceScript + " install");
             sh.getEnv().remove("ES_JAVA_HOME");
-            assertCommand(serviceScript + " start");
+            assertCommand(startCommand(DEFAULT_ID));
             assertStartedAndStop();
         } finally {
             FileUtils.rm(alternateJdk);
@@ -220,7 +249,7 @@ public void test33JavaChanged() throws Exception {
     public void test80JavaOptsInEnvVar() throws Exception {
         sh.getEnv().put("ES_JAVA_OPTS", "-Xmx2g -Xms2g");
         sh.run(serviceScript + " install");
-        assertCommand(serviceScript + " start");
+        assertCommand(startCommand(DEFAULT_ID));
         assertStartedAndStop();
         sh.getEnv().remove("ES_JAVA_OPTS");
     }
@@ -230,7 +259,7 @@ public void test81JavaOptsInJvmOptions() throws Exception {
             append(tempConf.resolve("jvm.options"), "-Xmx2g" + System.lineSeparator());
             append(tempConf.resolve("jvm.options"), "-Xms2g" + System.lineSeparator());
             sh.run(serviceScript + " install");
-            assertCommand(serviceScript + " start");
+            assertCommand(startCommand(DEFAULT_ID));
             assertStartedAndStop();
         });
     }

qa/packaging/src/test/java/org/elasticsearch/packaging/util/Platforms.java

@@ -50,6 +50,9 @@ public static boolean isSystemd() {
     }
 
     public static boolean isDocker() {
+        if (WINDOWS) {
+            return new Shell().runIgnoreExitCode("where docker").isSuccess();
+        }
         return new Shell().runIgnoreExitCode("which docker").isSuccess();
     }