Skip to content

Commit

Permalink
Devops: Fix the Docker builds (#6445)
Browse files Browse the repository at this point in the history
In a recent commit, the `aiida-prepare.sh` startup script was updated to
switch from the deprecated `verdi quicksetup` to `verdi presto`. This
left the builds broken since there were a few bugs in the implementation
of `verdi presto`, but also because the behavior of the commands is not
identical:

* The option `--profile` for `verdi quicksetup` was not correctly
  renamed to `--profile-name` for `verdi presto`.
* The `detect_postgres_config` utility returned the database hostname
  under the key `database_host` instead of `database_hostname`.
* The `detect_rabbitmq_config` utility returned a dictionary whose keys
  were not prefixed with `broker_` as is expected by the implementation
  of the `RabbitmqBroker` plugin.
* The `detect_rabbitmq_config` utility now checks an environment
  variable for each connection parameter before resorting to the default.
  This is necessary to allow the `aiida-core-base` to define the hostname
  of RabbitMQ which is `messaging` instead of the default `locahost`.
  The `verdi presto` command intentionally does not expose options to
  configure RabbitMQ connection parameters.
* The `aiida-core-base` case now defines the hostnames of the PSQL and
  RabbitMQ services through environment variables as they are set to
  the key in the docker compose file, which are `database` and
  `messaging`, respectively, instead of `localhost`.
* The `_docker_service_wait` fixture now prints the log output captured
  from `docker compose` if the health check times out or fails.
* The healthcheck for the RabbitMQ service is enabled in the docker
  compose of `aiida-core-base` and the healthchecks for both the PSQL
  and RabbitMQ services are corrected. These checks are necessary to
  ensure the `aiida-prepare.sh` is not called before they are up. If
  called before, `verdi presto` is called before the services are up and
  the profile creation will fail or will be configured without broker.
* The healthchecks for `aiida-core-base` do not work for the case of
  `aiida-core-with-services` as there the services are part of the main
  container. Here the `aiida-prepare.sh` is called as soon as the
  services startup scripts have started, but they don't have a health
  check. Therefore, a `sleep` is added before `verdi presto` is called.
  There should be a better solution for this using s6, but this will
  have to do for the moment.
  • Loading branch information
sphuber authored Jun 4, 2024
1 parent 2fa7a53 commit 3404c01
Show file tree
Hide file tree
Showing 7 changed files with 56 additions and 18 deletions.
12 changes: 10 additions & 2 deletions .docker/aiida-core-base/s6-assets/init/aiida-prepare.sh
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,19 @@ verdi config set warnings.development_version False
# If the environment variable `SETUP_DEFAULT_AIIDA_PROFILE` is not set, set it to `true`.
if [[ ${SETUP_DEFAULT_AIIDA_PROFILE:-true} == true ]] && ! verdi profile show ${AIIDA_PROFILE_NAME} &> /dev/null; then

# For the container that includes the services, this script is called as soon as the RabbitMQ startup script has
# been launched, but it can take a while for the service to come up. If ``verdi presto`` is called straight away
# it is possible it tries to connect to the service before that and it will configure the profile without a broker.
sleep 5

# Create AiiDA profile.
verdi presto \
--profile "${AIIDA_PROFILE_NAME:-default}" \
--verbosity info \
--profile-name "${AIIDA_PROFILE_NAME:-default}" \
--email "${AIIDA_USER_EMAIL:-aiida@localhost}" \
--use-postgres
--use-postgres \
--postgres-hostname "${AIIDA_POSTGRES_HOSTNAME:-localhost}" \
--postgres-password "${AIIDA_POSTGRES_PASSWORD:-password}"

# Setup and configure local computer.
computer_name=localhost
Expand Down

This file was deleted.

Empty file.
10 changes: 6 additions & 4 deletions .docker/docker-compose.aiida-core-base.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ services:
# volumes:
# - aiida-postgres-db:/var/lib/postgresql/data
healthcheck:
test: [CMD-SHELL, pg_isready]
test: [CMD, pg_isready, -U, postgres]
interval: 5s
timeout: 5s
retries: 10
Expand All @@ -23,14 +23,16 @@ services:
# volumes:
# - aiida-rmq-data:/var/lib/rabbitmq/
healthcheck:
test: rabbitmq-diagnostics check_port_connectivity
test: [CMD, rabbitmq-diagnostics, check_running]
interval: 30s
timeout: 30s
retries: 10

aiida:
image: ${REGISTRY:-}${AIIDA_CORE_BASE_IMAGE:-aiidateam/aiida-core-base}${TAG:-}
environment:
AIIDA_POSTGRES_HOSTNAME: database
AIIDA_BROKER_HOST: messaging
RMQHOST: messaging
TZ: Europe/Zurich
SETUP_DEFAULT_AIIDA_PROFILE: 'true'
Expand All @@ -39,8 +41,8 @@ services:
depends_on:
database:
condition: service_healthy
#messaging:
# condition: service_healthy
messaging:
condition: service_healthy

#volumes:
# aiida-postgres-db:
Expand Down
29 changes: 22 additions & 7 deletions .docker/tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,7 @@ def docker_compose(docker_services):
def _docker_service_wait(docker_services):
"""Container startup wait."""

# using `docker_compose` fixture would
# trigger a separate container
# using `docker_compose` fixture would trigger a separate container
docker_compose = docker_services._docker_compose

def is_container_ready():
Expand All @@ -53,11 +52,27 @@ def is_container_ready():
return False
return '✔ broker:' in output and 'Daemon is running' in output

docker_services.wait_until_responsive(
timeout=600.0,
pause=2,
check=lambda: is_container_ready(),
)
try:
docker_services.wait_until_responsive(
timeout=300.0,
pause=10,
check=lambda: is_container_ready(),
)
except Exception:
print('Timed out waiting for the profile and daemon to be up and running.')

try:
docker_compose.execute('exec -T aiida verdi status').decode().strip()
except Exception as exception:
print(f'Output of `verdi status`:\n{exception}')

try:
docker_compose.execute('exec -T aiida verdi profile show').decode().strip()
except Exception as exception:
print(f'Output of `verdi status`:\n{exception}')

print(docker_compose.execute('logs').decode().strip())
raise


@pytest.fixture
Expand Down
19 changes: 17 additions & 2 deletions src/aiida/brokers/rabbitmq/defaults.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,16 @@

from __future__ import annotations

import os
import typing as t

from aiida.common.extendeddicts import AttributeDict
from aiida.common.log import AIIDA_LOGGER

__all__ = ('BROKER_DEFAULTS',)

LOGGER = AIIDA_LOGGER.getChild('brokers.rabbitmq.defaults')

LAUNCH_QUEUE = 'process.queue'
MESSAGE_EXCHANGE = 'messages'
TASK_EXCHANGE = 'tasks'
Expand All @@ -32,11 +36,22 @@ def detect_rabbitmq_config() -> dict[str, t.Any] | None:
"""
from kiwipy.rmq.threadcomms import connect

connection_params = dict(BROKER_DEFAULTS)
connection_params = {
'protocol': os.getenv('AIIDA_BROKER_PROTOCOL', BROKER_DEFAULTS['protocol']),
'username': os.getenv('AIIDA_BROKER_USERNAME', BROKER_DEFAULTS['username']),
'password': os.getenv('AIIDA_BROKER_PASSWORD', BROKER_DEFAULTS['password']),
'host': os.getenv('AIIDA_BROKER_HOST', BROKER_DEFAULTS['host']),
'port': os.getenv('AIIDA_BROKER_PORT', BROKER_DEFAULTS['port']),
'virtual_host': os.getenv('AIIDA_BROKER_VIRTUAL_HOST', BROKER_DEFAULTS['virtual_host']),
'heartbeat': os.getenv('AIIDA_BROKER_HEARTBEAT', BROKER_DEFAULTS['heartbeat']),
}

LOGGER.info(f'Attempting to connect to RabbitMQ with parameters: {connection_params}')

try:
connect(connection_params=connection_params)
except ConnectionError:
return None

return connection_params
# The profile configuration expects the keys of the broker config to be prefixed with ``broker_``.
return {f'broker_{key}': value for key, value in connection_params.items()}
2 changes: 1 addition & 1 deletion src/aiida/cmdline/commands/cmd_presto.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ def detect_postgres_config(
echo.echo_critical(f'Unable to automatically create the PostgreSQL user and database: {exception}')

return {
'database_host': postgres_hostname,
'database_hostname': postgres_hostname,
'database_port': postgres_port,
'database_name': database_name,
'database_username': database_username,
Expand Down

0 comments on commit 3404c01

Please sign in to comment.