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.

Sign up for GitHub

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

Jump to bottom

feat(helm): add podmonitor #154

Merged
merged 1 commit into from
Mar 28, 2024
Merged

feat(helm): add podmonitor #154

merged 1 commit into from
Mar 28, 2024

Conversation

aslafy-z
Copy link
Contributor

@aslafy-z aslafy-z commented Mar 27, 2024 •
edited
Loading

Add support for podMonitor in helm chart.

Testing done:

  • podmonitor generated
    image

  • prometheus discovered it
    image

@aslafy-z aslafy-z marked this pull request as ready for review March 27, 2024 14:24
@aslafy-z aslafy-z requested a review from a team as a code owner March 27, 2024 14:24
@rbtr rbtr added type/enhancement New feature or request area/infra Test, Release, or CI Infrastructure labels Mar 27, 2024
vakalapa
vakalapa requested changes Mar 27, 2024
View reviewed changes
Copy link
Contributor

@vakalapa vakalapa left a comment

Choose a reason for hiding this comment

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

thank you for your contribution, can you add a "testing done" section in description along with some screenshots of the pod monitor if possible. That way we will know if this integration is successful.

deploy/manifests/controller/helm/retina/templates/daemonset.yaml Outdated Show resolved Hide resolved
deploy/manifests/controller/helm/retina/templates/podmonitor.yaml Outdated Show resolved Hide resolved
@aslafy-z aslafy-z requested a review from vakalapa March 27, 2024 16:33
rbtr
rbtr reviewed Mar 27, 2024
View reviewed changes
Copy link
Collaborator

@rbtr rbtr left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM, thanks for adding this! Can you include evidence of test in the PR description like @vakalapa mentioned? Something like a screenshot of the PodMonitor being discovered in Prometheus would suffice 🙂

@aslafy-z
Copy link
Contributor Author

I updated the description @rbtr @vakalapa

@rbtr rbtr enabled auto-merge (squash) March 27, 2024 17:42
rbtr
rbtr previously approved these changes Mar 27, 2024
View reviewed changes
Copy link
Collaborator

@rbtr rbtr left a comment

Choose a reason for hiding this comment

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

thanks @aslafy-z! LGTM

auto-merge was automatically disabled March 27, 2024 21:33

Head branch was pushed to by a user without write access

@aslafy-z aslafy-z requested a review from rbtr March 27, 2024 21:33
@aslafy-z
Copy link
Contributor Author

I just fixed conflicts @vakalapa @rbtr

@rbtr rbtr enabled auto-merge (squash) March 27, 2024 21:35
@aslafy-z
feat(helm): add podmonitor
471cc7b 
Signed-off-by: Zadkiel AHARONIAN <[email protected]>
auto-merge was automatically disabled March 28, 2024 08:43

Head branch was pushed to by a user without write access

@rbtr rbtr enabled auto-merge (squash) March 28, 2024 14:49
@rbtr rbtr added the priority/1 P1 label Mar 28, 2024
vakalapa
vakalapa approved these changes Mar 28, 2024
View reviewed changes
Copy link
Contributor

@vakalapa vakalapa left a comment

Choose a reason for hiding this comment

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

thanks for the contribution!

@rbtr rbtr merged commit 70ea129 into microsoft:main Mar 28, 2024
15 checks passed
@aslafy-z aslafy-z deleted the patch-1 branch March 28, 2024 16:16
hainenber pushed a commit to hainenber/retina that referenced this pull request Mar 28, 2024
@aslafy-z @hainenber
feat(helm): add podmonitor (microsoft#154)
ee18e70 
Add support for podMonitor in helm chart.

Testing done:
- podmonitor generated

![image](https://github.com/microsoft/retina/assets/8191198/357892cb-79eb-4355-80a8-06fb57c20d8a)

- prometheus discovered it

![image](https://github.com/microsoft/retina/assets/8191198/94f99bab-fd1f-45f2-8c2c-13aa2ba25a57)

Signed-off-by: Zadkiel AHARONIAN <[email protected]>
hainenber pushed a commit to hainenber/retina that referenced this pull request Apr 4, 2024
@aslafy-z @hainenber
feat(helm): add podmonitor (microsoft#154)
e935b29 
Add support for podMonitor in helm chart.

Testing done:
- podmonitor generated

![image](https://github.com/microsoft/retina/assets/8191198/357892cb-79eb-4355-80a8-06fb57c20d8a)

- prometheus discovered it

![image](https://github.com/microsoft/retina/assets/8191198/94f99bab-fd1f-45f2-8c2c-13aa2ba25a57)

Signed-off-by: Zadkiel AHARONIAN <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rbtr rbtr rbtr approved these changes

@vakalapa vakalapa vakalapa approved these changes

Assignees
No one assigned
Labels
area/infra Test, Release, or CI Infrastructure priority/1 P1 type/enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@aslafy-z @rbtr @vakalapa