manageiq icon indicating copy to clipboard operation
manageiq copied to clipboard

Allow run_single_worker to start non-rails workers

Open agrare opened this issue 2 years ago • 4 comments

Currently we have a separate provider_worker script which is able to start non-rails workers by passing options and secrets to stdin then exec'ing the worker. Having this as a standalone script presents some issues namely it would require changes to the systemd service which is not easily configurable.

If we move the logic for starting a non-rails worker into the ::Runner class it allows us to

  1. Use the standard run_single_worker
  2. Removes the duplication present in run_single_worker and provider_worker (checking roles, worker class names, etc...)
  3. Allows us to use Settings to enable/disable using a rails worker
  4. Allows the provider to define the options that will be passed to their worker if they want

Downsides:

  1. This requires that provider authors with a non-rails worker define a WorkerClass::Runner to define the cmdline
  2. I think that's it, maybe when we have a significant number of non-rails workers we could add back the option to run the script directly

https://github.com/ManageIQ/manageiq/issues/21992

agrare avatar Sep 19 '22 16:09 agrare

Example provider worker class:

app/models/manageiq/providers/vmware/infra_manager/event_catcher.rb

 class ManageIQ::Providers::Vmware::InfraManager::EventCatcher < ManageIQ::Providers::BaseManager::EventCatcher
  self.rails_worker = worker_settings.fetch(:rails_worker, true)

   require_nested :Runner
 end

app/models/manageiq/providers/vmware/infra_manager/event_catcher/runner.rb

class ManageIQ::Providers::Vmware::InfraManager::EventCatcher::Runner < ManageIQ::Providers::BaseManager::EventCatcher::Runner
  private

  # We could add this to a mixin included in all provider worker runner classes
  def worker_options
    ems          = ExtManagementSystem.find(@cfg[:ems_id])
    all_managers = [ems] + ems.child_managers

    super.merge(
      :ems => all_managers.map do |manager|
        manager.attributes.merge(
          "endpoints"       => manager.endpoints,
          "authentications" => manager.authentications
        )
      end
    )
  end

  def worker_cmdline
    "bundle exec ruby #{ManageIQ::Providers::Vmware::Engine.root.join("workers/event_catcher/worker")}"
  end
end

With this I can run my non-rails event catcher exactly the same way as the normal rails one:

lib/workers/bin/run_single_worker.rb --ems-id=2 --role=event ManageIQ::Providers::Vmware::InfraManager::EventCatcher

agrare avatar Sep 19 '22 16:09 agrare

Checked commit https://github.com/agrare/manageiq/commit/c105ea7d64bd7f160767fdb3b3117b212729dab1 with ruby 2.6.10, rubocop 1.28.2, haml-lint 0.35.0, and yamllint 2 files checked, 0 offenses detected Everything looks fine. :trophy:

miq-bot avatar Sep 19 '22 16:09 miq-bot

If we move the logic for starting a non-rails worker into the ::Runner class it allows us to

What do you mean by the this? Do you have pseudo code of what that would look like before/after?

jrafanie avatar Sep 19 '22 21:09 jrafanie

If we move the logic for starting a non-rails worker into the ::Runner class it allows us to

What do you mean by the this? Do you have pseudo code of what that would look like before/after?

:point_up: https://github.com/ManageIQ/manageiq/pull/22114#issuecomment-1251229243

Ah you mean the difference from how it worked with provider_worker script, I force pushed to the branch so I can't find what the systemd unit file would have looked like but we would have had to change the ExecStart from the standard run_single_worker to provider_worker meaning there would be no option to switch.

agrare avatar Sep 19 '22 21:09 agrare