Merge lp:~rvb/maas/add-power-params-doc into lp:~maas-committers/maas/trunk

[Raphaël Badin]

Add documentation about the power types and the related power parameters.

Here is how it looks like: http://people.canonical.com/~rvb/power_types_doc.png

The tests are minimal but testing the exact formatting would be a bit pointless I think. A test that the whole API doc can be translated from rST into HTML already exists.

[Julian Edwards (julian-edwards)]

I don't think we should be documenting this like this, other than some
examples perhaps.

This is because clusters can and will have varying power types available
- it's not a heterogeneous environment, especially during staged cluster
upgrades.

What I think we should do is implement an API function that returns
available power types and each one's parameters based on the cluster and
encourage API users to derive from that when editing/adding nodes.

[Raphaël Badin (rvb)]

> I don't think we should be documenting this like this, other than some
> examples perhaps.
>
> This is because clusters can and will have varying power types available
> - it's not a heterogeneous environment, especially during staged cluster
> upgrades.
>
> What I think we should do is implement an API function that returns
> available power types and each one's parameters based on the cluster and
> encourage API users to derive from that when editing/adding nodes.

An API call is probably needed indeed since some of the power types are registered dynamically.

But I still think it's worth documentation all the built-in power types. I'll change the introduction sentence to make it clear that those are the *built-in* power types and that others can be dynamically registered using clusters.

What do you say?

[Julian Edwards (julian-edwards)]

On 24/04/14 16:53, Raphaël Badin wrote:
>> I don't think we should be documenting this like this, other than some
>> examples perhaps.
>>
>> This is because clusters can and will have varying power types available
>> - it's not a heterogeneous environment, especially during staged cluster
>> upgrades.
>>
>> What I think we should do is implement an API function that returns
>> available power types and each one's parameters based on the cluster and
>> encourage API users to derive from that when editing/adding nodes.
>
> An API call is probably needed indeed since some of the power types are registered dynamically.
>
> But I still think it's worth documentation all the built-in power types. I'll change the introduction sentence to make it clear that those are the *built-in* power types and that others can be dynamically registered using clusters.
>
> What do you say?
>

Well all power types are built in, that's how MAAS rolls. I still
prefer a simple example of the more common types and an example of how
to use this new API call in conjunction with adding a node.

But this is just my opinion, see what others say too.

[Raphaël Badin (rvb)]

> Well all power types are built in, that's how MAAS rolls.

Not really, if you're using a custom-made cluster controller that registers an extra power type, that power type will be available in MAAS and it's not "built-in".

> I still
> prefer a simple example of the more common types and an example of how
> to use this new API call in conjunction with adding a node.

I really think documenting all the power types and power parameters we know about is better.

[Raphaël Badin (rvb)]

> But I still think it's worth documentation all the built-in power types. I'll
> change the introduction sentence to make it clear that those are the *built-
> in* power types and that others can be dynamically registered using clusters.

Done:

78 + line(
79 + "This is the list of the built-in power types and their "
80 + "associated power parameters. Note that other power types "
81 + "can be registered, at runtime, by cluster controllers with "
82 + "extra capabilities.")

[Gavin Panella (allenap)]

I think having some static docs on maas.ubuntu.com has some value.

However, I also think we ought to make all the docs available via the MAAS UI too, including dynamically (re)generating the code-derived docs. This would complement the API call that Julian suggests.

Once we have the docs-in-UI, the docs on maas.ubuntu.com should then have a section explaining to use the docs on your local MAAS installation for the most accurate information.

Right now these docs appear to be LISP by the count of braces.

review: Approve

=== modified file 'src/maasserver/api.py'
--- src/maasserver/api.py	2014-04-22 09:08:02 +0000
+++ src/maasserver/api.py	2014-04-24 08:03:14 +0000
@@ -149,6 +149,7 @@
     describe_resource,
     find_api_resources,
     generate_api_docs,
+    generate_power_types_doc,
     )
 from maasserver.clusterrpc.power_parameters import (
     get_all_power_types_from_clusters,
@@ -334,6 +335,8 @@
         :param power_type: The new power type for this node. If you use the
             default value, power_parameters will be set to the empty string.
             Available to admin users.
+            See the `Power types`_ section for a list of the available power
+            types.
         :type power_type: unicode
         :param power_parameters_{param1}: The new value for the 'param1'
             power parameter.  Note that this is dynamic as the available
@@ -342,6 +345,8 @@
             parameter is 'power_address' so one would want to pass 'myaddress'
             as the value of the 'power_parameters_power_address' parameter.
             Available to admin users.
+            See the `Power types`_ section for a list of the available power
+            parameters for each power type.
         :type power_parameters_{param1}: unicode
         :param power_parameters_skip_check: Whether or not the new power
             parameters for this node should be checked against the expected
@@ -2293,6 +2298,10 @@
                     line("  ", docline, sep="")
                 line()
 
+    line()
+    line()
+    line(generate_power_types_doc())
+
     return output.getvalue()
 
 
 
=== modified file 'src/maasserver/apidoc.py'
--- src/maasserver/apidoc.py	2014-03-27 15:12:48 +0000
+++ src/maasserver/apidoc.py	2014-04-24 08:03:14 +0000
@@ -19,6 +19,8 @@
     "generate_api_docs",
     ]
 
+from cStringIO import StringIO
+from functools import partial
 from inspect import getdoc
 from itertools import izip_longest
 
@@ -32,6 +34,7 @@
 from piston.doc import generate_doc
 from piston.handler import BaseHandler
 from piston.resource import Resource
+from provisioningserver.power_schema import JSON_POWER_TYPE_PARAMETERS
 
 
 def accumulate_api_resources(resolver, accumulator):
@@ -67,6 +70,49 @@
     return accumulator
 
 
+def generate_power_types_doc():
+    """Generate ReST documentation for the supported power types.
+
+    The documentation is derived from the `JSON_POWER_TYPE_PARAMETERS`
+    object.
+    """
+    output = StringIO()
+    line = partial(print, file=output)
+
+    line('Power types')
+    line('-----------')
+    line()
+    line("This is the list of the supported power types and their "
+         "associated power parameters.  Note that the list of usable "
+         "power types for a particular cluster might be a subset of this "
+         "list if the cluster in question is from an older version of "
+         "MAAS.")
+    line()
+    for item in JSON_POWER_TYPE_PARAMETERS:
+        title = "%s (%s)" % (item['name'], item['description'])
+        line(title)
+        line('=' * len(title))
+        line('')
+        line("Power parameters:")
+        line('')
+        for field in item['fields']:
+            field_description = []
+            field_description.append(
+                "* %s (%s)." % (field['name'], field['label']))
+            choices = field.get('choices', [])
+            if len(choices) > 0:
+                field_description.append(
+                    " Choices: %s" % ', '.join(
+                        "'%s' (%s)" % (choice[0], choice[1])
+                        for choice in choices))
+            default = field.get('default', '')
+            if default is not '':
+                field_description.append("  Default: '%s'." % default)
+            line(''.join(field_description))
+        line('')
+    return output.getvalue()
+
+
 def generate_api_docs(resources):
     """Generate ReST documentation objects for the ReST API.
 
 
=== modified file 'src/maasserver/tests/test_apidoc.py'
--- src/maasserver/tests/test_apidoc.py	2014-03-27 15:16:18 +0000
+++ src/maasserver/tests/test_apidoc.py	2014-04-24 08:03:14 +0000
@@ -25,6 +25,7 @@
     )
 from django.core.exceptions import ImproperlyConfigured
 from django.core.urlresolvers import reverse
+from maasserver import apidoc
 from maasserver.api_support import (
     operation,
     OperationsHandler,
@@ -35,6 +36,7 @@
     describe_resource,
     find_api_resources,
     generate_api_docs,
+    generate_power_types_doc,
     )
 from maasserver.testing.factory import factory
 from maasserver.testing.testcase import MAASServerTestCase
@@ -42,6 +44,8 @@
 from piston.doc import HandlerDocumentation
 from piston.handler import BaseHandler
 from piston.resource import Resource
+from provisioningserver.power_schema import make_json_field
+from testtools.matchers import ContainsAll
 
 
 class TestFindingResources(MAASServerTestCase):
@@ -306,3 +310,29 @@
             "name": "ExampleHandler",
             }
         self.assertEqual(expected, describe_resource(resource))
+
+
+class TestGeneratePowerTypesDoc(MAASServerTestCase):
+    """Tests for `generate_power_types_doc`."""
+
+    def test__generate_power_types_doc_generates_doc(self):
+        doc = generate_power_types_doc()
+        self.assertThat(doc, ContainsAll(["Power types", "IPMI"]))
+
+    def test__generate_power_types_doc_generates_describes_power_type(self):
+        name = factory.make_name('name')
+        description = factory.make_name('description')
+        param_name = factory.make_name('param_name')
+        param_description = factory.make_name('param_description')
+        json_fields = [{
+            'name': name,
+            'description': description,
+            'fields': [
+                make_json_field(param_name, param_description),
+            ],
+        }]
+        self.patch(apidoc, "JSON_POWER_TYPE_PARAMETERS", json_fields)
+        doc = generate_power_types_doc()
+        self.assertThat(
+            doc,
+            ContainsAll([name, description, param_name, param_description]))
 
=== modified file 'src/provisioningserver/power_schema.py'
--- src/provisioningserver/power_schema.py	2014-04-22 08:27:47 +0000
+++ src/provisioningserver/power_schema.py	2014-04-24 08:03:14 +0000
@@ -164,7 +164,7 @@
     },
     {
         'name': 'virsh',
-        'description': 'virsh (virtual systems)',
+        'description': 'Virsh (virtual systems)',
         'fields': [
             make_json_field('power_address', "Power address"),
             make_json_field('power_id', "Power ID"),