Add Dockerfile #110

solsson · 2017-07-06T04:53:53Z

No description provided.

edenhill · 2017-07-06T21:01:40Z

Great stuff!

Can you add an example to the README how to use it?

ctrochalakis · 2017-07-07T06:18:55Z

Dockerfile

@@ -0,0 +1,18 @@
+FROM debian:stable-20170620


Hello,

Is there a reason you are using the date tag and not just debian:stable?

I thought because the release is so recent that I wanted to stress the upgrade. If we remove the date tag I guess that debian:stretch makes more sense. Years from now they'll move :stable to next release, and the build might break.

we build master anyway so the code moves too

edenhill · 2017-07-08T08:58:01Z

LGTM.

@ctrochalakis mind a final review?

ctrochalakis · 2017-07-09T15:09:44Z

Dockerfile

+
+COPY . /usr/src/kafkacat
+
+RUN runDeps=''; \


You could drop runDeps since it's empty, also, you could avoid setting -ex and use && between commands.

Is this just a matter of conventions, or does it have any practical implications?

I'll drop the runDeps. This whole block is just copy paste from a convention I tend to use on debian.

ctrochalakis · 2017-07-09T15:12:42Z

README.md

+
+# Running in docker
+
+There is no official image build yet but you may run `docker build -t kafkacat .` or use for example `solsson/kafkacat@sha256:3075a6ca2d8431910cf01af13acd2ef9542c8ed48547733eb5c4321364ef0b66`.


It is better not to mention a specific docker image since it will become outdated soon enough.

ctrochalakis · 2017-07-09T15:16:15Z

README.md

+
+There is no official image build yet but you may run `docker build -t kafkacat .` or use for example `solsson/kafkacat@sha256:3075a6ca2d8431910cf01af13acd2ef9542c8ed48547733eb5c4321364ef0b66`.
+
+Example:


Personally I'd find the example easier to follow if it avoided detaching kafkacat instances. It's closer to how you 'd run kafkacat if it was locally installed, no need to call docker logs.

ctrochalakis · 2017-07-09T15:20:30Z

README.md

+docker run --rm kafkacat --help
+echo 1 > example.log
+docker run --name test-producer -d -v $(pwd)/example.log:/logs/example.log --entrypoint /bin/bash --net=host kafkacat \
+  -c 'tail -f /logs/example.log | kafkacat -b mybroker -t logs -P'


You can do that without passing example.log as a volume (assuming detach is dropped):

tail -f example.log | docker run -i --name test-producer --net=host kafkacat -b mybroker -t logs -P

I just thought this was a more interesting use case, but maybe only because I was working with log aggregation in Kubernetes.

solsson · 2017-07-09T18:27:14Z

I'll be on vacation for two weeks and resume this work once I'm back. Feel free to push to the PR.

…uild" This reverts commit cafd5e2.

solsson · 2017-07-20T03:27:04Z

@ctrochalakis thanks for the review. I've committed new suggestions.

Also in celebration of https://github.com/edenhill/librdkafka/releases/tag/v0.11.0 I've built:

$ docker run solsson/kafkacat@sha256:1266d140c52cb39bf314b6f22b6d7a01c4c9084781bc779fdfade51214a713a8 help
Error: -b <broker,..> missing

Usage: kafkacat <options> [file1 file2 .. | topic1 topic2 ..]]
kafkacat - Apache Kafka producer and consumer tool
https://github.com/edenhill/kafkacat
Copyright (c) 2014-2015, Magnus Edenhill
Version 1.3.1 (JSON) (librdkafka 0.11.0 builtin.features=gzip,snappy,sasl,regex,lz4,sasl_plain,plugins)

and then in b04279c reverted the version change.

…mage

…ibrdkafka

edenhill

I suspect librdkafka is not getting compiled with openssl or libsasl2 support.
It would probably makes sense to install libssl-dev and libsasl2-dev with buildDeps

