-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Fixes kafka stateful set configuration for proper directory entry in … #2951
Fixes kafka stateful set configuration for proper directory entry in … #2951
Conversation
…overrides section in run server
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: aedeph If they are not already assigned, you can assign the PR to them by writing The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/assign @kow3ns |
Once you do this it will complain about lost+found if your on ext file systems |
@@ -74,7 +74,7 @@ spec: | |||
- "exec kafka-server-start.sh /opt/kafka/config/server.properties --override broker.id=${HOSTNAME##*-} \ | |||
--override listeners=PLAINTEXT://:9093 \ | |||
--override zookeeper.connect=zk-0.zk-svc.default.svc.cluster.local:2181,zk-1.zk-svc.default.svc.cluster.local:2181,zk-2.zk-svc.default.svc.cluster.local:2181 \ | |||
--override log.dir=/var/lib/kafka \ | |||
--override log.dirs=/var/lib/kafka \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should be changed to /var/lib/kafka/data or something different than the base mount in the example here: https://github.com/kubernetes/contrib/blob/master/statefulsets/kafka/kafka.yaml#L168
this is to avoid the lost+found complaints from kafka.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are probably right, but I guess, that it is more about permissions managing on external volume and pod itself, rather than getting the only proper directory path value in start configuration. Besides, I consider the whole yaml as a template, and complaint is quite straightforward to proper understanding of the issue and exact solution to it.
My PR is more about misleading key, which did mislead me to lose actual data.
Issues go stale after 90d of inactivity. If this issue is safe to close now please do so with Send feedback to sig-testing, kubernetes/test-infra and/or fejta. |
Stale issues rot after 30d of inactivity. If this issue is safe to close now please do so with Send feedback to sig-testing, kubernetes/test-infra and/or fejta. |
Rotten issues close after 30d of inactivity. Send feedback to sig-testing, kubernetes/test-infra and/or fejta. |
@fejta-bot: Closed this PR. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
…overrides section in run server
Motivation: according to trunc code (which describes in quite a vague fashion in official documentation) log.dir section of configuration is used as a topic log directory if and only if log.dirs is not present. However, default /opt/kafka_2.11-0.10.2.0/config/server.properties from current gcr.io/google-samples/k8skafka contains entry of log.dirs=/tmp/kafka-logs, so override does nothing useful and in fact is very misleading, producing probable data loss, when mounting external volume to /var/lib/kafka.