tests: use service_started condition

[itzg]

No description provided.

[AlexTMjugador]

I've just noticed that CI is now failing due to errors that I suspect are related to a race condition during container startup. For what it's worth, I tried reproducing the failure locally by running:

repository-root$ docker build -t itzg/minecraft-server:test-java17-graalvm-42 --build-arg BASE_IMAGE=container-registry.oracle.com/graalvm/jdk:17-ol9 .
repository-root/tests/setuponlytests/generic-packs$ IMAGE_TO_TEST=itzg/minecraft-server:test-java17-graalvm-42 docker compose run --rm -e SETUP_ONLY=true -e DEBUG=false mc

But I was unsuccessful on my computer.

I also reviewed the relevant Docker documentation which states that the service_started condition is the default when no condition is explicitly specified, and I also remember that being true from previous experiences. This implies that this PR will not actually resolve the underlying issue, since it doesn't change that default and the container may still start but need some extra time before it is actually ready to serve requests.

Accordingly, I was about to submit a follow-up PR that introduces healthchecks and switches to the service_healthy condition instead, which should ensure that the Nginx container is ready to handle requests before the Minecraft container starts. I've attached the diff for my changes below.

Proposed changes diff

diff --git a/tests/setuponlytests/generic-packs/docker-compose.yml b/tests/setuponlytests/generic-packs/docker-compose.yml
index 52a3c9c..2e6bd70 100644
--- a/tests/setuponlytests/generic-packs/docker-compose.yml
+++ b/tests/setuponlytests/generic-packs/docker-compose.yml
@@ -3,9 +3,15 @@ services:
     image: nginx
     volumes:
       - ./web:/usr/share/nginx/html
+    healthcheck:
+      test: ["CMD", "curl", "--fail", "http://localhost/configs.zip"]
+      interval: 3s
+      timeout: 5s
+      retries: 3
   mc:
     depends_on:
-      - web
+      web:
+        condition: service_healthy
     image: ${IMAGE_TO_TEST:-itzg/minecraft-server}
     environment:
       EULA: "true"
diff --git a/tests/setuponlytests/icon-gif-multiframe/docker-compose.yml b/tests/setuponlytests/icon-gif-multiframe/docker-compose.yml
index be0d575..590b18f 100644
--- a/tests/setuponlytests/icon-gif-multiframe/docker-compose.yml
+++ b/tests/setuponlytests/icon-gif-multiframe/docker-compose.yml
@@ -3,9 +3,16 @@ services:
     image: nginx
     volumes:
       - ./web:/usr/share/nginx/html
+    healthcheck:
+      test:
+        ["CMD", "curl", "--fail", "http://localhost/motion-tween-example.gif"]
+      interval: 3s
+      timeout: 5s
+      retries: 3
   mc:
     depends_on:
-      - web
+      web:
+        condition: service_healthy
     image: ${IMAGE_TO_TEST:-itzg/minecraft-server}
     environment:
       EULA: "true"
diff --git a/tests/setuponlytests/icon-png-atscale/docker-compose.yml b/tests/setuponlytests/icon-png-atscale/docker-compose.yml
index 7ac20b6..b179d58 100644
--- a/tests/setuponlytests/icon-png-atscale/docker-compose.yml
+++ b/tests/setuponlytests/icon-png-atscale/docker-compose.yml
@@ -3,9 +3,21 @@ services:
     image: nginx
     volumes:
       - ./web:/usr/share/nginx/html
+    healthcheck:
+      test:
+        [
+          "CMD",
+          "curl",
+          "--fail",
+          "http://localhost/4737386_minecraft_squircle_icon.png",
+        ]
+      interval: 3s
+      timeout: 5s
+      retries: 3
   mc:
     depends_on:
-      - web
+      web:
+        condition: service_healthy
     image: ${IMAGE_TO_TEST:-itzg/minecraft-server}
     environment:
       EULA: "true"
diff --git a/tests/setuponlytests/icon-png-scale/docker-compose.yml b/tests/setuponlytests/icon-png-scale/docker-compose.yml
index 7ac20b6..b179d58 100644
--- a/tests/setuponlytests/icon-png-scale/docker-compose.yml
+++ b/tests/setuponlytests/icon-png-scale/docker-compose.yml
@@ -3,9 +3,21 @@ services:
     image: nginx
     volumes:
       - ./web:/usr/share/nginx/html
+    healthcheck:
+      test:
+        [
+          "CMD",
+          "curl",
+          "--fail",
+          "http://localhost/4737386_minecraft_squircle_icon.png",
+        ]
+      interval: 3s
+      timeout: 5s
+      retries: 3
   mc:
     depends_on:
-      - web
+      web:
+        condition: service_healthy
     image: ${IMAGE_TO_TEST:-itzg/minecraft-server}
     environment:
       EULA: "true"
diff --git a/tests/setuponlytests/packwiz/docker-compose.yml b/tests/setuponlytests/packwiz/docker-compose.yml
index d4ff501..63a85b8 100644
--- a/tests/setuponlytests/packwiz/docker-compose.yml
+++ b/tests/setuponlytests/packwiz/docker-compose.yml
@@ -3,9 +3,15 @@ services:
     image: nginx
     volumes:
       - ./web:/usr/share/nginx/html
+    healthcheck:
+      test: ["CMD", "curl", "--fail", "http://localhost/pack.toml"]
+      interval: 3s
+      timeout: 5s
+      retries: 3
   mc:
     depends_on:
-      - web
+      web:
+        condition: service_healthy
     image: ${IMAGE_TO_TEST:-itzg/minecraft-server}
     environment:
       EULA: "true"

Would you like me to submit a PR with the proposed patch, or would you prefer to apply it another way? Feel free to let me know, and thank you! ❤️

[itzg]

Thanks. There wasn't a problem before the Oracle Linux change (other than the alpine failures due to netty native library issues -- working on that here). Please also investigate why the Linux change affected it. Those tests have worked fine for years.

In any case, yes please just submit a PR instead of posting a patch in a comment.