Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs: fix update instructions #793

Merged
merged 2 commits into from
Nov 8, 2024
Merged

Conversation

rti
Copy link
Contributor

@rti rti commented Oct 11, 2024

This ensures that docker compose always checks for new images when starting the WBS Deploy stack. Actually, we expected this behavior being present already. It is also already documented like that. That's why I consider this a fix and not a feat.

@rti rti requested a review from a team October 11, 2024 08:26
Copy link
Contributor

@lorenjohnson lorenjohnson left a comment

Choose a reason for hiding this comment

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

Hey @rti, I have a thought on this. With pull_policy: always new images will be pulled even after just doing a docker compose stop and docker compose up, so on every ordinary reboot. So I think this pull_policy solution actually changes the behaviour we stated and makes patch and minor updates automatic, and no longer optional.

docker compose pull is another way to achieve this, such that the user determines explicitly when they want to receive updates and the images don't re-pull every time. I think this aligns with our current explanations in the README and I'd prefer adding it to the instructions at https://github.com/wmde/wikibase-release-pipeline/tree/main/deploy#minor-and-patch-updates-for-wbs-images, such that instead of:

docker compose down
docker compose up

We make the instructions there:

docker compose down
docker compose pull
docker compose up

And leave the service configuration as they are. If we want to expand there README you could document this option to always pull and/or make it an env var, i.e. pull_policy: ${PULL_POLICY}, with a default of missing and a note about the option for fully automatic updates (on restart) by setting it it always (https://docs.docker.com/reference/compose-file/services/#pull_policy.

Hope that makes sense.

@rti
Copy link
Contributor Author

rti commented Oct 14, 2024

Thanks. Good catch. That does indeed make sense. Will update the PR accordingly.

@rti rti force-pushed the fix-deploy-auto-update-on-restart branch from 4346037 to 534e82c Compare October 14, 2024 06:16
@rti rti requested a review from lorenjohnson October 14, 2024 06:22
@rti rti changed the title fix: always pull image on docker compose up docs: fix update instructions Oct 24, 2024
deploy/README.md Show resolved Hide resolved
@rti rti requested a review from lorenjohnson November 7, 2024 12:09
Copy link
Contributor

@lorenjohnson lorenjohnson left a comment

Choose a reason for hiding this comment

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

Please see final discussion really, and amend or merge as you see fit

@rti
Copy link
Contributor Author

rti commented Nov 8, 2024

Thanks lot for your thoughts and the review.

@rti rti merged commit 18ce9b8 into main Nov 8, 2024
@rti rti deleted the fix-deploy-auto-update-on-restart branch November 8, 2024 07:09
rti added a commit that referenced this pull request Nov 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants