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

correct the docker pushes #1063

Merged
merged 3 commits into from
Nov 12, 2024
Merged

correct the docker pushes #1063

merged 3 commits into from
Nov 12, 2024

Conversation

jmhodges
Copy link
Contributor

@jmhodges jmhodges commented Nov 8, 2024

So, since #1060, we're not able to do deploys. They error like as in:
https://github.com/mozilla-services/autograph/actions/runs/11748424011/job/32732461427

I thought the sha I was removing was the hash of the image, but it's
the git commit's sha!

The real problem seems to be we are pushing to both dockerhub and to
Google Artifact Registry (GAR) at the same time after our builds on
main. We wanted only to push to GAR and then on release copy them to
dockerhub.

That happens becuase the docker/metadata-action action gets called
with both Docker Hub and GAR in its images argument.

So, to fix this, I thought really hard about what we want to happen.

We currently have two separate deploy systems. In one, we need to push
non-latest tags to Docker Hub only when we want a staging deploy to
occur. In the other, we need to push to a semver-like tag to Google
Artifact Registry only when we want a staging release to occur.
Production deploys in both require a human to intervene and change the
expected deployed version.

In the deploy system that uses GAR, we need a push to latest on each
commit to effect a dev deploy.

We also use the latest on Docker Hub to represent the current main
branch's code for use in other people's projects.

So, let's sum that up in terms of tags.

On each commit to main, we want these docker tags and only
these docker tags pushed up:

  • :latest to GAR
  • :sha-<the git commit's sha> to GAR
  • :latest to Docker Hub

On each release, we want these docker tags and only these docker tags
pushed up:

  • :<git tag of release> to GAR
  • :<git tag of release> to Docker Hub

Those :<git tag of release> will be copies of the sha-<the git commit's sha> tag that was made on commit to main and pushed to GAR.

This patch does that by breaking up single docker/metadata-action
action call to two. One for GAR and one for Docker Hub; both with
different sets of tags depending on whether this is a "push" (commit on
main) or "release" event.

But doing just that doesn't quite solve it. Becuse we use
docker/build-push-action to both build and push to GAR in one go. To
avoid building on release, we have to skip that step, but that step is
what was doing all the work of talking to GAR. So, we also need to, only
on release, "copy" the previously made sha- tag to the :<git tag of release> tag within GAR itself.

This may be the point where we should split this one GitHub Actions
workflow into separate deploy and a release workflows. There is a
lot of thinking and conditional checks. It would cost a significant
amount of duplication between these two actions to do so. But I could be
told to do so.

(The crane install would be especially annoying; maybe we
should just go install it, anyway, since this install instructions were
for a non-Go environment)

So, since #1060, we're not able to do deploys. They error like as in:
https://github.com/mozilla-services/autograph/actions/runs/11748424011/job/32732461427

I thought the `sha` I was removing was the hash of the image, but it's
the git commit's sha!

The real problem seems to be we are pushing to both dockerhub and to
Google Artifact Registry (GAR) at the same time after our builds on
`main`. We wanted only to push to GAR and then on release copy them to
dockerhub.

That happens becuase the `docker/metadata-action` action gets called
with both Docker Hub and GAR in its `images` argument.

So, to fix this, I thought really hard about what we want to happen.

We currently have two separate deploy systems. In one, we need to push
non-`latest` tags to Docker Hub only when we want a staging deploy to
occur. In the other, we need to push to a semver-like tag to Google
Artifact Registry only when we want a staging release to occur.
Production deploys in both require a human to intervene and change the
expected deployed version.

In the deploy system that uses GAR, we need a push to `latest` on each
commit to effect a dev deploy.

We also use the `latest` on Docker Hub to represent the current `main`
branch's code for use in other people's projects.

So, let's sum that up in terms of tags.

On each commit to `main`, we want these docker tags and only
these docker tags pushed up:

* `:latest` to `GAR`
* `:sha-<the git commit's sha>` to GAR
* `:latest` to Docker Hub

On each release, we want these docker tags and only these docker tags
pushed up:

* `:<git tag of release>` to GAR
* `:<git tag of release>` to Docker Hub

Those `:<git tag of release>` will be copies of the `sha-<the git
commit's sha>` tag that was made on commit to `main` and pushed to GAR.

This patch does that by breaking up single `docker/metadata-action`
action call to two. One for GAR and one for Docker Hub; both with
different sets of tags depending on whether this is a "push" (commit on
main) or "release" event.

But doing just that doesn't quite solve it. Becuse we use
`docker/build-push-action` to both build and push to GAR in one go. To
avoid building on release, we have to skip that step, but that step is
what was doing all the work of talking to GAR. So, we also need to, only
on release, "copy" the previously made `sha-` tag to the `:<git tag of
release>` tag within GAR itself.

This may be the point where we should split this one GitHub Actions
workflow into separate `deploy` and a `release` workflows. There is a
lot of thinking and conditional checks. It would cost a significant
amount of duplication between these two actions to do so. But I could be
told to do so.

(The crane install would be especially annoying; maybe we
should just go install it, anyway, since this install instructions were
for a non-Go environment)
@jmhodges jmhodges marked this pull request as ready for review November 9, 2024 00:08
@jmhodges jmhodges requested review from a team as code owners November 9, 2024 00:08
@jmhodges jmhodges requested review from say-yawn and removed request for a team November 9, 2024 00:08
Copy link
Contributor

@bhearsum bhearsum left a comment

Choose a reason for hiding this comment

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

This may be the point where we should split this one GitHub Actions
workflow into separate deploy and a release workflows. There is a
lot of thinking and conditional checks. It would cost a significant
amount of duplication between these two actions to do so. But I could be
told to do so.

There is little to nothing in common between pushes to main and releases after this change besides boilerplate, so I would be totally on board with splitting these up. It would improve comprehensibility even with the duplication IMO.

@jmhodges jmhodges requested a review from jbuck November 12, 2024 16:29
@jmhodges
Copy link
Contributor Author

I noticed the SRC now in the crane calls needs to be updated

@jmhodges jmhodges merged commit 14144d7 into main Nov 12, 2024
14 checks passed
@jmhodges jmhodges deleted the correct-aws-staging-deploy branch November 12, 2024 16:53
jmhodges added a commit to mozilla-services/autograph-edge that referenced this pull request Nov 19, 2024
This is a port of
mozilla-services/autograph#1063 (and
mozilla-services/autograph#1060).

Currently, we're doing a deploy to one of our staging environments on
every merge to main this repo. That wasn't desired.

Along the way, we also build arm64 versions of the docker image for
convenience. See mozilla-services/autograph#1059
jmhodges added a commit to mozilla-services/autograph-edge that referenced this pull request Nov 19, 2024
This is a port of
mozilla-services/autograph#1063 (and
mozilla-services/autograph#1060).

Currently, we're doing a deploy to one of our staging environments on
every merge to main this repo. That wasn't desired.

Along the way, we also build arm64 versions of the docker image for
convenience. See mozilla-services/autograph#1059
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.

5 participants