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

Only create service with serviceMonitor #1317

Merged
merged 5 commits into from
Sep 13, 2024

Conversation

in0rdr
Copy link
Contributor

@in0rdr in0rdr commented Sep 12, 2024

Only create the service when serviceMonitor.create is set. The service is useless otherwise.

Background: I have a project where I can only use the prometheusRules.enabled=true but only want to use the prometheus rules, not the service monitor. Also, it creates a conflict in our deployment, because we already have a similar service with that name vault-monitoring.

@in0rdr in0rdr requested review from pree, eyenx and a team as code owners September 12, 2024 13:30
@github-actions github-actions bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Sep 12, 2024
@in0rdr
Copy link
Contributor Author

in0rdr commented Sep 12, 2024

I don't get what the check tries to tell me here:

Error: template: vault-monitoring/templates/service.yaml:1:14: executing "vault-monitoring/templates/service.yaml" at <.Values.serviceMonitor.create>: nil pointer evaluating interface {}.create

Copy link
Member

@eyenx eyenx left a comment

Choose a reason for hiding this comment

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

CI needs fixing

Only create the service when `serviceMonitor.create` is set. The service
is useless otherwise. I have a project where I can only use the
`prometheusRules.enabled=true` but only want to use the prometheus
rules, not the service monitor. Also, it creates a conflict in our
deployment because we already have a similar service with that name
`vault-monitoring`.
@in0rdr
Copy link
Contributor Author

in0rdr commented Sep 13, 2024

I quickly tested these CI commands offline. It seems be working when I set --target-branch to my feature branch. The ct lint command then returns chart version not ok. Needs a version bump!, but at least no more nil pointer. Not sure how that's supposed to work though and I don't think messing with the config file would be useful?

The `ct lint` command fails with a nil pointer. I assume it cannot find
some directory.
@in0rdr in0rdr force-pushed the feat/vault-monitoring/conditional-service-monitor branch from f5685d4 to d791995 Compare September 13, 2024 06:59
@in0rdr
Copy link
Contributor Author

in0rdr commented Sep 13, 2024

I think there's a bug in that "Pluto" magic stuff.

$ tail -n1 ./hack/pluto.sh
helm template $chart_dir | pluto detect -
$ helm template ./charts/vault-monitoring
Error: template: vault-monitoring/templates/service.yaml:1:14: executing "vault-monitoring/templates/service.yaml" at <.Values.serviceMonitor.create>: nil pointer evaluating interface {}.create

Use --debug flag to render out invalid YAML

@in0rdr
Copy link
Contributor Author

in0rdr commented Sep 13, 2024

@eyenx the .Values path was indeed wrong. CI is always right 😛

@in0rdr in0rdr merged commit b9eba17 into main Sep 13, 2024
3 checks passed
@in0rdr in0rdr deleted the feat/vault-monitoring/conditional-service-monitor branch September 13, 2024 07:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants