Unify VM Operations queue handling and set queue_name #19544
Conversation
agrare
force-pushed
the
set_queue_name_for_vm_operations
branch
2 times, most recently
from
8db3148
to
0beb804
Compare
agrare
force-pushed
the
set_queue_name_for_vm_operations
branch
from
0beb804
to
0195916
Compare
|
Some comments on commit agrare@0195916 spec/models/vm_spec.rb
|
|
Checked commit agrare@0195916 with ruby 2.5.5, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0 app/models/vm_or_template/operations/snapshot.rb
|
| @@ -336,6 +337,15 @@ def run_command_via_parent(verb, options = {}) | |||
| ext_management_system.send(verb, self, options) | |||
| end | |||
|
|
|||
| def run_command_via_task(task_options, queue_options) | |||
| MiqTask.generic_action_with_callback(task_options, command_queue_options(queue_options)) | |||
| end | |||
There was a problem hiding this comment.
Do we want to return only the task ID or the full task object here? I would generally prefer the latter, unless the UI needs the former for some reason.
https://github.com/ManageIQ/manageiq/blob/master/app/models/miq_task.rb#L303
There was a problem hiding this comment.
This maintains compatibility with what is currently being returned, https://github.com/ManageIQ/manageiq/pull/19544/files#diff-6002b94a939143e9dcfa4873f9fe003bL87
I'm okay changing it but we need to find and update all the callers as well.
|
I don't suppose there's any chance of documenting these methods is there? I know that you understand it all at a glance, but it would help me and potentially future maintainers. :) |
|
I'd love to document all of the "interface" methods (e.g. start, start_queue) but that is unrelated to the changes being made here. Would rather go through all the operations not just the ones that I happened to have to update here and have a PR just to add documentation. |
| } | ||
|
|
||
| MiqTask.generic_action_with_callback(task_opts, queue_opts) | ||
| run_command_via_queue(task_opts, :method_name => "rename", :args => [new_name]) |
There was a problem hiding this comment.
This looks like a typo, should have been run_command_via_task, right?
The params do match the _task signature, not _queue,
and so does the code (MiqQueue.put vs MiqTask.generic_action_with_callback).
And, so does UI travis ;)... https://travis-ci.org/ManageIQ/manageiq-ui-classic/jobs/615790473#L2060
(
- run_command_via_queue(task_opts, :method_name => "rename", :args => [new_name])
+ run_command_via_task(task_opts, :method_name => "rename", :args => [new_name])passes, creating a PR (#19553))
Move the queue handling for VM operations into common methods, set the
queue_name, and queue operations after check_policy_prevent.
Any operations run from a check_policy_prevent_callback were being run in the context of the automate role which was preventing these ems_operations from being run on an operations worker.
Issue: #19543