docker-compose to docker compose #7626

gobbledy-gook · 2023-03-10T14:07:10Z

Closes #7594
Closes #7621

~~Depends on~~:

Update docker and docker-compose across production cluster #7676 (merged)

It tries to upgrade from current docker-compose command to docker compose.

Technical

Basically what I have done: Using git grep docker-compose and making necessary edits. Also the previous feedbacks were taken into consideration of not changing all the .yml files to .yaml files.

Testing

% pytest tests/test_docker_compose.py -- #7626 (comment)

When merging this PR, let's try:

pytest the old version of this file on the old codebase and make sure it passes.
pytest the new version of this file on the old codebase and make sure it fails.
pytest the old version of this file on the new codebase and make sure it fails.
pytest the new version of this file on the new codebase and make sure it passes.

Screenshot

Stakeholders

@cclauss @scottbarnes

scottbarnes

Thanks so much for all your work so far, @gobbledy-gook.

I made a first pass through the entire file. This is good progress.

Also, I am sorry for the ambiguity when I asked you to 'push an updated PR'. I should have made it more clear that you should push a new commit to your existing PR, possibly using --force if necessary. :)

For all the yaml -> yml suggestions that have no comment and merely revert the change, that is just because the change was outside of the scope of Docker Compose.

.github/workflows/python_tests.yml

compose.infogami-local.yaml

compose.override.yaml

compose.production.yaml

pyproject.toml

scripts/infobase-server

scripts/setup_olserver.sh

scripts/solr_builder/Dockerfile

openlibrary/templates/check_ins/reading_goal_progress.html

gobbledy-gook · 2023-03-13T05:50:21Z

Just to clarify, is there anything expected from my side at the moment ??

scottbarnes · 2023-03-13T06:43:47Z

@gobbledy-gook, my apologies for leaving you hanging.

Have you tried completely re-building and running the development server with all of these changes? If not, you should try that and let us know how it goes. :)

Although there is more to do besides that, it may be more on me at this moment. I will try to set aside time tomorrow to look at the Jenkins stuff and report back how that goes for me. But first, sleep. :)

scottbarnes · 2023-03-14T05:12:24Z

I tried this PR out in Gitpod, and it seems good:

gitpod /workspace/openlibrary (docker-compose) $ docker compose version
Docker Compose version v2.16.0-gitpod.0

The development server also came up without apparent error and seems to work fine, both inside Gitpod and on my local computer.

So I think we are good to go with Gitpod and the local development environment.

With respect to Jenkins and scripts/solr_builder/Dockerfile, I did some pretty straight forward updates to that Dockerfile (updating to Ubuntu Jammy and the latest version of Docker Engine). The process for doing whatever it is doing with Solr is taking quite a while, and could still fail in some way, so I will let that run over night. If it finishes successfully will share some suggested edits to that file.

Is there any reason scripts/solr_builder/Dockerfile shouldn't use Jammy? I didn't try to get it to run on Xenial just because it appears Docker stopped supporting it.

I think with Gitpod and the Jenkins / solr_builder stuff out of the way, the last remaining task may be updating, to the extent necessary, Docker on any system that runs scripts which would use the docker compose module.

scripts/solr_builder/Dockerfile

gobbledy-gook · 2023-03-15T09:12:36Z

I didn't intend to close the PR, I am trying to reopen it. I apologise for it !

compose.production.yaml

openlibrary/plugins/openlibrary/readbooks.py

openlibrary/solr/db_load_works.py

openlibrary/templates/borrow_admin.html

openlibrary/templates/borrow_admin_no_update.html

scripts/oldump.sh

scripts/setup_olserver.sh

scripts/solr_builder/psql-import-chunk.sh

scripts/solr_builder/psql-import-in-chunks.sh

static/images/categories/sci-fi.svg

scottbarnes · 2023-03-15T21:19:35Z

@gobbledy-gook, I think we are getting close to being able to test this out. :)

We have a few ways forward here:

You can trawl through the feedback, implement the suggestions (or say why you think they shouldn't be implemented, which is a completely valid response), and
rebase against an updated master branch (this can sometimes be troublesome for people: I followed this guide to help me through my first experience with that: https://www.digitalocean.com/community/tutorials/how-to-rebase-and-update-a-pull-request)

OR

If you get stuck we can schedule a Zoom call and work on it together.

OR

I can just make the changes.

How would you like to proceed?

gobbledy-gook · 2023-03-18T12:02:41Z

Sincerely apologise for not being able to do something in past two day, got busy with my academics! Looking it now and will keep updated !

gobbledy-gook · 2023-03-20T01:26:41Z

@cclauss @scottbarnes I have a doubt. When I don't see any necessary changes to be done how do I make sure that there are actually no changes needed ? What is a good method to check ?

I ask this because currently I don't see any changes that should be made. That issue of unnecessary (deleted files), I feel has been resolved now

scottbarnes · 2023-03-20T06:14:13Z

@gobbledy-gook, this looks really close. I think the stuff from this comment still needs to be added to scripts/solr_builder/Dockerfile, but I think that is it.

Great work on the rebasing by the way. I think this is just one step away from being able to deploy to a staging environmennt for testing. This issue is a bit hard for us to test on our own because of the need to test it in Jenkins, but we can get help with that once scripts/solr_builder/Dockerfile is updated.

Edit: my mistake, there are a few other unresolved suggestions to process (e.g. to openlibrary/plugins/openlibrary/code.py), but once those are done, then I believe it's ready for testing. :D

gobbledy-gook · 2023-03-20T09:07:47Z

As mentioned by @scottbarnes DockerFIle and openlibrary/plugins/openlibrary/code.py updated 👍🏻

scripts/setup_olserver.sh

gobbledy-gook · 2023-03-23T03:04:35Z

Thanks so much for all your work so far, @gobbledy-gook.

I made a first pass through the entire file. This is good progress.

Also, I am sorry for the ambiguity when I asked you to 'push an updated PR'. I should have made it more clear that you should push a new commit to your existing PR, possibly using --force if necessary. :)

For all the yaml -> yml suggestions that have no comment and merely revert the change, that is just because the change was outside of the scope of Docker Compose.

Are there still changes to be made ?? @scottbarnes

scottbarnes · 2023-03-23T04:33:24Z

I think we are there, @gobbledy-gook. Thanks for all your hard work. :D

There are a lot of changes, so I will make another pass through, test everything out the best I can locally, and if it's okay with you, I will squash everything and then we can hopefully get this more fully tested on the Solr staging server.

gobbledy-gook · 2023-03-23T08:23:45Z

Please give honest opinion about this @scottbarnes @cclauss after having done all this, I feel like this shouldn't have taken so long. I made many mistakes and hence the time delay... But that's how we learn I guess

scottbarnes · 2023-03-24T00:37:42Z

@gobbledy-gook, barring any additional input from @cclauss or @cdrini, I think we are good to go, but if for some reason you wish to make further updates to this PR, make sure you first pull the code down from your branch on GitHub so that you get the slightly modified code I pushed to the branch.

With respect code and other things not always being perfect every time, you need only look at my own pull requests to see how many mistakes I make. It turns out perfection isn't easy! That's why we have wizards like @cclauss and @cdrini and others to help us. (And I will tell you a secret: even they make mistakes sometimes, as shocking as this may be.) :)

I think there were lots of little gotchas in here, and I know it would have helped you had I given more background about which YAML references to update, etc., right at the start. Sometimes communication is hard because we might have a specific thing in our heads, and as obvious as this fact should be, it turns out not everyone can read our minds every time with exact accuracy.

Having said that, I made some minor changes to the PR, as you can see if you look at the files changed tab.

A few things that may make things smoother going forward:

when making very small changes to a PR, you may want to consider git --amend so that you don't create an new commit. For an exercise that demonstrates this, see Git Kata: Amending commits.
each time you accept a suggestion an GitHub, it creates a new commit, which can lead to a lot of commits. One way to reduce this is to click "Add suggestion to batch" instead of "Commit suggestion". This isn't great documentation, but there are some pictures here
if you end up with lots of little commits, try squashing them into one commit, or perhaps a few choice commits where each one is a feature or component of the whole PR. You can do this with git rebase -i and using the squash keyword in conjunction with one or more picks. For an exercise that demonstrates this, see Git Kata: Squash commits.
I am not entirely sure how, but some files that weren't related, such as openlibrary/templates/account/mybooks.html ended up as a part of this PR. I am far from a git expert, but I think this may have happened while merging from master (which could happen with a git pull upstream master while on your feature branch, as git pull will automatically merge. In any event, I removed these files by rebasing, squashing everything into a single commit as I did it, then using git reset --soft HEAD^ to reset the state of the repository and moving the pointer back to the previous commit, and then unstaging the extraneous files. For more info you can do a Google search for the differences between git reset --hard, --mixed and --soft. You can also read more about removing a file from a previous commit.

Finally, git can be confusing! I've reference the Git Kata exercises a few times. I have found them helpful: https://github.com/eficode-academy/git-katas/

Now, with bated breath, we await feedback from @cclauss before we pester @cdrini to put this on the Solr staging server. :)

compose.infogami-local.yaml

compose.override.yaml

cclauss · 2023-03-24T06:40:22Z

scripts/setup_olserver.sh

+# Remove any old versions and install newer versions of Docker Engine and Docker Compose.
+# See https://docs.docker.com/engine/install/ubuntu/ for any possible changes.
+sudo apt-get remove docker docker-engine docker.io containerd runc
+DOCKER_VERSION=5:20.10.7~3-0~ubuntu-focal


This needs to be updated but I am unclear on how to create DOCKER_VERSION that matches our current Docker versions.

See #7706

In scripts/setup_olserver.sh it is 5:20.10.7~3-0~ubuntu-focal but in scripts/solr_builder/Dockerfile it is 5:20.10.23~3-0~ubuntu-jammy. Is it supposed to be like that ? And here (Current Docker Verisons) it is 23.0.1.

So we need to change it to 23.0.1 ?

Yep switch to 5:23.0.6-1~ubuntu.20.04~focal According to chatgpt, apt-cache madison docker-ce displays all possible versions.

tests/test_docker_compose.py

cclauss

LGTM.

Let's get @cdrini to review...

gobbledy-gook · 2023-03-30T19:20:15Z

Any updates here ?!

scottbarnes · 2023-03-30T20:21:17Z

Any updates here ?!

Alas, at the end of the day, @cdrini is still only human, and as with much of the Open Library team, has more to do than can be done. I know this isn't a satisfactory answer, but at least we are in the queue to get this PR tested further, but I am not sure how soon that will be.

As with you, I am excited to see this get merged, though, so my fingers are crossed in the hope that we won't need to do anything more.

This PR: - replaces references to "docker-compose" with "docker compose"; - removes the "docker-" prefix from Compose files; - changes Compose file suffixes to "yaml"; and - updates any filenames. E.g. "docker-compose.yml" > "compose.yaml". Docker guidance prefers 'compose.yaml', and this PR follows that: https://docs.docker.com/compose/compose-file/03-compose-file/ Co-authored-by: Scott Barnes <scottreidbarnes@gmail.com>

cdrini

Awesome work @gobbledy-gook &co! This was a meaty one 😊 I'm going to merge this now since the core is working, any comments can be done in a future PR. Thank you for your patience @gobbledy-gook !

scottbarnes · 2023-05-23T21:52:58Z

@gobbledy-gook, thanks for all your hard work on this. Your changes have been deployed and now Open Library is using the fruits of your labor. Even better, Open Library can now also use the most recent versions of Docker Compose. 🎉

scottbarnes requested changes Mar 10, 2023

View reviewed changes

cclauss reviewed Mar 10, 2023

View reviewed changes

openlibrary/templates/check_ins/reading_goal_progress.html Outdated Show resolved Hide resolved

mekarpeles assigned scottbarnes Mar 13, 2023

scottbarnes reviewed Mar 14, 2023

View reviewed changes

scripts/solr_builder/Dockerfile Show resolved Hide resolved

gobbledy-gook closed this Mar 15, 2023

gobbledy-gook force-pushed the docker-compose_to_docker_compose branch from b353922 to 7619cf8 Compare March 15, 2023 08:54

gobbledy-gook reopened this Mar 15, 2023