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

Install prometheus on local clusters (2/4) #529

Merged
merged 1 commit into from
Jun 23, 2021
Merged

Install prometheus on local clusters (2/4) #529

merged 1 commit into from
Jun 23, 2021

Conversation

kuritka
Copy link
Collaborator

@kuritka kuritka commented Jun 18, 2021 •
edited
Loading

related to #124

I want to check metrics that will be implemented. That's why I'm adding the option to install prometheus on both clusters with make deploy-prometheus.

Such deployed prometheus scrapes metrics of k8gb operator and is accessible directly from the browser:

Co-authored-by: Timofey Ilinykh [email protected]

Signed-off-by: kuritka [email protected]

@kuritka kuritka force-pushed the prometheus2-2 branch 2 times, most recently from 1171a8d to bcfbd59 Compare June 18, 2021 08:29
k0da
k0da requested changes Jun 18, 2021
View reviewed changes
@kuritka kuritka force-pushed the prometheus2-2 branch 2 times, most recently from f7eb3c4 to bdd64ab Compare June 18, 2021 13:51
k0da
k0da previously approved these changes Jun 18, 2021
View reviewed changes
Copy link
Collaborator

@k0da k0da left a comment

Choose a reason for hiding this comment

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

LGTM

ytsarev
ytsarev reviewed Jun 18, 2021
View reviewed changes
Copy link
Member

@ytsarev ytsarev left a comment

Choose a reason for hiding this comment

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

@kuritka is it intentionally without prometheus operator? just long-term thinking of ServiceMonitor CRDs

@kuritka
Copy link
Collaborator Author

kuritka commented Jun 18, 2021

@ytsarev - no operator was the intention, because we are on two local test-clusters. I didn't want to deliver something too extensive , just to check the metrics on the k8gb controller. But we can extend it at any time.

@k0da
Copy link
Collaborator

k0da commented Jun 18, 2021

I'm against pulling p8s operator here, we have single app in a single ns, we can simply maintain rule in a config map. We're not scalling testing clusters.

@ytsarev
Copy link
Member

ytsarev commented Jun 18, 2021

@kuritka so it is for querying metrics with prom UI, correct?

@k0da more context: https://docs.openshift.com/container-platform/4.1/applications/operator_sdk/osdk-monitoring-prometheus.html. Not a fond of pulling either but discussing what's the most relevant

somaritane
somaritane reviewed Jun 18, 2021
View reviewed changes
somaritane
somaritane reviewed Jun 18, 2021
View reviewed changes
@kuritka kuritka force-pushed the prometheus2-2 branch 4 times, most recently from 09fdad9 to ae93116 Compare June 21, 2021 16:18
@kuritka
Copy link
Collaborator Author

kuritka commented Jun 21, 2021

@kuritka so it is for querying metrics with prom UI, correct?

@k0da more context: https://docs.openshift.com/container-platform/4.1/applications/operator_sdk/osdk-monitoring-prometheus.html. Not a fond of pulling either but discussing what's the most relevant

@ytsarev , yes.

@k0da
Copy link
Collaborator

k0da commented Jun 21, 2021 •
edited
Loading

@ytsarev JFYI "github.com/operator-framework/operator-sdk/pkg/metrics" is no longer there. And https://github.com/kubernetes-sigs/controller-runtime/tree/master/pkg/metrics doesn't have anything related to ServiceMonitor

@ytsarev
Copy link
Member

ytsarev commented Jun 21, 2021

@k0da yeah, it might be all deprecated with the major 1.0 release of operator-sdk
@somaritane we have ServiceMonitor mentioned in https://github.com/k8gb-io/k8gb/blob/master/docs/metrics.md IIRC it was even a part of the controller startup, is that correct?

@somaritane
Copy link
Contributor

@somaritane we have ServiceMonitor mentioned in https://github.com/k8gb-io/k8gb/blob/master/docs/metrics.md IIRC it was even a part of the controller startup, is that correct?
@ytsarev Lots of things have changed with 1.0 Operator SDK migration, and the mentioned document definitely needs an update. AFAIK, ServiceMonitor can be created automatically during operator deployment together with auth proxy Service, since it is part of the default kustomize scaffolding of operator deployment generated by OSDK.
We've got rid of k8gb kustomize deployment config at some point because it was not used. But I don't think we've taken support for Prometheus Operator to account when created helm chart.
If we plan to support Prometheus Operator, we need to add the creation of the ServiceMonitor and respective Service object into the helm templates.

somaritane
somaritane suggested changes Jun 22, 2021
View reviewed changes
Copy link
Contributor

