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

Add an external controller metrics service #751

Open
wants to merge 3 commits into
base: v2
Choose a base branch
from

Conversation

vladsf
Copy link

@vladsf vladsf commented Dec 20, 2024

If a LINSTOR controller is external (piraeus chart installed with .Values.externalController.url set) then the external controller requires an Endpoint and an extra Service to make metrics available via ServiceMonitor.

This PR addresses the issue.

@vladsf vladsf force-pushed the add-ext-controller-metrics branch from fdf02f9 to bec5dce Compare December 20, 2024 18:40
Copy link
Member

@WanzenBug WanzenBug left a comment

Choose a reason for hiding this comment

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

Thanks! I left a few comments

charts/piraeus/templates/external-metrics-service.yaml Outdated Show resolved Hide resolved
charts/piraeus/templates/external-metrics-service.yaml Outdated Show resolved Hide resolved
charts/piraeus/templates/external-metrics-service.yaml Outdated Show resolved Hide resolved
Copy link
Member

@WanzenBug WanzenBug left a comment

Choose a reason for hiding this comment

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

I'm a bit slow today: actually, this whole thing should be moved to https://github.com/piraeusdatastore/helm-charts/tree/main/charts/linstor-cluster

There, we already have the externalController logic and even deploy a monitoring.

Sorry for the confusion. This helm chart also needs to be somehow moved there eventually.

@vladsf
Copy link
Author

vladsf commented Dec 23, 2024

I'm a bit slow today: actually, this whole thing should be moved to https://github.com/piraeusdatastore/helm-charts/tree/main/charts/linstor-cluster

There, we already have the externalController logic and even deploy a monitoring.

Sorry for the confusion. This helm chart also needs to be somehow moved there eventually.

I have not found externalController logic there. I guess I should abandon this PR?

@WanzenBug
Copy link
Member

I have not found externalController logic there. I guess I should abandon this PR?

Yes. Move it over to the other repo. There, users can already set linstorCluster.externalController.url. The chart here is just for deploying the operator, but stuff like setting up the LinstorCluster resource and setting up monitoring is controlled over there.

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.

2 participants