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

dockerize kbrowse and add instructions to README #29

Merged
merged 5 commits into from
Oct 29, 2018

Conversation

williamla
Copy link
Collaborator

No description provided.

Copy link
Contributor

@ptuckey ptuckey left a comment

Choose a reason for hiding this comment

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

Right on!

build.sh Outdated
rm -rf ${ROOT}/docker-files/*
cd ${ROOT}/docker-files/

curl -L -o master.zip https://github.com/PandoraMedia/kbrowse/archive/master.zip
Copy link
Contributor

Choose a reason for hiding this comment

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

What do you think about having this build from local source instead?

My thinking is that this could be a good first step towards #2.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, doing some testing on a clean pull and realized I had done a few things out of order between getting this running locally, then working this back into a commit. New changes pending.

docker build -t kbrowse .

# Run on default port 4000
docker run -p 4000:4000 kbrowse
Copy link
Contributor

Choose a reason for hiding this comment

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

I tried testing this PR locally, but ran into a couple of issues - not sure if they're particular to my local setup (I'm using "docker-machine version 0.14.0, build 89b8332").

  1. kbrowse -> kafka networking

By default, kbrowse uses "localhost:9092" for bootstrap servers. With #2, it would be nice to set up with docker-compose so that we bring up both kbrowse and kafka/zk containers, and configure kbrowse to consume from the kafka container.

Since we don't have a kafka container yet, I figured I'd test this via ./run-zookeeper-and-kafka, and then connecting to that with the docker run command. However, I'm getting a connection error on the "docker run" command, which I think is because the kbrowse container resolves "localhost" to itself, rather than the host.

I tried the suggestion at https://stackoverflow.com/a/24326540, which was to use --network="host", so that the docker container shares the host network, but I had the same issue.

Any thoughts on this?

  1. Browsing web console from host

When I use a custom config that points to a kafka broker hosted elsewhere, it looks like things started up just fine. I then wanted to try browsing to the web console from my host machine, so I tried my docker IP on port 4000, but got back an ERR_EMPTY_RESPONSE.

Were you able to browse to your web console?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If you're on a mac, the --network="host" directive won't work due to the way docker runs on the OS (docker/for-mac#1031). I might try to pull in a script I was looking at yesterday to get a full kafka env up with docker-compose, but that bit's still new to me.

For browsing the web console, I was able to reproduce the error you're seeing so will have to look into it further. It's strange because, of course, it was working just fine when I had created this a few weeks back... ;) Thanks for the quick review and test.

Copy link
Collaborator Author

@williamla williamla Oct 20, 2018

Choose a reason for hiding this comment

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

Fixed the browse issue. I had missed including the resource directory when I changed the docker config to perform the build from local source, so that resulted in misconfigured logging where jetty seemed to get stuck in a selector loop like what's described here: jetty/jetty.project#435. This would also explain why the console was so chatty with logging level at DEBUG.

You should be able to kbrowse now!

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks William - appreciate you iterating on this!

I am indeed on OSX, which explains the network issue.

I tried running the latest version (with a custom config pointing at an external Kafka broker) - as you say, the chatty logs go away - however, I'm not able to confirm that the web console is available - here is my output:

ptuckey-m01:kbrowse ptuckey$ docker run -p 4000:4000 kbrowse
+ trap 'echo exited ./launch.sh' HUP INT QUIT TERM KILL
+ export 'APP_ROOT=/app'
+ mkdir -p config
+ cp default.yml /app/config/default.yml
+ export 'cmd=java -jar /app/kbrowse-*-SNAPSHOT-standalone.jar server'
+ java -jar /app/kbrowse-0.0.1-SNAPSHOT-standalone.jar server

I tried browsing to my docker IP at port 4000, but am still receiving ERR_EMPTY_RESPONSE.

I'm supportive of landing as is, but I think the README should qualify that it's Linux-only atm (assuming that's the case). Might be handy if it mentioned how to browse to the web console in that case too.

I would request then, the following changes, prior to merge:

  1. Mention OS support in Docker section of README
  2. Provide example of browsing to web console in Docker context
  3. Can you please confirm that "./run-tests" succeeds? (Do we need to add "npm" to Dockerfile?)

At a higher level... the fuller docker compose solution seems like more work, for sure, so no need to include here (unless you would prefer). When you mention "pull in a script"... just a reminder that we want to be copyright-conscience.

Copy link
Collaborator Author

@williamla williamla Oct 23, 2018

Choose a reason for hiding this comment

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

I'm on a Mac so the current implementation works independent of OS. Looking at the command you pasted above, it doesn't look like you pasted in the custom config on the terminal:

# Run with local custom config (custom.yml)
docker run -p 4000:4000 -v $PWD/config/custom.yml:/app/default.yml kbrowse

This -v command basically maps the file located at ./config/custom.yml to the docker's default.yml file, which then within the docker container gets copied and picked up for use at runtime. Once that happens, you'll see a 'KBrowse Ready...' message with the host and port info.

And run-tests should succeed, but the container build is primarily targeted for build and run atm. I'll defer integrating the tests into the docker build until I've had more time to come back to this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I took a look at the existing test set up and it might make sense to refactor this when the docker-compose step is completed like you mentioned in #2 . I tried running the script as the build process given that the test builds it anyways, but got the following error. Not worth the effort to chase this down atm, but it might be related to the slim java container I'm using and the env it's expecting.

Step 7/15 : RUN ./run-tests
 ---> Running in c251e65b6ef2
Downloading Leiningen to /root/.lein/self-installs/leiningen-2.6.1-standalone.jar now...
Connecting to github.com (192.30.253.112:443)
Connecting to github-production-release-asset-2e65be.s3.amazonaws.com (54.231.121.91:443)
leiningen-2.6.1-stan   7% |**                             |  1087k  0:00:12 ETA
leiningen-2.6.1-stan 100% |*******************************| 14983k  0:00:00 ETA

Retrieving lein-ring/lein-ring/0.12.4/lein-ring-0.12.4.pom from clojars
Retrieving org/clojure/data.xml/0.0.8/data.xml-0.0.8.pom from central
Retrieving org/clojure/pom.contrib/0.1.2/pom.contrib-0.1.2.pom from central
Retrieving org/sonatype/oss/oss-parent/7/oss-parent-7.pom from central
Retrieving leinjacker/leinjacker/0.4.2/leinjacker-0.4.2.pom from clojars
Retrieving org/clojure/core.contracts/0.0.1/core.contracts-0.0.1.pom from central
Retrieving org/clojure/pom.contrib/0.0.26/pom.contrib-0.0.26.pom from central
Retrieving org/sonatype/oss/oss-parent/5/oss-parent-5.pom from central
Retrieving org/clojure/core.unify/0.5.3/core.unify-0.5.3.pom from central
Retrieving lein-cljfmt/lein-cljfmt/0.5.7/lein-cljfmt-0.5.7.pom from clojars
Retrieving cljfmt/cljfmt/0.5.7/cljfmt-0.5.7.pom from clojars
Retrieving org/clojure/clojure/1.7.0/clojure-1.7.0.pom from central
Retrieving org/clojure/tools.reader/1.0.0-alpha4/tools.reader-1.0.0-alpha4.pom from central
Retrieving org/clojure/clojure/1.4.0/clojure-1.4.0.pom from central
Retrieving rewrite-clj/rewrite-clj/0.5.2/rewrite-clj-0.5.2.pom from clojars
Retrieving org/clojure/tools.reader/0.10.0/tools.reader-0.10.0.pom from central
Retrieving rewrite-cljs/rewrite-cljs/0.4.3/rewrite-cljs-0.4.3.pom from clojars
Retrieving org/clojure/clojurescript/1.7.228/clojurescript-1.7.228.pom from central
Retrieving com/google/javascript/closure-compiler/v20151216/closure-compiler-v20151216.pom from central
Retrieving com/google/javascript/closure-compiler-parent/v20151216/closure-compiler-parent-v20151216.pom from central
Retrieving org/sonatype/oss/oss-parent/9/oss-parent-9.pom from central
Retrieving org/clojure/google-closure-library/0.0-20151016-61277aea/google-closure-library-0.0-20151016-61277aea.pom from central
Retrieving org/clojure/google-closure-library-third-party/0.0-20151016-61277aea/google-closure-library-third-party-0.0-20151016-61277aea.pom from central
Retrieving org/clojure/data.json/0.2.6/data.json-0.2.6.pom from central
Retrieving org/mozilla/rhino/1.7R5/rhino-1.7R5.pom from central
Retrieving org/clojure/tools.reader/1.0.0-alpha1/tools.reader-1.0.0-alpha1.pom from central
Retrieving org/clojure/tools.reader/1.0.0-alpha3/tools.reader-1.0.0-alpha3.pom from central
Retrieving meta-merge/meta-merge/1.0.0/meta-merge-1.0.0.pom from clojars
Retrieving com/googlecode/java-diff-utils/diffutils/1.2.1/diffutils-1.2.1.pom from central
Retrieving org/clojure/core.contracts/0.0.1/core.contracts-0.0.1.jar from central
Retrieving org/clojure/core.unify/0.5.3/core.unify-0.5.3.jar from central
Retrieving org/clojure/data.xml/0.0.8/data.xml-0.0.8.jar from central
Retrieving org/clojure/tools.reader/1.0.0-alpha4/tools.reader-1.0.0-alpha4.jar from central
Retrieving org/clojure/clojure/1.7.0/clojure-1.7.0.jar from central
Retrieving org/clojure/clojurescript/1.7.228/clojurescript-1.7.228.jar from central
Retrieving com/google/javascript/closure-compiler/v20151216/closure-compiler-v20151216.jar from central
Retrieving org/clojure/google-closure-library/0.0-20151016-61277aea/google-closure-library-0.0-20151016-61277aea.jar from central
Retrieving org/clojure/google-closure-library-third-party/0.0-20151016-61277aea/google-closure-library-third-party-0.0-20151016-61277aea.jar from central
Retrieving org/clojure/data.json/0.2.6/data.json-0.2.6.jar from central
Retrieving org/mozilla/rhino/1.7R5/rhino-1.7R5.jar from central
Retrieving com/googlecode/java-diff-utils/diffutils/1.2.1/diffutils-1.2.1.jar from central
Retrieving lein-ring/lein-ring/0.12.4/lein-ring-0.12.4.jar from clojars
Retrieving lein-cljfmt/lein-cljfmt/0.5.7/lein-cljfmt-0.5.7.jar from clojars
Retrieving leinjacker/leinjacker/0.4.2/leinjacker-0.4.2.jar from clojars
Retrieving cljfmt/cljfmt/0.5.7/cljfmt-0.5.7.jar from clojars
Retrieving rewrite-cljs/rewrite-cljs/0.4.3/rewrite-cljs-0.4.3.jar from clojars
Retrieving rewrite-clj/rewrite-clj/0.5.2/rewrite-clj-0.5.2.jar from clojars
Retrieving meta-merge/meta-merge/1.0.0/meta-merge-1.0.0.jar from clojars
Exception in thread "main" java.lang.StackOverflowError
	at java.io.UnixFileSystem.getBooleanAttributes0(Native Method)
	at java.io.UnixFileSystem.getBooleanAttributes(UnixFileSystem.java:242)
	at java.io.File.exists(File.java:819)
	at sun.reflect.GeneratedMethodAccessor34.invoke(Unknown Source)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, I should have clarified - I was testing against a custom kafka broker by editing "default.yml", and then re-building/re-running. I've now tested using the README example style (including a custom config with the --volume arg), and received the same output as before. I waited longer this time, and eventually received a TimeoutException:

org.apache.kafka.common.errors.TimeoutException: Timeout expired while fetching topic metadata

I since discovered that I can't ping the custom broker from within a docker container, so I suspect the same root issue (can't use --network=host).

So, if I'm understanding correctly, this PR adds the ability to build/run KBrowse in Docker, but can only connect to a publicly-available Kafka cluster.

Do I have that right?

If so, I'd request that the README Docker section make mention of that limitation, to avoid confusion - or, if I'm wrong, then can you describe the type of network connection you're making, to help me troubleshoot? (since you're on OSX, I'm assuming you also can't --network=host)

Thanks for your help!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There is no defined or set limitation to what you should be able to connect to. How are you pinging the broker? And does that ping work from your Mac terminal directly? I wasn't able to ping kafka from directly inside the container, but I think that behavior is expected. Pinging zookeeper did work, ie:
echo dump | nc zookeeper_host 2181 | grep brokers

I'm able to connect to our internal zookeeper that isn't publicly accessible. Do you have firewall rules on kafka? If you're able to run the kbrowse jar directly and connect to kafka, then it should be no different connecting via this container.

Copy link
Contributor

Choose a reason for hiding this comment

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

Turns out we have some network conflicts at work, which interfered with Docker container traffic. This required a workaround in Docker config. I also wound up installing Docker for Mac (and updating OS version). Things are happier now, including the fact that "localhost" on the host resolves to the Docker IP (previously "localhost:4000" wasn't working for me from OSX).

The other networking gotcha on OSX is connecting to the host from the Docker container. The current suggested workaround is to use host.docker.internal:
https://docs.docker.com/docker-for-mac/networking/#i-cannot-ping-my-containers

This additional example might be useful?

# Docker for Mac 18.03+
docker run -e KAFKA_BOOTSTRAP_SERVERS='local=host.docker.internal:9092' -p 4000:4000 kbrowse

docker build -t kbrowse .

# Run on default port 4000
docker run -p 4000:4000 kbrowse
Copy link
Contributor

Choose a reason for hiding this comment

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

Turns out we have some network conflicts at work, which interfered with Docker container traffic. This required a workaround in Docker config. I also wound up installing Docker for Mac (and updating OS version). Things are happier now, including the fact that "localhost" on the host resolves to the Docker IP (previously "localhost:4000" wasn't working for me from OSX).

The other networking gotcha on OSX is connecting to the host from the Docker container. The current suggested workaround is to use host.docker.internal:
https://docs.docker.com/docker-for-mac/networking/#i-cannot-ping-my-containers

This additional example might be useful?

# Docker for Mac 18.03+
docker run -e KAFKA_BOOTSTRAP_SERVERS='local=host.docker.internal:9092' -p 4000:4000 kbrowse

@ptuckey ptuckey merged commit 9b68428 into PandoraMedia:master Oct 29, 2018
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.

2 participants