Skip to content

Commit db982d4

Browse files
kilokangkilokang
authored
[All] Fix low data mode inheritance (#632)
* fix inheritance in nri-prometheus * fix inheritance in newrelic-logging * remove nits * fix templating a false string which breaks helm's falsiness * fix nri-prometheus configmap not templating in lowDataMode if config is empty. * fix typos * remove failing tests as they are a NTH out of scope * fix trim after indenting * mergeOverwrite gives precedence from right to left * add lowDataMode to infrastructure v3 chart * bump (again) infrastructure chart version * remove unneeded agent common config templating * document README * nit picking
1 parent 65dda97 commit db982d4

21 files changed

+120
-40
lines changed

‎charts/newrelic-infrastructure-v3/Chart.yaml

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
apiVersion: v2
22
name: newrelic-infrastructure-v3
33
description: A Helm chart to deploy the New Relic Kubernetes monitoring solution
4-
version: 3.0.3
4+
version: 3.0.4
55
appVersion: 3.0.0
66
kubeVersion: ">=1.16.0-0"
77
home: https://docs.newrelic.com/docs/kubernetes-pixie/kubernetes-integration/get-started/introduction-kubernetes-integration/

‎charts/newrelic-infrastructure-v3/README.md

+2-1
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,10 @@ Kubernetes: `>=1.16.0-0`
2121

2222
| Key | Type | Default | Description |
2323
|-----|------|---------|-------------|
24+
| lowDataMode | bool | `nil` | Enables low data mode sending less data by incrementing the interval from "15s" (the default when `lowDataMode` is `false` or `nil`) to "30s". Can be superseded by `.common.config.interval` |
2425
| common | object | See `values.yaml` | Config that applies to all instances of the solution: kubelet, ksm, control plane and sidecars. |
2526
| common.agentConfig | object | `{}` | Config for the Infrastructure agent. Will be used by the forwarder sidecars and the agent running integrations. See: https://docs.newrelic.com/docs/infrastructure/install-infrastructure-agent/configuration/infrastructure-agent-configuration-settings/ |
26-
| common.config.interval | string | `"15s"` | How often the integration should collect and report data. Intervals larger than 40s are not supported and will cause the NR UI to not behave properly. |
27+
| common.config.interval | string | `nil` | How often the integration should collect and report data. Intervals larger than 40s are not supported and will cause the NR UI to not behave properly. This value takes precedence over `lowDataMode` but if this value is undefined its default from 15s (without low data mode) to 30s (with low data mode enabled) |
2728
| common.config.timeout | string | `"30s"` | Timeout for the different APIs contacted by the integration: Kubernetes, KSM, Kubelet, etc. |
2829
| controlPlane | object | See `values.yaml` | Configuration for the DaemonSet that collects metrics from the control plane. |
2930
| controlPlane.affinity | object | Deployed only in master nodes. | Affinity for the control plane DaemonSet. |

‎charts/newrelic-infrastructure-v3/ci/test-cplane-kind-deployment-values.yaml

-1
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ common:
88
# not valid, the Identity Api doesn't return an AgentID and the server from the Agent takes to long to respond
99
is_forward_only: true
1010
config:
11-
interval: 30s
1211
timeout: 180s
1312

1413
customAttributes: '{"new":"relic","loren":"ipsum"}'

‎charts/newrelic-infrastructure-v3/ci/test-values.yaml

-1
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ common:
88
# not valid, the Identity Api doesn't return an AgentID and the server from the Agent takes to long to respond
99
is_forward_only: true
1010
config:
11-
interval: 30s
1211
timeout: 180s
1312

1413
customAttributes: '{"new":"relic","loren":"ipsum"}'

‎charts/newrelic-infrastructure-v3/templates/_helpers.tpl

+33
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,31 @@ Returns fargate
153153
{{- end -}}
154154
{{- end -}}
155155

156+
{{/*
157+
Returns lowDataMode
158+
*/}}
159+
{{- define "newrelic.lowDataMode" -}}
160+
{{/* `get` will return "" (empty string) if value is not found, and the value otherwise, so we can type-assert with kindIs */}}
161+
{{- if (get .Values "lowDataMode" | kindIs "bool") -}}
162+
{{- if .Values.lowDataMode -}}
163+
{{/*
164+
We want only to return when this is true, returning `false` here will template "false" (string) when doing
165+
an `(include "newrelic-logging.lowDataMode" .)`, which is not an "empty string" so it is `true` if it is used
166+
as an evaluation somewhere else.
167+
*/}}
168+
{{- .Values.lowDataMode -}}
169+
{{- end -}}
170+
{{- else -}}
171+
{{/* This allows us to use `$global` as an empty dict directly in case `Values.global` does not exists */}}
172+
{{- $global := index .Values "global" | default dict -}}
173+
{{- if get $global "lowDataMode" | kindIs "bool" -}}
174+
{{- if $global.lowDataMode -}}
175+
{{- $global.lowDataMode -}}
176+
{{- end -}}
177+
{{- end -}}
178+
{{- end -}}
179+
{{- end -}}
180+
156181
{{/*
157182
Returns the list of namespaces where secrets need to be accessed by the controlPlane integration to do mTLS Auth
158183
*/}}
@@ -188,3 +213,11 @@ Returns Custom Attributes even if formatted as a json string
188213
{{- define "newrelic.customAttributes" -}}
189214
{{- merge (include "newrelic.customAttributesWithoutClusterName" . | fromJson) (dict "clusterName" (include "newrelic.cluster" .)) | toJson }}
190215
{{- end -}}
216+
217+
{{- define "newrelic.integrationConfigDefaults" -}}
218+
{{- if include "newrelic.lowDataMode" . -}}
219+
interval: 30s
220+
{{- else -}}
221+
interval: 15s
222+
{{- end -}}
223+
{{- end -}}

‎charts/newrelic-infrastructure-v3/templates/control-plane/configmap.yaml

+5-4
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,18 @@
11
{{- if .Values.controlPlane.enabled -}}
2+
---
23
apiVersion: v1
34
kind: ConfigMap
45
metadata:
5-
namespace: {{ .Release.Namespace }}
66
labels:
77
{{- include "newrelic.labels" . | nindent 4 }}
88
name: {{ template "newrelic.fullname" . }}-control-plane
9+
namespace: {{ .Release.Namespace }}
910
data:
1011
nri-kubernetes.yml: |
11-
{{- .Values.common.config | toYaml | nindent 4 }}
12+
{{- (merge .Values.common.config (include "newrelic.integrationConfigDefaults" . | fromYaml)) | toYaml | nindent 4 }}
1213
controlPlane:
1314
enabled: true
14-
{{- with .Values.controlPlane.config }}
15-
{{- . | toYaml | nindent 6 }}
15+
{{- if .Values.controlPlane.config }}
16+
{{- toYaml .Values.controlPlane.config | nindent 6 }}
1617
{{- end }}
1718
{{- end }}

‎charts/newrelic-infrastructure-v3/templates/ksm/configmap.yaml

+5-4
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,18 @@
11
{{- if .Values.ksm.enabled -}}
2+
---
23
apiVersion: v1
34
kind: ConfigMap
45
metadata:
5-
namespace: {{ .Release.Namespace }}
66
labels:
77
{{- include "newrelic.labels" . | nindent 4 }}
88
name: {{ template "newrelic.fullname" . }}-ksm
9+
namespace: {{ .Release.Namespace }}
910
data:
1011
nri-kubernetes.yml: |
11-
{{- .Values.common.config | toYaml | nindent 4 }}
12+
{{- (merge .Values.common.config (include "newrelic.integrationConfigDefaults" . | fromYaml)) | toYaml | nindent 4 }}
1213
ksm:
1314
enabled: true
14-
{{- with .Values.ksm.config }}
15-
{{- . | toYaml | nindent 6 }}
15+
{{- if .Values.ksm.config }}
16+
{{- toYaml .Values.ksm.config | nindent 6 }}
1617
{{- end }}
1718
{{- end }}

‎charts/newrelic-infrastructure-v3/templates/kubelet/configmap.yaml

+5-4
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,18 @@
11
{{- if .Values.kubelet.enabled -}}
2+
---
23
apiVersion: v1
34
kind: ConfigMap
45
metadata:
5-
namespace: {{ .Release.Namespace }}
66
labels:
77
{{- include "newrelic.labels" . | nindent 4 }}
88
name: {{ template "newrelic.fullname" . }}-kubelet
9+
namespace: {{ .Release.Namespace }}
910
data:
1011
nri-kubernetes.yml: |
11-
{{- .Values.common.config | toYaml | nindent 4 }}
12+
{{- (merge .Values.common.config (include "newrelic.integrationConfigDefaults" . | fromYaml)) | toYaml | nindent 4 }}
1213
kubelet:
1314
enabled: true
14-
{{- with .Values.kubelet.config }}
15-
{{- . | toYaml | nindent 6 }}
15+
{{- if .Values.kubelet.config }}
16+
{{- toYaml .Values.kubelet.config | nindent 6 }}
1617
{{- end }}
1718
{{- end }}

‎charts/newrelic-infrastructure-v3/values.yaml

+7-2
Original file line numberDiff line numberDiff line change
@@ -35,16 +35,21 @@ images:
3535
common:
3636
# Configuration entries that apply to all instances of the integration: kubelet, ksm and control plane.
3737
config:
38-
# -- How often the integration should collect and report data.
3938
# Intervals larger than 40s are not supported and will cause the NR UI to not behave properly.
40-
interval: 15s
39+
# @default -- `15s` if `lowDataMode == false`, `30s` otherwise.
40+
# interval: 15s
4141
# -- Timeout for the different APIs contacted by the integration: Kubernetes, KSM, Kubelet, etc.
4242
timeout: 30s
4343
# -- Config for the Infrastructure agent.
4444
# Will be used by the forwarder sidecars and the agent running integrations.
4545
# See: https://docs.newrelic.com/docs/infrastructure/install-infrastructure-agent/configuration/infrastructure-agent-configuration-settings/
4646
agentConfig: {}
4747

48+
# lowDataMode -- (bool) Enables low data mode sending less data by incrementing the interval from "15s" (the default when `lowDataMode` is `false` or `nil`) to "30s".
49+
# Will be ignored if `.common.config.interval` is set.
50+
# @default -- false
51+
# lowDataMode: false
52+
4853
# -- Configuration for the DaemonSet that collects metrics from the Kubelet
4954
# @default -- See `values.yaml`
5055
kubelet:

‎charts/newrelic-logging/Chart.yaml

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
apiVersion: v1
22
description: A Helm chart to deploy New Relic Kubernetes Logging as a DaemonSet, supporting both Linux and Windows nodes and containers
33
name: newrelic-logging
4-
version: 1.10.6
4+
version: 1.10.7
55
appVersion: 1.10.0
66
home: https://github.com/newrelic/kubernetes-logging
77
icon: https://newrelic.com/assets/newrelic/source/NewRelic-logo-square.svg

‎charts/newrelic-logging/ci/test-override-global-lowdatamode.yaml

+3
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
global:
2+
lowDataMode: true
3+
lowDataMode: false

‎charts/newrelic-logging/ci/test-with-empty-global.yaml

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
global: {}

‎charts/newrelic-logging/ci/test-with-empty-values.yaml

Whitespace-only changes.

‎charts/newrelic-logging/templates/_helpers.tpl

+17-5
Original file line numberDiff line numberDiff line change
@@ -153,12 +153,24 @@ Returns fargate
153153
Returns lowDataMode
154154
*/}}
155155
{{- define "newrelic-logging.lowDataMode" -}}
156-
{{- if .Values.global }}
157-
{{- if .Values.global.lowDataMode }}
158-
{{- .Values.global.lowDataMode -}}
156+
{{/* `get` will return "" (empty string) if value is not found, and the value otherwise, so we can type-assert with kindIs */}}
157+
{{- if (get .Values "lowDataMode" | kindIs "bool") -}}
158+
{{- if .Values.lowDataMode -}}
159+
{{/*
160+
We want only to return when this is true, returning `false` here will template "false" (string) when doing
161+
an `(include "newrelic-logging.lowDataMode" .)`, which is not an "empty string" so it is `true` if it is used
162+
as an evaluation somewhere else.
163+
*/}}
164+
{{- .Values.lowDataMode -}}
165+
{{- end -}}
166+
{{- else -}}
167+
{{/* This allows us to use `$global` as an empty dict directly in case `Values.global` does not exists */}}
168+
{{- $global := index .Values "global" | default dict -}}
169+
{{- if get $global "lowDataMode" | kindIs "bool" -}}
170+
{{- if $global.lowDataMode -}}
171+
{{- $global.lowDataMode -}}
159172
{{- end -}}
160-
{{- else if .Values.lowDataMode }}
161-
{{- .Values.lowDataMode -}}
173+
{{- end -}}
162174
{{- end -}}
163175
{{- end -}}
164176

