Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update v2 migration script - quote empty values #1315

Merged
merged 1 commit into from
Jan 11, 2021

Conversation

pmalek-sumo
Copy link
Contributor

@pmalek-sumo pmalek-sumo commented Jan 11, 2021

Description

When using yq to write empty values (the ones with only |- or without values at all) it writes no values at all causing some helm templates to fail to render:

Error: template: sumologic/templates/statefulset.yaml:19:28: executing "sumologic/templates/statefulset.yaml" at <include (print $.Template.BasePath "/configmap.yaml") .>: error calling include: template: sumologic/templates/configmap.yaml:15:5: executing "sumologic/templates/configmap.yaml" at <tpl (.Files.Glob "conf/*.conf").AsConfig .>: error calling tpl: error during tpl function execution for "buffer.output.conf: |-\n  compress {{ .Values.fluentd.buffer.compress | quote }}\n  flush_interval {{ .Values.fluentd.buffer.flushInterval | quote }}\n  flush_thread_count {{ .Values.fluentd.buffer.numThreads | quote }}\n  chunk_limit_size {{ .Values.fluentd.buffer.chunkLimitSize | quote }}\n  total_limit_size {{ .Values.fluentd.buffer.totalLimitSize | quote }}\n  queued_chunks_limit_size {{ .Values.fluentd.buffer.queueChunkLimitSize | quote }}\n  overflow_action drop_oldest_chunk\n  retry_max_interval {{ .Values.fluentd.buffer.retryMaxInterval | quote }}\n  retry_forever {{ .Values.fluentd.buffer.retryForever | quote }}\n  {{- .Values.fluentd.buffer.extraConf | nindent 2 }}\ncommon.conf: |-\n  # Prevent fluentd from handling records containing its own logs.\n  <match fluentd.**>\n    @type null\n  </match>\n  # expose the Fluentd metrics to Prometheus\n  <source>\n    @type prometheus\n    metrics_path /metrics\n    port 24231\n  </source>\n  <source>\n    @type prometheus_output_monitor\n  </source>\n  <source>\n    @type http\n    port 9880\n    bind 0.0.0.0\n  </source>\n  {{- if .Values.fluentd.logLevel }}\n  <system>\n    log_level {{ .Values.fluentd.logLevel }}\n  </system>\n  {{- end }}": template: sumologic/templates/statefulset.yaml:11:49: executing "sumologic/templates/statefulset.yaml" at <2>: invalid value; expected string

hence this PR adds explicit quoting of those empty values.


Testing performed
  • Redeploy fluentd and fluentd-events pods
  • Confirm events, logs, and metrics are coming in

@pmalek-sumo pmalek-sumo added this to the v2.0 milestone Jan 11, 2021
@pmalek-sumo pmalek-sumo self-assigned this Jan 11, 2021
@pmalek-sumo pmalek-sumo requested a review from a team January 11, 2021 16:50
@@ -10,29 +10,29 @@ kube-prometheus-stack:
commonLabels: {}
kubeApiServer:
serviceMonitor:
interval:
interval: ""
Copy link
Contributor

Choose a reason for hiding this comment

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

Is an empty string acceptable as a value for interval?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know but what I do know is that those values are checked and conditionally applied in kube-prometheus-stack chart

    {{- if .Values.kubeControllerManager.serviceMonitor.interval }}
    interval: {{ .Values.kubeControllerManager.serviceMonitor.interval }}
    {{- end }}

https://github.com/prometheus-community/helm-charts/blob/c3f8b974f7511a772405e1fbf28dfe8483a9a04e/charts/kube-prometheus-stack/templates/exporters/kube-controller-manager/servicemonitor.yaml#L21-L23

@pmalek-sumo pmalek-sumo merged commit 3bcfbad into main Jan 11, 2021
@pmalek-sumo pmalek-sumo deleted the v2-migration-script-quote-empty-values branch January 11, 2021 19:11
@pmalek-sumo pmalek-sumo linked an issue Jan 13, 2021 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Prepare 1.x to 2.x migration script
4 participants