From 079fb1c9ec5f904d2bec065c7b1179da9517ca31 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9gis=20Behmo?= Date: Tue, 5 Oct 2021 00:29:26 +0200 Subject: [PATCH 1/3] fix: bypass build to accelerate "local start" Previously, we were building all images every time we ran a "local start" command. This was causing unnecessary rebuild. Here, instead, we make use of the `docker-compose up --build`. This means that only the required images will be rebuilt. --- CHANGELOG.md | 2 ++ tutor/commands/compose.py | 4 +--- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 32db045cd0..12adfbc6a5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,8 @@ Note: Breaking changes between versions are indicated by "💥". ## Unreleased +- [Improvement] Faster `tutor local start` by building only necessary images. + ## v12.1.5 (2021-10-25) - 💥[Improvement] Change the `settheme` command such that, by default, a custom theme is assigned to the LMS and the CMS, both in production and development mode. diff --git a/tutor/commands/compose.py b/tutor/commands/compose.py index 6495a1a968..f0924191ca 100644 --- a/tutor/commands/compose.py +++ b/tutor/commands/compose.py @@ -65,13 +65,11 @@ def run_job(self, service: str, command: str) -> int: @click.argument("services", metavar="service", nargs=-1) @click.pass_obj def start(context: Context, detach: bool, services: List[str]) -> None: - command = ["up", "--remove-orphans"] + command = ["up", "--remove-orphans", "--build"] if detach: command.append("-d") config = tutor_config.load(context.root) - # Rebuild Docker images with a `build: ...` context. - context.docker_compose(context.root, config, "build") # Start services context.docker_compose(context.root, config, *command, *services) From 02536e0f9f96bbcb2c613f4956b70c93740de194 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9gis=20Behmo?= Date: Mon, 27 Sep 2021 11:48:17 +0200 Subject: [PATCH 2/3] refactor: better runner inheritance architecture Before, custom `docker_compose_func` arguments had to be passed to job runners. This was not very elegant. Also, it prevented us from loading custom job files in development. Here, we adopt a better object-oriented approach, where context classes are ordered hierarchically. This paves the way for loading `dev/docker-compose.jobs.yml` files in `tutor dev init` commands -- which will be necessary to fix permissions in dev/local mode. --- tutor/bindmounts.py | 15 +++----- tutor/commands/compose.py | 72 ++++++++++++++++++--------------------- tutor/commands/context.py | 29 ++++++++++++---- tutor/commands/dev.py | 57 +++++++++++++++++-------------- tutor/commands/local.py | 55 +++++++++++++++++------------- tutor/jobs.py | 14 ++++++++ 6 files changed, 137 insertions(+), 105 deletions(-) diff --git a/tutor/bindmounts.py b/tutor/bindmounts.py index 2324d61f4e..0fa18986f0 100644 --- a/tutor/bindmounts.py +++ b/tutor/bindmounts.py @@ -1,22 +1,19 @@ import os -from typing import Callable, List, Tuple +from typing import List, Tuple import click -from mypy_extensions import VarArg from .exceptions import TutorError -from .types import Config +from .jobs import BaseComposeJobRunner from .utils import get_user_id def create( - root: str, - config: Config, - docker_compose_func: Callable[[str, Config, VarArg(str)], int], + runner: BaseComposeJobRunner, service: str, path: str, ) -> str: - volumes_root_path = get_root_path(root) + volumes_root_path = get_root_path(runner.root) volume_name = get_name(path) container_volumes_root_path = "/tmp/volumes" command = """rm -rf {volumes_path}/{volume_name} @@ -33,9 +30,7 @@ def create( if not os.path.exists(volumes_root_path): os.makedirs(volumes_root_path) - docker_compose_func( - root, - config, + runner.docker_compose( "run", "--rm", "--no-deps", diff --git a/tutor/commands/compose.py b/tutor/commands/compose.py index f0924191ca..0e39e247ab 100644 --- a/tutor/commands/compose.py +++ b/tutor/commands/compose.py @@ -1,8 +1,7 @@ import os -from typing import Callable, List +from typing import List import click -from mypy_extensions import VarArg from .. import bindmounts from .. import config as tutor_config @@ -12,19 +11,10 @@ from .. import jobs from ..types import Config from .. import utils -from .context import Context +from .context import BaseJobContext -class ComposeJobRunner(jobs.BaseJobRunner): - def __init__( - self, - root: str, - config: Config, - docker_compose_func: Callable[[str, Config, VarArg(str)], int], - ): - super().__init__(root, config) - self.docker_compose_func = docker_compose_func - +class ComposeJobRunner(jobs.BaseComposeJobRunner): def run_job(self, service: str, command: str) -> int: """ Run the "{{ service }}-job" service from local/docker-compose.jobs.yml with the @@ -45,9 +35,7 @@ def run_job(self, service: str, command: str) -> int: if not utils.is_a_tty(): run_command += ["-T"] job_service_name = "{}-job".format(service) - return self.docker_compose_func( - self.root, - self.config, + return self.docker_compose( *run_command, job_service_name, "sh", @@ -57,6 +45,11 @@ def run_job(self, service: str, command: str) -> int: ) +class BaseComposeContext(BaseJobContext): + def job_runner(self, config: Config) -> ComposeJobRunner: + raise NotImplementedError + + @click.command( short_help="Run all or a selection of services.", help="Run all or a selection of services. Docker images will be rebuilt where necessary.", @@ -64,22 +57,22 @@ def run_job(self, service: str, command: str) -> int: @click.option("-d", "--detach", is_flag=True, help="Start in daemon mode") @click.argument("services", metavar="service", nargs=-1) @click.pass_obj -def start(context: Context, detach: bool, services: List[str]) -> None: +def start(context: BaseComposeContext, detach: bool, services: List[str]) -> None: command = ["up", "--remove-orphans", "--build"] if detach: command.append("-d") - config = tutor_config.load(context.root) # Start services - context.docker_compose(context.root, config, *command, *services) + config = tutor_config.load(context.root) + context.job_runner(config).docker_compose(*command, *services) @click.command(help="Stop a running platform") @click.argument("services", metavar="service", nargs=-1) @click.pass_obj -def stop(context: Context, services: List[str]) -> None: +def stop(context: BaseComposeContext, services: List[str]) -> None: config = tutor_config.load(context.root) - context.docker_compose(context.root, config, "stop", *services) + context.job_runner(config).docker_compose("stop", *services) @click.command( @@ -103,7 +96,7 @@ def reboot(context: click.Context, detach: bool, services: List[str]) -> None: ) @click.argument("services", metavar="service", nargs=-1) @click.pass_obj -def restart(context: Context, services: List[str]) -> None: +def restart(context: BaseComposeContext, services: List[str]) -> None: config = tutor_config.load(context.root) command = ["restart"] if "all" in services: @@ -117,15 +110,15 @@ def restart(context: Context, services: List[str]) -> None: command += ["cms", "cms-worker"] else: command.append(service) - context.docker_compose(context.root, config, *command) + context.job_runner(config).docker_compose(*command) @click.command(help="Initialise all applications") @click.option("-l", "--limit", help="Limit initialisation to this service or plugin") @click.pass_obj -def init(context: Context, limit: str) -> None: +def init(context: BaseComposeContext, limit: str) -> None: config = tutor_config.load(context.root) - runner = ComposeJobRunner(context.root, config, context.docker_compose) + runner = context.job_runner(config) jobs.initialise(runner, limit_to=limit) @@ -141,10 +134,15 @@ def init(context: Context, limit: str) -> None: @click.argument("email") @click.pass_obj def createuser( - context: Context, superuser: str, staff: bool, password: str, name: str, email: str + context: BaseComposeContext, + superuser: str, + staff: bool, + password: str, + name: str, + email: str, ) -> None: config = tutor_config.load(context.root) - runner = ComposeJobRunner(context.root, config, context.docker_compose) + runner = context.job_runner(config) command = jobs.create_user_command(superuser, staff, name, email, password=password) runner.run_job("lms", command) @@ -164,18 +162,18 @@ def createuser( ) @click.argument("theme_name") @click.pass_obj -def settheme(context: Context, domains: List[str], theme_name: str) -> None: +def settheme(context: BaseComposeContext, domains: List[str], theme_name: str) -> None: config = tutor_config.load(context.root) - runner = ComposeJobRunner(context.root, config, context.docker_compose) + runner = context.job_runner(config) domains = domains or jobs.get_all_openedx_domains(config) jobs.set_theme(theme_name, domains, runner) @click.command(help="Import the demo course") @click.pass_obj -def importdemocourse(context: Context) -> None: +def importdemocourse(context: BaseComposeContext) -> None: config = tutor_config.load(context.root) - runner = ComposeJobRunner(context.root, config, context.docker_compose) + runner = context.job_runner(config) fmt.echo_info("Importing demo course") jobs.import_demo_course(runner) @@ -207,11 +205,9 @@ def run(context: click.Context, args: List[str]) -> None: ) @click.argument("path") @click.pass_obj -def bindmount_command(context: Context, service: str, path: str) -> None: +def bindmount_command(context: BaseComposeContext, service: str, path: str) -> None: config = tutor_config.load(context.root) - host_path = bindmounts.create( - context.root, config, context.docker_compose, service, path - ) + host_path = bindmounts.create(context.job_runner(config), service, path) fmt.echo_info( "Bind-mount volume created at {}. You can now use it in all `local` and `dev` commands with the `--volume={}` option.".format( host_path, path @@ -265,7 +261,7 @@ def logs(context: click.Context, follow: bool, tail: bool, service: str) -> None @click.argument("command") @click.argument("args", nargs=-1) @click.pass_obj -def dc_command(context: Context, command: str, args: List[str]) -> None: +def dc_command(context: BaseComposeContext, command: str, args: List[str]) -> None: config = tutor_config.load(context.root) volumes, non_volume_args = bindmounts.parse_volumes(args) volume_args = [] @@ -282,9 +278,7 @@ def dc_command(context: Context, command: str, args: List[str]) -> None: ) volume_arg = "{}:{}".format(host_bind_path, volume_arg) volume_args += ["--volume", volume_arg] - context.docker_compose( - context.root, config, command, *volume_args, *non_volume_args - ) + context.job_runner(config).docker_compose(command, *volume_args, *non_volume_args) def add_commands(command_group: click.Group) -> None: diff --git a/tutor/commands/context.py b/tutor/commands/context.py index 3d24220b2b..b85fb3bd70 100644 --- a/tutor/commands/context.py +++ b/tutor/commands/context.py @@ -1,15 +1,30 @@ +from ..jobs import BaseJobRunner from ..types import Config -def unimplemented_docker_compose(root: str, config: Config, *command: str) -> int: - raise NotImplementedError +class Context: + """ + Context object that is passed to all subcommands. + The project `root` is passed to all subcommands of `tutor`; that's because + it is defined as an argument of the top-level command. For instance: + + tutor --root=... local run ... + """ -# pylint: disable=too-few-public-methods -class Context: def __init__(self, root: str) -> None: self.root = root - self.docker_compose_func = unimplemented_docker_compose - def docker_compose(self, root: str, config: Config, *command: str) -> int: - return self.docker_compose_func(root, config, *command) + +class BaseJobContext(Context): + """ + Specialized context that subcommands may use. + + For instance `dev`, `local` and `k8s` define custom runners to run jobs. + """ + + def job_runner(self, config: Config) -> BaseJobRunner: + """ + Return a runner capable of running docker-compose/kubectl commands. + """ + raise NotImplementedError diff --git a/tutor/commands/dev.py b/tutor/commands/dev.py index b867503ba4..df6f678bd8 100644 --- a/tutor/commands/dev.py +++ b/tutor/commands/dev.py @@ -9,36 +9,43 @@ from ..types import Config from .. import utils from . import compose -from .context import Context -def docker_compose(root: str, config: Config, *command: str) -> int: - """ - Run docker-compose with dev arguments. - """ - args = [] - for folder in ["local", "dev"]: - # Add docker-compose.yml and docker-compose.override.yml (if it exists) - # from "local" and "dev" folders (but not docker-compose.prod.yml) - args += [ - "-f", - tutor_env.pathjoin(root, folder, "docker-compose.yml"), - ] - override_path = tutor_env.pathjoin(root, folder, "docker-compose.override.yml") - if os.path.exists(override_path): - args += ["-f", override_path] - return utils.docker_compose( - *args, - "--project-name", - str(config["DEV_PROJECT_NAME"]), - *command, - ) +class DevJobRunner(compose.ComposeJobRunner): + def docker_compose(self, *command: str) -> int: + """ + Run docker-compose with dev arguments. + """ + args = [] + for folder in ["local", "dev"]: + # Add docker-compose.yml and docker-compose.override.yml (if it exists) + # from "local" and "dev" folders (but not docker-compose.prod.yml) + args += [ + "-f", + tutor_env.pathjoin(self.root, folder, "docker-compose.yml"), + ] + override_path = tutor_env.pathjoin( + self.root, folder, "docker-compose.override.yml" + ) + if os.path.exists(override_path): + args += ["-f", override_path] + return utils.docker_compose( + *args, + "--project-name", + str(self.config["DEV_PROJECT_NAME"]), + *command, + ) + + +class DevContext(compose.BaseComposeContext): + def job_runner(self, config: Config) -> DevJobRunner: + return DevJobRunner(self.root, config) @click.group(help="Run Open edX locally with development settings") -@click.pass_obj -def dev(context: Context) -> None: - context.docker_compose_func = docker_compose +@click.pass_context +def dev(context: click.Context) -> None: + context.obj = DevContext(context.obj.root) @click.command( diff --git a/tutor/commands/local.py b/tutor/commands/local.py index 700f82f622..d417ce0762 100644 --- a/tutor/commands/local.py +++ b/tutor/commands/local.py @@ -11,33 +11,40 @@ from .. import exceptions from . import compose from .config import save as config_save_command -from .context import Context - - -def docker_compose(root: str, config: Config, *command: str) -> int: - """ - Run docker-compose with local and production yml files. - """ - args = [] - override_path = tutor_env.pathjoin(root, "local", "docker-compose.override.yml") - if os.path.exists(override_path): - args += ["-f", override_path] - return utils.docker_compose( - "-f", - tutor_env.pathjoin(root, "local", "docker-compose.yml"), - "-f", - tutor_env.pathjoin(root, "local", "docker-compose.prod.yml"), - *args, - "--project-name", - get_typed(config, "LOCAL_PROJECT_NAME", str), - *command - ) + + +class LocalJobRunner(compose.ComposeJobRunner): + def docker_compose(self, *command: str) -> int: + """ + Run docker-compose with local and production yml files. + """ + args = [] + override_path = tutor_env.pathjoin( + self.root, "local", "docker-compose.override.yml" + ) + if os.path.exists(override_path): + args += ["-f", override_path] + return utils.docker_compose( + "-f", + tutor_env.pathjoin(self.root, "local", "docker-compose.yml"), + "-f", + tutor_env.pathjoin(self.root, "local", "docker-compose.prod.yml"), + *args, + "--project-name", + get_typed(self.config, "LOCAL_PROJECT_NAME", str), + *command + ) + + +class LocalContext(compose.BaseComposeContext): + def job_runner(self, config: Config) -> LocalJobRunner: + return LocalJobRunner(self.root, config) @click.group(help="Run Open edX locally with docker-compose") -@click.pass_obj -def local(context: Context) -> None: - context.docker_compose_func = docker_compose +@click.pass_context +def local(context: click.Context) -> None: + context.obj = LocalContext(context.obj.root) @click.command(help="Configure and run Open edX from scratch") diff --git a/tutor/jobs.py b/tutor/jobs.py index f9b0adfff3..6a326e92a2 100644 --- a/tutor/jobs.py +++ b/tutor/jobs.py @@ -10,6 +10,10 @@ class BaseJobRunner: + """ + A job runner is responsible for getting a certain task to complete. + """ + def __init__(self, root: str, config: Config): self.root = root self.config = config @@ -25,6 +29,11 @@ def render(self, *path: str) -> str: return rendered def run_job(self, service: str, command: str) -> int: + """ + Given a (potentially large) string command, run it with the + corresponding service. Implementations will differ depending on the + deployment strategy. + """ raise NotImplementedError def iter_plugin_hooks( @@ -33,6 +42,11 @@ def iter_plugin_hooks( yield from plugins.iter_hooks(self.config, hook) +class BaseComposeJobRunner(BaseJobRunner): + def docker_compose(self, *command: str) -> int: + raise NotImplementedError + + def initialise(runner: BaseJobRunner, limit_to: Optional[str] = None) -> None: fmt.echo_info("Initialising all services...") if limit_to is None or limit_to == "mysql": From d73d6732d55294d4fedff1b59ccea207a008f609 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9gis=20Behmo?= Date: Mon, 27 Sep 2021 12:32:28 +0200 Subject: [PATCH 3/3] feat: make it possible to override jobs in dev Previously, job declarations were always loaded from local/docker-compose.yml and local/docker-compose.jobs.yml. This meant that it was not possible to override job declarations in dev mode. It is now the case, with dev/docker-compose.jobs.yml and dev/docker-compose.jobs.override.yml. Neither of these files exist yet... But who knows? we might need this feature one day. In any case the code is much cleaner now. --- CHANGELOG.md | 1 + tutor/commands/compose.py | 32 +++++++++++----- tutor/commands/dev.py | 41 +++++++++------------ tutor/commands/local.py | 30 ++++++--------- tutor/templates/dev/docker-compose.jobs.yml | 3 ++ 5 files changed, 56 insertions(+), 51 deletions(-) create mode 100644 tutor/templates/dev/docker-compose.jobs.yml diff --git a/CHANGELOG.md b/CHANGELOG.md index 12adfbc6a5..df1cf2e630 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ Note: Breaking changes between versions are indicated by "💥". ## Unreleased +- [Feature] Make it possible to override job configuration in development: if they exist, `dev/docker-compose.jobs.yml` and `dev/docker-compose.jobs.override.yml` will be loaded when running jobs. - [Improvement] Faster `tutor local start` by building only necessary images. ## v12.1.5 (2021-10-25) diff --git a/tutor/commands/compose.py b/tutor/commands/compose.py index 0e39e247ab..a78274c6bb 100644 --- a/tutor/commands/compose.py +++ b/tutor/commands/compose.py @@ -15,6 +15,24 @@ class ComposeJobRunner(jobs.BaseComposeJobRunner): + def __init__(self, root: str, config: Config): + super().__init__(root, config) + self.project_name = "" + self.docker_compose_files: List[str] = [] + self.docker_compose_job_files: List[str] = [] + + def docker_compose(self, *command: str) -> int: + """ + Run docker-compose with the right yml files. + """ + args = [] + for docker_compose_path in self.docker_compose_files: + if os.path.exists(docker_compose_path): + args += ["-f", docker_compose_path] + return utils.docker_compose( + *args, "--project-name", self.project_name, *command + ) + def run_job(self, service: str, command: str) -> int: """ Run the "{{ service }}-job" service from local/docker-compose.jobs.yml with the @@ -22,15 +40,11 @@ def run_job(self, service: str, command: str) -> int: service does not exist, run the service from good old regular docker-compose.yml. """ - run_command = [ - "-f", - tutor_env.pathjoin(self.root, "local", "docker-compose.jobs.yml"), - ] - override_path = tutor_env.pathjoin( - self.root, "local", "docker-compose.jobs.override.yml" - ) - if os.path.exists(override_path): - run_command += ["-f", override_path] + run_command = [] + for docker_compose_path in self.docker_compose_job_files: + path = tutor_env.pathjoin(self.root, docker_compose_path) + if os.path.exists(path): + run_command += ["-f", path] run_command += ["run", "--rm"] if not utils.is_a_tty(): run_command += ["-T"] diff --git a/tutor/commands/dev.py b/tutor/commands/dev.py index df6f678bd8..62fbaeaa46 100644 --- a/tutor/commands/dev.py +++ b/tutor/commands/dev.py @@ -1,4 +1,3 @@ -import os from typing import List import click @@ -6,35 +5,29 @@ from .. import config as tutor_config from .. import env as tutor_env from .. import fmt -from ..types import Config -from .. import utils +from ..types import Config, get_typed from . import compose class DevJobRunner(compose.ComposeJobRunner): - def docker_compose(self, *command: str) -> int: + def __init__(self, root: str, config: Config): """ - Run docker-compose with dev arguments. + Load docker-compose files from dev/ and local/ """ - args = [] - for folder in ["local", "dev"]: - # Add docker-compose.yml and docker-compose.override.yml (if it exists) - # from "local" and "dev" folders (but not docker-compose.prod.yml) - args += [ - "-f", - tutor_env.pathjoin(self.root, folder, "docker-compose.yml"), - ] - override_path = tutor_env.pathjoin( - self.root, folder, "docker-compose.override.yml" - ) - if os.path.exists(override_path): - args += ["-f", override_path] - return utils.docker_compose( - *args, - "--project-name", - str(self.config["DEV_PROJECT_NAME"]), - *command, - ) + super().__init__(root, config) + self.project_name = get_typed(self.config, "DEV_PROJECT_NAME", str) + self.docker_compose_files += [ + tutor_env.pathjoin(self.root, "local", "docker-compose.yml"), + tutor_env.pathjoin(self.root, "dev", "docker-compose.yml"), + tutor_env.pathjoin(self.root, "local", "docker-compose.override.yml"), + tutor_env.pathjoin(self.root, "dev", "docker-compose.override.yml"), + ] + self.docker_compose_job_files += [ + tutor_env.pathjoin(self.root, "local", "docker-compose.jobs.yml"), + tutor_env.pathjoin(self.root, "dev", "docker-compose.jobs.yml"), + tutor_env.pathjoin(self.root, "local", "docker-compose.jobs.override.yml"), + tutor_env.pathjoin(self.root, "dev", "docker-compose.jobs.override.yml"), + ] class DevContext(compose.BaseComposeContext): diff --git a/tutor/commands/local.py b/tutor/commands/local.py index d417ce0762..8f9b97cbab 100644 --- a/tutor/commands/local.py +++ b/tutor/commands/local.py @@ -1,4 +1,3 @@ -import os from time import sleep import click @@ -6,7 +5,7 @@ from .. import config as tutor_config from .. import env as tutor_env from .. import fmt -from ..types import get_typed, Config +from ..types import Config, get_typed from .. import utils from .. import exceptions from . import compose @@ -14,26 +13,21 @@ class LocalJobRunner(compose.ComposeJobRunner): - def docker_compose(self, *command: str) -> int: + def __init__(self, root: str, config: Config): """ - Run docker-compose with local and production yml files. + Load docker-compose files from local/. """ - args = [] - override_path = tutor_env.pathjoin( - self.root, "local", "docker-compose.override.yml" - ) - if os.path.exists(override_path): - args += ["-f", override_path] - return utils.docker_compose( - "-f", + super().__init__(root, config) + self.project_name = get_typed(self.config, "LOCAL_PROJECT_NAME", str) + self.docker_compose_files += [ tutor_env.pathjoin(self.root, "local", "docker-compose.yml"), - "-f", tutor_env.pathjoin(self.root, "local", "docker-compose.prod.yml"), - *args, - "--project-name", - get_typed(self.config, "LOCAL_PROJECT_NAME", str), - *command - ) + tutor_env.pathjoin(self.root, "local", "docker-compose.override.yml"), + ] + self.docker_compose_job_files += [ + tutor_env.pathjoin(self.root, "local", "docker-compose.jobs.yml"), + tutor_env.pathjoin(self.root, "local", "docker-compose.jobs.override.yml"), + ] class LocalContext(compose.BaseComposeContext): diff --git a/tutor/templates/dev/docker-compose.jobs.yml b/tutor/templates/dev/docker-compose.jobs.yml new file mode 100644 index 0000000000..3e50ba37b6 --- /dev/null +++ b/tutor/templates/dev/docker-compose.jobs.yml @@ -0,0 +1,3 @@ +version: "3.7" +services: {% if not patch("dev-docker-compose-jobs-services") %}{}{% endif %} + {{ patch("dev-docker-compose-jobs-services")|indent(4) }}