‎charts/newrelic-logging/values.yaml

+1-1
Original file line numberDiff line numberDiff line change
@@ -228,4 +228,4 @@ daemonSet:
228228
# annotations are not included. The plugin.type, plugin.version and plugin.source attributes are minified
229229
# into the plugin.source attribute.
230230
# Can be set as a global: global.lowDataMode
231-
lowDataMode: false
231+
# lowDataMode: false

‎charts/nri-prometheus/Chart.yaml

+1-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ apiVersion: v1
22
appVersion: 2.11.0
33
description: A Helm chart to deploy the New Relic Prometheus OpenMetrics integration
44
name: nri-prometheus
5-
version: 1.12.0
5+
version: 1.12.1
66
engine: gotpl
77
home: https://github.com/newrelic/nri-prometheus
88
icon: https://newrelic.com/assets/newrelic/source/NewRelic-logo-square.svg

‎charts/nri-prometheus/ci/test-lowdatamode-values.yaml

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
global:
2+
licenseKey: 1234567890abcdef1234567890abcdef12345678
3+
cluster: test-cluster
4+
5+
lowDataMode: true

‎charts/nri-prometheus/ci/test-override-global-lowdatamode.yaml

+6
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
global:
2+
licenseKey: 1234567890abcdef1234567890abcdef12345678
3+
cluster: test-cluster
4+
5+
lowDataMode: true
6+
lowDataMode: false

‎charts/nri-prometheus/templates/_helpers.tpl

+17-5
Original file line numberDiff line numberDiff line change
@@ -119,12 +119,24 @@ Return the customSecretLicenseKey
119119
Returns lowDataMode
120120
*/}}
121121
{{- define "nri-prometheus.lowDataMode" -}}
122-
{{- if .Values.global }}
123-
{{- if .Values.global.lowDataMode }}
124-
{{- .Values.global.lowDataMode -}}
122+
{{/* `get` will return "" (empty string) if value is not found, and the value otherwise, so we can type-assert with kindIs */}}
123+
{{- if (get .Values "lowDataMode" | kindIs "bool") -}}
124+
{{- if .Values.lowDataMode -}}
125+
{{/*
126+
We want only to return when this is true, returning `false` here will template "false" (string) when doing
127+
an `(include "nri-prometheus.lowDataMode" .)`, which is not an "empty string" so it is `true` if it is used
128+
as an evaluation somewhere else.
129+
*/}}
130+
{{- .Values.lowDataMode -}}
131+
{{- end -}}
132+
{{- else -}}
133+
{{/* This allows us to use `$global` as an empty dict directly in case `Values.global` does not exists */}}
134+
{{- $global := index .Values "global" | default dict -}}
135+
{{- if get $global "lowDataMode" | kindIs "bool" -}}
136+
{{- if $global.lowDataMode -}}
137+
{{- $global.lowDataMode -}}
125138
{{- end -}}
126-
{{- else if .Values.lowDataMode }}
127-
{{- .Values.lowDataMode -}}
139+
{{- end -}}
128140
{{- end -}}
129141
{{- end -}}
130142

