project-zot / helm-charts

7 stars 19 forks source link

add service monitor #42

Closed batazor closed 3 months ago

batazor commented 4 months ago

What type of PR is this?

Which issue does this PR fix: close #41

What does this PR do / Why do we need it:

If an issue # is not available please add repro steps and logs showing the issue:

Testing done on this change:

Automation added to e2e:

Will this break upgrades or downgrades?

Does this PR introduce any user-facing change?:

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

rchincha commented 4 months ago

@batazor can you pls take a look at the CI failures?

batazor commented 4 months ago

@batazor can you pls take a look at the CI failures?

done

rchincha commented 4 months ago

@batazor upgrade test failed as follows.

Get the application URL by running these commands: export NODE_PORT=$(kubectl get --namespace zot-e817pun8up -o jsonpath="{.spec.ports[0].nodePort}" services zot-e817pun8up) export NODE_IP=$(kubectl get nodes --namespace zot-e817pun8up -o jsonpath="{.items[0].status.addresses[0].address}") echo http://$NODE_IP:$NODE_PORT Error: UPGRADE FAILED: template: zot/templates/servicemonitor.yaml:1:18: executing "zot/templates/servicemonitor.yaml" at <.Values.metrics.enabled>: nil pointer evaluating interface {}.enabled

batazor commented 4 months ago

@rchincha Yes, I added support case handling when values.yaml was not set. Thanks!

rchincha commented 3 months ago

@batazor Still fails.

 Get the application URL by running these commands:
  export NODE_PORT=$(kubectl get --namespace zot-ue4ohhrx84 -o jsonpath="{.spec.ports[0].nodePort}" services zot-ue4ohhrx84)
  export NODE_IP=$(kubectl get nodes --namespace zot-ue4ohhrx84 -o jsonpath="{.items[0].status.addresses[0].address}")
  echo http://$NODE_IP:$NODE_PORT
Error: UPGRADE FAILED: template: zot/templates/servicemonitor.yaml:1:34: executing "zot/templates/servicemonitor.yaml" at <.Values.metrics.enabled>: nil pointer evaluating interface {}.enabled
===================================
batazor commented 3 months ago

@rchincha Okay, one more try, added even more complete variable checking

rchincha commented 3 months ago

@batazor

https://github.com/project-zot/helm-charts/pull/43 ^ had to pick whatever was approved and ready. Your PR may require one more version bump.

batazor commented 3 months ago

@rchincha no problem

rchincha commented 3 months ago

@batazor why close this PR?

batazor commented 3 months ago

I don't understand the problems that appear during testing, I haven't experienced such problems locally

rchincha commented 3 months ago

@batazor

@Andreea-Lupu noted that:

{{- if and .Values.metrics .Values.metrics.enabled .Values.metrics.serviceMonitor .Values.metrics.serviceMonitor.enabled }}

replaced by (as multi-line) appears to fix this

{{- if .Values.metrics }}
{{- if .Values.metrics.enabled }}
{{- if .Values.metrics.serviceMonitor }}
{{- if .Values.metrics.serviceMonitor.enabled }}