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

ci: Push docker images to dockerhub from github action for e2e tests #42467

Merged
merged 6 commits into from
Dec 21, 2022

Conversation

emmatyping
Copy link
Contributor

This should push images to dockerhub with a -tmp tag while we make sure this workflow works. In a couple of days this will become the default for pushing images for self-hosted to DockerHub.

@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Dec 19, 2022
@emmatyping emmatyping marked this pull request as ready for review December 20, 2022 01:05
@emmatyping emmatyping requested a review from a team as a code owner December 20, 2022 01:05
Copy link
Member

@joshuarli joshuarli left a comment

Choose a reason for hiding this comment

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

Can you move your Push built docker image step into here? So that we have an explicit step for it rather than it being mashed under Run Sentry self-hosted e2e CI. It'd also be more obvious that the explicit step's skipped if it isn't on the master/main branch.

@emmatyping
Copy link
Contributor Author

emmatyping commented Dec 20, 2022

Can you move your Push built docker image step into here?

I'd like to keep that centralized/consistent as well, so that we can keep tagging and pushing to dockerhub consistent as well. Looking at relay/snuba/sentry there were several different ways that images were getting tagged.

@joshuarli
Copy link
Member

Can you move your Push built docker image step into here?

I'd like to keep that centralized/consistent as well, so that we can keep tagging and pushing to dockerhub consistent as well. Looking at relay/snuba/sentry there were several different ways that images were getting tagged.

Snuba and relay have cloudbuild.yaml for tagging and pushing images. If you want to be consistent in that stuff to tag+push images also exist inside the source repo, then you should follow my recommendation. I don't see how your approach keeps things consistent.

@emmatyping
Copy link
Contributor Author

Snuba and relay have cloudbuild.yaml for tagging and pushing images
I don't see how your approach keeps things consistent.

We are planning on removing testing and image pushing from cloudbuild.yml/GCB across Sentry/Relay/Snuba and replacing it with a centralized Github Action (the one referenced here). That is the plan to make everything consistent. More context is in the issue about centralizing e2e CI/image pushing. Perhaps renaming the job would make it clearer that it also pushes images?

@joshuarli
Copy link
Member

Snuba and relay have cloudbuild.yaml for tagging and pushing images
I don't see how your approach keeps things consistent.

We are planning on removing testing and image pushing from cloudbuild.yml/GCB across Sentry/Relay/Snuba and replacing it with a centralized Github Action (the one referenced here). That is the plan to make everything consistent. More context is in the issue about centralizing e2e CI/image pushing. Perhaps renaming the job would make it clearer that it also pushes images?

Oh cool, that's the context I was missing - thanks!

Yeah, if you could rename it at some point, that would make me feel better.

@joshuarli joshuarli self-requested a review December 20, 2022 22:29
@emmatyping emmatyping merged commit f0f6268 into master Dec 21, 2022
@emmatyping emmatyping deleted the ethanhs/push-images-dockerhub branch December 21, 2022 19:23
@github-actions github-actions bot locked and limited conversation to collaborators Jan 6, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Scope: Backend Automatically applied to PRs that change backend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants