-
Notifications
You must be signed in to change notification settings - Fork 69
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
Service for VMware content library item deployment. #629
Conversation
:method_name => "library_item_deploy", | ||
:args => {}, | ||
:role => "ems_operations", | ||
#:queue_name => manager.queue_name_for_ems_operations, |
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.
We still need this so that the deployment will be run on the operations worker
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.
Yeah. I would uncomment it.
Added this as it would not get executed off of the queue when testing locally.
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.
If you're using simulate_queue_worker
it should still run it
} | ||
|
||
task_id = MiqTask.generic_action_with_callback(task_options, queue_options) | ||
task = MiqTask.wait_for_taskid(task_id) |
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 don't know much about how the Service provision side works but should we be waiting for the task to finish in a separate state?
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.
The wait here is to make sure that the library_item_deploy
is done when automate moves from execute
to next step. Otherwise, when automate moves to next step check_completed
, library_item_deploy
might still be in the queue and not being executed yet.
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.
This will lock-up a worker while the content library is being deployed? Even once we move to async on the ems_operations side
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 think so.
I can remove the wait here then rely on check_completed
to do the work right.
@tinaafitz Any idea?
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 think that makes more sense
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.
app/models/manageiq/providers/vmware/infra_manager/ovf_service.rb
Outdated
Show resolved
Hide resolved
app/models/manageiq/providers/vmware/infra_manager/ovf_service.rb
Outdated
Show resolved
Hide resolved
end | ||
|
||
def refresh_target | ||
target_folder || target_host || target_resource_pool |
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.
So we drop targeted refreshes anyway now that we're doing streaming refresh, https://github.com/ManageIQ/manageiq-providers-vmware/blob/master/app/models/manageiq/providers/vmware/infra_manager/refresh_worker/runner.rb#L22-L29
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 noticed this refresh was being skipped in the testing. Any change should be done here?
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.
Yeah you can drop the queue_refresh
here since it is just going to be queued and dropped, now that we're in the vmware repo we can make the assumption that we are using streaming refresh.
def351f
to
6ab9227
Compare
Checked commit lfu@6ab9227 with ruby 2.6.3, rubocop 0.69.0, haml-lint 0.28.0, and yamllint spec/models/manageiq/providers/vmware/infra_manager/ovf_service_spec.rb
|
@miq-bot add_label changelog/yes |
To support VMware content library OVF template provisioning from Service/Catalog.
Depends on #617.
ManageIQ/manageiq#19718
@miq-bot add_label enhancement, jansa/no