Skip to content

Commit

Permalink
fix: bind-mount in dev-specific services
Browse files Browse the repository at this point in the history
The -m/--mount option makes it possible to bind-mount volumes at runtime. The
volumes are declared in a local/docker-compose.tmp.yml file. The problem with
this approach is when we want to bind-mount a volume to a service which is
specific to the dev context. For instance: the "learning" service when the MFE
plugin is enabled.

In such a case, starting the service triggers a call to `docker-compose stop`
in the local context. This call fails because the "learning" service does not
exist in the local context. Note that this issue only seems to occur with
docker-compose v1.

To resolve this issue, we create two additional filters for
the dev context, which emulate the behaviour of the local context. With this approach, we convert the -m/--mount arguments right after they are parsed. Because they are parsed just once, we can get rid of the de-duplication logic initially introduced with the COMPOSE_CLI_MOUNTS context.

Close #711. Close also overhangio/tutor-mfe#57.
  • Loading branch information
regisb committed Jul 29, 2022
1 parent 8345b7a commit 585495e
Show file tree
Hide file tree
Showing 7 changed files with 174 additions and 142 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,9 @@ Every user-facing change should have an entry in this changelog. Please respect

## Unreleased

- [Bugfix] Fix `tutor dev start -m /path/to/frontend-app-learning` by introducing dev-specific `COMPOSE_DEV_TMP` and `COMPOSE_DEV_JOBS_TMP` filters (by @regisb).
- [Bugfix] Log the shell commands that Tutor executes more accurately. (by @kdmccormick)
- [Fix] `tutor dev quickstart` would fail under certain versions of docker-compose due to a bug in the logic that handled volume mounting. (by @kdmccormick)
- [Bugfix] `tutor dev quickstart` would fail under certain versions of docker-compose due to a bug in the logic that handled volume mounting. (by @kdmccormick)
- [Bugfix] The `tutor k8s start` command will succeed even when `k8s-override` and `kustomization-patches-strategic-merge` are not specified. (by @edazzocaisser)
- [Fix] `kubectl wait` checks deployments instead of pods as it could hang indefinitely if there are extra pods in a broken state. (by @keithgg)

Expand Down
28 changes: 13 additions & 15 deletions tests/commands/test_compose.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import typing as t
import unittest
from io import StringIO
from unittest.mock import patch

from click.exceptions import ClickException

Expand Down Expand Up @@ -42,25 +44,21 @@ def test_mount_option_parsing(self) -> None:
with self.assertRaises(ClickException):
param("lms,:/path/to/edx-platform:/openedx/edx-platform")

def test_compose_local_tmp_generation(self) -> None:
@patch("sys.stdout", new_callable=StringIO)
def test_compose_local_tmp_generation(self, _mock_stdout: StringIO) -> None:
"""
Ensure that docker-compose.tmp.yml is correctly generated, even when
mounts are processed more than once.
Ensure that docker-compose.tmp.yml is correctly generated.
"""
param = compose.MountParam()
mount_args: t.Tuple[t.List[compose.MountParam.MountType], ...] = (
# Auto-mounting of edx-platform to lms* and cms*
param("/path/to/edx-platform"),
# Manual mounting of some other folder to mfe and lms
param("mfe,lms:/path/to/something-else:/openedx/something-else"),
mount_args: t.List[compose.MountParam.MountType] = []
# Auto-mounting of edx-platform to lms* and cms*
mount_args += param.convert_implicit_form("/path/to/edx-platform")
# Manual mounting of some other folder to mfe and lms
mount_args += param.convert_explicit_form(
"mfe,lms:/path/to/something-else:/openedx/something-else"
)

# In some cases, process_mount_arguments ends up being called more
# than once. To ensure that we can handle that situation, we call it
# multiple times here.
compose.process_mount_arguments(mount_args)
compose.process_mount_arguments(mount_args)
compose.process_mount_arguments(mount_args)
# Mount volumes
compose.mount_tmp_volumes(mount_args, compose.BaseComposeContext(""))

compose_file: t.Dict[str, t.Any] = hooks.Filters.COMPOSE_LOCAL_TMP.apply({})
actual_services: t.Dict[str, t.Any] = compose_file["services"]
Expand Down
Loading

0 comments on commit 585495e

Please sign in to comment.