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

docker: root directory is /dist/docker/bin #558

Merged
merged 1 commit into from
Aug 8, 2015

Conversation

mreiferson
Copy link
Member

Is there a reason that / is set to /dist/docker/bin for the docker image?

I would like to use nsq_to_file and output messages to /dev/null to avoid the queues filling up my server disks while testing, which is not possible because of this.

@mreiferson
Copy link
Member

innnnnteresting

cc @paddyforan

@mreiferson mreiferson added the bug label Mar 20, 2015
@Bondza
Copy link
Author

Bondza commented Mar 20, 2015

Scratch that, it does not seem to work at all. I tried running it directly on the server instead of in the docker image. Like this:

./nsq_to_file -lookupd-http-address=10.97.155.18:4161 -channel=dummy -filename-format=null -output-dir=/dev

Which results in:

2015/03/20 14:32:40 LOOKUPD: querying http://10.97.155.18:4161/topics
2015/03/20 14:32:40 INF    1 [hello/dummy] querying nsqlookupd http://10.97.155.18:4161/lookup?topic=hello
2015/03/20 14:32:40 INF    1 [hello/dummy] (10.127.164.124:4150) connecting to nsqd
2015/03/20 14:32:40 INFO: opening /dev/null
2015/03/20 14:32:40 syncing 1 records to disk
2015/03/20 14:32:40 ERROR: failed syncing messages - fsync: invalid argument

Which is the same error message I got when trying to run this in the docker image.

@jehiah
Copy link
Member

jehiah commented Mar 20, 2015

@Bondza you probably want to use nsq_tail which will be more effective to just drop messages.

@mreiferson
Copy link
Member

this probably has to do with the way nsq_to_file is opening the file...

@Bondza
Copy link
Author

Bondza commented Mar 20, 2015

Yes, I ended up using nsq_tail instead. It is not quite as convenient though.
Because I need to run one instance per topic I would like to drop messages from, whereas I could just use the -topic-pattern argument for nsq_to_file to drop messages from all topics.

By the way, the docs for nsq_to_file is missing information about the topic pattern argument.

@mreiferson mreiferson changed the title Docker image root directory nsq_to_file: cannot write to /dev/null Apr 26, 2015
@reeze
Copy link

reeze commented May 4, 2015

I didn't try docker. it works on OS X btw.

➜  nsq git:(master) ✗ ./nsq_to_file --lookupd-http-address=127.0.0.1:4161 -channel=test -filename-format=null -output-dir=/dev
2015/05/04 21:33:54 LOOKUPD: querying http://127.0.0.1:4161/topics
2015/05/04 21:33:54 INF    1 [message_topi/test] querying nsqlookupd http://127.0.0.1:4161/lookup?topic=message_topi
2015/05/04 21:33:54 INF    1 [message_topi/test] (127.0.0.1:4150) connecting to nsqd
2015/05/04 21:33:54 INF    2 [test/test] querying nsqlookupd http://127.0.0.1:4161/lookup?topic=test
2015/05/04 21:33:54 INF    2 [test/test] (127.0.0.1:4150) connecting to nsqd
2015/05/04 21:33:54 INFO: opening /dev/null
2015/05/04 21:33:54 syncing 1 records to disk

@mreiferson
Copy link
Member

I don't feel strongly about this one - it could come down to filesystem, etc.

Semantically, I'm not sure we should make it easier to route topics to /dev/null - it seems like ephemeral channels/topics should be sufficient for the original use case here.

Now, whether or not the docker image should be rooted the way it is could be a useful conversation. I'm going to update the title (again) to reflect that.

@mreiferson mreiferson changed the title nsq_to_file: cannot write to /dev/null docker: root directory is /dist/docker/bin May 5, 2015
@paddycarver
Copy link
Contributor

I looked through this (rather late, I apologise) and there's an easy fix. If we add the binaries one at a time, instead of the entire directory, it behaves the exact same way, but doesn't overwrite the entire filesystem (which is baaad). This lets people use /bin/sh, /tmp, etc. while maintaining backwards compatibility. I can work up a tiny Dockerfile-only patch for it at some point, if this is a thing we want to support.

I'll test it a bit more for backwards-compatibility when it's not 3am, but....

paddyforan in ~/go/src/github.com/bitly/nsq on master!$ docker build -t nsqtest .
Sending build context to Docker daemon 86.52 MB
Sending build context to Docker daemon
Step 0 : FROM busybox
 ---> 4986bf8c1536
Step 1 : ADD dist/docker/bin/nsq_pubsub /nsq_pubsub
 ---> 1b7a70d46969
Removing intermediate container 31965c14a265
Step 2 : ADD dist/docker/bin/nsq_stat /nsq_stat
 ---> 5c3ccf993b3e
Removing intermediate container 3bdcad95228a
Step 3 : ADD dist/docker/bin/nsq_tail /nsq_tail
 ---> 6e3d3a55ad7e
Removing intermediate container 6476349f2321
Step 4 : ADD dist/docker/bin/nsq_to_file /nsq_to_file
 ---> dbff70736f59
Removing intermediate container 587c9f471f67
Step 5 : ADD dist/docker/bin/nsq_to_http /nsq_to_http
 ---> c1997e563991
Removing intermediate container d829cfedf682
Step 6 : ADD dist/docker/bin/nsq_to_nsq /nsq_to_nsq
 ---> ca0ff223814a
Removing intermediate container 58af97e82124
Step 7 : ADD dist/docker/bin/nsqadmin /nsqadmin
 ---> e79160717703
Removing intermediate container e64b846b2374
Step 8 : ADD dist/docker/bin/nsqd /nsqd
 ---> c4c75ea47a0c
Removing intermediate container e8ac589e871c
Step 9 : ADD dist/docker/bin/nsqlookupd /nsqlookupd
 ---> f304844bb5cc
Removing intermediate container 1ebfeced6524
Step 10 : ADD dist/docker/bin/to_nsq /to_nsq
 ---> 1525690e9018
Removing intermediate container 82e004948035
Step 11 : EXPOSE 4150 4151 4160 4161 4170 4171
 ---> Running in 67ec94b7c85a
 ---> 82380b10ef30
Removing intermediate container 67ec94b7c85a
Step 12 : VOLUME /data
 ---> Running in 58f3c60e4d60
 ---> a0587cec333b
Removing intermediate container 58f3c60e4d60
Step 13 : VOLUME /etc/ssl/certs
 ---> Running in a7349737de73
 ---> 67ed5006bc98
Removing intermediate container a7349737de73
Successfully built 67ed5006bc98
paddyforan in ~/go/src/github.com/bitly/nsq on master!$ docker run -it nsqtest /nsqd
[nsqd] 2015/06/12 06:52:54.050040 nsqd v0.3.5 (built w/go1.4.2)
[nsqd] 2015/06/12 06:52:54.050134 ID: 967
[nsqd] 2015/06/12 06:52:54.050206 NSQ: persisting topic/channel metadata to nsqd.967.dat
[nsqd] 2015/06/12 06:52:54.105979 TCP: listening on [::]:4150
[nsqd] 2015/06/12 06:52:54.106140 HTTP: listening on [::]:4151
^C[nsqd] 2015/06/12 06:52:55.534081 NSQ: persisting topic/channel metadata to nsqd.967.dat
[nsqd] 2015/06/12 06:52:55.536285 TCP: closing [::]:4150
[nsqd] 2015/06/12 06:52:55.537425 HTTP: closing [::]:4151
[nsqd] 2015/06/12 06:52:55.588831 NSQ: closing topics
[nsqd] 2015/06/12 06:52:55.589173 LOOKUP: closing
[nsqd] 2015/06/12 06:52:55.589209 ID: closing
paddyforan in ~/go/src/github.com/bitly/nsq on master![1] $ docker run -it nsqtest /bin/sh
/ # ls
bin          dev          home         lib64        media        nsq_pubsub   nsq_tail     nsq_to_http  nsqadmin     nsqlookupd   proc         run          sys          to_nsq       var
data         etc          lib          linuxrc      mnt          nsq_stat     nsq_to_file  nsq_to_nsq   nsqd         opt          root         sbin         tmp          usr
/ # %
paddyforan in ~/go/src/github.com/bitly/nsq on master!$ docker run -it nsqtest /bin/sh
/ # echo "test" > /tmp/testing
/ # cat /tmp/testing
test
/ # %
paddyforan in ~/go/src/github.com/bitly/nsq on master!$ git --no-pager diff
diff --git a/Dockerfile b/Dockerfile
index 138c60d..c56cb52 100644
--- a/Dockerfile
+++ b/Dockerfile
@@ -1,6 +1,15 @@
 FROM busybox

