Refactor VM naming method in provisioning task to support call from automate #16897
Conversation
app/models/miq_provision.rb
Outdated
| options[:vm_target_name] = new_name | ||
| options[:vm_target_hostname] = get_hostname(new_name) | ||
|
|
||
| update(:description => self.class.get_description(self, new_name), :options => options) |
There was a problem hiding this comment.
Any reason to use update rather than update_attributes?
There was a problem hiding this comment.
Nope, I think either works here so I'll change it if you prefer.
|
@bdunne Wanted to mention the smaller refactoring in the test as well to not use |
spec/models/miq_provision_spec.rb
Outdated
| end | ||
|
|
||
| it "Updates the request description with only name parameter passed" do | ||
| expect(@pr).to receive(:update_description_from_tasks) |
There was a problem hiding this comment.
Should there be more to this than just checking that the method was called?
app/models/miq_provision.rb
Outdated
| end | ||
|
|
||
| def update_vm_name(new_name, update_request: true) | ||
| new_name = get_vm_full_name(new_name, true) |
There was a problem hiding this comment.
Should we call the class method directly? (even though it seems strange that it is a class method)
There was a problem hiding this comment.
Sure, I can remove the instance method. It is a class method because the logic used from both MiqRequest and MiqRequestTask.
gmcculloug
force-pushed
the
provision_vm_name
branch
2 times, most recently
from
109131d
to
3eacbb8
Compare
gmcculloug
force-pushed
the
provision_vm_name
branch
from
3eacbb8
to
0818e15
Compare
|
@bdunne Changes pushed. |
|
Checked commits gmcculloug/manageiq@1b85d4a~...0818e15 with ruby 2.3.3, rubocop 0.52.0, haml-lint 0.20.0, and yamllint 1.10.0 |
Refactor VM naming method in provisioning task to support call from automate (cherry picked from commit ee6cb2d) https://bugzilla.redhat.com/show_bug.cgi?id=1539750
|
Gaprindashvili backport details: |
Refactor VM naming method in provisioning task to support call from automate (cherry picked from commit ee6cb2d) https://bugzilla.redhat.com/show_bug.cgi?id=1539752
|
Fine backport details: |
Refactor VM naming method in provisioning task to support call from automate (cherry picked from commit ee6cb2d) https://bugzilla.redhat.com/show_bug.cgi?id=1539752
VM naming during provisioning runs when the provision task is initially created. This is generally not an issue with Lifecycle provisioning. For Service provisioning this can be an issue because the dialog field data gets pushed down to the tasks after they are created (seems obvious) and therefore after the naming method has run.
Therefore it is difficult for the dialog field data to be used as part of the VM naming.
Furthermore, the naming logic at task creation supports an auto-incrementing feature, with synchronization to ensure uniqueness, which is not available from other calls.
This PR does not change the location of when the naming code runs, it make the vm_naming method available to an external caller so they can generate the name at a later time in the provisioning process and take advantage of the auto-incrementing name feature.
Note: The first commit is refactoring. The main changes are in the second commit.
Links [Optional]
Steps for Testing/QA [Optional]
In automate override one of the VM provisioning state-machine methods, like the
Provisionmethod and to call the newly exposedupdate_vm_namemethod and pass any name. It can contain the enumeration sequence ($n{#}) used to auto-increment the name. For example, "clone_test_$n{4}" should generate a VM with a name like "clone_test_0001".