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

Preparing to add Docker deployment with circle #22

Merged
merged 1 commit into from Aug 28, 2019

Conversation

@vsoch
Copy link
Contributor

vsoch commented Aug 24, 2019

This PR won't work until #15 is merged, but after that it should give us a good start to adding a deployment for the sourcecred/widgets container. As with sourcecred/sourcecred, the DOCKER_LOGIN and DOCKER_PASSWORD will need to be added to the CircleCI environment.

Signed-off-by: Vanessa Sochat vsochat@stanford.edu

@vsoch

This comment has been minimized.

Copy link
Contributor Author

vsoch commented Aug 27, 2019

@Beanow I think this repo needs to be set up to 1) be connected to Circle, and 2) trigger on pull requests and cancel redundant builds, and 3) add DOCKER_LOGIN and DOCKER_PASSWORD.

@Beanow

This comment has been minimized.

Copy link
Member

Beanow commented Aug 27, 2019

Yes. I've done manual builds for both to dockerhub now as a placeholder. https://hub.docker.com/u/sourcecred

  1. we have circle connected
  2. will be part of the config file
  3. need to look a bit into the proper security procedure here, but I should have access to configure this
@Beanow

This comment has been minimized.

Copy link
Member

Beanow commented Aug 27, 2019

Considering we currently don't have tagged releases yet, should we have both :latest and :dev point to master?

@vsoch

This comment has been minimized.

Copy link
Contributor Author

vsoch commented Aug 27, 2019

That would make sense, is there any versioning? If not, I would suggest that we use the commit (first 12 characters) as a tag, along with latest and dev, and we change the trigger for deploy to be on merge to master.

@Beanow

This comment has been minimized.

Copy link
Member

Beanow commented Aug 27, 2019

That would make sense, is there any versioning? If not, I would suggest that we use the commit (first 12 characters) as a tag, along with latest and dev, and we change the trigger for deploy to be on merge to master.

There isn't any versioning at the moment. I feel like if there's a strong need to pin versions right now, we should work towards versioning instead of using commit sha's.

Alternatively you can pin using docker's image sha's instead. For example:
sourcecred/widgets@sha256:dc9392f56afe44ea11c516d171018473a433144565e5aad6ab1651bc80744a30
is the current release.

Beanow added a commit that referenced this pull request Aug 27, 2019
Result of rebasing #22
@Beanow Beanow force-pushed the good-labs:add/docker-deploy branch from 7f7a447 to 4a1188e Aug 27, 2019
@Beanow

This comment has been minimized.

Copy link
Member

Beanow commented Aug 27, 2019

Rebasing against master to include existing circle config.

@vsoch

This comment has been minimized.

Copy link
Contributor Author

vsoch commented Aug 27, 2019

The goal of the version is to link the builds to the code - the Docker tag is already provided by Docker Hub and doesn't do that. I strongly suggest that we use a git commit until there is any other kind of (code) versioning in place.

@vsoch

This comment has been minimized.

Copy link
Contributor Author

vsoch commented Aug 27, 2019

Did you change it? The config isn't valid...

@vsoch

This comment has been minimized.

Copy link
Contributor Author

vsoch commented Aug 27, 2019

You can't put filters under the jobs like that (at least according to the validator.) Please don't do any more pushes, I'll see if I can fix it.

@vsoch

This comment has been minimized.

Copy link
Contributor Author

vsoch commented Aug 27, 2019

oup you beat me, but it didn't trigger a build so something is up - I guess I won't butt in if you are working on it.

@vsoch

This comment has been minimized.

Copy link
Contributor Author

vsoch commented Aug 27, 2019

Can you please stop pushing? I mean - let me fix it, wasn't this my PR?

@Beanow

This comment has been minimized.

Copy link
Member

Beanow commented Aug 27, 2019

Can you please stop pushing? I mean - let me fix it, wasn't this my PR?

Yes, sorry. I had made changes because in master there was an existing config. I tried to integrate it. I'll stop touching and give feedback instead.

@vsoch

This comment has been minimized.

Copy link
Contributor Author

vsoch commented Aug 27, 2019

Cool thanks :)

@vsoch

This comment has been minimized.

Copy link
Contributor Author

vsoch commented Aug 27, 2019

hmm why didn't it trigger? I made the filter to ignore all but master - this is add/docker-deploy so it should have run (what did I miss?)

@Beanow

This comment has been minimized.

Copy link
Member

Beanow commented Aug 27, 2019

I wonder if it's the config error previously that "stoppped" this PR from triggering. I can run a manual rebuild, see if that restarts it for commits.

@vsoch

This comment has been minimized.

Copy link
Contributor Author

vsoch commented Aug 27, 2019

Oh I see - it's because the workflow is still set up wrong, let me see if I can fix it.

@vsoch vsoch force-pushed the good-labs:add/docker-deploy branch from b02f783 to 98dfb7e Aug 27, 2019
@vsoch

This comment has been minimized.

Copy link
Contributor Author

vsoch commented Aug 27, 2019

Can you look in settings and see if workflows was turned off?

@Beanow

This comment has been minimized.

Copy link
Member

Beanow commented Aug 27, 2019

Workflows are turned on.

Build forked pull requests

Run builds for pull requests from forks. CircleCI will automatically update the commit status shown on GitHub's pull request page.

This is not though. But I assume this does not include PRs on this repository.

@Beanow

This comment has been minimized.

Copy link
Member

Beanow commented Aug 27, 2019

I've not changed settings though. And the previous commits did trigger. As well as having workflows.

@Beanow

This comment has been minimized.

Copy link
Member

Beanow commented Aug 27, 2019

Actually Build forked pull requests may be the problem. The build that did trigger was possibly because I had pushed to origin first instead of your branch. Can you try updating now @vsoch?

@vsoch

This comment has been minimized.

Copy link
Contributor Author

vsoch commented Aug 27, 2019

Ah yes, that's probably it. Always need to double check those settings! Let me rebase and push again.

@vsoch vsoch force-pushed the good-labs:add/docker-deploy branch from 8f7fba6 to 918aed2 Aug 27, 2019
@vsoch

This comment has been minimized.

Copy link
Contributor Author

vsoch commented Aug 27, 2019

There we go!

That's what I was referring to in #22 (comment) - sorry next time I'll try to be more explicit "in the settings there is a checkbox for building on forked pull requests, etc."

@vsoch

This comment has been minimized.

Copy link
Contributor Author

vsoch commented Aug 27, 2019

That was so speedy!

#!/bin/bash -eo pipefail
DOCKER_TAG=$(echo "${CIRCLE_SHA1}" | head -c 12)
echo "Version that would be used for Docker tag is ${DOCKER_TAG}"
Version that would be used for Docker tag is 918aed2c62f0

So - this will deploy latest, dev, and the commit. The Docker container will still have a sha sum, but with the commit we can (somewhat) link the container to the code, with the Docker sha it's not possible. When widgets has a version proper, we can change that around.

@Beanow

This comment has been minimized.

Copy link
Member

Beanow commented Aug 27, 2019

Got it. Auto-cancel redundant builds is also not enabled yet. Though these are separate workflows so probably not ones we would auto-cancel. I think it would be neat to combine them into one workflow again so it shows up in github as 1 check.

@vsoch

This comment has been minimized.

Copy link
Contributor Author

vsoch commented Aug 27, 2019

It runs faster to have them run in parallel, so this would be my preference. I would also check auto cancel redundant builds (I usually do) it wouldn't be for different PRs/ branches, but just if, for example, I updated a branch on one PR. It's good to do given that we don't want to waste our free cycles on something that has been changed.

@vsoch

This comment has been minimized.

Copy link
Contributor Author

vsoch commented Aug 27, 2019

Were these manual pushes? https://hub.docker.com/r/sourcecred/sourcecred/tags I don't see that the PR for the automated build was merged yet (same for this repo!) https://github.com/sourcecred/sourcecred/pulls

@vsoch

This comment has been minimized.

Copy link
Contributor Author

vsoch commented Aug 27, 2019

okay updated - there are still two jobs in the workflow, not sure if you intend for there to be one? I'm not sure how that would work, since they are separate. Anyway, take a look.

@Beanow

This comment has been minimized.

Copy link
Member

Beanow commented Aug 27, 2019

That resolves the issue and looks good for the deploy one too 👍
image

Haven't tested, but is what I assumed a single workflow would look like.

workflows:
  version: 2
  commit:
    jobs:
      - test
      - docker/publish:
          deploy: false
          image: sourcecred/widgets
          tag: latest
          filters:
            branches:
              ignore:
                - master
          after_build:
            - run:
                name: Preview Docker Tag for Build
                command: |
                   DOCKER_TAG=$(echo "${CIRCLE_SHA1}" | head -c 12)
                   echo "Commit that would be used for Docker tag is ${DOCKER_TAG}"
      - docker/publish:
          image: sourcecred/widgets
          tag: latest
          filters:
            branches:
              ignore: /.*/
            tags:
              only: /^v.*/
          after_build:
            - run:
                name: Publish Docker Tag with Sourcecred Version (for Widgets)
                command: |
                   DOCKER_TAG=$(echo "${CIRCLE_SHA1}" | head -c 12)
                   echo "Commit for Docker tag is ${DOCKER_TAG}"
                   docker tag sourcecred/widgets:latest sourcecred/widgets:${DOCKER_TAG}
                   docker tag sourcecred/widgets:latest sourcecred/widgets:dev
@Beanow Beanow dismissed their stale review Aug 27, 2019

issue addressed

…ile PR merged first

Signed-off-by: Vanessa Sochat <vsochat@stanford.edu>
@vsoch vsoch force-pushed the good-labs:add/docker-deploy branch from 47dbacd to ad68f47 Aug 27, 2019
@vsoch

This comment has been minimized.

Copy link
Contributor Author

vsoch commented Aug 27, 2019

okay just tried a variation of that

@Beanow

This comment has been minimized.

Copy link
Member

Beanow commented Aug 27, 2019

Looks good! Thank you @vsoch.

I've started looking into the credential handling (DOCKER_LOGIN and DOCKER_PASSWORD). It looks like Docker hub does not have some kind of equivalent of GitHubs deploy keys. So I believe the closest thing would be to create a new user specifically for each build pipeline to ensure if credentials were to be compromised it doesn't grant access to every project of that developer.

The only alternative that I found which has other authentication schemes is using their build system. https://docs.docker.com/docker-hub/builds/

Do you have any experience and suggestions for a secure setup for it?

@vsoch

This comment has been minimized.

Copy link
Contributor Author

vsoch commented Aug 27, 2019

It's pretty crappy, to be honest - the dummy user is the best thing I've thought of. I used to do automated builds, but they got very buggy after the UI change in Docker Hub and annoying to manage, so now I don't.

@Beanow

This comment has been minimized.

Copy link
Member

Beanow commented Aug 27, 2019

Yeah I'm starting to notice, without 2FA docker/hub-feedback#358 (shame on them) and with a breach not long ago. The precaution of deploy specific users seems about right.

@decentralion would you like me to generate these users?

@decentralion

This comment has been minimized.

Copy link
Member

decentralion commented Aug 28, 2019

Go for it @Beanow

I wonder if we should set up a shared password manager for people with maintainerly duties for SourceCred. Wrote some thoughts here. sourcecred/sourcecred#1333

@Beanow

This comment has been minimized.

Copy link
Member

Beanow commented Aug 28, 2019

As with the main repo, I've added a deploy user on Docker hub and CircleCI which has permissions only for this repo.
The user I've verified by logging in with it locally and testing:

$ docker push sourcecred/widgets:dev
The push refers to repository [docker.io/sourcecred/widgets]
...
dev: digest: sha256:dc9392f56afe44ea11c516d171018473a433144565e5aad6ab1651bc80744a30 size: 3264

$ docker push sourcecred/sourcecred:dev
The push refers to repository [docker.io/sourcecred/sourcecred]
...
errors:
denied: requested access to the resource is denied
unauthorized: authentication required
@Beanow

This comment has been minimized.

Copy link
Member

Beanow commented Aug 28, 2019

Same as sourcecred/sourcecred#1320 (comment) we're not leveraging --cache-from here either. But I think we can address this in a new PR.

Letting this run some builds let's us see how it behaves.

@Beanow
Beanow approved these changes Aug 28, 2019
@Beanow Beanow merged commit c33b8eb into sourcecred:master Aug 28, 2019
2 checks passed
2 checks passed
ci/circleci: publish-1 Your tests passed on CircleCI!
Details
ci/circleci: test Your tests passed on CircleCI!
Details
@vsoch

This comment has been minimized.

Copy link
Contributor Author

vsoch commented Aug 28, 2019

Success! https://hub.docker.com/r/sourcecred/widgets/tags

There is a tiny bug - because we don't explicitly push all tags for sourcecred widgets, it looks like it only pushes latest. I'll open a PR to fix this.

Did you see that the nightly failed?
https://circleci.com/gh/sourcecred/widgets/60

@Beanow

This comment has been minimized.

Copy link
Member

Beanow commented Aug 28, 2019

Oh I had just found the same, pushed something now.

@vsoch

This comment has been minimized.

Copy link
Contributor Author

vsoch commented Aug 28, 2019

oh boo you beat me 😆

Beanow added a commit that referenced this pull request Aug 28, 2019
Deferred from final comments on #22
Beanow added a commit that referenced this pull request Aug 28, 2019
Deferred from final comments on #22
Beanow added a commit that referenced this pull request Aug 28, 2019
Deferred from final comments on #22
Beanow added a commit that referenced this pull request Aug 28, 2019
Deferred from final comments on #22
Beanow added a commit that referenced this pull request Aug 28, 2019
Deferred from final comments on #22
Beanow added a commit that referenced this pull request Aug 28, 2019
Deferred from final comments on #22
Beanow added a commit that referenced this pull request Aug 28, 2019
Deferred from final comments on #22
@Beanow

This comment has been minimized.

Copy link
Member

Beanow commented Aug 28, 2019

Did you see that the nightly failed?
https://circleci.com/gh/sourcecred/widgets/60

I did @vsoch, this is related to #21

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.