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

chore: split packages #1496

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

kristof-mattei
Copy link

@kristof-mattei kristof-mattei commented Oct 24, 2023

Description

In #1407 the database packages were split, meaning the database choice is now a compilation choice, and thus we need individual packages.

In the PR the only package deployed is the one with DATABASE_BACKEND=spanner.

This PR changes that. Unfortunately the way Docker works it means we'll need to add 2 repos (as we can't rename old ones).

The proposal is:

Go from 2 repos (DOCKERHUB_REPO & UTILS_DOCKERHUB_REPO) to 3 (DOCKERHUB_REPO_SPANNER, DOCKERHUB_REPO_MYSQL & DOCKERHUB_REPO_UTILS).

The package for the Spanner DB would go to the DOCKERHUB_REPO_SPANNER, the one for MySQL DB to DOCKERHUB_REPO_MYSQL, and I did rename the variable for the utils repo from UTILS_DOCKERHUB_REPO to DOCKERHUB_REPO_UTILS to match the same naming structure.

That's it. It does not bring latest tags for the normal builds. That will go in another PR (should this one be accepted).

What needs to be done before merging this in?

Create the following repos in hub.docker.com (names are proposals):

Add the following Environment Variables into CircleCI:

  • DOCKERHUB_REPO_SPANNER=https://hub.docker.com/r/mozilla/syncstorage-rs-spanner
  • DOCKERHUB_REPO_MYSQL=https://hub.docker.com/r/mozilla/syncstorage-rs-mysql
  • DOCKERHUB_REPO_UTILS=https://hub.docker.com/r/mozilla/sync-spanner-py-utils

Ensure the user $DOCKER_USER has access to those 3 repos ^ for pushing.

And after merging this in?

Testing

  1. Kick off a build in CircleCI with the changes above and see if it succeeds.
  2. Fake tag the tip of the branch and push that to ensure it deploys the package in the respective repos.

Issue(s)

Notes

  1. I copied the following line, as I'm not sure it has meaning (some comment parser?):
    https://github.com/mozilla-services/syncstorage-rs/compare/master...kristof-mattei:syncstorage-rs:feature/split-docker-packages?expand=1#diff-78a8a19706dbd2a4425dd72bdab0502ed7a2cef16365ab7030a5a0588927bf47R454

  2. My formatter made the YAML formatting more consistent. There is a view in GitHub to not display those whitespace changes. If this is not desired let me know & I'll patch them out.

@WAdama
Copy link

WAdama commented Oct 26, 2023

I would second that proposal!

@plaes
Copy link

plaes commented Dec 28, 2023

@kristof-mattei The # touch: 1676417203 in the .circleci/config.yml file was probably added to re-trigger or retry circleci actions, so it could be removed.

@jackyzy823
Copy link

I'd sugguest to use tag (instead of different image) and add README, like:

  1. "mozilla/syncstorage-rs:0.14.4" aka "mozilla/syncstorage-rs:0.14.4-spanner"
  2. "mozilla/syncstorage-rs:0.14.4-mysql"
  3. "mozilla/syncstorage-rs:0.14.4-tools"

@kristof-mattei
Copy link
Author

I'd sugguest to use tag (instead of different image) and add README, like:

  1. "mozilla/syncstorage-rs:0.14.4" aka "mozilla/syncstorage-rs:0.14.4-spanner"
  2. "mozilla/syncstorage-rs:0.14.4-mysql"
  3. "mozilla/syncstorage-rs:0.14.4-tools"

Unfortunately that doesn't fit Docker's treatment of tags. Not passing in a tag implies latest. So when you pull mozilla/syncstorage-rs which one do you get?

@tillmann
Copy link

tillmann commented Jan 14, 2024

Unfortunately that doesn't fit Docker's treatment of tags.

I don't think that is accurate. It is actually quite common to distinguish different configurations of an image via tags. It is even considered bad practice to not supply a tag ("rely on the latest-tag") when deploying a container.

E.g. the rust images: https://hub.docker.com/_/rust/ there are tags for various ubuntu-flavor as well as alpine-based images. latest just seems to point to the most common (stable?) one (in your case that would be "spanner" iiuc). (I guess there is plenty of more examples, just looked at wordpress: https://hub.docker.com/_/wordpress).

Please correct me if I'm wrong, I just stumbled upon this project with the intention of hosting an instance myself.

Another data-point: https://docs.docker.com/develop/dev-best-practices/

When building images, always tag them with useful tags which codify version information, intended destination (prod or test, for instance), stability, or other information that's useful when deploying the application in different environments. Don't rely on the automatically-created latest tag.

@pasbec
Copy link

pasbec commented Feb 26, 2024

Hi, First of all thanks for providing this software!

Just wanted to check before spending more time on this: Is this pull request still valid and will updated SQL docker images be provided in the proposed or some other way?

I'm currently planing to run my own conatiner but having to stick with version 0.13.7 forever wouldn't be a good idea.

@kristof-mattei
Copy link
Author

Unfortunately that doesn't fit Docker's treatment of tags.

I don't think that is accurate. It is actually quite common to distinguish different configurations of an image via tags. It is even considered bad practice to not supply a tag ("rely on the latest-tag") when deploying a container.

E.g. the rust images: https://hub.docker.com/_/rust/ there are tags for various ubuntu-flavor as well as alpine-based images. latest just seems to point to the most common (stable?) one (in your case that would be "spanner" iiuc). (I guess there is plenty of more examples, just looked at wordpress: https://hub.docker.com/_/wordpress).

Please correct me if I'm wrong, I just stumbled upon this project with the intention of hosting an instance myself.

Another data-point: https://docs.docker.com/develop/dev-best-practices/

When building images, always tag them with useful tags which codify version information, intended destination (prod or test, for instance), stability, or other information that's useful when deploying the application in different environments. Don't rely on the automatically-created latest tag.

Late reply but I sit corrected. Let me find some time to see how this impacts this PR. Will need to delete those new repo references.

Thanks.

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.

None yet

6 participants