You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@deltacloud.apache.org by David Lutterkort <lu...@redhat.com> on 2013/04/02 02:26:51 UTC
Re: [PATCH] added Template subcollections to SystemTemplate
On Thu, 2013-03-28 at 12:19 +1100, Koper, Dies wrote:
> A bit of background on this patch:
> It contains my latest work on implementing cimi systems and system
> templates for mock and fgcp.
> It is not complete and contains some debugging statements. Its main
> purpose is to show you what I have and what issues I'm seeing.
>
> It applies to master (fcffad13e66175152cf8a43d615b79727902b5ee) and
> requires "[PATCH] CIMI schema: tolerate nil hash_map attributes" to
> solve a nil error for system templates with fgcp.
>
> Issues I'm having:
>
> (1)
> With mock, even unsupported subcollections are shown when retrieving
> systems. With fgcp they are also shown, and their href urls are broken:
This is a shortcoming of how we populate models right now - there's no
way to suppress collections that the driver doesn't support. In fact,
the code goes through great length to make sure we generate the href for
empty collections (since that href is needed to add the first element to
an empty collection)
The easiest way to 'fix' this for now is to comment those unsupported
collections out in system.rb.
A better fix is to add a mechanism similar to what we do for $select to
models; call that the 'exclude' mechanism. The various generate_xxx
methods should set the that exclude parameter based on collections that
are declared in Rabbit, but not available (Rabbit knows that, Michal:
how do we get that info out of Rabbit ?)
Similar to how CIMI::Model::Resource has @select_attrs it should also
have @exclude_attrs that are consulted in prepare and cause the
corresponding collection to be set to nil (which will remove it from the
XML/JSON output)
> (2)
> With mock (not with fgcp), when I list a system's volumes, its id is not
> generated correctly:
>
> d:\projects>curl --user mockuser:mockpassword
> http://localhost:3001/cimi/systems/system2/volumes?format=xml
> <Collection xmlns="http://schemas.dmtf.org/cimi/1"
> resourceURI="http://schemas.dmtf.org/cimi/1/SystemVolumeCollection">
> <id>http://localhost:3001/cimi/system/system2/volumes</id>
> <count>1</count>
> <SystemVolume>
> <id>http://localhost:3001/cimi/volumes?id=sysvol1</id>
> ...
>
> That should be http://localhost:3001/cimi/volumes/sysvol1.
> Note my comment in mock_driver_cimi_methods.rb#system_volumes about the
> 1st arg I pass to convert_cimi_mock_urls:
>
> #FIXME: with ":volumes", delete url becomes
> 'http://localhost:3001/cimi/volumes?id=sysvol1'
> #with ":system_volume" or ":system_volumes", undefined method
> `system_volume_url' for #<CIMI::Collections::Systems:0x44fe338> in
> mock_driver_cimi_methods.rb:261
> volumes.map{|vol|convert_cimi_mock_urls(:volumes, vol,
> opts[:env])}.flatten
This seems to be a Rabbit problem with the names of the subcollections;
somehow URL helpers for the subcollections do not get generated
correctly. Michal ?
BTW, attached is a patch that addresses a few URL conversion problems I
found in the mock driver; feel free to commit if it doesn't make things
worse ;)
David
RE: [PATCH] added Template subcollections to SystemTemplate
Posted by "Koper, Dies" <di...@fast.au.fujitsu.com>.
> BTW, attached is a patch that addresses a few URL conversion problems
> I
> found in the mock driver; feel free to commit if it doesn't make things
> worse ;)
ACK & pushed - I've confirmed things seem to not have gotten worse :)
Cheers,
Dies Koper
> -----Original Message-----
> From: David Lutterkort [mailto:lutter@redhat.com]
> Sent: Tuesday, 2 April 2013 11:27 AM
> To: dev@deltacloud.apache.org
> Cc: mfojtik@redhat.com
> Subject: Re: [PATCH] added Template subcollections to SystemTemplate
>
> On Thu, 2013-03-28 at 12:19 +1100, Koper, Dies wrote:
> > A bit of background on this patch:
> > It contains my latest work on implementing cimi systems and system
> > templates for mock and fgcp.
> > It is not complete and contains some debugging statements. Its main
> > purpose is to show you what I have and what issues I'm seeing.
> >
> > It applies to master (fcffad13e66175152cf8a43d615b79727902b5ee) and
> > requires "[PATCH] CIMI schema: tolerate nil hash_map attributes" to
> > solve a nil error for system templates with fgcp.
> >
> > Issues I'm having:
> >
> > (1)
> > With mock, even unsupported subcollections are shown when retrieving
> > systems. With fgcp they are also shown, and their href urls are broken:
>
> This is a shortcoming of how we populate models right now - there's no
> way to suppress collections that the driver doesn't support. In fact,
> the code goes through great length to make sure we generate the href for
> empty collections (since that href is needed to add the first element
> to
> an empty collection)
>
> The easiest way to 'fix' this for now is to comment those unsupported
> collections out in system.rb.
>
> A better fix is to add a mechanism similar to what we do for $select to
> models; call that the 'exclude' mechanism. The various generate_xxx
> methods should set the that exclude parameter based on collections that
> are declared in Rabbit, but not available (Rabbit knows that, Michal:
> how do we get that info out of Rabbit ?)
>
> Similar to how CIMI::Model::Resource has @select_attrs it should also
> have @exclude_attrs that are consulted in prepare and cause the
> corresponding collection to be set to nil (which will remove it from the
> XML/JSON output)
>
> > (2)
> > With mock (not with fgcp), when I list a system's volumes, its id is
> not
> > generated correctly:
> >
> > d:\projects>curl --user mockuser:mockpassword
> > http://localhost:3001/cimi/systems/system2/volumes?format=xml
> > <Collection xmlns="http://schemas.dmtf.org/cimi/1"
> >
> resourceURI="http://schemas.dmtf.org/cimi/1/SystemVolumeCollection">
> > <id>http://localhost:3001/cimi/system/system2/volumes</id>
> > <count>1</count>
> > <SystemVolume>
> > <id>http://localhost:3001/cimi/volumes?id=sysvol1</id>
> > ...
> >
> > That should be http://localhost:3001/cimi/volumes/sysvol1.
> > Note my comment in mock_driver_cimi_methods.rb#system_volumes about
> the
> > 1st arg I pass to convert_cimi_mock_urls:
> >
> > #FIXME: with ":volumes", delete url becomes
> > 'http://localhost:3001/cimi/volumes?id=sysvol1'
> > #with ":system_volume" or ":system_volumes", undefined method
> > `system_volume_url' for #<CIMI::Collections::Systems:0x44fe338> in
> > mock_driver_cimi_methods.rb:261
> > volumes.map{|vol|convert_cimi_mock_urls(:volumes, vol,
> > opts[:env])}.flatten
>
> This seems to be a Rabbit problem with the names of the subcollections;
> somehow URL helpers for the subcollections do not get generated
> correctly. Michal ?
>
> BTW, attached is a patch that addresses a few URL conversion problems
> I
> found in the mock driver; feel free to commit if it doesn't make things
> worse ;)
>
> David
RE: [PATCH] added Template subcollections to SystemTemplate
Posted by "Koper, Dies" <di...@fast.au.fujitsu.com>.
Cheers, that worked!
I've pushed your patch as David ACKed it.
I've just send a patch to the ML with the changes for mock ([PATCH] mock: fix href urls in mock system and subcollections. Also include start action for newly created system), and I also found good use for it with fgcp ([PATCH] FGCP: use url_for helpers for Rabbit subcollections).
Regards,
Dies Koper
> -----Original Message-----
> From: Michal Fojtik [mailto:mfojtik@redhat.com]
> Sent: Friday, 5 April 2013 2:00 AM
> To: Koper, Dies
> Subject: Re: [PATCH] added Template subcollections to SystemTemplate
>
> On 04/04/2013 03:38 PM, Koper, Dies wrote:
>
> Hi Dies,
>
> > Hi Michal,
> >
> >> To use this method, you will need to pass 'context' parameter and then
> >> call the helper using:
> >>
> >> context.system_volume_url(system_id, volume_id)
>
> I tried hard to fix it :-) I found we completely lack support for
> generating URL helpers for all sub-collections. I have patch[1] to fix
> it. However I was not able to make the syntax the same as in your
> example, without making url generation code super long :-)
>
> So once you have my patch applied you can use this syntax:
>
> context.system_volume_url(:id => system_id, :ent_id => volume_id)
>
> Does this work for you?
>
> [1] http://tracker.deltacloud.org/set/408
>
> -- Michal
>
> >
> > This is what I tried: (you can do the same in master)
> >
> > diff --git
> a/server/lib/deltacloud/drivers/mock/mock_driver_cimi_methods.rb
> b/server/lib/deltacloud/drivers/mock/mock_driver_cimi_metho
> > index b0d4f9a..f236935 100644
> > ---
> a/server/lib/deltacloud/drivers/mock/mock_driver_cimi_methods.rb
> > +++
> b/server/lib/deltacloud/drivers/mock/mock_driver_cimi_methods.rb
> > @@ -80,7 +80,8 @@ module Deltacloud::Drivers::Mock
> > end
> > #FIXME: with ":machines", delete url becomes
> 'http://localhost:3001/cimi/machines?id=sysmach1'
> > #with ":system_machine"/":system_machines", undefined method
> `system_machine_url' for #<CIMI::Collections::Systems:0x44fe338> in
> > - machines.map{|mach|convert_cimi_mock_urls(:machines, mach,
> opts[:env])}.flatten
> > +# machines.map{|mach|convert_cimi_mock_urls(:machines, mach,
> opts[:env])}.flatten
> > + machines.map{|mach|convert_urls(mach, opts[:env])}.flatten
> > end
> >
> > def system_volumes(credentials, opts={})
> > @@ -337,10 +338,20 @@ module Deltacloud::Drivers::Mock
> > return s
> > end
> > return s unless u.scheme == 'http' && u.host ==
> 'cimi.example.org'
> > - _, coll, id = u.path.split("/")
> > + puts u.path
> > + _, coll, id, sub_coll, sub_id = u.path.split("/")
> > + puts coll
> > + puts id
> > + puts sub_coll
> > + puts sub_id
> > method = "#{coll.singularize}_url"
> > + method = "#{coll.singularize}_#{sub_coll.singularize}_url" if
> sub_coll
> > + puts "method: #{method}"
> > if context.respond_to?(method)
> > - context.send(method, id)
> > + s = context.send(method, id) unless sub_coll
> > + s = context.send(method, id, sub_id) if sub_coll
> > + puts "s: #{s}"
> > + s
> > else
> > s
> > end
> >
> > So I'm trying to change rewrite_url to also cater for urls with
> subcollections.
> >
> > The debugging output shows that the context does not respond to system
> related methods such as system_machine_url.
> >
> > What now?
> >
> > Regards,
> > Dies Koper
> >
> >
> >> -----Original Message-----
> >> From: Michal Fojtik [mailto:mfojtik@redhat.com]
> >> Sent: Wednesday, 3 April 2013 7:31 PM
> >> To: dev@deltacloud.apache.org
> >> Subject: Re: [PATCH] added Template subcollections to SystemTemplate
> >>
> >> On 04/02/2013 02:26 AM, David Lutterkort wrote:
> >>
> >>> This seems to be a Rabbit problem with the names of the subcollections;
> >>> somehow URL helpers for the subcollections do not get generated
> >>> correctly. Michal ?
> >>
> >> Sry, I overlooked this one :)
> >>
> >> I checked the patch and seems you are trying to use 'system_volume_url'
> >> helper in driver (not in Sinatra context).
> >>
> >> To use this method, you will need to pass 'context' parameter and then
> >> call the helper using:
> >>
> >> context.system_volume_url(system_id, volume_id)
> >>
> >> (check David patch for example :-)
> >>
> >> -- Michal
> >>
> >>>
> >>> BTW, attached is a patch that addresses a few URL conversion problems
> >> I
> >>> found in the mock driver; feel free to commit if it doesn't make things
> >>> worse ;)
> >>>
> >>> David
> >>>
> >>
> >>
> >> --
> >>
> >> Michal Fojtik <mf...@redhat.com>
> >> Deltacloud API, CloudForms
> >
>
>
> --
>
> Michal Fojtik <mf...@redhat.com>
> Deltacloud API, CloudForms
RE: [PATCH] added Template subcollections to SystemTemplate
Posted by "Koper, Dies" <di...@fast.au.fujitsu.com>.
Hi Michal,
> To use this method, you will need to pass 'context' parameter and then
> call the helper using:
>
> context.system_volume_url(system_id, volume_id)
This is what I tried: (you can do the same in master)
diff --git a/server/lib/deltacloud/drivers/mock/mock_driver_cimi_methods.rb b/server/lib/deltacloud/drivers/mock/mock_driver_cimi_metho
index b0d4f9a..f236935 100644
--- a/server/lib/deltacloud/drivers/mock/mock_driver_cimi_methods.rb
+++ b/server/lib/deltacloud/drivers/mock/mock_driver_cimi_methods.rb
@@ -80,7 +80,8 @@ module Deltacloud::Drivers::Mock
end
#FIXME: with ":machines", delete url becomes 'http://localhost:3001/cimi/machines?id=sysmach1'
#with ":system_machine"/":system_machines", undefined method `system_machine_url' for #<CIMI::Collections::Systems:0x44fe338> in
- machines.map{|mach|convert_cimi_mock_urls(:machines, mach, opts[:env])}.flatten
+# machines.map{|mach|convert_cimi_mock_urls(:machines, mach, opts[:env])}.flatten
+ machines.map{|mach|convert_urls(mach, opts[:env])}.flatten
end
def system_volumes(credentials, opts={})
@@ -337,10 +338,20 @@ module Deltacloud::Drivers::Mock
return s
end
return s unless u.scheme == 'http' && u.host == 'cimi.example.org'
- _, coll, id = u.path.split("/")
+ puts u.path
+ _, coll, id, sub_coll, sub_id = u.path.split("/")
+ puts coll
+ puts id
+ puts sub_coll
+ puts sub_id
method = "#{coll.singularize}_url"
+ method = "#{coll.singularize}_#{sub_coll.singularize}_url" if sub_coll
+ puts "method: #{method}"
if context.respond_to?(method)
- context.send(method, id)
+ s = context.send(method, id) unless sub_coll
+ s = context.send(method, id, sub_id) if sub_coll
+ puts "s: #{s}"
+ s
else
s
end
So I'm trying to change rewrite_url to also cater for urls with subcollections.
The debugging output shows that the context does not respond to system related methods such as system_machine_url.
What now?
Regards,
Dies Koper
> -----Original Message-----
> From: Michal Fojtik [mailto:mfojtik@redhat.com]
> Sent: Wednesday, 3 April 2013 7:31 PM
> To: dev@deltacloud.apache.org
> Subject: Re: [PATCH] added Template subcollections to SystemTemplate
>
> On 04/02/2013 02:26 AM, David Lutterkort wrote:
>
> > This seems to be a Rabbit problem with the names of the subcollections;
> > somehow URL helpers for the subcollections do not get generated
> > correctly. Michal ?
>
> Sry, I overlooked this one :)
>
> I checked the patch and seems you are trying to use 'system_volume_url'
> helper in driver (not in Sinatra context).
>
> To use this method, you will need to pass 'context' parameter and then
> call the helper using:
>
> context.system_volume_url(system_id, volume_id)
>
> (check David patch for example :-)
>
> -- Michal
>
> >
> > BTW, attached is a patch that addresses a few URL conversion problems
> I
> > found in the mock driver; feel free to commit if it doesn't make things
> > worse ;)
> >
> > David
> >
>
>
> --
>
> Michal Fojtik <mf...@redhat.com>
> Deltacloud API, CloudForms
Re: [PATCH] added Template subcollections to SystemTemplate
Posted by Michal Fojtik <mf...@redhat.com>.
On 04/02/2013 02:26 AM, David Lutterkort wrote:
> This seems to be a Rabbit problem with the names of the subcollections;
> somehow URL helpers for the subcollections do not get generated
> correctly. Michal ?
Sry, I overlooked this one :)
I checked the patch and seems you are trying to use 'system_volume_url'
helper in driver (not in Sinatra context).
To use this method, you will need to pass 'context' parameter and then
call the helper using:
context.system_volume_url(system_id, volume_id)
(check David patch for example :-)
-- Michal
>
> BTW, attached is a patch that addresses a few URL conversion problems I
> found in the mock driver; feel free to commit if it doesn't make things
> worse ;)
>
> David
>
--
Michal Fojtik <mf...@redhat.com>
Deltacloud API, CloudForms
Re: [PATCH] added Template subcollections to SystemTemplate
Posted by David Lutterkort <lu...@redhat.com>.
On Tue, 2013-04-02 at 10:41 +0200, Michal Fojtik wrote:
> On 04/02/2013 02:26 AM, David Lutterkort wrote:
> > On Thu, 2013-03-28 at 12:19 +1100, Koper, Dies wrote:
> >> A bit of background on this patch:
> >> It contains my latest work on implementing cimi systems and system
> >> templates for mock and fgcp.
> >> It is not complete and contains some debugging statements. Its main
> >> purpose is to show you what I have and what issues I'm seeing.
> >>
> >> It applies to master (fcffad13e66175152cf8a43d615b79727902b5ee) and
> >> requires "[PATCH] CIMI schema: tolerate nil hash_map attributes" to
> >> solve a nil error for system templates with fgcp.
> >>
> >> Issues I'm having:
> >>
> >> (1)
> >> With mock, even unsupported subcollections are shown when retrieving
> >> systems. With fgcp they are also shown, and their href urls are broken:
> >
> > This is a shortcoming of how we populate models right now - there's no
> > way to suppress collections that the driver doesn't support. In fact,
> > the code goes through great length to make sure we generate the href for
> > empty collections (since that href is needed to add the first element to
> > an empty collection)
> >
> > The easiest way to 'fix' this for now is to comment those unsupported
> > collections out in system.rb.
> >
> > A better fix is to add a mechanism similar to what we do for $select to
> > models; call that the 'exclude' mechanism. The various generate_xxx
> > methods should set the that exclude parameter based on collections that
> > are declared in Rabbit, but not available (Rabbit knows that, Michal:
> > how do we get that info out of Rabbit ?)
>
> How about this?
>
> module CIMI::Helper
>
> def supported_collections
> CIMI::Service::CloudEntryPoint.create(self).entities
> end
>
> end
>
> All 'Base' classes are in fact 'Sinatra::Base' and you can use all
> defined Sinatra helpers in the Rabbit operation control block.
>
> Having this, you can do then following in all Service models:
>
> context.supported_collections.include? Collection
I don't think this is quite right - what I am looking for is for an easy
way to determine which of the subcollections of, for example, System are
supported by the current driver - for example, whether
System.credentials should be populated or not.
Rabbit knows that because of the various with_capability statements on
operations for each subcollection. When the service object (or the
driver, in the case of systems) sets up a model object, we'd like to
completely omit System.credentials if the driver doesn't support it.
If I can get my hands on which collections are supported according to
Rabbit, it would be easy enough to write some code to omit unsupported
collections.
David
Re: [PATCH] added Template subcollections to SystemTemplate
Posted by Michal Fojtik <mf...@redhat.com>.
On 04/02/2013 02:26 AM, David Lutterkort wrote:
> On Thu, 2013-03-28 at 12:19 +1100, Koper, Dies wrote:
>> A bit of background on this patch:
>> It contains my latest work on implementing cimi systems and system
>> templates for mock and fgcp.
>> It is not complete and contains some debugging statements. Its main
>> purpose is to show you what I have and what issues I'm seeing.
>>
>> It applies to master (fcffad13e66175152cf8a43d615b79727902b5ee) and
>> requires "[PATCH] CIMI schema: tolerate nil hash_map attributes" to
>> solve a nil error for system templates with fgcp.
>>
>> Issues I'm having:
>>
>> (1)
>> With mock, even unsupported subcollections are shown when retrieving
>> systems. With fgcp they are also shown, and their href urls are broken:
>
> This is a shortcoming of how we populate models right now - there's no
> way to suppress collections that the driver doesn't support. In fact,
> the code goes through great length to make sure we generate the href for
> empty collections (since that href is needed to add the first element to
> an empty collection)
>
> The easiest way to 'fix' this for now is to comment those unsupported
> collections out in system.rb.
>
> A better fix is to add a mechanism similar to what we do for $select to
> models; call that the 'exclude' mechanism. The various generate_xxx
> methods should set the that exclude parameter based on collections that
> are declared in Rabbit, but not available (Rabbit knows that, Michal:
> how do we get that info out of Rabbit ?)
How about this?
module CIMI::Helper
def supported_collections
CIMI::Service::CloudEntryPoint.create(self).entities
end
end
All 'Base' classes are in fact 'Sinatra::Base' and you can use all
defined Sinatra helpers in the Rabbit operation control block.
Having this, you can do then following in all Service models:
context.supported_collections.include? Collection
> Similar to how CIMI::Model::Resource has @select_attrs it should also
> have @exclude_attrs that are consulted in prepare and cause the
> corresponding collection to be set to nil (which will remove it from the
> XML/JSON output)
>
>> (2)
>> With mock (not with fgcp), when I list a system's volumes, its id is not
>> generated correctly:
>>
>> d:\projects>curl --user mockuser:mockpassword
>> http://localhost:3001/cimi/systems/system2/volumes?format=xml
>> <Collection xmlns="http://schemas.dmtf.org/cimi/1"
>> resourceURI="http://schemas.dmtf.org/cimi/1/SystemVolumeCollection">
>> <id>http://localhost:3001/cimi/system/system2/volumes</id>
>> <count>1</count>
>> <SystemVolume>
>> <id>http://localhost:3001/cimi/volumes?id=sysvol1</id>
>> ...
>>
>> That should be http://localhost:3001/cimi/volumes/sysvol1.
>> Note my comment in mock_driver_cimi_methods.rb#system_volumes about the
>> 1st arg I pass to convert_cimi_mock_urls:
>>
>> #FIXME: with ":volumes", delete url becomes
>> 'http://localhost:3001/cimi/volumes?id=sysvol1'
>> #with ":system_volume" or ":system_volumes", undefined method
>> `system_volume_url' for #<CIMI::Collections::Systems:0x44fe338> in
>> mock_driver_cimi_methods.rb:261
>> volumes.map{|vol|convert_cimi_mock_urls(:volumes, vol,
>> opts[:env])}.flatten
>
> This seems to be a Rabbit problem with the names of the subcollections;
> somehow URL helpers for the subcollections do not get generated
> correctly. Michal ?
>
> BTW, attached is a patch that addresses a few URL conversion problems I
> found in the mock driver; feel free to commit if it doesn't make things
> worse ;)
>
> David
>
--
Michal Fojtik <mf...@redhat.com>
Deltacloud API, CloudForms