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 helm/template and values to make the metrics port configurable #121

Closed
odcheck opened this issue Oct 8, 2024 · 4 comments
Closed

Comments

@odcheck
Copy link
Contributor

odcheck commented Oct 8, 2024

Default node_exporter port is 9100 which will hammer it, if using also port 9100 for k8s-ephemeral-storage-metrics

like e.g. in chart/templates/DeployType.yaml

      containers:
        - name: metrics
          image: {{ .Values.image.repository }}:{{ .Values.image.tag }}
          imagePullPolicy: {{ .Values.image.imagePullPolicy }}
          resources:
            {{- toYaml .Values.resources | nindent 12 }}
          ports:
            - name: metrics
              containerPort: {{ .Values.metricsPort | default 9100 }}

          livenessProbe:
            failureThreshold: 10
            httpGet:
              path: /metrics
              port: {{ .Values.metricsPort | default 9100 }}
              
          readinessProbe:
            failureThreshold: 10
            httpGet:
              path: /metrics
              port: {{ .Values.metricsPort | default 9100 }}
              scheme: HTTP              

and e.g. in metrics.yaml

kind: Service
metadata:
  name: k8s-ephemeral-storage-metrics
  namespace: "{{ $.Release.Namespace }}"
  labels:
    {{- include "chart.labels" . | nindent 6 }}
spec:
  type: ClusterIP
  selector:
  {{- include "chart.selectorLabels" . | nindent 6 }}
  ports:
    - name: metrics
      port: {{ .Values.metricsPort | default 9100 }}

then it could be used in values like this e.g.

image:
  repository: ghcr.io/jmcgrath207/k8s-ephemeral-storage-metrics
  tag: 1.13.0
  imagePullPolicy: IfNotPresent
  imagePullSecrets: []

# -- Set port number for metrics port (default 9100)
metricsPort:
@jmcgrath207
Copy link
Owner

Hi @odcheck ,

Could you expand on this?

Default node_exporter port is 9100 which will hammer it

Not against adding customizing for the metrics port here, but I am confused on the performance impact leaving it the way it is.

The this metrics service should have a different IP address than the node exporters (since they are different service objects).

If you're not seeing that behavior can you show me some debug output?

Thanks!

@odcheck
Copy link
Contributor Author

odcheck commented Oct 8, 2024

😅 you are absolutely right and I was completely on the wrong track. So there is no loss of performance whatsoever ..
However, it would still be practical to make the port generally configurable by using the values file.

Thank you for the quick feedback!

@jmcgrath207
Copy link
Owner

Sweet! Np, Yeah if you want to make that configurable in a PR, I am all for it.

Let's keep it under here and remove the default, since the values are the default already if nothing is supplied from your values.yaml.

https://github.com/jmcgrath207/k8s-ephemeral-storage-metrics/blob/master/chart/values.yaml#L28

{{ .Values.metrics.port }}

Also change this one.
https://github.com/jmcgrath207/k8s-ephemeral-storage-metrics/blob/master/chart/templates/metrics.yaml#L61

@odcheck
Copy link
Contributor Author

odcheck commented Oct 8, 2024

@odcheck odcheck closed this as completed Oct 9, 2024
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

No branches or pull requests

2 participants