Skip to content

Commit

Permalink
Support aiidalab/full-stack (#149)
Browse files Browse the repository at this point in the history
* Switch to 'aiidalab/full-stack:latest' as default image.

* Switch to jovyan as the default user.

Selecting a different user is currently creating problems, see
aiidalab/aiidalab-docker-stack#297 .

* Fix debug message w.r.t. the ~/.conda directory.

* Skip check if wait-for-services script is not present.

Script is not provided on the full-stack image.

* Test against the old and the revised image.

Allow test parametrization w.r.t. the default image.

* CI: Increase test timeout.
  • Loading branch information
csadorf authored Sep 29, 2022
1 parent 7ae7dc9 commit 49e66d1
Show file tree
Hide file tree
Showing 7 changed files with 46 additions and 12 deletions.
7 changes: 5 additions & 2 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,15 @@ jobs:
needs: [pre-commit]

runs-on: ubuntu-latest
timeout-minutes: 10
timeout-minutes: 30

strategy:
fail-fast: false
matrix:
python-version: ['3.8', '3.9', '3.10']
default-image:
- '' # use the application default
- aiidalab/aiidalab-docker-stack:latest

steps:

Expand All @@ -52,7 +55,7 @@ jobs:
- name: Run tests
run: |
pytest -v --slow
pytest -v --slow --default-image=${{ matrix.default-image }}
coverage xml
- name: Upload coverage to Codecov
Expand Down
9 changes: 6 additions & 3 deletions aiidalab_launch/instance.py
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,8 @@ def _run_post_start(self) -> None:
LOGGER.warning(
"Failed to ensure ~/.conda directory is owned by the system user."
)
LOGGER.debug("The ~/.conda directory is owned by the system user.")
else:
LOGGER.debug("The ~/.conda directory is owned by the system user.")

def stop(self, timeout: float | None = None) -> None:
self._requires_container()
Expand Down Expand Up @@ -270,7 +271,7 @@ def exec_create(self, cmd: str, privileged: bool = False) -> str:
return self.client.api.exec_create(
self.container.id,
cmd,
user=None if privileged else self.profile.system_user,
user="root" if privileged else self.profile.system_user,
workdir=None if privileged else f"/home/{self.profile.system_user}",
)["Id"]
except docker.errors.APIError:
Expand All @@ -291,7 +292,9 @@ async def _init_scripts_finished(container: Container) -> None:
result = await loop.run_in_executor(
None, container.exec_run, "wait-for-services"
)
if result.exit_code != 0:
if result.exit_code in (126, 127):
LOGGER.debug("Container does not support wait-for-services script.")
elif result.exit_code != 0:
raise FailedToWaitForServices(
"Failed to check for init processes to complete."
)
Expand Down
14 changes: 12 additions & 2 deletions aiidalab_launch/profile.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import toml
from docker.models.containers import Container

from .core import LOGGER
from .util import docker_mount_for, get_docker_env, is_volume_readonly

MAIN_PROFILE_NAME = "default"
Expand All @@ -30,7 +31,7 @@ def _default_port() -> int: # explicit function required to enable test patchin
return DEFAULT_PORT


_DEFAULT_IMAGE = "aiidalab/aiidalab-docker-stack:latest"
_DEFAULT_IMAGE = "aiidalab/full-stack:latest"


def _valid_volume_name(source: str) -> None:
Expand Down Expand Up @@ -65,7 +66,7 @@ class Profile:
name: str = MAIN_PROFILE_NAME
port: int | None = field(default_factory=_default_port)
default_apps: list[str] = field(default_factory=lambda: ["aiidalab-widgets-base"])
system_user: str = "aiida"
system_user: str = "jovyan"
image: str = _DEFAULT_IMAGE
home_mount: str | None = None
extra_mounts: set[str] = field(default_factory=set)
Expand Down Expand Up @@ -93,6 +94,14 @@ def __post_init__(self):
self.extra_mounts.remove(extra_mount)
self.extra_mounts.add(f"{extra_mount}:rw")

if (
self.image.split(":")[0] == "aiidalab/full-stack"
and self.system_user != "jovyan"
):
LOGGER.warning(
"Resetting the system user may create issues for this image!"
)

def container_name(self) -> str:
return f"{CONTAINER_PREFIX}{self.name}"

Expand Down Expand Up @@ -127,6 +136,7 @@ def environment(self, jupyter_token: str) -> dict:
"AIIDALAB_DEFAULT_APPS": " ".join(self.default_apps),
"JUPYTER_TOKEN": str(jupyter_token),
"SYSTEM_USER": self.system_user,
"NB_USER": self.system_user,
}

def dumps(self) -> str:
Expand Down
15 changes: 15 additions & 0 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,16 @@ def docker_client():
pytest.skip("docker not available")


@pytest.fixture(autouse=True)
def _select_default_image(monkeypatch_session, pytestconfig):
_default_image = pytestconfig.getoption("default_image")
if _default_image is not None:
monkeypatch_session.setattr(
aiidalab_launch.profile, "_DEFAULT_IMAGE", _default_image
)
yield None


@pytest.fixture(scope="session", autouse=True)
def _pull_docker_image(docker_client):
try:
Expand Down Expand Up @@ -250,6 +260,11 @@ def pytest_addoption(parser):
default=False,
help="Enable long running tests.",
)
parser.addoption(
"--default-image",
action="store",
help="Select the default image to test against.",
)


def pytest_configure(config):
Expand Down
4 changes: 2 additions & 2 deletions tests/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -161,11 +161,11 @@ def test_status(self, started_instance):
assert started_instance.profile.home_mount in result.output
assert started_instance.url() in result.output

def test_exec(self):
def test_exec(self, started_instance):
runner: CliRunner = CliRunner()
result: Result = runner.invoke(cli.cli, ["exec", "--", "whoami"])
assert result.exit_code == 0
assert "aiida" in result.output
assert started_instance.profile.system_user in result.output

def test_logs(self):
runner: CliRunner = CliRunner()
Expand Down
2 changes: 1 addition & 1 deletion tests/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
port = 8888
default_apps = [ "aiidalab-widgets-base",]
system_user = "aiida"
image = "aiidalab/aiidalab-docker-stack:latest"
image = "aiidalab/full-stack:edge"
home_mount = "aiidalab_default_home"
"""
}
Expand Down
7 changes: 5 additions & 2 deletions tests/test_instance.py
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ async def test_profile_configuration_changes(instance):
assert not any(instance.configuration_changes())

# Change image
instance.profile.image = "aiidalab/aiidalab-docker-stack:1234"
instance.profile.image = "aiidalab/full-stack:1234"
assert "Profile configuration has changed." in instance.configuration_changes()
instance.profile = deepcopy(original_profile)
assert not any(instance.configuration_changes())
Expand Down Expand Up @@ -159,7 +159,10 @@ def test_instance_host_ports(self, started_instance):

def test_instance_exec_create(self, docker_client, started_instance):
exec_id = started_instance.exec_create(cmd="whoami")
assert docker_client.api.exec_start(exec_id).decode().strip() == "aiida"
assert (
docker_client.api.exec_start(exec_id).decode().strip()
== started_instance.profile.system_user
)

exec_id_privileged = started_instance.exec_create(cmd="whoami", privileged=True)
assert (
Expand Down

0 comments on commit 49e66d1

Please sign in to comment.