-
Notifications
You must be signed in to change notification settings - Fork 896
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 runners needed for Prometheus alert collection #15864
Conversation
@miq-bot add_label wip |
Waiting on worker PRs to be merged. I'll mark this wip until those are merged. Please un-wip when those are merged. Then it will show back up on my list. |
lib/workers/miq_worker_types.rb
Outdated
@@ -29,13 +29,15 @@ | |||
"ManageIQ::Providers::Kubernetes::ContainerManager::EventCatcher" => %i(manageiq_default), | |||
"ManageIQ::Providers::Kubernetes::ContainerManager::MetricsCollectorWorker" => %i(manageiq_default), | |||
"ManageIQ::Providers::Kubernetes::ContainerManager::RefreshWorker" => %i(manageiq_default), | |||
"ManageIQ::Providers::Kubernetes::MonitoringManager::EventCatcher" => %i(kubernetes openshift), |
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.
@jrafanie what's the impact of using kubernetes openshift
here for the mapping? I see everything else uses manageiq_default
.
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.
@blomquisg Currently, we have tight coupling to the manageiq-api and vmware provider gems, see here, so you'll get NameError/LoadError when core code tries to load Api and vmware. For now, we need to use manageiq_default here to be safe.
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.
Thanks, updated
@blomquisg @gtanzillo could you please review? The dependent code was merged |
@miq-bot remove_label wip |
44db1d8
to
11ebdfe
Compare
Checked commits moolitayer/manageiq@0a54bf0~...11ebdfe with ruby 2.3.3, rubocop 0.47.1, and haml-lint 0.20.0 |
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.
LGTM 👍 @blomquisg are you good with this?
ping @blomquisg |
Add new Prometheus workers so they can be started.
Depends on:
ManageIQ/manageiq-providers-kubernetes#40
ManageIQ/manageiq-providers-openshift#22