lordgamez commented on a change in pull request #1154:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1154#discussion_r686110080



##########
File path: charts/stable/minifi-daemonset/templates/_helpers.tpl
##########
@@ -0,0 +1,56 @@
+{{/* vim: set filetype=mustache: */}}
+{{/*
+Expand the name of the chart.
+*/}}
+{{- define "minifi-logging-daemonset.name" -}}

Review comment:
       I think the name `minifi-logging-daemonset` is too specific for this 
generic template. It does not require a specific logging use case.

##########
File path: charts/stable/minifi-daemonset/templates/_helpers.tpl
##########
@@ -0,0 +1,56 @@
+{{/* vim: set filetype=mustache: */}}
+{{/*
+Expand the name of the chart.
+*/}}
+{{- define "minifi-logging-daemonset.name" -}}
+{{- default .Chart.Name .Values.nameOverride | trunc 63 | trimSuffix "-" -}}
+{{- end -}}
+
+{{/*
+Create a default fully qualified app name.
+We truncate at 63 chars because some Kubernetes name fields are limited to 
this (by the DNS naming spec).
+If release name contains chart name it will be used as a full name.
+*/}}
+{{- define "minifi-logging-daemonset.fullname" -}}
+{{- if .Values.fullnameOverride -}}
+{{- .Values.fullnameOverride | trunc 63 | trimSuffix "-" -}}
+{{- else -}}
+{{- $name := default .Chart.Name .Values.nameOverride -}}
+{{- if contains $name .Release.Name -}}
+{{- .Release.Name | trunc 63 | trimSuffix "-" -}}
+{{- else -}}
+{{- printf "%s-%s" .Release.Name $name | trunc 63 | trimSuffix "-" -}}
+{{- end -}}
+{{- end -}}
+{{- end -}}
+
+{{/*
+Create chart name and version as used by the chart label.
+*/}}
+{{- define "minifi-logging-daemonset.chart" -}}
+{{- printf "%s-%s" .Chart.Name .Chart.Version | replace "+" "_" | trunc 63 | 
trimSuffix "-" -}}
+{{- end -}}
+
+{{/*
+Common labels
+*/}}
+{{- define "minifi-logging-daemonset.labels" -}}

Review comment:
       Shouldn't `minifi-logging-daemonset.labels` and 
`minifi-logging-daemonset.serviceAccountName` templates be reused in the 
daemonset template?

##########
File path: charts/stable/minifi-daemonset/README.md
##########
@@ -0,0 +1,61 @@
+# Apache MiNiFi Daemonset

Review comment:
       I think storing the helm charts in the `/kubernetes/charts` or 
`/helm/charts` would be more appropriate than `/charts`

##########
File path: charts/stable/minifi-daemonset/templates/daemonset.yaml
##########
@@ -0,0 +1,82 @@
+apiVersion: apps/v1
+kind: DaemonSet
+metadata:
+  labels:
+    app.kubernetes.io/managed-by: JMO
+    k8s-app: {{ .Release.Name }}
+  name: {{ .Release.Name }}
+  namespace: {{ .Release.Namespace }}
+spec:
+  revisionHistoryLimit: 10
+  selector:
+    matchLabels:
+      name: {{ .Release.Name }}
+  template:
+    metadata:
+      labels:
+        name: {{ .Release.Name }}
+        restart: "0"
+    spec:
+      containers:
+        - image: {{ .Values.image.repository }}:{{ .Values.image.tag }}
+          imagePullPolicy: {{ .Values.image.pullPolicy }}
+          name: {{ .Release.Name }}
+          resources:
+            limits:
+              cpu: {{ .Values.resources.limits.cpu }}
+              memory: {{ .Values.resources.limits.memory }}
+            requests:
+              cpu: {{ .Values.resources.requests.cpu }}
+              memory: {{ .Values.resources.requests.memory }}
+          terminationMessagePath: /dev/termination-log
+          terminationMessagePolicy: File
+          volumeMounts:
+            - mountPath: /opt/minifi/minifi-current/conf/config.yml
+              name: minificonfig
+              subPath: config.yml
+            - mountPath: /opt/minifi/minifi-current/conf/minifi-log.properties
+              name: minificonfig
+              subPath: minifi-log.properties
+            - mountPath: /var/log
+              name: varlog
+            - mountPath: /var/lib/docker/containers
+              name: varlibdockercontainers
+              readOnly: true
+            - mountPath: /var/log/journal
+              name: varlogjournal
+              readOnly: true
+      dnsPolicy: ClusterFirst
+      {{- if .Values.imagePullSecrets }}
+      imagePullSecrets:
+        {{- range .Values.imagePullSecrets }}
+        - name: {{ .name }}
+        {{- end }}
+      {{- end }}
+      restartPolicy: Always
+      schedulerName: default-scheduler
+      serviceAccount: {{ .Values.serviceAccount | quote }}
+      serviceAccountName: {{ .Values.serviceAccountName | quote }}
+      terminationGracePeriodSeconds: 30
+      tolerations:
+        - operator: Exists
+      volumes:
+        - configMap:
+            defaultMode: 420
+            name: {{ .Release.Name }}-config
+          name: minificonfig
+        - hostPath:
+            path: /var/log
+            type: ""
+          name: varlog
+        - hostPath:
+            path: /var/lib/docker/containers
+            type: ""
+          name: varlibdockercontainers
+        - hostPath:
+            path: /var/log/journal
+            type: ""
+          name: varlogjournal

Review comment:
       Are these necessary to be mounted in a generic minifi daemonset 
deployment?

##########
File path: charts/stable/minifi-daemonset/templates/daemonset.yaml
##########
@@ -0,0 +1,82 @@
+apiVersion: apps/v1
+kind: DaemonSet
+metadata:
+  labels:
+    app.kubernetes.io/managed-by: JMO

Review comment:
       What does `JMO` refer to in this context?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to