-
Notifications
You must be signed in to change notification settings - Fork 16.8k
[incubator/fluentd] Generalize chart to work with different backends #5357
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
apiVersion: apps/v1beta1 | ||
kind: Deployment | ||
apiVersion: extensions/v1beta1 | ||
kind: DaemonSet | ||
metadata: | ||
name: {{ template "fluentd.fullname" . }} | ||
labels: | ||
|
@@ -18,6 +18,8 @@ spec: | |
labels: | ||
app: {{ template "fluentd.name" . }} | ||
release: {{ .Release.Name }} | ||
annotations: | ||
checksum/config: {{ include (print $.Template.BasePath "/configmap.yaml") . | sha256sum }} | ||
spec: | ||
{{- if .Values.image.pullSecrets }} | ||
imagePullSecrets: | ||
|
@@ -29,15 +31,26 @@ spec: | |
- name: {{ .Chart.Name }} | ||
image: "{{ .Values.image.repository }}:{{ .Values.image.tag }}" | ||
imagePullPolicy: {{ .Values.image.pullPolicy }} | ||
securityContext: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be dependent on whether the logs are mounted. |
||
runAsNonRoot: false | ||
runAsUser: 0 | ||
env: | ||
- name: OUTPUT_HOST | ||
value: {{ .Values.output.host | quote }} | ||
- name: OUTPUT_PORT | ||
value: {{ .Values.output.port | quote }} | ||
- name: OUTPUT_BUFFER_CHUNK_LIMIT | ||
value: {{ .Values.output.buffer_chunk_limit | quote }} | ||
- name: OUTPUT_BUFFER_QUEUE_LIMIT | ||
value: {{ .Values.output.buffer_queue_limit | quote }} | ||
{{- range $name, $value := .Values.env.open }} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not just have one less level here and put the env vars in env. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @rendhalver can you provide example what structure do you prefer? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. env: instead of env.open: |
||
{{- if not (empty $value) }} | ||
- name: {{ $name | quote }} | ||
value: {{ $value | quote }} | ||
{{- end }} | ||
{{- end }} | ||
{{- $secret_name := include "fluentd.fullname" . }} | ||
{{- range $name, $value := .Values.env.secret }} | ||
{{- if not ( empty $value) }} | ||
- name: {{ $name | quote }} | ||
valueFrom: | ||
secretKeyRef: | ||
name: {{ $secret_name }} | ||
key: {{ $name | quote }} | ||
{{- end }} | ||
{{- end }} | ||
resources: | ||
{{ toYaml .Values.resources | indent 12 }} | ||
ports: | ||
|
@@ -49,23 +62,29 @@ spec: | |
- name: http-input | ||
containerPort: 9880 | ||
protocol: TCP | ||
livenessProbe: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why remove this? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because liveness probe required config to provide the endpoint for the check. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't see how that achieves the same ends. |
||
httpGet: | ||
# Use percent encoding for query param. | ||
# The value is {"log": "health check"}. | ||
# the endpoint itself results in a new fluentd | ||
# tag 'fluentd.pod-healthcheck' | ||
path: /fluentd.pod.healthcheck?json=%7B%22log%22%3A+%22health+check%22%7D | ||
port: 9880 | ||
initialDelaySeconds: 5 | ||
timeoutSeconds: 1 | ||
volumeMounts: | ||
- name: varlog | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See above. You are basically turning this into the fluentd-elasticsearch chart. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @rendhalver would it be problem if we will mount logs inside container that do not require them? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why? |
||
mountPath: /var/log | ||
readOnly: true | ||
- name: varlogpos | ||
mountPath: /mnt/pos | ||
- name: varlibdockercontainers | ||
mountPath: /var/lib/docker/ | ||
readOnly: true | ||
- name: config-volume-{{ template "fluentd.fullname" . }} | ||
mountPath: /etc/fluent/config.d | ||
mountPath: {{ .Values.configDir }} | ||
volumes: | ||
- name: config-volume-{{ template "fluentd.fullname" . }} | ||
configMap: | ||
name: {{ template "fluentd.fullname" . }} | ||
- name: varlog | ||
hostPath: | ||
path: /var/log | ||
- name: varlogpos | ||
emptyDir: {} | ||
- name: varlibdockercontainers | ||
hostPath: | ||
path: /var/lib/docker/ | ||
- name: config-volume-{{ template "fluentd.fullname" . }} | ||
configMap: | ||
name: {{ template "fluentd.fullname" . }} | ||
{{- with .Values.nodeSelector }} | ||
nodeSelector: | ||
{{ toYaml . | indent 8 }} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
apiVersion: v1 | ||
kind: Secret | ||
metadata: | ||
name: {{ include "fluentd.fullname" . }} | ||
labels: | ||
app: {{ template "fluentd.name" . }} | ||
chart: {{ template "fluentd.chart" . }} | ||
release: {{ .Release.Name }} | ||
heritage: {{ .Release.Service }} | ||
type: Opaque | ||
data: | ||
{{- range $name, $value := .Values.env.secret }} | ||
{{- if not (empty $value) }} | ||
{{ $name }}: {{ $value | b64enc }} | ||
{{- end }} | ||
{{- end }} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,11 +9,15 @@ image: | |
# - secret1 | ||
# - secret2 | ||
|
||
output: | ||
host: elasticsearch-client.default.svc.cluster.local | ||
port: 9200 | ||
buffer_chunk_limit: 2M | ||
buffer_queue_limit: 8 | ||
env: | ||
open: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @osterman There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree with @osterman. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @osterman @rendhalver can you provide the example of the structure that would you prefer? |
||
OUTPUT_HOST: elasticsearch-client.default.svc.cluster.local | ||
OUTPUT_PORT: 9200 | ||
OUTPUT_BUFFER_CHUNK_LIMIT: 2M | ||
OUTPUT_BUFFER_QUEUE_LIMIT: 8 | ||
FLUENT_UID: 0 | ||
FLUENTD_OPT: "" | ||
secret: | ||
|
||
service: | ||
type: ClusterIP | ||
|
@@ -40,6 +44,7 @@ ingress: | |
# hosts: | ||
# - http-input.local | ||
|
||
configDir: /etc/fluent/config.d | ||
configMaps: | ||
general.conf: | | ||
# Prevent fluentd from handling records containing its own logs. Otherwise | ||
|
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.
Why are you switching this to a DaemonSet?
There is an fluentd-elasticsearch chart that does exactly this.
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.
that's true.
I'd like to make chart general enough for the different use of fluentd.
there are already
fluentd-elasticsearch
,fluentd-clouwatch
.I use fluentd chart with datadog.
It seems to be a mistake to create a chart for each type of backend.
What do you think about?
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.
I like the idea but if we are going to replace all three charts with one it needs to be flexible and configurable enough to do what each of those chart do already without reducing the functionality the current Charts.
I wrote this one to replace logstash in my Elastic-Stack cluster so it didn't need to be a DaemonSet or need access to node logs.
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.
Two things...
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.
If the DaemonSet can be still used as part of a Service then that works for me.
We use this chart to route logs from devices outside our cluster to an ES cluster in Kube so I would like to keep that functionality.
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.
(nitpick:
deamonset.yaml
misspelled)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.
A
Service
works by selectingPods
according to aselector
so it's compatible withDaemonSets
which create pods. AService
doesn't care (or know) how thePods
were created.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.
OK awesome. Thank you.