fog / fog-xenserver

Module for the 'fog' gem to support XENSERVER
MIT License
16 stars 22 forks source link

clone_vm/clone_server takes reference as first parameter #50

Closed djaara closed 9 years ago

djaara commented 9 years ago

Server#clone called clone_vm with wrong parameter order causing subsequent API call failure.

plribeiro3000 commented 9 years ago

Damm. Good catch.

Can you take care of the few style warnings?

Thank you!

PS: what are your feedback on fog-xenserver over the old code from fog. Something else we can improve?

djaara commented 9 years ago

Yep, can check style warnings.

For a while I fight with VM creation from template, but I found correct approach in git log. Than I run into problems with following code:

template = connection.tempaltes.find_by_name 'template_name'
vm = template.clone 'vm_name'
vm.save
vm.provision
vm.start

Where vm.start failed due to missing disk (as vm.save created new template without disk - so at this time I had 2 templates; first one originally cloned and second one without disk which was converted to VM), so I realise that vm.save is not safe operation and removed it from code.

Issue I'm fight right now is that after vm.start I don't have access to vm.guest_metrics.networks (even after vm.reload) - returns empty Hash. If I try to load vm from another connection, networks hash is populated correctly.

All my issues seems to be related to lack of documentation.

coveralls commented 9 years ago

Coverage Status

Coverage remained the same at 94.07% when pulling b7b75ac4f150ced7a8aa9526a6db03200906fbbc on djaara:master into a107ca72ecfccbe123769eff3109526c7deaf2ad on fog:master.

plribeiro3000 commented 9 years ago

Hmmm. Thank you for the feedback. We have some documentation in the wiki but most are not updated. Gotta fix them.

The point with vm.save now is that it is pretty much what the XENAPI does. No magic behind the scenes. This decision make it a little more verbose to create a vm but at least now we have a better control over it. Before we didn't have much options and if you wanted to create something different you were on your own.

This new design follows the `XENAPI´ one. A good look at here should help you a lot. =)

coveralls commented 9 years ago

Coverage Status

Coverage remained the same at 94.07% when pulling 56128e2fb3aa5b6e1ab4872134c210c6e55684ac on djaara:master into a107ca72ecfccbe123769eff3109526c7deaf2ad on fog:master.

plribeiro3000 commented 9 years ago

Thx @djaara !

plribeiro3000 commented 9 years ago

Release as 0.2.2. Thanks for your time and efforts. :heart:

djaara commented 9 years ago

Great! Thanks.

One more thing - is there any alternative to original Fog::Compute::XenServer::Server.refresh?

def refresh
  Fog::Logger.deprecation(
    'This method is deprecated. Use #reload instead.'
  )
  data = service.get_record( reference, 'VM' )
  merge_attributes( data )
  true
end

At this moment, I've replaced refresh calls with:

vm = connection.servers.get vm.reference

But there might be better solution.

plribeiro3000 commented 9 years ago

Yeah, there is reload Since fog-core already defines reload there wasn't much sense to keep refresh. =)

djaara commented 9 years ago

I forgot to mention, that reload does not work for me:

With #reload I'm experiencing problems with accessing latest guest_metrics. I'm getting guest_metrics with data related to original template, not to provisioned/started VM.

plribeiro3000 commented 9 years ago

Hmm. i suspect it might be a bug somewhere when instantiating the objects. Perhaps you could open a issue here for this behavior and provide more info like whats the value of data here and the value of new_attributes here.

I suspect it might be related to the associations stuff and the merge_attributes method defined here. If you can provide more info we can find the issue and fix it.

EDIT: Perhaps the identity is not being overwrited with the new value and thus reload is getting data from the template instead.