On 05/05/2011 07:35 AM, Mark McLoughlin wrote:
Hi Mo,
On Thu, 2011-05-05 at 00:41 -0400, Mo Morsi wrote:
> Introduces a generic web puppet resource type to access various web
> apis from puppet. Implements a provider of this type using curl to
> issue the requests.
>
> Then uses this provider to issue requests to the conductor api to
> create providers and hardware profiles, analyzing the results for
> errors
Tricky stuff! Even though curl.rb is considerably more complex than
exec-ing the curl command lines, the resulting web resources are much
nicer. Cool stuff :)
Thanks. Agree, on the backend its a little more complicated than just
calling out to curl, but the result is the interface is much more
cleaner and we can do alot more in terms of verifying the result and
output to make sure the operation proceeded as intended.
ACK from my FWIW, but some comments inline
> diff --git a/recipes/aeolus_recipe/lib/puppet/provider/web/curl.rb
b/recipes/aeolus_recipe/lib/puppet/provider/web/curl.rb
> new file mode 100644
> index 0000000..b5f4468
> --- /dev/null
> +++ b/recipes/aeolus_recipe/lib/puppet/provider/web/curl.rb
....
> +# Puppet provider definition
> +Puppet::Type.type(:web).provide :curl do
> + desc "Use curl to access web resources"
> +
> + def http_method
> + @request_method
> + end
> +
> + def http_method=(request_method)
> + begin
> + # Set request method and generate a unique session key
> + @request_method = request_method
> + session = "/tmp/#{UUID.new.generate}"
> +
> + # Invoke a login request if necessary
...
I'm a bit surprised to see all this happening in http_method=
Care to explain why, just for my own education? :)
Sure. When it comes to puppet resource types and providers you can only
perform operations in properties, for example in the 'service' provider
type, setting the 'ensure' property to 'running' will invoke 'service
<name> start', while setting the 'enable' property to 'true'
will invoke
'chkconfig <name> on'.
Honestly wasn't sure what the best name for the property to actual
perform the operation in in this case, if you have a better suggestion
than 'http_method', I'm all ears.
> +define aeolus::conductor::provider($type="",$url=""){
> + web{ "provider-$name":
> + uri => "https://localhost/conductor/admin/providers",
> + http_method => 'post',
> + parameters => { 'provider[name]' => $name,
'provider[url]' => $url,
> + 'provider[provider_type_codename]' => $type },
> + returns => '200',
> + verify => '.*Provider added.*',
> + follow => true,
> + unless => { 'http_method' => 'get',
> + 'uri' =>
'https://localhost/conductor/admin/providers',
> + 'verify' => ".*$name.*" },
Would something like this be better:
unless => { http_method => 'get',
uri =>
'https://localhost/conductor/admin/providers',
parameters => { 'q' => $name },
verify =>
".*/admin/providers/.*>$name</a>.*" }
i.e. run a search query and use a more specific regexp?
Well it actually does run a search query as is right now. And the nice
thing is that we can set the regex to whatever we want to look for in
the aeolus-configuration recipe without modifying the backend web
provider itself.
At some point though I would like to expand this to not only be able to
supply a regex but an xpath location at which that regex should appear.
Also, now that I type that ... it looks like "unless" doesn't re-use the
web type, right? i.e. parameters are automatically provided? It'd be
cool if it re-used the same type somehow
It does reuse the web type. You can specify the parameters as you
suggested as is right now.
At some point I would like to add an 'if' parameter, eg only invoke the
web request if the 'if' request yields true.
> +define aeolus::conductor::hwp($memory='', $cpu='',
$storage='', $architecture=''){
> + web{ "hwp-$name":
> + uri =>
"https://localhost/conductor/admin/hardware_profiles",
> + http_method => 'post',
> + parameters => {'hardware_profile[name]' => $name,
> + 'hardware_profile[memory_attributes][value]' =>
$memory,
> + 'hardware_profile[cpu_attributes][value]' =>
$cpu,
> + 'hardware_profile[storage_attributes][value]' =>
$storage,
> + 'hardware_profile[architecture_attributes][value]' =>
$architecture,
> + 'hardware_profile[memory_attributes][name]' =>
'memory',
> + 'hardware_profile[memory_attributes][unit]' =>
'MB',
> + 'hardware_profile[cpu_attributes][name]' =>
'cpu',
> + 'hardware_profile[cpu_attributes][unit]' =>
'count',
> + 'hardware_profile[storage_attributes][name]' =>
'storage',
> + 'hardware_profile[storage_attributes][unit]' =>
'GB',
> + 'hardware_profile[architecture_attributes][name]' =>
'architecture',
> + 'hardware_profile[architecture_attributes][unit]' =>
'label',
> + 'commit' => 'Save'},
> + returns => '200',
> + #verify => '.*Hardware profile added.*',
> + follow => true,
> + unless => { 'http_method' => 'get',
> + 'uri' =>
'https://localhost/conductor/admin/hardware_profiles',
> + 'verify' => ".*$name.*" },
Is there something weird going on with GET on the hardware_profiles
resource?
Without 'Accept: text/html', I'm seeing:
The error occurred while evaluating nil.[]):
app/controllers/admin/hardware_profiles_controller.rb:206:in
`build_hardware_profile'
app/controllers/admin/hardware_profiles_controller.rb:14:in `index'
app/controllers/admin/hardware_profiles_controller.rb:11:in `index'
compass (0.10.4)
lib/compass/app_integration/rails/actionpack2/action_controller.rb:7:in `process'
i.e. it's following the "format.js" path and breaking. Is that a
conductor bug?
Not fully sure what the issue your describing here. Do we need to
specify this additional header in the request for this to work? How does
format.js play into this?
It seemed to be fully working when I tried it out, not sure what the
difference between our environments is. If there is an error in the
conductor logs due to this request, I'm tempted to say its an issue on
that side (since the request looks like it should be valid). We'll need
to debug this further tho.
If you supply 'Accept: text/html' does the commented out
"verify" above
start working? An accept header seems like a reasonable parameter for
the web type to support
The reason verify is commented out in this case, is that even if the
request succeeds, the conductor wui doesn't display 'HWP added' or
anything similar. This should be added to the conductor so that we can
verify the operation here, or better yet we should provide a restful api
in the conductor so that we can verify the resulting http status code.
> diff --git a/recipes/aeolus_recipe/manifests/defaults.pp
b/recipes/aeolus_recipe/manifests/defaults.pp
> index f5c74c3..57b79f6 100644
> --- a/recipes/aeolus_recipe/manifests/defaults.pp
> +++ b/recipes/aeolus_recipe/manifests/defaults.pp
> @@ -8,3 +8,16 @@
>
> Package {provider => 'rpm'}
>
> +$admin_user='admin'
> +$admin_password='password'
> +
> +# Setup the default login/logout targets for web requests
> +Web{
> + login => { 'http_method' => 'post',
> + 'uri' =>
'https://localhost/conductor/user_session',
> + 'user_session[login]' => "$admin_user",
> + 'user_session[password]' =>
"$admin_password",
> + 'commit' => 'submit' },
> + logout => { 'http_method' => 'post',
> + 'uri' =>
'https://localhost/conductor/logout' }
A GET on /logout will work too, a POST isn't needed
Ah good point, will change this before I push the patch.
Thanks for the feedback / ACK.
-Mo