Add tests for all images by nineinchnick · Pull Request #147 · trinodb/docker-images

nineinchnick · 2022-09-08T14:17:47Z

Extracted from #143

Partially reverts #140 - restore using JDK 11 for Spark and Phoenix images. Tests were failing without this. This was also raised in trinodb/trino#13733

nineinchnick · 2022-09-09T09:05:28Z

It seems that the testing/dns image didn't work at all until I upgraded the base image. It has very few pulls reported on the package page: https://github.com/trinodb/docker-images/pkgs/container/testing%2Fdns and is not referenced in https://github.com/trinodb/trino

Maybe we can remove it?

hashhar · 2022-09-10T11:13:39Z

DNS image is not used (as confirmed by @sopel39 sometime ago).

hashhar

Some comments.

bin/test.sh

testing/centos7-oj11/Dockerfile

testing/spark3.0-iceberg/Dockerfile

testing/phoenix5/Dockerfile

nineinchnick · 2022-10-10T08:15:04Z

@hashhar a gentle reminder that I'm waiting for your response in a few comments.

hashhar · 2022-10-10T08:30:33Z

Thanks for the reminder @nineinchnick. Replied and resolved some of them.

bin/test.sh

testing/centos7-oj11/Dockerfile

hashhar · 2022-10-11T18:11:38Z

testing/spark3-delta/Dockerfile

+HEALTHCHECK --interval=10s --timeout=5s --start-period=10s \
+    CMD curl -f http://localhost:10213/


will this still show up as healthy when under heavy load on CI? e.g. longer timeout ?

Same for other images in this commit " Add tests for spark images "

The timeout is defined here as 5s. There's a retries parameter with a default value of 3: https://docs.docker.com/engine/reference/builder/#healthcheck. So the container will be marked as unhealthy after 3 failures (timeouts). But I'm not sure what you mean by "show up" - nothing except the tests here use these health checks.

We use testcontainers for tests - does it remove containers when healthchecks fail? Or are they only tested during startup?

Product tests use health checks for these images from this repo:

hive3.1-hive

hdp2.6-hive

hdp3.1-hive

but they configure/override the healthcheck. Also, I'm not changing them in this PR. Other images, like spark3-delta, used in EnvSinglenodeDeltaLakeOss, don't wait for a health check, but for an open port: waitingFor(forSelectedPorts(SPARK_THRIFT_PORT)).
All other containers seem to have similar conditions.

% pwd /Users/jwas/src/trino % ag forHealthcheck --java --silent plugin/trino-accumulo/src/test/java/io/trino/plugin/accumulo/TestingAccumuloServer.java 58: accumuloContainer.waitingFor(Wait.forHealthcheck().withStartupTimeout(Duration.ofMinutes(10))); testing/trino-product-tests-launcher/src/main/java/io/trino/tests/product/launcher/env/common/Hadoop.java 33:import static org.testcontainers.containers.wait.strategy.Wait.forHealthcheck; 88: .waitingForAll(forSelectedPorts(10000), forHealthcheck()) // HiveServer2 testing/trino-product-tests-launcher/src/main/java/io/trino/tests/product/launcher/env/common/Standard.java 53:import static org.testcontainers.containers.wait.strategy.Wait.forHealthcheck; 154: .waitingForAll(forLogMessage(".*======== SERVER STARTED ========.*", 1), forHealthcheck()) testing/trino-product-tests-launcher/src/main/java/io/trino/tests/product/launcher/env/environment/EnvSinglenodeCompatibility.java 38:import static org.testcontainers.containers.wait.strategy.Wait.forHealthcheck; 82: .waitingForAll(forLogMessage(".*======== SERVER STARTED ========.*", 1), forHealthcheck())

and I checked all configs to see which hadoop base image they provide.

Then I checked:

