[cloudstack] servers.get will now find VM in projects for normal users #2713

Merged
merged 1 commit into from Apr 9, 2014

6 participants

@loa

We have noticed that fog will not find VMs towards Cloudstack 3.0.6 with a normal user account if the VM was deployed in a project.

This issue only occurs for non-admin users.

To find VMs for a normal user you need to send 2 requests. One without projectid in case the VM is deployed in the default view and one with projectid set to -1 to search in all projects.

This pull-request proposes a solution where the second request will be made in case the first won't return the VM.

Following script was used to test this behaviour:

#!/usr/bin/env ruby
require 'fog'

# Precreate a vm in default view and specify id here
vm_in_default_view = 'b6c88916-55d6-456b-a81b-b1ed6c478fdb'

# Create a vm in a project and specify ids here
project = '4e86ebcb-a4ae-4da2-a407-795dab48d409'
vm_in_project = '589a324d-ae24-4a61-b5e8-6a24075603b8'

# Use other VMs for testing with admin account
vm_in_default_view = '445ee23f-d0d1-49c5-8f6b-f7e3e1684633'
project = '091ecbe5-f107-46e3-ad6d-62d7b9d6ee98'
vm_in_project = '4321de7f-ec47-4332-8144-e0cc3bed6c45'

## CASE A
puts
puts "Check if we can find the machine in the default view"
vm = Fog::Compute[:Cloudstack].list_virtual_machines('id' => vm_in_default_view)
if not vm['listvirtualmachinesresponse'].empty?
  puts "[A]: VM in default view found, " + vm['listvirtualmachinesresponse']['virtualmachine'][0]['name']
else
  puts "[A]: VM in default view NOT found"
end

## CASE B
puts
puts "Check if we can find the machine in the project"
vm = Fog::Compute[:Cloudstack].list_virtual_machines('id' => vm_in_project)
if not vm['listvirtualmachinesresponse'].empty?
  puts "[B]: VM in project found, " + vm['listvirtualmachinesresponse']['virtualmachine'][0]['name']
else
  puts "[B]: VM in project NOT found"
end

## CASE C
puts
puts "Check if we can find the machine in the project with projectid specified"
vm = Fog::Compute[:Cloudstack].list_virtual_machines('id' => vm_in_project, 'projectid' => project)
if not vm['listvirtualmachinesresponse'].empty?
  puts "[C]: VM in project found, " + vm['listvirtualmachinesresponse']['virtualmachine'][0]['name']
else
  puts "[C]: VM in project NOT found"
end

## CASE D
puts
puts "Check if we can find the machine in the project with projectid set to '-1'"
vm = Fog::Compute[:Cloudstack].list_virtual_machines('id' => vm_in_project, 'projectid' => '-1')
if not vm['listvirtualmachinesresponse'].empty?
  puts "[D]: VM in project found, " + vm['listvirtualmachinesresponse']['virtualmachine'][0]['name']
else
  puts "[D]: VM in project NOT found"
end

## CASE E
puts
puts "Check if we can find the machine in the default view with projectid set to '-1'"
vm = Fog::Compute[:Cloudstack].list_virtual_machines('id' => vm_in_default_view, 'projectid' => '-1')
if not vm['listvirtualmachinesresponse'].empty?
  puts "[E]: VM in default view found, " + vm['listvirtualmachinesresponse']['virtualmachine'][0]['name']
else
  puts "[E]: VM in default view NOT found"
end

Following is the result for admin user

Check if we can find the machine in the default view
[A]: VM in default view found, esup-1247-3

Check if we can find the machine in the project
[B]: VM in project found, esup-1247-4

Check if we can find the machine in the project with projectid specified
[C]: VM in project found, esup-1247-4

Check if we can find the machine in the project with projectid set to '-1'
[D]: VM in project found, esup-1247-4

Check if we can find the machine in the default view with projectid set to '-1'
[E]: VM in default view NOT found

Following is the result for a normal user

Check if we can find the machine in the default view
[A]: VM in default view found, esup-1247-1

Check if we can find the machine in the project
[B]: VM in project NOT found

Check if we can find the machine in the project with projectid specified
[C]: VM in project found, esup-1247-2

Check if we can find the machine in the project with projectid set to '-1'
[D]: VM in project found, esup-1247-2

Check if we can find the machine in the default view with projectid set to '-1'
[E]: VM in default view NOT found
@coveralls

Coverage Status

Coverage remained the same when pulling d7ccd0e on Loa:cloudstack-list_virtual_machines into 5007aac on fog:master.

@geemus
fog member

@dm1try could you take a look and review? Thanks!

@loa

@dm1try @geemus it would be really nice if we can get some attention to this since it's hurting us a lot.

We are not concerned how it's fixed more of the functionality of fog which we find broken in case you happen to use a normal user account towards cloudstack where VMs are provisioned in projects. Ofc the root cause is the non-logical api given to us by cloudstack.

@geemus
fog member

@Loa - sorry for the delay, I've never used cloudstack myself, so reticent to merge potentially breaking changes.

@bdorry - could you perhaps take a look?

@loa
loa commented Apr 9, 2014

@geemus @bdorry I'm sorry to bother you guys but we are bleeding.

@i11
i11 commented Apr 9, 2014

Any update guys?
This is a real issue for us as well!

@geemus
fog member

@Loa sorry. Quick question, could we JUST do the -1 check to avoid having to do 2 checks?

@mindjiver

Might be good to check if that would have any performance implications, if the default would need to search through all the projects? Other than that I think it would be alright to do -1 on the first API call. Any comments @Loa ?

@loa
loa commented Apr 9, 2014

@geemus @mindjiver sorry no, you need to do both requests.

projectid = -1 will search in all projects but not in the default view.

So to be able to either have VMs in default view or projects you have to do both. I don't know of any other ways of crafting the request, anyway no other way that wouldn't require more than one request anyway.

Performance wise I start with the default request and check if it found anything, which will always work for admin users.

If you check the output I given in my example you can see exactly this weird behavior...

@bdorry
fog member

@Loa @geemus you have to do both requests, it's not intuitive behavior from the Cloudstack API

It looks ok to me, my main concern would be performance implications. I don't know if it would make sense to do the all project check first, or default view search first. Logically I'd say default view first (as it's implemented), but that may not be optimal for every user.

I don't know that there is a good default behavior for all users.

@loa
loa commented Apr 9, 2014

@mindjiver @bdorry
Currently a request without projectid set for an admin user will search everything anyway.

So actually you can see this as an performance upgrade for normal users (on cloudstack side, not for the user that needs to do 2 requests).

@geemus
fog member

Thanks for clarification.

@geemus geemus merged commit c115fec into fog:master Apr 9, 2014

1 check passed

Details default The Travis CI build passed
@loa
loa commented Apr 9, 2014

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment