Skip to content
This repository has been archived by the owner on Aug 1, 2024. It is now read-only.

Remove docker-compose from devstack #1172

Merged
merged 23 commits into from
Sep 14, 2023
Merged

Conversation

rgraber
Copy link
Contributor

@rgraber rgraber commented Sep 11, 2023

Replace docker-compose with docker compose in all instances. Docker-compose is no longer supported. For macs, this requires installing the docker compose plugin.
Also adds provisioning tests for analyticsapi and insights to the PR checks. Coursegraph needs its neo4j image updated before tests will pass so I left it out.


I've completed each of the following or determined they are not applicable:

  • Made a plan to communicate any major developer interface changes (or N/A)

@rgraber rgraber force-pushed the rsgraber/20230831-rm-docker-compose branch from 53a1c8e to dfb7318 Compare September 11, 2023 18:44
@rgraber rgraber marked this pull request as ready for review September 13, 2023 18:39
@@ -57,6 +57,9 @@ jobs:
brew install lima docker
limactl start --name=default template://docker
echo "DOCKER_HOST=unix:///Users/runner/.lima/default/sock/docker.sock" >> $GITHUB_ENV
# install the docker compose plugin
mkdir -p ~/.docker/cli-plugins
Copy link
Contributor Author

@rgraber rgraber Sep 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is where the docker compose plugin is made discoverable

@@ -28,7 +28,7 @@ jobs:
os:
- ubuntu-20.04 # Ubuntu 20.04 "Focal Fossa"
python-version: [ '3.8' ]
services: [ discovery+lms+forum ,registrar+lms, ecommerce+lms, edx_notes_api+lms, credentials+lms, xqueue]
services: [ discovery+lms+forum ,registrar+lms, ecommerce+lms, edx_notes_api+lms, credentials+lms, xqueue, analyticsapi+insights+lms]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add analytics and insights to provisioning tests

@@ -1,4 +1,4 @@
-c constraints.txt

docker-compose # For launching the devstack Docker containers
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove the PyPI docker-compose, which pinned PyYAML to a version that didn't work on Apple Silicon

@@ -48,7 +48,7 @@ jobs:
sudo apt update
sudo apt install docker-ce containerd.io
docker version
docker-compose --version
docker compose --version
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[inform] this spaced version also works with podman

➜  docker compose --version
podman version 4.6.2

Copy link
Contributor

@dianakhuang dianakhuang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this looks good. Feel free to wait for Tim to test on Linux and verify that we don't need to do any soft linking shenanigans there.

@timmc-edx
Copy link
Contributor

docs/getting_started.rst has a "Prerequisites" section that describes what you need to install for Linux and Mac. I think we'll want to update that as well.

For Linux, developers on Ubuntu (and Debian) should ensure they've uninstalled docker.io and docker-compose from the main Ubuntu repositories (this is why I was inadvertently using an old version) and instead install docker-ce and docker-compose-plugin from the official Docker package repository: https://docs.docker.com/engine/install/ubuntu/ -- not sure what the minimum version would be, although maybe we don't need to include that.

@timmc-edx
Copy link
Contributor

Optionally, we may also want to give a bit of advice somewhere in the docs on how to work with older versions of devstack that still use docker-compose. What works for me on Linux, and will probably also work on Mac, is to make this tiny shell script and put it on the PATH under the name docker-compose:

#!/bin/bash

docker compose "$@"

@rgraber
Copy link
Contributor Author

rgraber commented Sep 14, 2023

Optionally, we may also want to give a bit of advice somewhere in the docs on how to work with older versions of devstack that still use docker-compose. What works for me on Linux, and will probably also work on Mac, is to make this tiny shell script and put it on the PATH under the name docker-compose:

#!/bin/bash

docker compose "$@"

Do we want to give advice for older devstack or just add to the solve-by-updating section of troubleshooting?

@timmc-edx
Copy link
Contributor

I was actually thinking of mentioning it in the "Developing on Open edX named release branches" file, but it's probably worth mentioning on the troubleshooting page too, yeah.

@rgraber
Copy link
Contributor Author

rgraber commented Sep 14, 2023

I was actually thinking of mentioning it in the "Developing on Open edX named release branches" file, but it's probably worth mentioning on the troubleshooting page too, yeah.

There's already a recommendation in there for keeping devstack on master. I worry adding stuff about workarounds for older devstacks will make things more confusing rather than less. I'm also not sure what exact bug you'd get if you didn't update devstack (to put in the troubleshooting guide).

@timmc-edx
Copy link
Contributor

The docs I'm looking at say to check out the named-release branch of devstack rather than staying on master. That means that if someone wants to work on Palm, they check out open-release/palm.master, and now the Makefile is referencing docker-compose. So if someone wants to work on both Palm and Quince, they'll need both docker-compose and docker compose to work.

@rgraber
Copy link
Contributor Author

rgraber commented Sep 14, 2023

The docs I'm looking at say to check out the named-release branch of devstack rather than staying on master. That means that if someone wants to work on Palm, they check out open-release/palm.master, and now the Makefile is referencing docker-compose. So if someone wants to work on both Palm and Quince, they'll need both docker-compose and docker compose to work.

Gotcha. Will update

@rgraber rgraber merged commit 676d333 into master Sep 14, 2023
11 checks passed
@rgraber rgraber deleted the rsgraber/20230831-rm-docker-compose branch September 14, 2023 17:49
@rgraber rgraber mentioned this pull request Sep 14, 2023
4 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants