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: make sure we log in for tagged-releases #1541

Merged
merged 1 commit into from Jan 10, 2020
Merged

Conversation

Beanow
Copy link
Contributor

@Beanow Beanow commented Jan 10, 2020

Over at #1514 (comment)
I suggested adding deploy: false to be less surprising with the tags we're pushing to DockerHub.
As mentioned in #1512, we actually need deploy: true to log in with DockerHub.

Test plan:
Compare with a push to master, to see our credentials are correct for pushing :dev.
https://circleci.com/gh/sourcecred/sourcecred/4422

With the tagged deploy.
https://circleci.com/gh/sourcecred/sourcecred/4426

Over at
#1514 (comment)
I suggested adding `deploy: false` to be less surprising with the
tags we're pushing to DockerHub.

As mentioned in #1512, we actually need `publish: true` to log in
with DockerHub.
Copy link
Contributor

@vsoch vsoch left a comment

Choose a reason for hiding this comment

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

I think this would resolve the issue!

@Beanow
Copy link
Contributor Author

Beanow commented Jan 10, 2020

@vsoch from #1512 (comment)

Indeed if we use the orb and put deploy to false it won't log us in, but we need to do that to have more explicit control about the pushes. Why not just add:

docker login -u "${DOCKER_LOGIN}" -p "${DOCKER_PASSWORD}" docker.io

If deploy is true, then we would just be pushing sourcecred/sourcecred:latest, so I think you could set deploy to true instead and then manually just push the custom tag. If you think it's unclear for the developer, we could add a note (comment) about that.

You're right we can use either option there.
In this case I'm pushing :latest twice. Once explicitly and once from the orb.

Open to suggestions from @decentralion if one would be preferable / less confusing. 😃
For now just want to make sure it works.

@Beanow Beanow merged commit c1f123f into master Jan 10, 2020
@Beanow Beanow deleted the pr/ci-remove-deploy-false branch January 10, 2020 22:57
@vsoch
Copy link
Contributor

vsoch commented Jan 10, 2020

It's redundant to have it push twice (with deploy as true and then the manual line) but I think it's actually okay because it's more transparent for the developer. There isn't actually any issue with doing the push twice because the layers will already be found to exist and won't be pushed again. Solid and quick work, @Beanow !

Beanow added a commit that referenced this pull request Jan 10, 2020
Over at
#1514 (comment)
I suggested adding `deploy: false` to be less surprising with the
tags we're pushing to DockerHub.

As mentioned in #1512, we actually need `publish: true` to log in
with DockerHub.
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

2 participants