‎charts/nri-prometheus/templates/configmap.yaml

+9-8
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,12 @@ apiVersion: v1
88
data:
99
config.yaml: |
1010
cluster_name: {{ include "nri-prometheus.cluster" . }}
11-
{{- if .Values.config }}
12-
{{- if (include "nri-prometheus.lowDataMode" .) }}
13-
{{ $lowDataDefault := .Files.Get "static/lowdatamodedefaults.yaml" | fromYaml -}}
14-
{{- mergeOverwrite .Values.config $lowDataDefault | toYaml | indent 4 -}}
15-
{{- else }}
16-
{{ toYaml .Values.config | indent 4 | trim }}
17-
{{- end }}
18-
{{- end }}
11+
12+
{{- $config := .Values.config -}}
13+
{{- $lowDataDefault := .Files.Get "static/lowdatamodedefaults.yaml" | fromYaml -}}
14+
15+
{{- if (include "nri-prometheus.lowDataMode" .) }}
16+
{{- mergeOverwrite $lowDataDefault .Values.config | toYaml | nindent 4 }}
17+
{{- else }}
18+
{{- toYaml .Values.config | trim | nindent 4 }}
19+
{{- end }}

‎charts/nri-prometheus/values.yaml

+1-1
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ prometheusScrape: "true"
2323
# If set, the `config.transformations` option present in this file will be ignored,
2424
# and instead a default one excluding KSM and cAdvisor metrics will be used.
2525
# Default: false
26-
lowDataMode: false
26+
# lowDataMode: false
2727

2828
nameOverride: ""
2929

0 commit comments

Comments
 (0)