Hi Dies,
looks good, though I don't understand the networks method at all:
On Fri, 2013-02-22 at 23:45 +1100, di...@fast.au.fujitsu.com wrote:
> diff --git a/server/lib/deltacloud/drivers/fgcp/fgcp_driver_cimi_methods.rb
> b/server/lib/deltacloud/drivers/fgcp/fgcp_driver_cimi_methods.rb
> new file mode 100644
> index 0000000..db26a05
> --- /dev/null
> +++ b/server/lib/deltacloud/drivers/fgcp/fgcp_driver_cimi_methods.rb
> @@ -0,0 +1,137 @@
> + def systems(credentials, opts={})
> + systems = []
> +
> + safely do
> + client = new_client(credentials)
> +
> + if opts and opts[:id]
> + vsys_config = client.get_vsys_configuration(opts[:id])
> + return CIMI::Model::System.new(
> + :id => opts[:id],
> + :name => vsys_config['vsys'][0]['vsysName'][0],
> +# :machines => "http://cimi.example.org/machines?realm_id=#{}";,
> +# :volumes => "http://cimi.example.org/volumes?realm_id=#{}";,
> +# :addresses =>
> "http://cimi.example.org/addresses?realm_id=#{}";,
> + :description => vsys_config['vsys'][0]['description'][0]
> + )
> + elsif xml = client.list_vsys['vsyss']
> +
> + return [] if xml.nil?
> + xml[0]['vsys'].each do |vsys|
> +
> + vsys_config = client.get_vsys_configuration(vsys['vsysId'][0])
> + vsys_description_el = vsys_config['vsys'][0]['description']
> + systems << CIMI::Model::System.new(
> + :id => vsys['vsysId'][0],
> + :name => vsys_config['vsys'][0]['vsysName'][0],
> +# :machines =>
> "http://cimi.example.org/machines?realm_id=#{vsys['vsysId'][0]}",
> +# :volumes =>
> "http://cimi.example.org/volumes?realm_id=#{vsys['vsysId'][0]}",
> +# :networks =>
> "http://cimi.example.org/networks?realm_id=#{vsys['vsysId'][0]}",
> +# :addresses =>
> "http://cimi.example.org/addresses?realm_id=#{vsys['vsysId'][0]}",
> + :description => vsys_description_el ? vsys_description_el[0] :
> nil
> + )
> + end
> + end
> + end
> + systems
> + end
This returns a single system or an array of systems, depending on
whether opts[:id] is set or not - it's cleaner to always return an
array. If opts[:id] is set, that just happens to contain only one entry.
> + def system_templates(credentials, opts={})
> + templates = []
> + safely do
> + client = new_client(credentials)
> +
> client.list_vsys_descriptor['vsysdescriptors'][0]['vsysdescriptor'].each do
> |desc|
> + templates << CIMI::Model::SystemTemplate.new(
> + :id => desc['vsysdescriptorId'][0],
> + :name => desc['vsysdescriptorName'][0],
> + :description => desc['description'][0]
> + )
This would be more readable as
templates =
client.list_vsys_descriptor['vsysdescriptors'][0]['vsysdescriptor'].collect do
|desc|
CIMI::Model::SystemTemplate.new(...)
end
> + return templates.last if opts and opts[:id] ==
> desc['vsysdescriptorId'][0]
Again, it's cleaner to always return an array here. And you could say:
templates = templates.select { |t| opts[:id] == t[:id] } if opts[:id]
> + end
> + end
> + templates
> + end
> +
> + def networks(credentials, opts={})
> + #list vsys
> + #list vsys config
> + # add network per network segment.
> + # add public network.
> + networks = []
> +
> + safely do
> + client = new_client(credentials)
> +
> + if opts and opts[:id]
> + #list vsys config
> + # add network segment.
> + # or public network?
> +
> + vsys_id = client.extract_vsys_id(opts[:id])
> + vsys_config = client.get_vsys_configuration(vsys_id)
> + vsys_config['vsys'][0]['vnets'][0]['vnet'].find do |vnet|
> +
> + networks << CIMI::Model::Network.new(
> + :id => opts[:id],
> + :name => opts[:id],
> + :state => 'STARTED',
> + :network_type => 'PRIVATE'
> + ) if vnet['networkId'][0] == opts[:id]
> + end
> + elsif xml = client.list_vsys['vsyss']
> +
> + return [] if xml.nil?
> + xml[0]['vsys'].each do |vsys|
> +
> + # use get_vsys_configuration (instead of
> get_vserver_configuration) to retrieve all vservers in one call
> + vsys_config = client.get_vsys_configuration(vsys['vsysId'][0])
> + vsys_config['vsys'][0]['vservers'][0]['vserver'].each do
> |vserver|
> +
> + # skip firewalls - they probably don't belong here and their
> new type ('firewall' instead of
> + # 'economy') causes errors when trying to map to available
> profiles)
> + unless determine_server_type(vserver) == 'FW'
> + # to keep the response time of this method acceptable,
> retrieve state
> + # only if required because state is filtered on
> + state_data = opts[:state] ? instance_state_data(vserver,
> client) : nil
> + # filter on state
> + if opts[:state].nil? or opts[:state] == state_data[:state]
> + instances << convert_to_instance(client, vserver,
> state_data)
> + end
> + end
> + end
> + end
> + end
> + end
> + end
Here, networks is never returned, and the elsif branch doesn't even
manipulate it.
David
