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 values file to move controller.serviceMonitor to prometheus.serviceMonitor #4351

Merged
merged 27 commits into from Sep 20, 2023

Conversation

shaun-nx
Copy link
Contributor

@shaun-nx shaun-nx commented Sep 13, 2023

Proposed changes

This PR moves the ServiceMonitor fields from controller.serviceMonitor to prometheus.serviceMonitor in the helm values files.
This is being done to bring the creation of ServiceMonitor closer to where the Prometheus Service itself is created.

Helm command used to deploy:

~/deployments/helm-chart » helm install prometheus \
--set controller.kind=deployment \
--set controller.nginxplus=true \
--set controller.image.repository=nginx-plus-ingress \
--set controller.image.tag="moveServiceMonitor" \
--set prometheus.service.create=true \
--set prometheus.serviceMonitor.create=true \
--set prometheus.serviceMonitor.labels.product="nic" \
--set controller.replicaCount=5 .

Checklist

Before creating a PR, run through this checklist and mark each as complete.

  • I have read the CONTRIBUTING doc
  • I have added tests that prove my fix is effective or that my feature works
  • I have checked that all unit tests pass after adding my changes
  • I have updated necessary documentation
  • I have rebased my branch onto main
  • I will ensure my PR is targeting the main branch and pulling from my branch from my own fork

@shaun-nx shaun-nx requested review from a team as code owners September 13, 2023 20:12
@github-actions github-actions bot added documentation Pull requests/issues for documentation helm_chart Pull requests that update the Helm Chart labels Sep 13, 2023
@shaun-nx shaun-nx changed the title Move service monitor Move ServiceMonitor from controller.serviceMonitor to prometheus.serviceMonitor Sep 13, 2023
@shaun-nx shaun-nx changed the title Move ServiceMonitor from controller.serviceMonitor to prometheus.serviceMonitor Update helm values file to move controller.serviceMonitor to prometheus.serviceMonitor Sep 13, 2023
@codecov
Copy link

codecov bot commented Sep 13, 2023

Codecov Report

Merging #4351 (aad352c) into main (372aeb9) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #4351   +/-   ##
=======================================
  Coverage   52.19%   52.19%           
=======================================
  Files          59       59           
  Lines       16929    16929           
=======================================
  Hits         8836     8836           
  Misses       7796     7796           
  Partials      297      297           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Contributor

@ADubhlaoich ADubhlaoich left a comment

Choose a reason for hiding this comment

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

LGTM!

@brianehlert brianehlert added this to the v3.3.0 milestone Sep 14, 2023
deployments/helm-chart/values.yaml Outdated Show resolved Hide resolved
deployments/helm-chart/values.yaml Outdated Show resolved Hide resolved
@shaun-nx shaun-nx merged commit 3b91d97 into main Sep 20, 2023
61 checks passed
@shaun-nx shaun-nx deleted the moveServiceMonitor branch September 20, 2023 11:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Pull requests/issues for documentation helm_chart Pull requests that update the Helm Chart
Projects
Status: Done 🚀
Development

Successfully merging this pull request may close these issues.

Update helm values file to move controller.serviceMonitor to prometheus.serviceMonitor
7 participants