adds chart for kafka by faraazkhan · Pull Request #144 · helm/charts

faraazkhan · 2016-10-22T01:53:41Z

Adds a chart for Kafka based on the awesome work done here: https://github.com/Yolean/kubernetes-kafka and here: https://github.com/kubernetes/charts/tree/master/incubator/zookeeper

k8s-ci-robot · 2016-10-22T01:53:41Z

Can a kubernetes member verify that this patch is reasonable to test? If so, please reply with "@k8s-bot ok to test" on its own line.

Regular contributors should join the org to skip this step.

prydonius · 2016-10-22T02:01:15Z

@k8s-bot ok to test

prydonius · 2016-10-22T02:01:30Z

@faraazkhan can you sign the Linux Foundation CLA?

faraazkhan · 2016-10-22T02:03:17Z

@prydonius I am trying. Added my github account and everything, it says, I am supposed to get an email for with the CLA but nothing so far.

faraazkhan · 2016-10-22T02:06:12Z

Also looks like we are running an old version of helm in Jenkins right now. I1021 19:04:55.614] [ERROR] templates/kafka-petset.yaml: namespace option is currently NOT supported doesnt seem right 😄

Update: Verified Jenkins is running a newer version Client: &version.Version{SemVer:"v2.0.0-beta.1", GitCommit:"1b792f990675702488899b94352168572c54c972", GitTreeState:"clean"} . I have tested this against Client: &version.Version{SemVer:"v2.0.0-alpha.5", GitCommit:"a324146945c01a1e2dd7eaf23caf0e55fabfd3d2", GitTreeState:"clean"}

Not sure why the error then 😞

prydonius · 2016-10-23T19:52:09Z

@faraazkhan Thanks for signing the CLAs! Namespace shouldn't be set in the manifests, since Helm manages which namespace a Chart's resources gets installed into using the helm install --namespace flag.

prydonius · 2016-10-23T19:53:49Z

can you add https://github.com/solsson/dockerfiles/tree/master/kafka

prydonius · 2016-10-23T19:56:41Z

Helm now supports specifying dependencies using a requirements.yaml file. Can you update this to remove this vendored version of zookeeper, and instead just add a requirements.yaml that can be used to bring in the dependency?

See https://github.com/kubernetes/helm/blob/master/docs/charts.md#managing-dependencies-with-requirementsyaml and https://github.com/kubernetes/charts/tree/master/incubator/patroni for an example.

prydonius · 2016-10-23T19:57:14Z

Can you remove these trailing lines?

faraazkhan · 2016-10-24T03:40:56Z

@prydonius Jenkins is currently failing with W1023 20:38:31.781] Error: deletion error count 1: no objects visited. I have seen this error, along with at least one more in the latest version of helm. This doesn't necessarily seem like an issue with the chart, but more with helm?

faraazkhan · 2016-10-24T04:07:45Z

@k8s-bot e2e test this

chrislovecnm

Multiple tweaks ... looks good

chrislovecnm · 2016-10-24T15:41:35Z

+
+## Pre Requisites:
+
+* Kubernetes 1.3 with alpha APIs enable


You are also using storage classes as well. You probably should mention both of those.

chrislovecnm · 2016-10-24T15:42:41Z

+
+### Connecting to Kafka
+
+You can connect to Kafka by running a simple pod in the K8s cluster like this with a configuration like this:


You may want to add a notes.txt file with these instructions.

chrislovecnm · 2016-10-24T15:45:30Z

+        - containerPort: 9092
+          name: kafka
+        resources:
+          requests:


We need a limits section in here as well.

chrislovecnm · 2016-10-24T15:46:12Z

+          mountPath: /tmp/zookeeper
+      volumes:
+      - name: workdir
+        emptyDir: {}


Dumb question but does this need persitence?

chrislovecnm · 2016-10-24T15:46:41Z

+  - metadata:
+      name: datadir
+      annotations:
+        volume.alpha.kubernetes.io/storage-class: anything


Should we have this setup to do both a PVC or an empty dir? Looking for an example ...

I think the empty dir can be removed. Want to leave the PVC in. I will push that out shortly.

chrislovecnm · 2016-10-24T15:47:59Z

+        pod.alpha.kubernetes.io/initialized: "true"
+        pod.alpha.kubernetes.io/init-containers: '[
+        ]'
+    spec:


We need health check probes http://kubernetes.io/docs/user-guide/liveness/

Still needs a ready probe. We can move into incubator without.

I could just add the same test for readiness as well. Not sure how valuable that would be though, in Kafka's situation.

chrislovecnm · 2016-10-24T15:50:53Z

@viglesiasce any ideas on the build failure "W1023 20:38:31.781] Error: deletion error count 1: no objects visited", do we need to bump the helm version we are using?

krisnova · 2016-10-24T15:52:29Z

+*.bak
+*.tmp
+*~
+# Various IDEs


Is it best practice to add proprietary file types to a global .ignore file? Shouldn't the user be managing these?

@kris-nova this is autogenned by helm.

Adds liveness probe Adds resource limits Improves README.md

faraazkhan · 2016-10-30T18:51:59Z

Ok. I have addressed the comments here . But still getting the Error: deletion error count 1: no objects visited on Jenkins. I also see this happen locally when I do a helm delete --purge my-release. It seems to be raised here, will try to dig into it further later, in the meanwhile any help is appreciated. @prydonius @viglesiasce @chrislovecnm

Update: Please note, despite the error all the resources seem to be deleted fine and I am able to reuse the helm release name even in the same namespace with no errors (so purge worked just fine)

chrislovecnm · 2016-10-30T19:43:24Z

@k8s-bot e2e test this

chrislovecnm

Still, have storage class issue, and readyprobe not defined, and we can merge this into incubator with these open items.

@viglesiasce any ideas about the build failures?

Thanks for the chart @faraazkhan!!!

chrislovecnm · 2016-10-30T19:44:32Z

+        pod.alpha.kubernetes.io/initialized: "true"
+        pod.alpha.kubernetes.io/init-containers: '[
+        ]'
+    spec:


Still needs a ready probe. We can move into incubator without.

viglesiasce · 2016-11-02T17:13:34Z

Sorry for the delay @faraazkhan! Will look into the failures today.

technosophos · 2016-11-02T20:24:47Z

With that deletion error, can you still see the resources in Kubernetes that were supposed to be deleted? That error message usually indicates that the deletion logic went to remove a resource from Kubernetes, and the resource was missing.

viglesiasce · 2016-11-02T20:43:36Z

One thing I noticed is that after deploy the kf and kf-broker services end up with no endpoints.

@faraazkhan is that expected?

@technosophos could that be an issue?

NAME                        ENDPOINTS                                                        AGE
ep/demo-app-promchart       10.140.5.17:8080,10.140.5.18:8080,10.140.5.25:8080 + 2 more...   1d
ep/demo-grafana             10.140.5.21:3000                                                 1d
ep/kubernetes               104.199.112.125:443                                              16d
ep/plinking-racoo-alerts    10.140.5.6:9093                                                  5d
ep/plinking-racoo-server    10.140.5.7:9090                                                  5d
ep/steely-panda-nginx       10.140.5.23:80                                                   21h
ep/voting-goose-kf          <none>                                                           5m
ep/voting-goose-kf-broker   <none>                                                           5m
ep/voting-goose-zk          10.140.1.11:2888,10.140.1.13:2888,10.140.1.14:2888 + 3 more...   5m
NAME                         CLUSTER-IP       EXTERNAL-IP   PORT(S)             AGE
svc/demo-app-promchart       10.143.244.167   <none>        80/TCP              1d
svc/demo-grafana             10.143.246.48    <none>        80/TCP              1d
svc/kubernetes               10.143.240.1     <none>        443/TCP             16d
svc/plinking-racoo-alerts    10.143.245.26    <none>        9093/TCP            5d
svc/plinking-racoo-server    10.143.244.45    <none>        80/TCP              5d
svc/steely-panda-nginx       10.143.242.164   <none>        80/TCP              21h
svc/voting-goose-kf          10.143.243.96    <none>        9092/TCP            5m
svc/voting-goose-kf-broker   None             <none>        9092/TCP            5m
svc/voting-goose-zk          None             <none>        2888/TCP,3888/TCP   5m

chrislovecnm · 2016-11-03T07:56:14Z

Houston that is a problem with the services ;) @faraazkhan did you monkey your selectors or @viglesiasce do yah think this is helm?

faraazkhan · 2016-11-10T02:37:32Z

@chrislovecnm @viglesiasce @technosophos sorry about the delay in responding, so I just pushed a few changes, that ensure that all the services created have endpoints. I see what @technosophos is saying but not sure what that resource could be, everything created in the helm chart seems to be destroyed correctly:

viglesiasce · 2016-11-11T00:45:58Z

Thanks @faraazkhan! Things are looking better wrt services. Can you add a NOTES file that explains next steps to the users.

faraazkhan · 2016-11-11T14:36:29Z

@viglesiasce Can you please be specific, how can I improve this? Maybe I am not following a convention notes vs NOTES https://github.com/kubernetes/charts/pull/144/files#diff-2654b4aa4eacf071f2e90debaad8b050R1

chrislovecnm · 2016-11-11T20:25:38Z

NOTES.txt file. Look at the charts that are not in incubator. They should have examples

prydonius · 2016-11-11T20:28:46Z

@@ -0,0 +1,36 @@
+You can connect to Kafka by running a simple pod in the K8s cluster like this with a configuration like this:


Helm will specifically look for a while called NOTES.txt, render it as a template, and print it out after installing/upgrading or running helm status on the release. This will allow you to print out working commands with the name of the release! See https://github.com/kubernetes/charts/blob/master/incubator/patroni/templates/NOTES.txt for an example :)

chrislovecnm

This is also and issue

oot@ip-172-20-82-63:/var/log# kubectl -n kafka describe svc
Name:           exacerbated-ar-kafka
Namespace:      kafka
Labels:         chart=kafka-0.1.0
            component=exacerbated-ar-kafka
            heritage=Tiller
            release=exacerbated-ar
Selector:       app=kafka,component=exacerbated-ar-kafka
Type:           ClusterIP
IP:         100.69.243.160
Port:           <unset> 9092/TCP
Endpoints:      <none>
Session Affinity:   None
No events.

Name:           exacerbated-ar-kafka-bro
Namespace:      kafka
Labels:         chart=kafka-0.1.0
            component=exacerbated-ar-kafka
            heritage=Tiller
            release=exacerbated-ar
Selector:       app=kafka,component=exacerbated-ar-kafka
Type:           ClusterIP
IP:         None
Port:           <unset> 9092/TCP
Endpoints:      <none>
Session Affinity:   None
No events.

Name:           exacerbated-ar-zk
Namespace:      kafka
Labels:         app=zk
            chart=zookeeper-0.1.1
            component=exacerbated-ar-zk
            heritage=Tiller
            release=exacerbated-ar
Selector:       app=zk,component=exacerbated-ar-zk
Type:           ClusterIP
IP:         None
Port:           client  2181/TCP
Endpoints:      100.96.3.3:2181,100.96.4.5:2181,100.96.5.2:2181
Port:           peer    2888/TCP
Endpoints:      100.96.3.3:2888,100.96.4.5:2888,100.96.5.2:2888
Port:           leader-election 3888/TCP
Endpoints:      100.96.3.3:3888,100.96.4.5:3888,100.96.5.2:3888
Session Affinity:   None
No events.

Name:           zookeeper
Namespace:      kafka
Labels:         <none>
Selector:       app=zk
Type:           ClusterIP
IP:         100.65.108.22
Port:           client  2181/TCP
Endpoints:      100.96.3.3:2181,100.96.4.5:2181,100.96.5.2:2181
Session Affinity:   None

You have two services that do not have endpoints. I am uncertain what the purpose of the services are.

chrislovecnm · 2016-11-12T21:57:57Z

+
+## Known Limitations
+
+* Namespace creation is not automated


This is an easy fix. Will need to happen for stable. This is may be a reason why unit tests are failing.

@faraazkhan any word on what these services are for?

viglesiasce · 2016-11-18T19:34:52Z

@k8s-bot ok to test

k8s-ci-robot · 2016-11-18T19:36:06Z

Jenkins Charts e2e failed for commit ca656b5. Full PR test history.

The magic incantation to run this job again is @k8s-bot e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

viglesiasce

Move notes.txt to templates/NOTES.txt
Update NOTES.txt to be templatized
Remove dependencies in notes on a particular namespace

viglesiasce · 2016-12-09T18:01:00Z

+  kind: Pod
+  metadata:
+    name: testclient
+    namespace: kafka


I wouldnt set the namespace explicitly here. Users may not have configured this namespace for installation.

You can make this a kubectl run command and avoid having the pod yaml here. Look at the postgresql chart as an example.

kubectl run client -it --rm --image solsson/kafka:0.10.0.1 --command -- bash

viglesiasce · 2016-12-09T18:09:01Z

+  To stop the listener session above press: Ctrl+C
+
+  To start an interactive message producer session:
+  kubectl -n kafka exec -ti testclient -- ./bin/kafka-console-producer.sh --broker-list my-release-broker-kf.kafka.svc.cluster.local:9092 --topic test1


You can simplify the dns name there and make it --broker-list {{ .Release.Name }}-kf:9092. Again this make the command work across namespaces.

F21 · 2016-12-19T22:31:13Z

Any update here?

prydonius · 2017-02-03T00:52:57Z

@faraazkhan any updates here?

which might not matter because we no longer have a loadbalancing service. These probes won't catch all failure modes, but if they fail we're pretty sure the container is malfunctioning. I found some sources recommending ./bin/kafka-topics.sh for probes but to me it looks risky to introduce a dependency to some other service for such things. One such source is helm/charts#144 The zookeeper probe is from https://kubernetes.io/docs/tasks/configure-pod-container/configure-liveness-readiness-probes/ An issue is that zookeeper's logs are quite verbose for every probe.

helm/charts#144 Beware: This image is 841 MB, compared to 301 for the current :latest.

