Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

clone_vm/clone_server takes reference as first parameter #50

Merged
merged 2 commits into from Apr 13, 2015

Conversation

djaara
Copy link
Contributor

@djaara djaara commented Apr 10, 2015

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

Server#clone called clone_vm with wrong parameter order causing
subsequent API call failure.
@@ -154,7 +154,7 @@ def can_be_cloned?

def clone(name)
raise "Clone Operation not Allowed" unless can_be_cloned?
self.reference = service.clone_vm(name, self.reference)
self.reference = service.clone_vm(self.reference, name)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant self detected.

@plribeiro3000
Copy link
Member

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
Copy link
Contributor Author

djaara commented Apr 10, 2015

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
Copy link

Coverage Status

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

@plribeiro3000
Copy link
Member

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
Copy link

Coverage Status

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

@plribeiro3000
Copy link
Member

Thx @djaara !

plribeiro3000 added a commit that referenced this pull request Apr 13, 2015
clone_vm/clone_server takes reference as first parameter
@plribeiro3000 plribeiro3000 merged commit ac17d4f into fog:master Apr 13, 2015
@plribeiro3000
Copy link
Member

Release as 0.2.2. Thanks for your time and efforts. ❤️

@djaara
Copy link
Contributor Author

djaara commented Apr 13, 2015

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
Copy link
Member

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

@djaara
Copy link
Contributor Author

djaara commented Apr 13, 2015

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
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants