-
Notifications
You must be signed in to change notification settings - Fork 7
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 prometheus to devstack plugin #123
Conversation
cddf4be
to
b2ee6d9
Compare
A note, the CI should succeed now. The fix, which caused it to fail previously got merged yesterday. If it doesn't succeed, we might have some other issues |
Build failed (check pipeline). Post https://review.rdoproject.org/zuul/buildset/e168d46b3afb49bd89b0c8148d12b6af ❌ stf-crc-latest-nightly_bundles RETRY_LIMIT in 8m 49s |
recheck |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks pretty clean to me and is quite self-contained. Didn't get a chance to test it, but it all looks sane.
In order to interact with podman, we need the newuidmap binary. I've seen this being present on centos vms by default, but on some ubuntu / debian vms this isn't the case and the plugin fails to pull images.
I was forced by the telemetry CI to add instalation of uidmap package for ubuntu and debian vms. I'll add this PR either as Depends-On or the jwysogla-prometheus_devstack branch directly into the local.conf into my proposed integration job for the telemetry-tempest-plugin here. This way this PR will get tested directly with the telemetry-tempest-plugin, which is the intended use for this. |
I broke ceilometer's devstack plugin when CEILOMETER_BACKEND=none. Great. I'm looking into it |
The Depends-On from openstack Zuul to this PR didn't seem to work. The old sg-core devstack plugin code is still being used there. I don't think we can make openstack Zuul to use this PR, so we will need to merge this before it can be used by openstack Zuul. I have a fix for the CI failure: https://review.opendev.org/c/openstack/ceilometer/+/900509 I'll try to gather reviews for it ASAP. |
Both parts of the devstack plugin will install the same "container executable" (most likely podman). I moved this to one function for both in order to de-duplicate the code
I decided for one more adjustment. I moved the "container executable" installation into one function shared by both parts of the devstack plugin to de-duplicate the code. This already fixes an issue from the previous commit, where I forgot to add the installation of the uidmap to the now non-existent preinstall_prometheus function. |
An example of a local.conf to deploy with the new changes:
|
I deployed devstack with the above local.conf and I was able to curl for metrics: https://paste.opendev.org/show/bh1JoQF7AQh5zKUsPOBY/ Note: You actually don't need to specify the "PROMETHEUS_SCRAPE_TARGETS" in local.conf to get metrics from sg-core. "sg-core" is the default target, so the only configuration needed to get metrics from ceilometer to prometheus is to specify "sg-core" as one of the ceilometer's backends for example by using the "CEILOMETER_BACKENDS" variable (default is gnocchi for backwards compatibility). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm hitting some issues with Devstack that are not related to this change, so I couldn't test run this one. Code looks good though Jaromir, thanks!
And thanks for the extra context to understand how to test this and the decision making process :)
As discussed with the osp squad on slack. I'm moving the prometheus devstack plugin into the sg-core devstack plugin. This is similar to the ceilometer devstack plugin, which also takes care about configuring gnocchi.
Recap on why we decided to make prometheus deployment part of the sg-core plugin
We need a way to deploy prometheus upstream to be able to use it in CI. As a first attempt I tried to create a new prometheus plugin and make it a part of the observabilityclient repository, because observabilityclient is basically just an openstack client for prometheus - this made sense. This didn't work, because devstack expects the plugin to be called the same as the repository (I was able to work around that for devstack, but this workaround couldn't be used for CI). It doesn't make sense to have a "python-observabilityclient" devstack plugin, which in fact just deploys prometheus. So we decided to add prometheus deployment to this plugin instead.
What the new proposed plugin can do
It can deploy sg-core in a container just like before. It can deploy prometheus in a similar way. You can set either the SG_CORE_ENABLE=false or PROMETHEUS_ENABLE=false to disable their deployment. By default both containers (sg-core and prometheus) are deployed. By default prometheus is configured to scrape sg-core. sg-core is by default configured to listen on tcp and expose metrics on prometheus endpoint. You can configure additional targets for scraping by prometheus. A configuration for the observabilityclient to communicate with this deployed prometheus is also created by default. This means, that it's enough to specify this plugin in the local.conf and to set the CEILOMETER_BACKEND=sg-core and you should get metrics going all the way from ceilometer to prometheus (the pieces for the CEILOMETER_BACKEND=sg-core are still on review and so they aren't merged yet).
I did several devstack deployments with different scrape-configs and *_ENABLE=false and I'm confident, that the plugin should work as intended, but feel free to try it out before approving if needed.
Depends-On: infrawatch/service-telemetry-operator#524