helm/charts#144 Beware: This image is 841 MB, compared to 301 for the current :latest. See README.md for upgrade instructions.

…-papertrail-system [T2A-143] fix oos-detection papertrail system name

[openebs]chore(release): add OpenEBS 2.0.0 YAML for AWS

googlebot added the cla: yes label Oct 22, 2016

prydonius reviewed Oct 23, 2016

View reviewed changes

Comment thread incubator/kafka/values.yaml Outdated

Copy link
Copy Markdown

Member

prydonius Oct 23, 2016

Choose a reason for hiding this comment

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

Can you remove these trailing lines?

faraazkhan added 3 commits October 23, 2016 21:50

adds chart for kafka

8520a3a

upgrading chart to using requirements.yaml for dependencies

612a49e

updates README.md

4456b6a

chrislovecnm suggested changes Oct 24, 2016

View reviewed changes

krisnova reviewed Oct 24, 2016

View reviewed changes

faraazkhan added 2 commits October 30, 2016 13:27

adds notes.txt

a9d52c5

Adds liveness probe Adds resource limits Improves README.md

Merge remote-tracking branch 'origin/master' into kafka

f8fc256

chrislovecnm reviewed Oct 30, 2016

View reviewed changes

adds readiness probe and removes empty dir

d57a539

ensure endpoints for all services and better testing instructions

ca656b5

chrislovecnm mentioned this pull request Nov 10, 2016

Support Kafka in PetSet kubernetes/kubernetes#23794

Closed

prydonius reviewed Nov 11, 2016

View reviewed changes

chrislovecnm suggested changes Nov 12, 2016

View reviewed changes

prydonius added awaiting review changes needed and removed awaiting review labels Nov 22, 2016

viglesiasce suggested changes Dec 9, 2016

View reviewed changes

viglesiasce merged commit 00e7aea into helm:master Feb 3, 2017

This was referenced Jun 27, 2017

Kafka 0.11.0.0 solsson/dockerfiles#4

Closed

Upgrade to Kafka 0.11 and test for open issues Yolean/kubernetes-kafka#30

Merged

solsson added a commit to solsson/dockerfiles that referenced this pull request Jul 26, 2017

Preserves the build used in the community helm chart

d0ddee5

helm/charts#144 Beware: This image is 841 MB, compared to 301 for the current :latest.

solsson added a commit to solsson/dockerfiles that referenced this pull request Jul 26, 2017

Preserves the build used in the community helm chart

26d5465

helm/charts#144 Beware: This image is 841 MB, compared to 301 for the current :latest. See README.md for upgrade instructions.

jordanjennings pushed a commit to jordanjennings/charts that referenced this pull request Nov 27, 2018

Merge pull request helm#144 from nkiraly/T2A-143/update-oos-detection…

af79620

…-papertrail-system [T2A-143] fix oos-detection papertrail system name

shubham14bajpai pushed a commit to shubham14bajpai/charts that referenced this pull request Sep 6, 2021

Merge pull request helm#144 from ranjithwingrider/gh-pages

a917341

[openebs]chore(release): add OpenEBS 2.0.0 YAML for AWS


		### Connecting to Kafka

		You can connect to Kafka by running a simple pod in the K8s cluster like this with a configuration like this:

		@@ -0,0 +1,36 @@
		You can connect to Kafka by running a simple pod in the K8s cluster like this with a configuration like this:

Conversation

faraazkhan commented Oct 22, 2016

Uh oh!

k8s-ci-robot commented Oct 22, 2016

Uh oh!

prydonius commented Oct 22, 2016

Uh oh!

prydonius commented Oct 22, 2016

Uh oh!

faraazkhan commented Oct 22, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

faraazkhan commented Oct 22, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

prydonius commented Oct 23, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

faraazkhan commented Oct 24, 2016

Uh oh!

faraazkhan commented Oct 24, 2016

Uh oh!

chrislovecnm left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chrislovecnm commented Oct 24, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

faraazkhan commented Oct 30, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chrislovecnm commented Oct 30, 2016

Uh oh!

chrislovecnm left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

viglesiasce commented Nov 2, 2016

Uh oh!

technosophos commented Nov 2, 2016

Uh oh!

viglesiasce commented Nov 2, 2016

Uh oh!

chrislovecnm commented Nov 3, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

faraazkhan commented Nov 10, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

viglesiasce commented Nov 11, 2016

Uh oh!

faraazkhan commented Nov 11, 2016

Uh oh!

faraazkhan commented Oct 22, 2016 •

edited

Loading

faraazkhan commented Oct 22, 2016 •

edited

Loading

faraazkhan commented Oct 30, 2016 •

edited

Loading

chrislovecnm commented Nov 3, 2016 •

edited

Loading

faraazkhan commented Nov 10, 2016 •

edited

Loading