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 overhangio#711. Close also overhangio/tutor-mfe#57.
  • Loading branch information
regisb authored and moonesque committed Jul 30, 2022
1 parent e5850eb commit ba2c241
Show file tree
Hide file tree
Showing 7 changed files with 225 additions and 106 deletions.
18 changes: 18 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,24 @@ 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)
- [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)
- [BugFix] `kubectl wait` checks deployments instead of pods as it could hang indefinitely if there are extra pods in a broken state. (by @keithgg)

## v14.0.3 (2022-07-09)

- [Bugfix] Build openedx-dev Docker image even when the host user is root, for instance on Windows. (by @regisb)
- [Bugfix] Patch nutmeg.1 release with [LTI 1.3 fix](https://github.com/openedx/edx-platform/pull/30716). (by @ormsbee)
- [Improvement] Make it possible to override k8s resources in plugins using `k8s-override` patch. (by @foadlind)

## v14.0.2 (2022-06-27)

- [Bugfix] Update problem with hint template so it works with newer python versions. (by @mariajgrimaldi)
- [Feature] Add default PYTHONBREAKPOINT to openedx/Dockerfile (by @Carlos-Muniz)
- [Bugfix] Fix smtp server port in `cms.yml` which was causing email sending failures in the Studio. (by @regisb)
- [Bugfix] Skip waiting for MongoDB if it is served using SRV records. (by @gabor-boros)
- [Improvement] Use `git am` instead of `cherry-pick` to simplify patching process.

## v14.0.1 (2022-06-13)
Expand Down
46 changes: 46 additions & 0 deletions tests/commands/test_compose.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
import unittest
from io import StringIO
from unittest.mock import patch

from click.exceptions import ClickException

from tutor.commands import compose
from tutor.commands.local import LocalContext


class ComposeTests(unittest.TestCase):
Expand Down Expand Up @@ -36,3 +39,46 @@ def test_mount_option_parsing(self) -> None:
)
with self.assertRaises(ClickException):
param("lms,:/path/to/edx-platform:/openedx/edx-platform")

@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.
"""
param = compose.MountParam()
mount_args = (
# Auto-mounting of edx-platform to lms* and cms*
param.convert_implicit_form("/path/to/edx-platform"),
# Manual mounting of some other folder to mfe and lms
param.convert_explicit_form(
"mfe,lms:/path/to/something-else:/openedx/something-else"
),
)
# Mount volumes
compose.mount_tmp_volumes(mount_args, LocalContext(""))

compose_file: t.Dict[str, t.Any] = hooks.Filters.COMPOSE_LOCAL_TMP.apply({})
actual_services: t.Dict[str, t.Any] = compose_file["services"]
expected_services: t.Dict[str, t.Any] = {
"cms": {"volumes": ["/path/to/edx-platform:/openedx/edx-platform"]},
"cms-worker": {"volumes": ["/path/to/edx-platform:/openedx/edx-platform"]},
"lms": {
"volumes": [
"/path/to/edx-platform:/openedx/edx-platform",
"/path/to/something-else:/openedx/something-else",
]
},
"lms-worker": {"volumes": ["/path/to/edx-platform:/openedx/edx-platform"]},
"mfe": {"volumes": ["/path/to/something-else:/openedx/something-else"]},
}
self.assertEqual(actual_services, expected_services)

compose_jobs_file: t.Dict[
str, t.Any
] = hooks.Filters.COMPOSE_LOCAL_JOBS_TMP.apply({})
actual_jobs_services: t.Dict[str, t.Any] = compose_jobs_file["services"]
expected_jobs_services: t.Dict[str, t.Any] = {
"cms-job": {"volumes": ["/path/to/edx-platform:/openedx/edx-platform"]},
"lms-job": {"volumes": ["/path/to/edx-platform:/openedx/edx-platform"]},
}
self.assertEqual(actual_jobs_services, expected_jobs_services)
Loading

0 comments on commit ba2c241

Please sign in to comment.