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

Extended docker support #66

Merged

Conversation

maxmeyer
Copy link
Contributor

@maxmeyer maxmeyer commented Mar 9, 2018

Preface

This incorparates all changes from #27 (/cc @dexafree) rebased to current master and reduces the image size by using the multistaged build feature from docker. This also reduces the surface for attacks by reducing the amount of installed packages.

Additions

  • Multistaged docker build - minimal Alpine Linux Image for the image
  • Build/Setup-scripts to make it easier for newcomers to get started
  • Extend documentation about how to run the docker container

@dexafree
Copy link
Contributor

dexafree commented Mar 9, 2018

Seems great to me.
Would you maybe consider using a Makefile instead of 3 different scripts for the setup?

I could help with that.

@maxmeyer
Copy link
Contributor Author

maxmeyer commented Mar 9, 2018

@dexafree I thought about that too, but in the end having those scripts is more obvious than a single file - also see https://githubengineering.com/scripts-to-rule-them-all/. But I'm happy to add a simple Makefile which can act as another entrypoint. This is what I do in my projects normally, having both the scripts and the Makefile.

@maxmeyer
Copy link
Contributor Author

maxmeyer commented Mar 9, 2018

@dexafree Done.

@maxmeyer maxmeyer force-pushed the feature/extend-docker-support branch from 8a4d66a to fb586cc Compare March 9, 2018 14:57

EXPOSE 8080

ENTRYPOINT ["/usr/bin/dumb-init", "--"]
Copy link
Contributor Author

@maxmeyer maxmeyer Mar 9, 2018

Choose a reason for hiding this comment

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

@RadhiFadlillah If you wonder, what this tool is for: https://github.com/Yelp/dumb-init:

dumb-init is a simple process supervisor and init system designed to run as PID 1 inside minimal container environments (such as Docker). It is deployed as a small, statically-linked binary written in C.

@RadhiFadlillah
Copy link
Collaborator

Thanks @maxmeyer.

BTW, I think we don't really need to add those additional scripts, especially since go command is already simple enough to use. For example, setup is just wrapper for go get and build is just wrapper for go build.

README.md Outdated
To build the project, please run the following command.

```sh
bin/build
Copy link
Collaborator

Choose a reason for hiding this comment

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

Here, we can just use go build -o shiori

README.md Outdated
command.

```sh
bin/setup
Copy link
Collaborator

Choose a reason for hiding this comment

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

Here, we can just use go get -d -v ./..

### Build the image

```bash
$ docker build -t shiori .
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If there's a decision for #70 this needs to be replaced by bin/docker/build.

@RadhiFadlillah RadhiFadlillah merged commit 6170197 into go-shiori:master Mar 10, 2018
@RadhiFadlillah
Copy link
Collaborator

Thanks a lot @maxmeyer, I will take a look at #70 later.

@maxmeyer
Copy link
Contributor Author

maxmeyer commented Mar 10, 2018

Thanks @RadhiFadlillah. I'm looking forward for the next release of shiori. 😄

@maxmeyer
Copy link
Contributor Author

@RadhiFadlillah Awesome! I just realized, that you just setup the automated docker build! 👍

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.

3 participants