-ADD dist/docker/bin/ /
+ADD dist/docker/bin/nsq_pubsub /nsq_pubsub
+ADD dist/docker/bin/nsq_stat /nsq_stat
+ADD dist/docker/bin/nsq_tail /nsq_tail
+ADD dist/docker/bin/nsq_to_file /nsq_to_file
+ADD dist/docker/bin/nsq_to_http /nsq_to_http
+ADD dist/docker/bin/nsq_to_nsq /nsq_to_nsq
+ADD dist/docker/bin/nsqadmin /nsqadmin
+ADD dist/docker/bin/nsqd /nsqd
+ADD dist/docker/bin/nsqlookupd /nsqlookupd
+ADD dist/docker/bin/to_nsq /to_nsq

 EXPOSE 4150 4151 4160 4161 4170 4171
paddyforan in ~/go/src/github.com/bitly/nsq on master!$

That looks promising. Downside, of course, is if a new binary is added to the release, we need to add it manually to the Dockerfile.

@mreiferson
Copy link
Member

That's an acceptable trade off (having to manually add new binaries). Let's do it!

Thanks for looking into this!

@paddycarver
Copy link
Contributor

PR incoming tonight, to finally finish this thing off.

At the same time, we can symlink the binaries to /bin, so you can just docker run nsqio/nsq nsqd and it will be correctly found. Any opinion here or there on whether that's desired behaviour? It's just extra lines in the Dockerfile, so I don't see the harm, but I won't bloat the PR if you'd rather just get this fix in.

@ploxiln
Copy link
Member

ploxiln commented Jun 18, 2015

That makes me think... could you just

ADD dist/docker/bin/* /

or

ADD dist/docker/bin/ /nsq_bin/
RUN cd / && ln -s nsq_bin/* .

@Bondza
Copy link
Author

Bondza commented Jun 19, 2015

Couldn't you put the binaries in /bin and instead symlink them to /?

That way you could

ADD dist/docker/bin/* /bin
RUN ln -s /bin/nsq_pubsub /
...
RUN ln -s /bin/to_nsq /

You would have to symlink each binary but any new binaries would automatically be added to /bin and possible to use like docker run nsqio/nsq any_new_binary

@mreiferson
Copy link
Member

@Bondza what about @ploxiln's suggestion, seems like the most future proof?

@ploxiln
Copy link
Member

ploxiln commented Jun 20, 2015

I'd just like to add that, if you use my second suggestion, you can additionally put links in bin, of course:

ADD dist/docker/bin/ /nsq_bin/
RUN cd /    && ln -s /nsq_bin/* . \
 && cd /bin && ln -s /nsq_bin/* .

And also, for docker, fewer directives ➡️ fewer layers ➡️ faster push/pull/run

@Bondza
Copy link
Author

Bondza commented Jun 20, 2015

Of course that works, I suggested it because I think it's a bit odd to place binaries in the root directory and not in /bin.

Assuming that all binaries are called nsq-something, you could have the binaries accessible from / and still put the binaries in /bin like this:

ADD dist/docker/bin/ /bin/
RUN ln -s /bin/*nsq* /

Anyways, thanks for fixing this!

@mreiferson
Copy link
Member

RFR @jehiah

/cc @ploxiln

jehiah added a commit that referenced this pull request Aug 8, 2015
docker: root directory is /dist/docker/bin
@jehiah jehiah merged commit a305368 into nsqio:master Aug 8, 2015
@mreiferson mreiferson deleted the docker_bin_558 branch August 8, 2015 21:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants