Bug #19990
closedFailed to update a compute vmware (VMware) instance node.test.domain: undefined method `[]' for nil:NilClass
Added by Christian Meißner over 7 years ago. Updated over 6 years ago.
Description
If I try to update the environment of a machine in a vmware compute instance i get the above error.
I attached the sanitized debug output of the following command.
hammer -u user -p pass -d host update --id 127 --environment production
If I use --environment-id
instead of --environment
it leads to the same error.
Other actions for example start
and stop
of a virtual machine works fine.
Files
hammer_cli_debug.log | hammer_cli_debug.log | 6.62 KB | Christian Meißner, 06/13/2017 05:00 AM | ||
production.log | production.log | 23.3 KB | Christian Meißner, 06/14/2017 01:10 AM | ||
production.log | production.log | 472 KB | Steven Miller, 06/15/2017 03:02 PM | ||
guiheader.txt | guiheader.txt | 3.78 KB | Header sent when submitted request to create new host | Steven Miller, 06/15/2017 03:02 PM | |
foremanpost-smiller-20180119.txt | foremanpost-smiller-20180119.txt | 2.83 KB | Steven Miller, 01/19/2018 02:25 PM |
Updated by Tomáš Strachota over 7 years ago
Hello Christian,
thank you for reporting the issue. Attached hammer logs show that the error occurs on the server side. May I ask you to attach relevant part of foreman logs too?
If you try to perform the same action in the UI, does it fail with the same error?
Updated by Christian Meißner over 7 years ago
- File production.log production.log added
Tomáš Strachota wrote:
Hello Christian,
thank you for reporting the issue. Attached hammer logs show that the error occurs on the server side. May I ask you to attach relevant part of foreman logs too?
Hi Tomas attached you can find the cut off of our production.log.
If you try to perform the same action in the UI, does it fail with the same error?
No, in the UI it works pretty good.
Updated by Tomáš Strachota over 7 years ago
- Project changed from Hammer CLI to Foreman
- Category changed from Hosts to Compute resources - VMware
I'm switching the component to VMWare as this is a server side API issue. The compute node orchestration fails on update via API because it misses some compute attribute data. UI works fine because it loads the compute attributes into the form when the page is rendered and then when user hits update the request is sent with required parameters. In API the only workaround is to send the fill set of current compute attributes with the update request.
Updated by Tomáš Strachota over 7 years ago
Most likely introduced with https://github.com/theforeman/foreman/pull/4146/
oVirt is probably affected too.
Updated by Marek Hulán over 7 years ago
- Translation missing: en.field_release set to 266
I suppose this is a candidate for 1.15.2 then. Please reset the Release if you disagree.
Updated by Steven Miller over 7 years ago
- File production.log production.log added
- File guiheader.txt guiheader.txt added
I am seeing the same bug on this provider, even when provisioning a new system from the GUI. I've attached my production logs, along with the final form data that is submitted when attempting to create a host. On the VMware side the actual VM is created, but then when it hits the "undefined method" error it rolls back the VMware machine (but not the record within foreman)
Updated by Daniel Lobato Garcia over 7 years ago
- Translation missing: en.field_release changed from 266 to 276
Updated by Daniel Lobato Garcia over 7 years ago
- Translation missing: en.field_release changed from 276 to 287
Updated by Marek Hulán about 7 years ago
Tomas, why do you think it was introduced by the linked PR? According to the trace, it seems that compute_attrs for a given VM are nil. Based on incoming parameters, they are part of API request. I don't see how attributes can become nil so far
Parameters: {"host"=>{"environment_id"=>20, "compute_attributes"=>{"volumes_attributes"=>{}}, "host_parameters_attributes"=>{}, "interfaces_attributes"=>{}}, "apiv"=>"v2", "id"=>"119"}
Updated by Tomáš Strachota about 7 years ago
Marek, the issue has actually been there even before the commit I mentioned, but this change eventually exposed the buggy code to users:
https://github.com/theforeman/foreman/pull/4146/files#diff-4443c0de1e4e867a30d3c878b9cb64e9R24
Before that it wasn't possible to update attribute of existing VMs on vmWare. From the traceback, the process fails on:
https://github.com/theforeman/foreman/blob/c952c782adac290fd5a1145090380424665f5fe1/app/models/compute_resources/foreman/model/vmware.rb#L434
(please note that the bug was reported against the foreman 1.15 and line numbers have been shifted since then)
So the issue isn't caused by compute_attributes being nil, but by volume attributes that can't be found. The code isn't prepared for partial updates and always requires the full set of compute attributes (including volumes and nics). The reason it works in the UI is that the host edit form pre-loads current compute attributes and therefore the attributes are complete.
Updated by Marek Hulán about 7 years ago
- Translation missing: en.field_release deleted (
287)
Now that makes more sense, thanks for clarification. I suppose this is API RFE then, removing from 1.15.4
Updated by Bhanu Prasad Ganguru about 7 years ago
Hi, we recently updated foreman to 1.15.6
And still seeing the same issue
is there a solution for this issue?
Bhanu
Updated by Adam Winberg about 7 years ago
Im not using hammer, but the workaround I use when using the API is to first do a GET to get the nodes 'vm_compute_attributes' and then attach that result to my PUT operation on the node.
In perl (just to get the idea):
my $client = REST::Client->new(); $client->setHost($foreman_host); # Get compute attributes, bug workaround: # http://projects.theforeman.org/issues/19990 $client->GET("/api/hosts/$fqdn/vm_compute_attributes", $headers); my $compute_attrs = decode_json($client->responseContent()); # Set the attributes for a new host my %json_data = ( host => { comment => "Updating the comment attribute", compute_attributes => $compute_attrs, } ); # Translate the perl hash in json notation my $data = encode_json(\%json_data); # Update parameter $client->PUT("/api/hosts/$fqdn",($data,$headers));
I'm not using hammer though...
Updated by Steven Miller almost 7 years ago
I recently upgraded my environment to 1.16.0 and still found the issue. I was able to eliminate the issue by commenting out the following line in line 457 of the app/models/compute_resources/foreman/model/vmware.rb (the line in the 1.16.0 codebase). I may submit a pull request at some point, but want to do a little more research to make sure this doesn't break anything unexpected. For us this fixes compute creating through the UI and API.
450 def save_vm(uuid, attr) 451 vm = find_vm_by_uuid(uuid) 452 vm.attributes.merge!(attr.deep_symbolize_keys) 453 #volumes are not part of vm.attributes so we have to set them seperately if needed 454 if attr.has_key?(:volumes_attributes) 455 vm.volumes.each do |vm_volume| 456 volume_attrs = attr[:volumes_attributes].values.detect {|vol| vol[:id] == vm_volume.id} 457 #vm_volume.size_gb = volume_attrs[:size_gb] 458 end 459 end 460 vm.save 461 end
Updated by Steven Miller almost 7 years ago
Forgot to mention that it appears the UI is passing the parameter "sizeGb" to specify volume size. It appear the older, non-React UI code still uses that parameter, but the newer view code does not...maybe...
Updated by Marek Hulán almost 7 years ago
- Status changed from New to Need more information
- Priority changed from High to Normal
The patch would disable the volume size adjusting. What I did not fully understand was the last comment, do you say that you are now able to reproduce this bug even using UI? The new react implementation work the same way.
Updated by Steven Miller almost 7 years ago
The patch I submitted is the only way we could foreman to work. The hard drive size is still set correctly, even with this line completely commented out. I have a more sophisticated patch that will check if size_gb is set, and if it is not, then just skip the assignment (line 457 in the example).
In our 1.16 install, the "size_gb" parameter is never passed through the web interface, and seems to break at this line. I've attached "foremanpost-smiller-20180119.txt", which contains all the POSTed variables from a web browser doing a VMware deployment (on a 1.16 install). I see that both "size" and "SizeGB" are set within the host[compute_attributes][scsi_controllers] attribute. I don't see a size_gb set on the UI end at least.
Updated by Dmitry Okun over 6 years ago
Steven Miller wrote:
The patch I submitted is the only way we could foreman to work. The hard drive size is still set correctly, even with this line completely commented out. I have a more sophisticated patch that will check if size_gb is set, and if it is not, then just skip the assignment (line 457 in the example).
In our 1.16 install, the "size_gb" parameter is never passed through the web interface, and seems to break at this line. I've attached "foremanpost-smiller-20180119.txt", which contains all the POSTed variables from a web browser doing a VMware deployment (on a 1.16 install). I see that both "size" and "SizeGB" are set within the host[compute_attributes][scsi_controllers] attribute. I don't see a size_gb set on the UI end at least.
I was also hitting the same error when attempting to provision a VMware VM. Moreover, my VMs were created and right away deleted via foreman. By commenting these lines out, I was able to create VMs via Foreman
Updated by Steven Miller over 6 years ago
- Pull request https://github.com/theforeman/foreman/pull/5577 added
Created a pull request for a slightly more sophisticated version of my fix. If volume_attrs is present as a has it will attempt to set size_gb, otherwise it will skip the assignment. Internal testing seems to work for us (both UI and API usage)
Updated by The Foreman Bot over 6 years ago
- Status changed from Need more information to Ready For Testing
Updated by The Foreman Bot over 6 years ago
- Assignee set to Ondřej Pražák
- Pull request https://github.com/theforeman/foreman/pull/5626 added
Updated by The Foreman Bot over 6 years ago
- Pull request https://github.com/theforeman/foreman/pull/5627 added
Updated by Daniel Lobato Garcia over 6 years ago
- Translation missing: en.field_release set to 330
Updated by Daniel Lobato Garcia over 6 years ago
- Copied to Bug #23752: VMWare allow save vm w/o size_gb attr (1.16.2) added
Updated by Anonymous over 6 years ago
- Status changed from Ready For Testing to Closed
- % Done changed from 0 to 100
Applied in changeset 1dbb2c5b319dbab023f3aaa98019d7b9cd0ee8f6.