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

prometheus: Fixes #197 permission issue for service monitoring #200

Merged

Conversation

kenplusplus
Copy link
Collaborator

  1. Move the service monitoring to the namespace of kepler align with deployment
  2. Add Role and RoleBinding to allow prometheus monitoring the namespace kepler

This has been tested on a local kubernetes cluster.

Signed-off-by: Lu Ken ken.lu@intel.com

…ervice monitoring

1. Move the service monitoring to the namespace of kepler align with deployment
2. Add Role and RoleBinding to allow prometheus monitoring the namespace kepler

This has been tested on a local kubernetes cluster.

Signed-off-by: Lu Ken <ken.lu@intel.com>
@SamYuan1990
Copy link
Collaborator

@kenplusplus , one question, did you have once try with https://github.com/sustainable-computing-io/kepler/blob/main/hack/cluster-deploy.sh#L25 ?

The thing I am rising up here:
as CI still WIP, so far it doesn't base on manifests/kubernetes/keplerExporter-serviceMonitor.yaml
but the kind cluster as a steps in CI process will create namespace monitoring
https://github.com/sustainable-computing-io/kepler/actions/runs/3073990395/jobs/4966476837#step:7:36
and deploy service monitor https://github.com/sustainable-computing-io/kepler/actions/runs/3073990395/jobs/4966476837#step:8:232

in my point of view, if we want user to use a quick solution to deploy promethus operator and kepler ref to [link] (https://prometheus-operator.dev/docs/prologue/quick-start/#deploy-kube-prometheus).
we'd better keep name space as monitoring ? and make manifests/kubernetes/keplerExporter-serviceMonitor.yaml been tested. (means in CI workflow, should not test https://github.com/sustainable-computing-io/kepler/blob/main/hack/cluster-deploy.sh#L25 but manifests/kubernetes/keplerExporter-serviceMonitor.yaml )

or we should make manifests/kubernetes/keplerExporter-serviceMonitor.yaml auto generated.
I don't want to see manifests/kubernetes/keplerExporter-serviceMonitor.yaml different with CI auto generation result and the file skipped in pipeline.

cc: @rootfs , @marceloamaral

@rootfs
Copy link
Contributor

rootfs commented Sep 17, 2022

/assign @SamYuan1990

@rootfs
Copy link
Contributor

rootfs commented Sep 17, 2022

If CI is able to query Kepler metrics, that'll be great. I previously tried the following to verify Kepler is working. If the new namespace works with the promtheus change, can we add this check in the CI?

export IP=$(kubectl get svc -n monitoring prometheus-k8s -ojsonpath='{.spec.clusterIP}')
curl "http://${IP}:9090/api/v1/query?query=node_energy_stat[200s]"

@SamYuan1990
Copy link
Collaborator

If CI is able to query Kepler metrics, that'll be great. I previously tried the following to verify Kepler is working. If the new namespace works with the promtheus change, can we add this check in the CI?

export IP=$(kubectl get svc -n monitoring prometheus-k8s -ojsonpath='{.spec.clusterIP}')
curl "http://${IP}:9090/api/v1/query?query=node_energy_stat[200s]"

ok... @rootfs , I hope a free discussion with items below before adjust CI for testing.

  1. which sample yaml (for deploy kepler on customer k8s cluster) will we provide to user ?
  2. the scope of this yaml file? (kepler pod, service ... ) will it including cluster role binding ?

@SamYuan1990
Copy link
Collaborator

If CI is able to query Kepler metrics, that'll be great. I previously tried the following to verify Kepler is working. If the new namespace works with the promtheus change, can we add this check in the CI?

export IP=$(kubectl get svc -n monitoring prometheus-k8s -ojsonpath='{.spec.clusterIP}')
curl "http://${IP}:9090/api/v1/query?query=node_energy_stat[200s]"

ok... @rootfs , I hope a free discussion with items below before adjust CI for testing.

  1. which sample yaml (for deploy kepler on customer k8s cluster) will we provide to user ?
  2. the scope of this yaml file? (kepler pod, service ... ) will it including cluster role binding ?

for me,
answer for
Q1, either manifests or we ask client to generate manifests will be fine.
Q2, ideally, I just want the file including kepler pod, service ... as k8s components. for namespace creation and role binding as rbac ... if possible, not including. they should some where setting when set up prometheus operator, as prometheus operator configuration.

@rootfs , @kenplusplus , @marceloamaral please let me know your comments.

@rootfs
Copy link
Contributor

rootfs commented Sep 18, 2022

Let's have this fix first, we'll go to the integration CI test case in a followup discussion. @kenplusplus please open up issues based on your use cases. Thanks.

@rootfs rootfs merged commit dcf1117 into sustainable-computing-io:main Sep 18, 2022
@kenplusplus
Copy link
Collaborator Author

Collaborator

@SamYuan1990

  1. I did not try the cluster_deploy.sh, I tried everything on our local kubernetes cluster.
  2. monitoring namespace will be created by prometheus.
  3. Keeping monitoring is an easy solution, but sound like the final direction is to use "kelper" At least, openshift alway do that.

I will try your CI and try to provide similiar fixing

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.

4 participants