Project

General

Profile

Actions

Bug #19990

closed

Failed to update a compute vmware (VMware) instance node.test.domain: undefined method `[]' for nil:NilClass

Added by Christian Meißner almost 7 years ago. Updated almost 6 years ago.

Status:
Closed
Priority:
Normal
Category:
Compute resources - VMware
Target version:

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

Related issues 1 (0 open1 closed)

Copied to Foreman - Bug #23752: VMWare allow save vm w/o size_gb attr (1.16.2)Closed05/31/2018Actions
Actions #1

Updated by Tomáš Strachota almost 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?

Actions #2

Updated by Christian Meißner almost 7 years ago

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.

Actions #3

Updated by Tomáš Strachota almost 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.

Actions #4

Updated by Tomáš Strachota almost 7 years ago

  • Target version set to 115
Actions #5

Updated by Tomáš Strachota almost 7 years ago

Most likely introduced with https://github.com/theforeman/foreman/pull/4146/
oVirt is probably affected too.

Actions #6

Updated by Marek Hulán almost 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 almost 7 years ago

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)

Actions #8

Updated by Daniel Lobato Garcia almost 7 years ago

  • translation missing: en.field_release changed from 266 to 276
Actions #9

Updated by Daniel Lobato Garcia over 6 years ago

  • translation missing: en.field_release changed from 276 to 287
Actions #10

Updated by Marek Hulán over 6 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"}
Actions #11

Updated by Tomáš Strachota over 6 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.

Actions #12

Updated by Marek Hulán over 6 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

Actions #13

Updated by Bhanu Prasad Ganguru over 6 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

Actions #15

Updated by Adam Winberg over 6 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...

Actions #16

Updated by Steven Miller over 6 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
Actions #17

Updated by Steven Miller over 6 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...

Actions #18

Updated by Marek Hulán about 6 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.

Actions #19

Updated by Steven Miller about 6 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.

Actions #20

Updated by Dmitry Okun about 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

Actions #21

Updated by Steven Miller almost 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)

Actions #22

Updated by The Foreman Bot almost 6 years ago

  • Status changed from Need more information to Ready For Testing
Actions #23

Updated by Tomer Brisker almost 6 years ago

  • Bugzilla link set to 1551014
Actions #24

Updated by The Foreman Bot almost 6 years ago

  • Assignee set to Ondřej Pražák
  • Pull request https://github.com/theforeman/foreman/pull/5626 added
Actions #25

Updated by The Foreman Bot almost 6 years ago

  • Pull request https://github.com/theforeman/foreman/pull/5627 added
Actions #26

Updated by Daniel Lobato Garcia almost 6 years ago

  • translation missing: en.field_release set to 330
Actions #27

Updated by Daniel Lobato Garcia almost 6 years ago

  • Copied to Bug #23752: VMWare allow save vm w/o size_gb attr (1.16.2) added
Actions #28

Updated by Anonymous almost 6 years ago

  • Status changed from Ready For Testing to Closed
  • % Done changed from 0 to 100
Actions

Also available in: Atom PDF