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

WIP: Migrate Makefile calls to use compose plugin syntax #1128

Closed

Conversation

VanDavv
Copy link

@VanDavv VanDavv commented Jul 24, 2023

I've encountered an issue while trying to setup devstack on Debian 12 with fresh Docker installation.

Official docker installation instructions available here encourage users to install docker along with two plugins: Buildx and Compose

This allows developers to both utilize multi-arch builds with Buildx and to utilize the yaml-based approach in setting up containers with Compose

However, the way to use the docker compose, as installed by default, is by calling

docker compose ...

Whereas the "old" way of using it, by installing a Python script, was by calling

docker-compose ...

This PR changes the calls from docker-compose to docker compose in the Makefile of the devstack.
People who might be affected negatively by this change are the developers who still use the docker compose installed outside the plugin. It can be mitigated by installing compose plugin by running

sudo apt-get install docker-compose-plugin

Without this PR, people who followed official Docker docs will have to install docker compose once again, as a Python script, to cover for the syntax change of the command

Here are my logs as I tried to use the Makefile with fresh docker and compose installation

$ make dev.pull.lms+frontend-app-learning
make[1]: Entering directory '/home/vandavv/dev/edx/edx-devstack'
docker-compose pull --include-deps $(echo lms+frontend-app-learning | tr + " ")
/bin/sh: 1: docker-compose: not found
make[1]: *** [Makefile:208: impl-dev.pull.lms+frontend-app-learning] Error 127
make[1]: Leaving directory '/home/vandavv/dev/edx/edx-devstack'
Would you like to assist devstack development by sending anonymous usage metrics to edX? Run `make metrics-opt-in` to learn more!
make: *** [Makefile:205: dev.pull.lms+frontend-app-learning] Error 2
$ docker compose version
Docker Compose version v2.19.1

With this PR, the command make dev.pull.lms+frontend-app-learning was completed successfully

EDIT

I have also updated different provisioning scripts to use the new non-hyphen syntax, but introducing those would require a lot of testing I believe. Will update this PR to be a draft PR and will test on my side whether there are any other implications of moving from docker-compose to docker compose


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)

@openedx-webhooks openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Jul 24, 2023
@openedx-webhooks
Copy link

openedx-webhooks commented Jul 24, 2023

Thanks for the pull request, @VanDavv! Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

  • supporting documentation
  • Open edX discussion forum threads
  • timeline information ("this must be merged by XX date", and why that is)
  • partner information ("this is a course on edx.org")
  • any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

This is currently a draft pull request. When it is ready for our review and all tests are green, click "Ready for Review", or remove "WIP" from the title, as appropriate.

⚠️ We can't start reviewing your pull request until you've submitted a signed contributor agreement or indicated your institutional affiliation. Please see the CONTRIBUTING file for more information. If you've signed an agreement in the past, you may need to re-sign. See The New Home of the Open edX Codebase for details.

Once you've signed the CLA, please allow 1 business day for it to be processed. After this time, you can re-run the CLA check by editing the PR title. If the problem persists, you can tag the @openedx/cla-problems team in a comment on your PR for further assistance.

@VanDavv VanDavv changed the title Migrate Makefile calls to use compose plugin syntax WIP: Migrate Makefile calls to use compose plugin syntax Jul 24, 2023
@VanDavv
Copy link
Author

VanDavv commented Jul 24, 2023

I've pushed the migrated provisioning scripts as I was successfully able to run make dev.provision with the updated files. Will leave it as is for now and fall back to Python script

@itsjeyd
Copy link

itsjeyd commented Jul 26, 2023

Hey @VanDavv, thank you for your contribution!

Please follow the steps for signing a Contributor Agreement that the PR bot mentioned above.

Once that's done we can proceed with enabling tests for this PR.

@itsjeyd itsjeyd added the waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. label Jul 26, 2023
@itsjeyd
Copy link

itsjeyd commented Aug 8, 2023

Hi @VanDavv, just checking in to see if you already had a chance to sign the Contributor Agreement?

@itsjeyd
Copy link

itsjeyd commented Sep 13, 2023

Hey @VanDavv, just checking in to see if you're still planning to continue working on this PR?

@itsjeyd
Copy link

itsjeyd commented Sep 26, 2023

Hi @VanDavv, just a quick update that we're closing this PR now because it is stale. You may reopen this pull request, or open a new one, when you have time to come back to this work.

@itsjeyd itsjeyd closed this Sep 26, 2023
@openedx-webhooks
Copy link

@VanDavv Even though your pull request wasn’t merged, please take a moment to answer a two question survey so we can improve your experience in the future.

1 similar comment
@openedx-webhooks
Copy link

@VanDavv Even though your pull request wasn’t merged, please take a moment to answer a two question survey so we can improve your experience in the future.

@itsjeyd itsjeyd added closed inactivity PR was closed because the author abandoned it and removed waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. labels Sep 26, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
closed inactivity PR was closed because the author abandoned it open-source-contribution PR author is not from Axim or 2U
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants