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

Build a Docker image from source code #810

Merged
merged 8 commits into from Jan 23, 2019
Merged

Build a Docker image from source code #810

merged 8 commits into from Jan 23, 2019

Conversation

dtinth
Copy link
Member

@dtinth dtinth commented Jan 23, 2019

Problems

  • We struggle with getting the GitHub Actions to work on our project, but it is not easy to debug Danger when running on GitHub Actions. We’ve tried setting DEBUG=* and --verbose to no avail.

  • We tried forking Danger JS, but since its Dockerfile always install danger-js from the npm registry (via npx danger@beta), it is not easy for us to modify the source code and debug Danger on GitHub actions.

  • Upon inspecting the npm registry I found that the beta tag is two major versions behind. danger@latest is at v7.0.2, whereas danger@beta is at v5.0.0-beta.24. That means people running Danger on GitHub Actions, following the documentation are running an utterly outdated version of Danger.

Solution

  • I updated the Dockerfile so that it always compile Danger from source code. This means:

    • People can use a specific version of Danger via uses = "danger/danger-js@v7.0.3" or use a cutting edge version via uses = "danger/danger-js@master".

    • People can fork danger-js and use it right away via uses = "<username>/danger-js@master".

  • By using the latest version of Danger. The problem is gone — our configuration works now.

Caveat

  • It will take a longer time to run Danger the first time, due to compiling TypeScript from source code (I used yarn run build:fast for faster build). This takes ~2 minutes.

    But subsequent runs will be faster, as Docker cache is used. For example, here, on subsequent runs ---> Using cache is present in all steps.

    Step 1/10 : FROM node:10-slim
    Status: Downloaded newer image for node:10-slim
    ---> 1ee14be408b9
    Step 2/10 : MAINTAINER Orta Therox
    ---> Using cache
    ---> 15e1c45a0159
    Step 3/10 : LABEL "com.github.actions.name"="Danger JS"
    ---> Using cache
    ---> c23ae56f8c2f
    Step 4/10 : LABEL "com.github.actions.description"="Runs JavaScript/TypeScript Dangerfiles"
    ---> Using cache
    ---> 106e82f2cc17
    Step 5/10 : LABEL "com.github.actions.icon"="zap"
    ---> Using cache
    ---> 14262429c7b7
    Step 6/10 : LABEL "com.github.actions.color"="blue"
    ---> Using cache
    ---> 9536a2e390ed
    Step 7/10 : RUN mkdir -p /usr/src/danger
    ---> Using cache
    ---> 3e3a0ce8c201
    Step 8/10 : COPY . /usr/src/danger
    ---> Using cache
    ---> de4f5756278f
    Step 9/10 : RUN cd /usr/src/danger &&   yarn &&   yarn run build:fast &&   chmod +x distribution/commands/danger.js &&   ln -s $(pwd)/distribution/commands/danger.js /usr/bin/danger
    ---> Using cache
    ---> beb1b82f53bd
    Step 10/10 : ENTRYPOINT ["danger", "ci"]
    ---> Using cache
    ---> 5383fc05ca7d
    Successfully built 5383fc05ca7d
    

@orta
Copy link
Member

@orta orta commented Jan 23, 2019

Coo, yeah, as long as it's caching I'm 👍 with this

@orta orta merged commit d725dcd into danger:master Jan 23, 2019
4 checks passed
@peril-staging
Copy link
Contributor

@peril-staging peril-staging bot commented Jan 23, 2019

Thanks for the PR @dtinth.

The Danger org conform to the Moya Community Continuity Guidelines, which means
that we want to offer any contributor the ability to control their destiny.

So, we've sent you an org invite - thanks dtinth 🎉

Generated by 🚫 dangerJS

@peril-staging
Copy link
Contributor

@peril-staging peril-staging bot commented Jan 27, 2019

Thanks for the PR @dtinth.

This PR has been shipped in v7.0.4 - CHANGELOG.

@devnoname120
Copy link

@devnoname120 devnoname120 commented Apr 22, 2020

@dtinth Are you able to cache the build in GitHub actions?

@dtinth
Copy link
Member Author

@dtinth dtinth commented Apr 22, 2020

We were able to cache the Docker layers while in beta. The workflow syntax has since changed and I did not update the project to use the new workflow. (We ended up creating our own system where results from multiple builds are aggregated together into a single comment.) So, I am not sure what the situation is now...

@f-meloni
Copy link
Member

@f-meloni f-meloni commented Apr 22, 2020

I’ve made several test with GitHub actions in order to improve the danger-swift compile time, and AFAIK there is no relevant speed improvement given by a layer cache on the second run of an action at the moment.

@orta
Copy link
Member

@orta orta commented Apr 22, 2020

Personally, I'd recommend not using the GitHub Action and just calling danger from npm/yarn unless you're not in a JavaScript project - the abstraction overhead of docker is definitely not worth it

@devnoname120
Copy link

@devnoname120 devnoname120 commented Apr 22, 2020

@orta are there plans to update the image on https://hub.docker.com? It would allow people to do uses: docker://dangersystems/danger-js in GitHub actions without building the container themselves.

@orta
Copy link
Member

@orta orta commented Apr 22, 2020

I've tried a few times, but not got it working and last time my creds got leaked by docker hub so I'm not really looking forwards to doing it again

https://github.com/danger/danger-js/blob/master/.github/workflows/uploadDockerImage.yml
https://github.com/danger/danger-js/actions/runs/79800580

@f-meloni
Copy link
Member

@f-meloni f-meloni commented Apr 22, 2020

I'm pretty sure I can fix it, by using the script we have on danger/swift. The only problem with that is that packages requires auth even for public packages, then while it could be used for example in CircleCI where you can specify the auth credentials to download an docker image, it can not be used on Github Actions sadtrombone

@devnoname120
Copy link

@devnoname120 devnoname120 commented Apr 22, 2020

Yes, GitHub registry requires auth so https://hub.docker.com is probably a better option.
@orta how did your credentials get leaked? Could we create another account just for this?

@dtinth
Copy link
Member Author

@dtinth dtinth commented Apr 22, 2020

Personally I agree with @orta, and the ubuntu-latest image comes with Yarn and npm built-in :) A lot of workflows I write are run: npx something nowadays.

@f-meloni
Copy link
Member

@f-meloni f-meloni commented Apr 22, 2020

That is valid only if you are using Actions, a Docker image can be used easily in any environment and CI that supports docker.
While I'm almost always in a CI env that has swift, then I can use swift run danger-swift, I have a lot of cases where I use a docker image of danger-swift to run danger, especially on circleci.
Is just a lot simpler.
Also with a docker image with Danger-js already builded in it you could not have to install it every time, and you could use it as base for a more complex Docker image that you use on your CI machine.

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

4 participants