-
Notifications
You must be signed in to change notification settings - Fork 62
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
Support publish VM #95
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -36,6 +36,14 @@ def clone_completed?(args) | |
phase_context = args[:phase_context] | ||
logger = args[:logger] | ||
|
||
if source.template? | ||
vm_clone_completed?(logger, phase_context, source) | ||
else | ||
template_clone_completed?(logger, phase_context, source) | ||
end | ||
end | ||
|
||
def vm_clone_completed?(logger, phase_context, source) | ||
source.with_provider_connection(VERSION_HASH) do |connection| | ||
vm = vm_service_by_href(phase_context[:new_vm_ems_ref], connection).get | ||
status = vm.status | ||
|
@@ -44,6 +52,15 @@ def clone_completed?(args) | |
end | ||
end | ||
|
||
def template_clone_completed?(logger, phase_context, source) | ||
source.with_provider_connection(VERSION_HASH) do |connection| | ||
template = template_service_by_href(phase_context[:new_vm_ems_ref], connection).get | ||
status = template.status | ||
logger.info("The status of the template being cloned is #{status}") | ||
status == OvirtSDK4::TemplateStatus::OK | ||
end | ||
end | ||
|
||
def destination_image_locked?(vm) | ||
vm.with_provider_object(VERSION_HASH) do |vm_proxy| | ||
vm_proxy.get.status == OvirtSDK4::VmStatus::IMAGE_LOCKED | ||
|
@@ -170,7 +187,7 @@ def get_template_proxy(template, connection) | |
end | ||
|
||
def get_vm_proxy(vm, connection) | ||
VmProxyDecorator.new(connection.system_service.vms_service.vm_service(vm.uid_ems), self) | ||
VmProxyDecorator.new(connection.system_service.vms_service.vm_service(vm.uid_ems), connection, self) | ||
end | ||
|
||
def collect_disks_by_hrefs(disks) | ||
|
@@ -205,6 +222,13 @@ def start_clone(source, clone_options, phase_context) | |
end | ||
end | ||
|
||
def make_template(source, clone_options, phase_context) | ||
source.with_provider_object(VERSION_HASH) do |rhevm_vm| | ||
template = rhevm_vm.make_template(clone_options) | ||
populate_phase_context(phase_context, template) | ||
end | ||
end | ||
|
||
def vm_start(vm, cloud_init) | ||
opts = {} | ||
vm.with_provider_object(VERSION_HASH) do |rhevm_vm| | ||
|
@@ -298,11 +322,13 @@ def host_deactivate(host) | |
end | ||
|
||
class VmProxyDecorator < SimpleDelegator | ||
attr_reader :service | ||
def initialize(vm, service) | ||
@obj = vm | ||
@service = service | ||
super(vm) | ||
attr_reader :ovirt_services, :connection | ||
|
||
def initialize(vm_service, connection, ovirt_services) | ||
@obj = vm_service | ||
@connection = connection | ||
@ovirt_services = ovirt_services | ||
super(vm_service) | ||
end | ||
|
||
def update_memory_reserve!(memory_reserve_size) | ||
|
@@ -326,7 +352,7 @@ def update_memory!(memory, limit) | |
|
||
def update_host_affinity!(dest_host_ems_ref) | ||
vm = get | ||
vm.placement_policy.hosts = [OvirtSDK4::Host.new(:id => service.uuid_from_href(dest_host_ems_ref))] | ||
vm.placement_policy.hosts = [OvirtSDK4::Host.new(:id => ovirt_services.uuid_from_href(dest_host_ems_ref))] | ||
update(vm) | ||
end | ||
|
||
|
@@ -405,6 +431,33 @@ def update_cloud_init!(content) | |
) | ||
) | ||
end | ||
|
||
def make_template(options) | ||
templates_service = connection.system_service.templates_service | ||
cluster = ovirt_services.cluster_from_href(options[:cluster], connection) | ||
if options[:storage] | ||
storage = ovirt_services.storage_from_href(options[:storage], connection) | ||
end | ||
vm = get | ||
vm.disk_attachments = connection.follow_link(vm.disk_attachments) | ||
template = build_template_from_hash(:name => options[:name], | ||
:vm => vm, | ||
:description => options[:description], | ||
:cluster => cluster, | ||
:storage => storage) | ||
templates_service.add(template) | ||
end | ||
|
||
def build_template_from_hash(args) | ||
options = { | ||
:name => args[:name], | ||
:description => args[:description], | ||
:vm => args[:vm], | ||
:cluster => args[:cluster], | ||
:storage_domain => args[:storage] && {:id => args[:storage].id} | ||
}.compact | ||
OvirtSDK4::Template.new(options) | ||
end | ||
end | ||
|
||
class TemplateProxyDecorator < SimpleDelegator | ||
|
@@ -437,6 +490,10 @@ def cluster_from_href(href, connection) | |
connection.system_service.clusters_service.cluster_service(uuid_from_href(href)).get | ||
end | ||
|
||
def storage_from_href(href, connection) | ||
connection.system_service.storage_domains_service.storage_domain_service(uuid_from_href(href)).get | ||
end | ||
|
||
def uuid_from_href(ems_ref) | ||
URI(ems_ref).path.split('/').last | ||
end | ||
|
@@ -476,6 +533,11 @@ def vm_service_by_href(href, connection) | |
connection.system_service.vms_service.vm_service(vm_uuid) | ||
end | ||
|
||
def template_service_by_href(href, connection) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here |
||
template_uuid = uuid_from_href(href) | ||
connection.system_service.templates_service.template_service(template_uuid) | ||
end | ||
|
||
# | ||
# Updates the amount memory of a virtual machine. | ||
# | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -33,15 +33,19 @@ def poll_clone_complete | |
end | ||
|
||
def customize_destination | ||
if destination_image_locked? | ||
_log.info("Destination image locked; re-queuing") | ||
requeue_phase | ||
if source.template? | ||
if destination_image_locked? | ||
_log.info("Destination image locked; re-queuing") | ||
requeue_phase | ||
else | ||
message = "Starting New #{destination_type} Customization" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why we use capital letter for "new" and "customization"? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. developer excuses #211: "it was like that before" seems like other providers (vmware and microsoft) have the exact message (with the capitals) |
||
_log.info("#{message} #{for_destination}") | ||
update_and_notify_parent(:message => message) | ||
configure_container | ||
signal :configure_disks | ||
end | ||
else | ||
message = "Starting New #{destination_type} Customization" | ||
_log.info("#{message} #{for_destination}") | ||
update_and_notify_parent(:message => message) | ||
configure_container | ||
signal :configure_disks | ||
signal :post_provision | ||
end | ||
end | ||
|
||
|
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 already fetch storage domain in different place. I think that we start to have similar responsibilities in
inventory
andovirt_services
. In my opinion there is no need to have two distinct object having the same responsibility.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'd might access RHV from different places of the provider. We can think of introducing a class that will be responsible for interacting between the provider to RHV via v4, I'm not sure this is the PR to introduce such.
We should first get into agreement if we'd prefer that method over letting each workflow/scenario interact independently using the engine's connection.
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 am not saying that we should fix it in this PR. I am saying that we already have similar code in different place. I think it would be better to define responsibilities of the classes we use to talk to the engine.
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'll track refactoring of ovirt_services and querying of engine's request by issue #100
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