% ag waitingFor testing/trino-product-tests-launcher testing/trino-product-tests-launcher/src/main/java/io/trino/tests/product/launcher/env/DockerContainer.java 323: public DockerContainer waitingForAll(WaitStrategy... strategies) 329: waitingFor(waitAllStrategy); testing/trino-product-tests-launcher/src/main/java/io/trino/tests/product/launcher/env/common/HydraIdentityProvider.java 60: .waitingFor(new SelectedPortWaitStrategy(5432)); 72: .waitingFor(Wait.forHttp("/healthz").forPort(3000).forStatusCode(200)); 93: .waitingFor(new WaitAllStrategy() testing/trino-product-tests-launcher/src/main/java/io/trino/tests/product/launcher/env/common/Kerberos.java 58: .waitingFor(DEFAULT_WAIT_STRATEGY); testing/trino-product-tests-launcher/src/main/java/io/trino/tests/product/launcher/env/common/Hadoop.java 88: .waitingForAll(forSelectedPorts(10000), forHealthcheck()) // HiveServer2 testing/trino-product-tests-launcher/src/main/java/io/trino/tests/product/launcher/env/common/Standard.java 134: .waitingFor(new WaitAllStrategy()) // don't wait 154: .waitingForAll(forLogMessage(".*======== SERVER STARTED ========.*", 1), forHealthcheck()) testing/trino-product-tests-launcher/src/main/java/io/trino/tests/product/launcher/env/common/Minio.java 80: .waitingFor(forSelectedPorts(MINIO_PORT)) testing/trino-product-tests-launcher/src/main/java/io/trino/tests/product/launcher/env/common/Kafka.java 79: .waitingFor(forSelectedPorts(2181)) 97: .waitingForAll(forSelectedPorts(9092), forLogMessage(".*started \\(kafka.server.KafkaServer\\).*", 1)) 113: .waitingFor(forSelectedPorts(SCHEMA_REGISTRY_PORT)) testing/trino-product-tests-launcher/src/main/java/io/trino/tests/product/launcher/env/environment/AbstractEnvSinglenodeLdap.java 80: .waitingFor(forSelectedPorts(LDAP_PORT)) testing/trino-product-tests-launcher/src/main/java/io/trino/tests/product/launcher/env/environment/EnvSinglenodeSparkHive.java 98: .waitingFor(forSelectedPorts(SPARK_THRIFT_PORT)); testing/trino-product-tests-launcher/src/main/java/io/trino/tests/product/launcher/env/environment/EnvMultinodeClickhouse.java 100: .waitingFor(forSelectedPorts(2181)) 121: .waitingFor(forSelectedPorts(httpPort, nativePort)) testing/trino-product-tests-launcher/src/main/java/io/trino/tests/product/launcher/env/environment/EnvSinglenodeCassandra.java 70: .waitingFor(forSelectedPorts(CASSANDRA_PORT)) testing/trino-product-tests-launcher/src/main/java/io/trino/tests/product/launcher/env/environment/EnvSinglenodeSqlserver.java 64: .waitingFor(forSelectedPorts(SQLSERVER_PORT)); testing/trino-product-tests-launcher/src/main/java/io/trino/tests/product/launcher/env/environment/EnvSinglenodeSparkIceberg.java 98: .waitingFor(forSelectedPorts(SPARK_THRIFT_PORT)); testing/trino-product-tests-launcher/src/main/java/io/trino/tests/product/launcher/env/environment/EnvSinglenodeCompatibility.java 82: .waitingForAll(forLogMessage(".*======== SERVER STARTED ========.*", 1), forHealthcheck()) testing/trino-product-tests-launcher/src/main/java/io/trino/tests/product/launcher/env/environment/EnvSinglenodePostgresql.java 70: .waitingFor(forSelectedPorts(POSTGRESQL_PORT)); testing/trino-product-tests-launcher/src/main/java/io/trino/tests/product/launcher/env/environment/EnvMultinodePhoenix5.java 61: .waitingFor(forSelectedPorts(ZOOKEEPER_PORT)) testing/trino-product-tests-launcher/src/main/java/io/trino/tests/product/launcher/env/environment/EnvMultinodeKerberosKudu.java 73: .waitingForAll( 103: .waitingFor(forSelectedPorts(KUDU_MASTER_PORT)); 123: .waitingFor(forSelectedPorts(initialKuduTserverPort)) testing/trino-product-tests-launcher/src/main/java/io/trino/tests/product/launcher/env/environment/EnvMultinodeMariadb.java 68: .waitingFor(forSelectedPorts(MARIADB_PORT)); testing/trino-product-tests-launcher/src/main/java/io/trino/tests/product/launcher/env/environment/EnvSinglenodeDeltaLakeOss.java 134: .waitingFor(forSelectedPorts(SPARK_THRIFT_PORT)); testing/trino-product-tests-launcher/src/main/java/io/trino/tests/product/launcher/env/environment/EnvSinglenodeMysql.java 67: .waitingFor(forSelectedPorts(MYSQL_PORT));

That DEFAULT_WAIT_STRATEGY for Kerberos is also a port check:

public static final WaitStrategy DEFAULT_WAIT_STRATEGY = new WaitAllStrategy() .withStrategy(forSelectedPorts(KERBEROS_PORT, KERBEROS_ADMIN_PORT)) .withStrategy(forLogMessage(".*krb5kdc entered RUNNING state.*", 1));

Indeed seems like Testcontainers only cares about the health/wait strategy when waiting for container to start - they aren't considered at all once containers start running.

Thanks for doing the deep investigation.

Turns out product tests look at healthchecks after all - see failures in https://github.com/trinodb/trino/actions/runs/3342925565/jobs/5536149519 which fail to start environment because Spark healthcheck appears to fail.

I thought that waitingFor() would override the health check:

DockerContainer container = new DockerContainer("ghcr.io/trinodb/testing/spark3-delta:" + hadoopImagesVersion, SPARK_CONTAINER_NAME) .withCopyFileToContainer(forHostPath(configDir.getPath("spark-defaults.conf")), "/spark/conf/spark-defaults.conf") .waitingFor(forSelectedPorts(SPARK_THRIFT_PORT));

Ok it does not remove the default health check but the failure is unexpected, I'm trying to reproduce this locally.

The healthcheck is a bit weird.

It responds with:

{ "Start": "2022-10-28T09:09:23.693071205Z", "End": "2022-10-28T09:09:23.75675015Z", "ExitCode": 0, "Output": " % Total % Received % Xferd Average Speed Time Time Time Current\n Dload Upload Total Spent Left Speed\n\r 0 0 0 0 0 0 0 0 --:--:-- --:--:-- --:--:-- 0\r100 44 0 44 0 0 8598 0 --:--:-- --:--:-- --:--:-- 8800\n\u0004\u0000\u0000\u0000\u0011Invalid status 71" },

But locally the exit code is always 0, I'll retry on CI.

.github/workflows/ci.yml

This is required to use the `--format` switch in `docker compose ps` and read the status of the health check in tests.

Currently used versions of Spark and Phoenix5 are not compatible with JDK 17 and should be using JDK 11.

cla-bot bot added the cla-signed label Sep 8, 2022

nineinchnick requested review from ebyhr, hashhar and wendigo September 8, 2022 14:17

nineinchnick force-pushed the tests branch 2 times, most recently from 51ab414 to 52ed97f Compare September 8, 2022 14:32

nineinchnick mentioned this pull request Sep 8, 2022

Build multi-arch images if PLATFORMS env var is set #143

Merged

nineinchnick force-pushed the tests branch 3 times, most recently from dd32323 to fc4b372 Compare September 9, 2022 08:52

hashhar reviewed Sep 10, 2022

View reviewed changes

bin/test.sh Show resolved Hide resolved

testing/centos7-oj11/Dockerfile Show resolved Hide resolved

testing/spark3.0-iceberg/Dockerfile Show resolved Hide resolved

testing/phoenix5/Dockerfile Outdated Show resolved Hide resolved

nineinchnick force-pushed the tests branch from fc4b372 to 3ec68e4 Compare September 28, 2022 09:17

wendigo approved these changes Oct 10, 2022

View reviewed changes

nineinchnick mentioned this pull request Oct 10, 2022

Add tests for the phoenix5 testing Docker image trinodb/trino#14543

Closed

Format all shell scripts using shfmt

4cf67fc

nineinchnick force-pushed the tests branch 2 times, most recently from ccb3406 to 58e3c78 Compare October 11, 2022 10:07

hashhar approved these changes Oct 11, 2022

View reviewed changes

Jan Waś added 8 commits October 11, 2022 20:55

Switch from Docker Compose v1 to v2

30750d1

This is required to use the `--format` switch in `docker compose ps` and read the status of the health check in tests.

Add tests for openldap images

62c9070

Restore centos7-oj11 image as base for spark and phoenix5

7e3b08a

Currently used versions of Spark and Phoenix5 are not compatible with JDK 17 and should be using JDK 11.

Add tests for spark images

cde299a

Note missing tests for the phoenix5 image

a740016

Enable hive3.1 tests

1b77e84

Add tests for accumulo image

229fff4

Upgrade base image for dns

e7c4361

Add tests for the dns image

34edc5e

nineinchnick force-pushed the tests branch from 58e3c78 to 34edc5e Compare October 11, 2022 18:56

hashhar merged commit fbe9270 into trinodb:master Oct 12, 2022

nineinchnick deleted the tests branch October 27, 2022 13:07

		HEALTHCHECK --interval=10s --timeout=5s --start-period=10s \
		CMD curl -f http://localhost:10213/

Conversation

nineinchnick commented Sep 8, 2022

Uh oh!

nineinchnick commented Sep 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hashhar commented Sep 10, 2022

Uh oh!

hashhar left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nineinchnick commented Oct 10, 2022

Uh oh!

hashhar commented Oct 10, 2022

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

nineinchnick commented Sep 9, 2022 •

edited

Loading