fixes #8295 - add vsphere memory and cpu hot add #2896
Conversation
| @@ -18,6 +18,12 @@ | |||
| <% checked = params[:host] && params[:host][:compute_attributes] && params[:host][:compute_attributes][:start] || '1' %> | |||
| <%= checkbox_f f, :start, { :checked => (checked == '1'), :help_inline => _("Power ON this machine"), :label => _('Start'), :label_size => "col-md-3" } if new_host and controller_name != 'compute_attributes' %> | |||
|
|
|||
| <% checked = params[:host] && params[:host][:compute_attributes] && params[:host][:compute_attributes][:memoryHotAddEnabled] || '0' %> | |||
| <%= checkbox_f f, :memoryHotAddEnabled, { :checked => (checked == '1'), :help_inline => _("Enable memory hot add"), :label => _('Memory hot add'), :label_size => "col-md-2" } if new_host and controller_name != 'compute_attributes' %> | |||
There was a problem hiding this comment.
I don't know if you want the and controller_name != 'compute_attributes' here, as that's to prevent the checkbox appearing on the compute profile pages. It's excluded from there for the "Power ON" checkbox as it's kind of only applicable to real hosts, not compute profiles, but I think these would be useful set on compute profiles too.
I think that means you can also get rid of the "checked" variable and simply leave :checked out, a bit like external_ip on in gce/_base.
There was a problem hiding this comment.
And lastly, I think you might want to move them above the Power ON line, up with the other hardware settings.
timogoebel
force-pushed
the
feature_vsphere_hot_add
branch
from
4abfb28
to
558ca17
Compare
theforeman-bot
added
Needs re-review
Needs testing
and removed
Waiting on contributor
labels
|
@domcleal: Thanks for reviewing! I added the suggested changes. :-) |
timogoebel
force-pushed
the
feature_vsphere_hot_add
branch
from
558ca17
to
128510f
Compare
| @@ -13,6 +13,8 @@ | |||
| <%= select_f f, :guest_id, compute_resource.guest_types, :first, :last, {}, { :label => _("Guest OS"), :class => "col-md-2", :disabled => !new_host } %> | |||
| <%= select_f f, :scsi_controller_type, compute_resource.scsi_controller_types, :first, :last, {}, { :label => _("SCSI controller"), :class => "col-md-2", :disabled => !new_host } %> | |||
| <%= select_f f, :hardware_version, compute_resource.vm_hw_versions, :first, :last, {}, { :label => _("Virtual H/W version"), :class => "col-md-2", :disabled => !new_host } %> | |||
| <%= checkbox_f f, :memoryHotAddEnabled, { :help_inline => _("Enable memory hot add"), :label => _('Memory hot add'), :label_size => "col-md-2", :disabled => !new_host } if new_host %> | |||
There was a problem hiding this comment.
is the if new_host required? If removed, will it show the current value of the setting when you edit the host, or will it only work during creation?
There was a problem hiding this comment.
It does pick up the current value, removed the requirement.
There was a problem hiding this comment.
Ah great. Now I see what it looks like, you might want to get rid of the help_inline, since it's just a copy of the text - unless there's something more to say.
There was a problem hiding this comment.
You are right, updated the help text to actually say something. Thanks for the hint.
|
@shlomizadok would you mind testing this out? |
timogoebel
force-pushed
the
feature_vsphere_hot_add
branch
2 times, most recently
from
afdacdd
to
d0b5874
Compare
|
@domcleal we're a VMware environment, if you want me to take a look. |
|
@brandonweeks testing is always appreciated 💯 |
|
@brandonweeks indeed, as Michael said, that'd be great. Confirmation that it works, makes sense, also works still with it off is needed. |
|
I'm sorry, it is not working for me... (I am using vSphere 5.5) |
|
@shlomizadok : Thanks for testing. This actually should work fine as we just deployed 200 VMs with this settings and they were picked up. However you need to select a supported Guest OS I think (e.g. RHEL 7). @marcohelmerich: Can you confirm? Sorry for posting this in German:
|
timogoebel
force-pushed
the
feature_vsphere_hot_add
branch
from
d0b5874
to
4d695c3
Compare
theforeman-bot
added
Needs re-review
Needs testing
and removed
Waiting on contributor
labels
|
I just fixed the broken indent of the "Start" option... |
|
If CPU and Memory Hot Plug is available for VMs depends on kernel support of that feature. |
|
Chose RHEL-6 64-bit. Still not working for me 😿 |
|
Hmm, running out of ideas at the momen. Do you use the same user for manual and foreman-deployment? vsphere Docu: Since vSphere API 4.0 That is pretty straightforward. There is not much room for doing anything wrong... |
Above, @timogoebel suggested RHEL 7 as the OS type, have you tried that? |
|
@domcleal : I think, that should not make any difference. Give me a sec to test this... |
|
@shlomizadok: I tested this again. Everything works like a charm. We basically just pass the correct options to fog. When I update the settings through vSphere GUI, they are updated in Foreman accordingly.
|
|
👍 works great. |
|
Great, we double checked everything and couldn't find an error :-) |
|
Merged as 4d695c3, thanks @timogoebel! |
This PR adds two new checkboxes to the vsphere new vm gui and allows to enable CPU and memory hot add.