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

[tempo-vulture] feat: add option to deploy podmonitor and add option to remove svc #3221

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

KyriosGN0
Copy link
Contributor

added option to opt out of using a service (still enabled by default) and add podMonitor which is disabled by default

@KyriosGN0
Copy link
Contributor Author

Hey @Whyeasy, @dgzlopes, can i get a review here? sorry for nagging

@KyriosGN0 KyriosGN0 force-pushed the feat-remove-svc-add-podmonitor branch from 4571d9c to a0b0c1e Compare July 18, 2024 23:12
@KyriosGN0
Copy link
Contributor Author

hey @Whyeasy @dgzlopesm, another ping...

@KyriosGN0
Copy link
Contributor Author

hey @Whyeasy @dgzlopes, can i please get a review here? thanks!

@KyriosGN0
Copy link
Contributor Author

@Whyeasy @dgzlopes, hey sorry to nag, but its been over a month....

@KyriosGN0
Copy link
Contributor Author

hey @Whyeasy @dgzlopes, another 2 weeks, another ping :(

@KyriosGN0
Copy link
Contributor Author

hey, @Whyeasy @dgzlopes, im really sorry that and i might sound disrecptpful, but i opened the PR almost 2 and half months ago, and i haven't gotten a one comment from as codeowner.
i would really like to merge this, since its a really simple change

@KyriosGN0
Copy link
Contributor Author

ping @Whyeasy @dgzlopes

@KyriosGN0 KyriosGN0 force-pushed the feat-remove-svc-add-podmonitor branch from 893ddfe to 49749fd Compare November 11, 2024 17:36
Signed-off-by: AvivGuiser <[email protected]>
@KyriosGN0
Copy link
Contributor Author

@zanhsieh i rebased according to new chart, is it possible to get a review ? (also from codeowners?)

@zanhsieh
Copy link
Collaborator

@zalegrala @Sheikh-Abubaker
Can you review this PR plz?

@zalegrala
Copy link
Contributor

Hi thanks for the PR. Can you add a little context about why this change is necessary?

@KyriosGN0
Copy link
Contributor Author

hey @zalegrala in general in my org we prefer the option to disable services and use pod-monitors instead of service monitor

@zalegrala
Copy link
Contributor

Hi, I'm a bit slow. Is there a reason we need the service for vulture at all?

@KyriosGN0
Copy link
Contributor Author

@zalegrala as far as i understand no, its entirely a "client" application, only reason for service is for service monitor.

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

Successfully merging this pull request may close these issues.

3 participants