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.

Sign up for GitHub

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

Jump to bottom

ci: Add docker auth to circleci config #695

Merged
merged 2 commits into from
Dec 11, 2020
Merged

ci: Add docker auth to circleci config #695

merged 2 commits into from
Dec 11, 2020

Conversation

haworku
Copy link
Contributor

@haworku haworku commented Dec 7, 2020

Summary

Update config to add auth.

Related Issues or PRs

closes #633

How To Test

  • CI should run successfully

@haworku haworku added the type: development Enhancements to the development workflow label Dec 7, 2020
@haworku haworku requested a review from eeeady December 7, 2020 22:00
@haworku
Copy link
Contributor Author

haworku commented Dec 7, 2020

@eeeady is this it? Or am I missing something else that needs to happen here

@haworku haworku self-assigned this Dec 9, 2020
@haworku haworku marked this pull request as ready for review December 10, 2020 16:51
@haworku haworku requested a review from pkarman December 10, 2020 16:52
ahobson
ahobson previously approved these changes Dec 10, 2020
View reviewed changes
Copy link
Contributor

@ahobson ahobson left a comment

Choose a reason for hiding this comment

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

Sure looks right to me. I don't know if we need to do any config changes in CircleCi though.

pkarman
pkarman previously approved these changes Dec 10, 2020
View reviewed changes
Copy link

@pkarman pkarman left a comment

Choose a reason for hiding this comment

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

Confirmed that CircleCI job passed and that env vars are set in CI correctly.

@eeeady
Copy link
Contributor

eeeady commented Dec 10, 2020

I forgot to comment before... sorry Hana!

It looks like it would work but when I check the workflow in CircleCI it doesn't seem like it's using the credentials. We might need to make changes so that the context stuff works. One moment.

@eeeady
Copy link
Contributor

eeeady commented Dec 10, 2020

So if you look at the job in circleci it looks good but if you click into "spin up environment" you'll see the lines:

Starting container circleci/node:12.16.2
Warning: No authentication provided, using CircleCI credentials for pulls from Docker Hub.
image is cached as circleci/node:12.16.2, but refreshing...
12.16.2: Pulling from circleci/node

so the job isn't using the auth we think you set up.

I think I can make a PR against yours OR I can pull this branch down and make changes. Do you have a preference @haworku ?

@eeeady eeeady dismissed stale reviews from pkarman and ahobson via 6af8cff December 10, 2020 23:29
@eeeady eeeady requested review from ahobson and pkarman December 10, 2020 23:30
@eeeady
Copy link
Contributor

eeeady commented Dec 10, 2020

We might need to change branch protections to block on "build" rather than "workflow" from circleci.

@eeeady
Copy link
Contributor

eeeady commented Dec 10, 2020

Made the branch protection change. Checking the build job for the output I want to see.

@eeeady
Copy link
Contributor

eeeady commented Dec 10, 2020

Output is expected. no warning:

Build-agent version 1.0.48308-8a85a016 (2020-12-10T14:02:10+0000)
Docker Engine Version: 19.03.13
Kernel Version: Linux 3d864dd0b6d4 4.15.0-1077-aws #81-Ubuntu SMP Wed Jun 24 16:48:15 UTC 2020 x86_64 Linux>
Starting container circleci/node:12.16.2
image is cached as circleci/node:12.16.2, but refreshing...
12.16.2: Pulling from circleci/node
Digest: sha256:7335cef9567718b3bfd03cdaaf60d457fd1ee39630d02fad3710f092e6cff072
Status: Image is up to date for circleci/node:12.16.2
pull stats: N/A
time to create container: 3.091s
using image circleci/node@sha256:7335cef9567718b3bfd03cdaaf60d457fd1ee39630d02fad3710f092e6cff072

@haworku
Copy link
Contributor Author

haworku commented Dec 11, 2020

@eeeady thanks for pointing to the place in the circleci build specifically where the warning was. Entirely missed that!

pkarman
pkarman approved these changes Dec 11, 2020
View reviewed changes
Copy link

@pkarman pkarman left a comment

Choose a reason for hiding this comment

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

good catch @eeeady

.circleci/config.yml
jobs:
- build:
context:
- org-global
Copy link

Choose a reason for hiding this comment

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

nice!

are there other repos that now need this same change?

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for asking! I've gotten most of our other repos. I think there are a few PRs that I still need to clean up.

@haworku haworku merged commit 26e61cd into main Dec 11, 2020
@haworku haworku deleted the hw-633-circleci branch December 11, 2020 17:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pkarman pkarman pkarman approved these changes

@eeeady eeeady Awaiting requested review from eeeady

@gidjin gidjin Awaiting requested review from gidjin

@sarboc sarboc Awaiting requested review from sarboc

@suzubara suzubara Awaiting requested review from suzubara

@tinyels tinyels Awaiting requested review from tinyels

@ahobson ahobson Awaiting requested review from ahobson

Assignees

@haworku haworku

Labels
type: development Enhancements to the development workflow
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[ci] Update circleci config for docker download rate limit
4 participants
@haworku @eeeady @ahobson @pkarman