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

Add alpine based Dockerfile #845

Merged
merged 1 commit into from
Dec 21, 2018
Merged

Add alpine based Dockerfile #845

merged 1 commit into from
Dec 21, 2018

Conversation

wallyqs
Copy link
Member

@wallyqs wallyqs commented Dec 6, 2018

Adds extra Dockerfile image that can be used to build the server from master.

docker build -f docker/Dockerfile.alpine -t <image:tag> .

Signed-off-by: Waldemar Quevedo wally@synadia.com

  • Link to issue, e.g. Resolves #NNN
  • Documentation added (if applicable)
  • Tests added
  • Branch rebased on top of current master (git pull --rebase origin master)
  • Changes squashed to a single commit (described here)
  • Build is green in Travis CI
  • You have certified that the contribution is your original work and that you license the work to the project under the Apache 2 license

Changes proposed in this pull request:

  • Add Dockerfile.alpine to build latest nats server image

/cc @nats-io/core

@kozlovic kozlovic self-requested a review December 6, 2018 23:48
Copy link
Member

@kozlovic kozlovic left a comment

Choose a reason for hiding this comment

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

LGTM

@kozlovic
Copy link
Member

kozlovic commented Dec 6, 2018

For the record: you would run this from your gnatsd directory. Whatever the state or your current branch is is what will be used to build the image.


RUN apk add --update ca-certificates

COPY --from=builder /gnatsd /gnatsd
Copy link
Member

Choose a reason for hiding this comment

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

I think we should create a dir, nats I think, have bin and conf dirs. sym link /nats/bin/gnatsd to /bin

Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry for the delay, I have applied the changes and this is how it looks now:

$ docker run --entrypoint /bin/sh -it wallyqs/nats-server:edge
...
/ # tree /nats/
/nats/
├── bin
│   └── gnatsd
└── conf
    └── gnatsd.conf

2 directories, 2 files
/ # ls -la /bin/ | grep gnatsd -A1 -B1
lrwxrwxrwx    1 root     root            12 Sep 11 20:23 getopt -> /bin/busybox
lrwxrwxrwx    1 root     root            16 Dec 21 17:31 gnatsd -> /nats/bin/gnatsd
lrwxrwxrwx    1 root     root            12 Sep 11 20:23 grep -> /bin/busybox
/ # ls -la /nats/*/*
-rwxr-xr-x    1 root     root       8932640 Dec 21 17:30 /nats/bin/gnatsd
-rw-r--r--    1 root     root           671 Dec 21 17:05 /nats/conf/gnatsd.conf

$ docker run wallyqs/nats-server:edge
[1] 2018/12/21 17:43:36.979863 [INF] Starting nats-server version 2.0.0-beta.12
[1] 2018/12/21 17:43:36.979973 [INF] Git commit [2bc515a]
[1] 2018/12/21 17:43:36.980124 [INF] Starting http monitor on 0.0.0.0:8222
[1] 2018/12/21 17:43:36.980349 [INF] Listening for client connections on 0.0.0.0:4222
[1] 2018/12/21 17:43:36.980404 [INF] Server id is NDEHGDUGIFECN7NUZUPOEDEDVOGAPEWHQSPEGGQNSMIZFVIO2MPZSURN
[1] 2018/12/21 17:43:36.980429 [INF] Server is ready
[1] 2018/12/21 17:43:36.981009 [INF] Listening for route connections on 0.0.0.0:6222
^C[1] 2018/12/21 17:43:37.816006 [INF] Server Exiting..

Copy link
Member

Choose a reason for hiding this comment

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

ok getting closer. We should symlink the binary to /bin/nats-server IMO.
Also move gnatsd to nats-server in prep for rename.

Copy link
Member

Choose a reason for hiding this comment

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

How would a user easily provide their own config to run this docker image? Is it easy?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sounds good on renaming, will make the changes.

About custom config, since we are using alpine then users could do something like this too now without having to mount volumes for example:

docker run --entrypoint /bin/sh wallyqs/nats-server:edge-2 -c '
cat > /nats/conf/gnatsd.conf <<CONF
  listen = 4222
  authorization {
    user = foo
    pass = bar
  }   
CONF
/bin/gnatsd -c /nats/conf/gnatsd.conf
'

Copy link
Member

Choose a reason for hiding this comment

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

That seems correct to me.

Copy link
Member Author

Choose a reason for hiding this comment

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

Using volumes then yes would also be possible to override with a local file, for example:

$ echo '
  listen = 4333
' > my-nats.conf

$ docker run -v $(pwd)/my-nats.conf:/nats/conf/nats.conf -it wallyqs/nats-server:edge
[1] 2018/12/21 20:12:19.113881 [INF] Starting nats-server version 2.0.0-beta.12
[1] 2018/12/21 20:12:19.114099 [INF] Listening for client connections on 0.0.0.0:4333

Copy link
Member

Choose a reason for hiding this comment

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

ok that looks good. Should we call it nats-server.conf?

Copy link
Member Author

Choose a reason for hiding this comment

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

sgtm!

Copy link
Member Author

Choose a reason for hiding this comment

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

ok updated so that it is now nats-server.conf

@coveralls
Copy link

coveralls commented Dec 8, 2018

Coverage Status

Coverage increased (+0.04%) to 91.607% when pulling 0c65fe7 on alpine-dockerfile into f10020e on master.

@wallyqs wallyqs force-pushed the alpine-dockerfile branch 3 times, most recently from 2e64caf to a7dfb61 Compare December 9, 2018 00:52
@wallyqs wallyqs force-pushed the alpine-dockerfile branch 6 times, most recently from 317f41e to 0101655 Compare December 21, 2018 19:52
Copy link
Member

@derekcollison derekcollison left a comment

Choose a reason for hiding this comment

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

Maybe have server conf be nats-server.conf, but otherwise LGTM. Nice..

To build it:

```
docker build -f docker/Dockerfile.alpine -t <image:tag> .
```

Signed-off-by: Waldemar Quevedo <wally@synadia.com>
@derekcollison
Copy link
Member

LGTM

@derekcollison derekcollison merged commit 9e215b6 into master Dec 21, 2018
@derekcollison derekcollison deleted the alpine-dockerfile branch December 21, 2018 20:35
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