edenhill · 2017-10-10T13:16:04Z

Dockerfile

  apt-get update && apt-get install -y $buildDeps --no-install-recommends; \
  rm -rf /var/lib/apt/lists/*; \
  \
  cd /usr/src/kafkacat; \
+  echo "Source versions:"; \
+  cat ./bootstrap.sh | grep github_download; \


Suggest grep ^github_download bootstrap.sh

solsson · 2017-10-10T13:59:53Z

I suspect librdkafka is not getting compiled with openssl or libsasl2 support.

Thanks. Fixed. Do we need libsasl2-2 and libssh2-1 to stick around as runtime dependency? Now they're apt-get removed. I don't have an ssl'd kafka to test with.

I also fixed the grep as suggested.

What do you think about 2778479 and 275fade? I think they help outsiders like me produce stable builds, but they also mean that there's another version number to maintain in source, and you don't need it if you plan to publish official docker builds triggered whenever librdkafka is released.

edenhill · 2017-10-10T14:02:31Z

I think fixating the versions is a good idea and it isn't too bad to update the 3-4 times librdkafka is released per year.

libsasl2-2 is most likely needed during runtime, libssh* is not afaik.

edenhill · 2017-10-10T14:33:18Z

Dockerfile

+  echo "Source versions:"; \
+  grep ^github_download ./bootstrap.sh; \
+  \
+  ./bootstrap.sh; \


We'll want to remove tmp-bootstrap before running bootstrap to make sure it gets a clean slate

edenhill · 2017-10-10T14:36:13Z

Dockerfile

+  rm -rf /usr/src/kafkacat/tmp-bootstrap; \
+  apt-get purge -y --auto-remove $buildDeps; \
+  rm /var/log/dpkg.log /var/log/alternatives.log /var/log/apt/*.log
+


Run kafkacat -V to verify that the binary still works after package removal.

It bailed on lack of libcrypto. I've added all libs in 723fc0f. The resulting layer totals 10.2 MB now instead of 6.6 prior to building with libssl+libsasl. Lacking a comprehensive set of tests for the build I found it warranted to take the safe approach.

Aye, add kafkacat -V as a final step which makes sure all required dependencies are in place.

solsson · 2017-10-11T05:51:25Z

I think we're done. Running it from https://hub.docker.com/r/solsson/kafkacat/ for my use cases, and it works.

edenhill · 2017-10-11T09:18:49Z

Great stuff, thanks for this @solsson !

solsson added 2 commits July 6, 2017 06:48

Adds Docker build

4c398ae

Runs kafkacat by default

2085928

solsson added a commit to solsson/docker-kafkacat that referenced this pull request Jul 6, 2017

Same as edenhill/kcat#110 but one less layer due to no need for COPY

f431caf

solsson added 3 commits July 7, 2017 07:02

Uses the convention from solsson/docker-kafkacat#1

940eff5

Exemplifies use of the docker build

219bf9b

Adds a check that you've found a proper image

91313fb

ctrochalakis reviewed Jul 7, 2017

View reviewed changes

Switches to moving tag, better for official images, and ...

d922e21

we build master anyway so the code moves too

ctrochalakis reviewed Jul 9, 2017

View reviewed changes

solsson added 6 commits July 20, 2017 04:43

Avoids mention that will get outdated

e42be5f

Removes an additional 100k

e8e23b2

Removes unused runDeps in response to edenhill#110 review

5d4ca26

Simplifies examples in response to edenhill#110 review

a71abcd

Makes it easier to spot the -i

57e113c

Runs specific version for hub.docker.com/r/solsson/kafkacat build

cafd5e2

solsson force-pushed the dockerfile branch from f23bd38 to cafd5e2 Compare July 20, 2017 03:17

Revert "Runs specific version for hub.docker.com/r/solsson/kafkacat b…

b04279c

…uild" This reverts commit cafd5e2.

solsson added a commit to StreamingMicroservicesPlatform/docker-kafka that referenced this pull request Jul 20, 2017

Use librdkafka 0.11.0 from edenhill/kcat#110

1b7a24e

solsson added a commit to StreamingMicroservicesPlatform/docker-kafka that referenced this pull request Jul 20, 2017

Use librdkafka 0.11.0 from edenhill/kcat#110

59cd5f1

solsson added 2 commits October 10, 2017 14:09

Debian slim works just as well, so no reason to use the larger base i…

1c3a0a8

…mage

Avoids some apt warnings, and shows that we build current master of l…

073f206

…ibrdkafka

edenhill requested changes Oct 10, 2017

View reviewed changes

Simplifies the version grep

c08f076

solsson added 4 commits October 10, 2017 15:35

librdkafka build now finds libssl and libsasl2 ok

fc97977

librdkafka build now finds liblz4 ok

0393379

Uses specific dependency versions for docker image build

2778479

Lets you override dependency versions at build time

275fade

solsson added 2 commits October 10, 2017 16:12

Keeps libsasl2 as runtime dependency

675bdb2

Keep runtime libs for the other -dev buildDeps too

723fc0f

edenhill requested changes Oct 10, 2017

View reviewed changes

solsson added 3 commits October 10, 2017 16:59

Avoids reuse of earlier bootstrap runs

21a9146

Excludes rpm sources from docker build

ed15858

Keeps only the deps required to pass a kafkacat -V test

ac6c085

edenhill merged commit 1215518 into edenhill:master Oct 11, 2017

FuriKuri mentioned this pull request Oct 19, 2017

Add build and usage alternative with docker #118

Closed

solsson mentioned this pull request Nov 27, 2017

How to run examples bitbouncer/kspp#7

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Dockerfile #110

Add Dockerfile #110

solsson commented Jul 6, 2017

edenhill commented Jul 6, 2017

ctrochalakis Jul 7, 2017

solsson Jul 7, 2017

edenhill commented Jul 8, 2017

ctrochalakis Jul 9, 2017

solsson Jul 9, 2017

solsson Jul 9, 2017

ctrochalakis Jul 9, 2017

ctrochalakis Jul 9, 2017

ctrochalakis Jul 9, 2017

solsson Jul 9, 2017

solsson commented Jul 9, 2017 •

edited

Loading

solsson commented Jul 20, 2017

edenhill left a comment

edenhill Oct 10, 2017

solsson commented Oct 10, 2017

edenhill commented Oct 10, 2017

edenhill Oct 10, 2017

solsson Oct 10, 2017

edenhill Oct 10, 2017

solsson Oct 10, 2017 •

edited

Loading

edenhill Oct 10, 2017

solsson Oct 10, 2017

solsson commented Oct 11, 2017

edenhill commented Oct 11, 2017


		# Running in docker

		There is no official image build yet but you may run `docker build -t kafkacat .` or use for example `solsson/kafkacat@sha256:3075a6ca2d8431910cf01af13acd2ef9542c8ed48547733eb5c4321364ef0b66`.


		There is no official image build yet but you may run `docker build -t kafkacat .` or use for example `solsson/kafkacat@sha256:3075a6ca2d8431910cf01af13acd2ef9542c8ed48547733eb5c4321364ef0b66`.

		Example:


		COPY . /usr/src/kafkacat

		RUN runDeps=''; \

Add Dockerfile #110

Add Dockerfile #110

Conversation

solsson commented Jul 6, 2017

edenhill commented Jul 6, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

edenhill commented Jul 8, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

solsson commented Jul 9, 2017 • edited Loading

solsson commented Jul 20, 2017

edenhill left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

solsson commented Oct 10, 2017

edenhill commented Oct 10, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

solsson Oct 10, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

solsson commented Oct 11, 2017

edenhill commented Oct 11, 2017

solsson commented Jul 9, 2017 •

edited

Loading

solsson Oct 10, 2017 •

edited

Loading