Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions cassandra_nodetool/tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,14 +50,14 @@ def dd_environment():
compose_file,
build=True,
service_name=common.CASSANDRA_CONTAINER_NAME,
waith_for_health=True,
wait_for_health=True,
):
cassandra_seed = get_container_ip("{}".format(common.CASSANDRA_CONTAINER_NAME))
env['CASSANDRA_SEEDS'] = cassandra_seed
with docker_run(
compose_file,
service_name=common.CASSANDRA_CONTAINER_NAME_2,
waith_for_health=True,
wait_for_health=True,
conditions=[WaitFor(create_keyspace, attempts=10, wait=10)],
):
yield common.CONFIG_INSTANCE, E2E_METADATA
1 change: 1 addition & 0 deletions datadog_checks_dev/changelog.d/23628.changed
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Make docker_run wait for Docker Compose service health by default.
13 changes: 6 additions & 7 deletions datadog_checks_dev/datadog_checks/dev/docker.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ def shared_logs(example_log_configs, mount_whitelist=None):
@contextmanager
def docker_run(
compose_file=None,
waith_for_health=False,
wait_for_health=True,
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Opt out TLS FIPS from health waiting

With this default set to True, dd_fips_environment now runs tls/tests/fips/docker-compose.yml through docker compose up --wait; Docker documents --wait as waiting for services to be running or healthy. That FIPS compose file has healthchecks that execute curl, but tls/tests/fips/Dockerfile only installs openssl and bash, so the services become unhealthy and the fixture fails before the previous sleep=20 path can yield. The non-FIPS TLS fixture was opted out in this commit, but the FIPS fixture still needs wait_for_health=False or a working healthcheck.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this was a real failure. We did not opt FIPS TLS out because unlike the non-FIPS fixture, this compose stack does not rely on a successful one-shot container; the services are meant to stay running and be health-checked.

Instead, we fixed the broken healthcheck by replacing the curl probe with an openssl s_client probe, using binaries already present in the Alpine-based image. I also validated the FIPS stack locally with docker compose up --wait --build.

build=False,
service_name=None,
up=None,
Expand All @@ -132,7 +132,7 @@ def docker_run(
compose_file (str):
A path to a Docker compose file. A custom tear
down is not required when using this.
waith_for_health (bool):
wait_for_health (bool):
Whether or not to wait for the health of the service to be healthy before yielding.
build (bool):
Whether or not to build images for when `compose_file` is provided
Expand Down Expand Up @@ -176,8 +176,7 @@ def docker_run(
composeFileArgs = {'compose_file': compose_file, 'build': build, 'service_name': service_name}
if capture is not None:
composeFileArgs['capture'] = capture
if waith_for_health:
composeFileArgs['waith_for_health'] = waith_for_health
composeFileArgs['wait_for_health'] = wait_for_health
set_up = ComposeFileUp(**composeFileArgs)
if down is not None:
tear_down = down
Expand Down Expand Up @@ -244,16 +243,16 @@ def __init__(
build: bool = False,
service_name: str | None = None,
capture: str | None = None,
waith_for_health: bool = False,
wait_for_health: bool = True,
):
self.compose_file = compose_file
self.build = build
self.service_name = service_name
self.capture = capture
self.waith_for_health = waith_for_health
self.wait_for_health = wait_for_health
self.command = ['docker', 'compose', '-f', self.compose_file, 'up', '-d', '--force-recreate']

if waith_for_health:
if wait_for_health:
self.command.append('--wait')

if self.build:
Expand Down
51 changes: 50 additions & 1 deletion datadog_checks_dev/tests/test_docker.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import tenacity

from datadog_checks.dev.ci import running_on_ci
from datadog_checks.dev.docker import compose_file_active, docker_run
from datadog_checks.dev.docker import ComposeFileUp, compose_file_active, docker_run
from datadog_checks.dev.subprocess import run_command

from .common import not_windows_ci
Expand Down Expand Up @@ -36,6 +36,33 @@ def test_up(self):


class TestDockerRun:
def test_wait_for_health_default(self):
set_up = self.get_set_up()

assert '--wait' in set_up.command

def test_wait_for_health_explicit_disable(self):
set_up = self.get_set_up(wait_for_health=False)

assert '--wait' not in set_up.command

def test_wait_for_health_typo_rejected(self):
compose_file = os.path.join(DOCKER_DIR, 'test_default.yaml')

with pytest.raises(TypeError, match='unexpected keyword argument'):
with docker_run(compose_file, waith_for_health=False):
pass

def get_set_up(self, **kwargs):
compose_file = os.path.join(DOCKER_DIR, 'test_default.yaml')

with mock.patch('datadog_checks.dev.docker.environment_run') as environment_run:
environment_run.return_value.__enter__.return_value = None
with docker_run(compose_file, **kwargs):
pass

return environment_run.call_args[1]['up']

@pytest.mark.parametrize(
"capture",
[
Expand Down Expand Up @@ -93,3 +120,25 @@ def test_retry_condition_failed_only_on_first_run(self):

with docker_run(up=up, down=mock.MagicMock(), attempts=3, conditions=[condition], attempts_wait=0):
assert condition.call_count == 2


class TestComposeFileUp:
def test_wait_for_health_default(self):
compose_file = os.path.join(DOCKER_DIR, 'test_default.yaml')

compose_file_up = ComposeFileUp(compose_file)

assert '--wait' in compose_file_up.command

def test_wait_for_health_explicit_disable(self):
compose_file = os.path.join(DOCKER_DIR, 'test_default.yaml')

compose_file_up = ComposeFileUp(compose_file, wait_for_health=False)

assert '--wait' not in compose_file_up.command

def test_wait_for_health_typo_rejected(self):
compose_file = os.path.join(DOCKER_DIR, 'test_default.yaml')

with pytest.raises(TypeError, match='unexpected keyword argument'):
ComposeFileUp(compose_file, waith_for_health=False)
2 changes: 1 addition & 1 deletion envoy/tests/docker/api_v3/Dockerfile-service
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
FROM envoyproxy/envoy:v1.31.0
RUN apt-get update && apt-get install -y python3 bash python3-pip
RUN python3 --version && pip3 --version
RUN pip3 install -q Flask==2.1.2 requests==2.28.1
RUN pip3 install -q Flask==2.1.2 requests==2.28.1 'Werkzeug<3'
RUN mkdir /code
ADD ./service.py /code
ADD ./start_service.sh /usr/local/bin/start_service.sh
Expand Down
4 changes: 3 additions & 1 deletion envoy/tests/docker/api_v3/service-envoy.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@ static_resources:
- match: {prefix: "/service"}
route: {cluster: local_service}
http_filters:
- name: envoy.router
- name: envoy.filters.http.router
typed_config:
"@type": type.googleapis.com/envoy.extensions.filters.http.router.v3.Router
clusters:
- name: local_service
connect_timeout: 0.25s
Expand Down
2 changes: 1 addition & 1 deletion harbor/tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ def dd_environment(e2e_instance):
if os.environ.get('HARBOR_USE_SSL'):
cert_dir = os.path.join(HERE, 'compose', 'common', 'cert')
e2e_metadata['docker_volumes'] = ['{}:/home/harbor/tests/compose/common/cert'.format(cert_dir)]
with docker_run(compose_file, conditions=conditions, attempts=5, waith_for_health=True):
with docker_run(compose_file, conditions=conditions, attempts=5, wait_for_health=True):
yield e2e_instance, e2e_metadata


Expand Down
1 change: 1 addition & 0 deletions http_check/tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ def dd_environment(mock_local_http_dns):
with docker_run(
os.path.join(HERE, 'compose', 'docker-compose.yml'),
build=True,
wait_for_health=False,
log_patterns=["start worker process"],
conditions=[WaitFor(call_endpoint, args=("https://127.0.0.1",))],
):
Expand Down
2 changes: 1 addition & 1 deletion kafka/tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,6 @@ def dd_environment():
service="kafka",
),
],
waith_for_health=True,
wait_for_health=True,
):
yield load_jmx_config(), {'use_jmx': True}
4 changes: 3 additions & 1 deletion kong/tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@ def dd_environment():
Start a kong cluster
"""
with docker_run(
compose_file=os.path.join(common.HERE, 'compose', 'docker-compose.yml'), endpoints=common.STATUS_URL
compose_file=os.path.join(common.HERE, 'compose', 'docker-compose.yml'),
wait_for_health=False,
endpoints=common.STATUS_URL,
):
yield common.openmetrics_instance

Expand Down
1 change: 1 addition & 0 deletions krakend/tests/docker/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ WORKDIR /app
COPY requirements.txt /app/requirements.txt
COPY api.py /app/api.py

RUN apt-get update && apt-get install -y --no-install-recommends curl && rm -rf /var/lib/apt/lists/*
RUN pip install -r requirements.txt

# Run the application
Expand Down
5 changes: 3 additions & 2 deletions krakend/tests/docker/docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,10 @@ services:
- ./krakend.json:/etc/krakend/krakend.json:ro
command: ["run", "-d", "-c", "/etc/krakend/krakend.json"]
depends_on:
- api
api:
condition: service_healthy
healthcheck:
test: ["CMD", "curl", "-f", "http://localhost:8080/__health"]
test: ["CMD", "wget", "-q", "--spider", "http://localhost:8080/__health"]
interval: 30s
timeout: 10s
retries: 3
Expand Down
5 changes: 3 additions & 2 deletions krakend/tests/lab/docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,12 @@ services:
- ../docker/krakend.json:/etc/krakend/krakend.json:ro
command: ["run", "-d", "-c", "/etc/krakend/krakend.json"]
depends_on:
- api
api:
condition: service_healthy
labels:
com.datadoghq.ad.checks: '{"krakend": {"instances": [{"openmetrics_endpoint": "http://localhost:9090/metrics"}], "logs": [{"source": "krakend", "service": "test-api", "type": "docker"}]}}'
healthcheck:
test: ["CMD", "curl", "-f", "http://localhost:8080/__health"]
test: ["CMD", "wget", "-q", "--spider", "http://localhost:8080/__health"]
interval: 30s
timeout: 10s
retries: 3
Expand Down
4 changes: 3 additions & 1 deletion marathon/tests/compose/docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,9 @@ services:
- MESOS_WORK_DIR=/var/tmp/mesos
- MESOS_MASTER=zk://zookeeper:2181/mesos
- MESOS_SWITCH_USER=0
- MESOS_CONTAINERIZERS=docker,mesos
- MESOS_CONTAINERIZERS=mesos
- MESOS_LAUNCHER=posix
- MESOS_SYSTEMD_ENABLE_SUPPORT=false
volumes:
- /sys/fs/cgroup:/sys/fs/cgroup
- /var/run/docker.sock:/run/docker.sock
Expand Down
2 changes: 1 addition & 1 deletion nifi/tests/docker/docker-compose.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ services:
SINGLE_USER_CREDENTIALS_USERNAME: admin
SINGLE_USER_CREDENTIALS_PASSWORD: ctsBtRBKHRAx69EqUghvvgEvjnaLjFEB
healthcheck:
test: ["CMD-SHELL", "curl -sk -o /dev/null -w '%{http_code}' https://localhost:8443/nifi-api/flow/about | grep -qv 000"]
test: ["CMD-SHELL", "curl -sk -o /dev/null -w '%{http_code}' https://$$(hostname):8443/nifi-api/flow/about | grep -qv 000"]
interval: 10s
timeout: 5s
retries: 30
Expand Down
4 changes: 4 additions & 0 deletions postgres/tests/compose/docker-compose-replication.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ services:
test: ["CMD-SHELL", "pg_isready -U datadog -d datadog_test && if [[ ! -e /tmp/container_ready.txt ]]; then exit 1; fi"]
interval: 1s
timeout: 5s
start_period: 60s
retries: 5
networks:
- pg-net
Expand All @@ -28,6 +29,7 @@ services:
test: ["CMD-SHELL", "pg_isready -p 5433 -U datadog -d datadog_test && if [[ ! -e /tmp/container_ready.txt ]]; then exit 1; fi"]
interval: 1s
timeout: 5s
start_period: 60s
retries: 5
networks:
- pg-net
Expand All @@ -47,6 +49,7 @@ services:
test: ["CMD-SHELL", "pg_isready -p 5434 -U datadog -d datadog_test && if [[ ! -e /tmp/container_ready.txt ]]; then exit 1; fi"]
interval: 1s
timeout: 5s
start_period: 60s
retries: 5
networks:
- pg-net
Expand All @@ -66,6 +69,7 @@ services:
test: ["CMD-SHELL", "pg_isready -p 5435 -U datadog -d datadog_test && if [[ ! -e /tmp/container_ready.txt ]]; then exit 1; fi"]
interval: 1s
timeout: 5s
start_period: 60s
retries: 5
networks:
- pg-net
Expand Down
1 change: 1 addition & 0 deletions postgres/tests/compose/docker-compose.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ services:
test: ["CMD-SHELL", "pg_isready -U datadog -d datadog_test && if [[ ! -e /tmp/container_ready.txt ]]; then exit 1; fi"]
interval: 1s
timeout: 5s
start_period: 60s
retries: 5
volumes:
- ./resources:/docker-entrypoint-initdb.d/
Expand Down
2 changes: 1 addition & 1 deletion prefect/tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ def dd_environment(instance: Callable[[str], dict[str, str | dict[str, list[str]
compose_file=str(COMPOSE_FILE_E2E),
conditions=conditions,
env_vars={"PREFECT_PORT": str(port)},
waith_for_health=True,
wait_for_health=True,
mount_logs=True,
):
yield (
Expand Down
2 changes: 1 addition & 1 deletion tls/tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ def clean_fips_environment():

@pytest.fixture(scope='session')
def dd_environment(instance_e2e, mock_local_tls_dns):
with docker_run(os.path.join(HERE, 'compose', 'docker-compose.yml'), build=True, sleep=20):
with docker_run(os.path.join(HERE, 'compose', 'docker-compose.yml'), build=True, wait_for_health=False, sleep=20):
e2e_metadata = {'docker_volumes': ['{}:{}'.format(CA_CERT, CA_CERT_MOUNT_PATH)]}
yield instance_e2e, e2e_metadata

Expand Down
4 changes: 2 additions & 2 deletions tls/tests/fips/docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ services:
- ./ca.key:/etc/ssl/private/server.key
command: ["/usr/local/bin/start-server.sh", "ECDHE-RSA-AES128-SHA256"]
healthcheck:
test: ["CMD", "curl", "-f", "https://localhost:443"]
test: ["CMD-SHELL", "printf '' | timeout 5 openssl s_client -connect localhost:443 -brief"]
interval: 30s
timeout: 10s
retries: 3
Expand All @@ -22,7 +22,7 @@ services:
- ./ca.key:/etc/ssl/private/server.key
command: ["/usr/local/bin/start-server.sh", "ECDHE-RSA-CHACHA20-POLY1305"]
healthcheck:
test: ["CMD", "curl", "-f", "https://localhost:443"]
test: ["CMD-SHELL", "printf '' | timeout 5 openssl s_client -connect localhost:443 -brief"]
interval: 30s
timeout: 10s
retries: 3
Loading