@somaritane somaritane left a comment

Choose a reason for hiding this comment

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

@kuritka thanks for the documentation and uninstall make target updates. Looking good, just a few improvement suggestions from my side.

@kuritka
Install prometheus on local clusters (2/4)
d4469e1 
related to #124

I want to check metrics that will be implemented. That's why I'm adding the option to install prometheus on both clusters with `make deploy-prometheus`.

Such deployed prometheus scrapes metrics of k8gb operator and is accessible directly from the browser:

 - http://127.0.0.1:8080 - test-gslb1
 - http://127.0.0.1:8081 - test-gslb2

Co-authored-by: Timofey Ilinykh <[email protected]>

Signed-off-by: kuritka <[email protected]>
@kuritka kuritka requested a review from ytsarev June 22, 2021 11:52
@kuritka kuritka requested review from somaritane and k0da June 22, 2021 11:52
somaritane
somaritane approved these changes Jun 22, 2021
View reviewed changes
Copy link
Contributor

@somaritane somaritane left a comment

Choose a reason for hiding this comment

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

@kuritka LGTM!

@kuritka kuritka merged commit 936fb40 into master Jun 23, 2021
kuritka pushed a commit that referenced this pull request Jun 30, 2021
@k0da @kuritka
Fix local playground setup
17d7c36 
Don't create service and don't point publish service to it. Istead use
node IPs as Ingress address

Signed-off-by: Dinar Valeev <[email protected]>

Install prometheus on local clusters (2/4) (#529)

related to #124

I want to check metrics that will be implemented. That's why I'm adding the option to install prometheus on both clusters with `make deploy-prometheus`.

Such deployed prometheus scrapes metrics of k8gb operator and is accessible directly from the browser:

 - http://127.0.0.1:8080 - test-gslb1
 - http://127.0.0.1:8081 - test-gslb2

Co-authored-by: Timofey Ilinykh <[email protected]>

Signed-off-by: kuritka <[email protected]>

backup, podCommand, add message to WithTestApp(string), testApp helm from 4.0.6 -> 5.0.1

minor changes

HitTestApp implemented
kuritka pushed a commit that referenced this pull request Jun 30, 2021
@k0da @kuritka
Fix local playground setup
91583b3 
Don't create service and don't point publish service to it. Istead use
node IPs as Ingress address

Signed-off-by: Dinar Valeev <[email protected]>

Install prometheus on local clusters (2/4) (#529)

related to #124

I want to check metrics that will be implemented. That's why I'm adding the option to install prometheus on both clusters with `make deploy-prometheus`.

Such deployed prometheus scrapes metrics of k8gb operator and is accessible directly from the browser:

 - http://127.0.0.1:8080 - test-gslb1
 - http://127.0.0.1:8081 - test-gslb2

Co-authored-by: Timofey Ilinykh <[email protected]>

Signed-off-by: kuritka <[email protected]>

backup, podCommand, add message to WithTestApp(string), testApp helm from 4.0.6 -> 5.0.1

minor changes

HitTestApp implemented

Signed-off-by: kuritka <[email protected]>
kuritka pushed a commit that referenced this pull request Jun 30, 2021
@k0da @kuritka
Fix local playground setup
34b2f4a 
Don't create service and don't point publish service to it. Istead use
node IPs as Ingress address

Signed-off-by: Dinar Valeev <[email protected]>

Install prometheus on local clusters (2/4) (#529)

related to #124

I want to check metrics that will be implemented. That's why I'm adding the option to install prometheus on both clusters with `make deploy-prometheus`.

Such deployed prometheus scrapes metrics of k8gb operator and is accessible directly from the browser:

 - http://127.0.0.1:8080 - test-gslb1
 - http://127.0.0.1:8081 - test-gslb2

Co-authored-by: Timofey Ilinykh <[email protected]>

Signed-off-by: kuritka <[email protected]>

backup, podCommand, add message to WithTestApp(string), testApp helm from 4.0.6 -> 5.0.1

minor changes

HitTestApp implemented

Signed-off-by: kuritka <[email protected]>
@kuritka kuritka deleted the prometheus2-2 branch July 19, 2021 21:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@somaritane somaritane somaritane approved these changes

@donovanmuller donovanmuller Awaiting requested review from donovanmuller donovanmuller is a code owner

@ytsarev ytsarev Awaiting requested review from ytsarev ytsarev is a code owner

@k0da k0da Awaiting requested review from k0da k0da is a code owner

Assignees
No one assigned
Labels
None yet
Projects
k8gb
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@kuritka @k0da @ytsarev @somaritane