Merge lp:~tycho-s/maas/add-missing-node-fields into lp:~maas-committers/maas/trunk

[Tycho Andersen]

Add cpu_count, memory, storage to the list of updateable fields via the API.

Add cpu_count, memory, storage to the list of updateable fields via the API.

[John A Meinel (jameinel)]

Aren't these fields ones that are pulled from the hardware inventory, and the next run of the tags processing is going to reset them to whatever the value was before?

[Julian Edwards (julian-edwards)]

Indeed - it's why they're not writable by the API or the UI, the are supposed
to come from hardware inventory. If that process is not working, we should
fix it, not paper over it.

However, I'll change my vote if you can justify it :)

review: Disapprove

[Gavin Panella (allenap)]

Fwiw, I would have approved this but for the reasons already mentioned.
It's a good change. Have a quick (<5 minute) pre-implementation call next
time, with any of us.

[Tycho Andersen (tycho-s)]

On Wed, Nov 27, 2013 at 11:01:54PM -0000, Julian Edwards wrote:
> Review: Disapprove
>
> Indeed - it's why they're not writable by the API or the UI, the are supposed
> to come from hardware inventory. If that process is not working, we should
> fix it, not paper over it.
>
> However, I'll change my vote if you can justify it :)

If I create a node via `maas-cli maas nodes new', there is currently
no way to set the hwinfo that I can see. I spoke with jtv, and he
seemed to think this was an oversight. That said, is there any reason
/not/ to support this? What happens if I add some RAM to a box?

[Tycho Andersen (tycho-s)]

On Wed, Nov 27, 2013 at 11:09:17PM -0000, Gavin Panella wrote:
> Fwiw, I would have approved this but for the reasons already mentioned.
> It's a good change. Have a quick (<5 minute) pre-implementation call next
> time, with any of us.

Yes, I did speak with jtv.

[Gavin Panella (allenap)]

> On Wed, Nov 27, 2013 at 11:01:54PM -0000, Julian Edwards wrote:
> > Review: Disapprove
> >
> > Indeed - it's why they're not writable by the API or the UI, the are
> supposed
> > to come from hardware inventory. If that process is not working, we should
> > fix it, not paper over it.
> >
> > However, I'll change my vote if you can justify it :)
>
> If I create a node via `maas-cli maas nodes new', there is currently
> no way to set the hwinfo that I can see. I spoke with jtv, and he
> seemed to think this was an oversight. That said, is there any reason
> /not/ to support this? What happens if I add some RAM to a box?

In theory, that would be done by a mini-commissioning cycle. However, we don't have one, though we can force a machine through a full commissioning cycle. That means deallocating the machine from the user, which may not be desirable. There's merit in letting people fix their own data while MAAS does not provide convenient means to update itself.

I don't want to flip-flop on this, so let's see what others think. Jeroen was in favour, I'm on the fence. Does anyone else want to chime in?

As a compromise, we could land this so that people using dailies can use it. However, we'd also file a bug against 14.04 to review its inclusion in the LTS; perhaps there would be a better way to do it at that point.

[Julian Edwards (julian-edwards)]

So, if you add a new node it is supposed to go through commissioning and that will set the hwinfo. However, your more compelling case is the one where you change the RAM, for example, and don't want to deallocate the machine. Until we have a better solution to do that automatically then I'm happy for this to land.

Thanks for doing it!

review: Approve

=== modified file 'src/maasserver/forms.py'
--- src/maasserver/forms.py	2013-11-25 10:33:08 +0000
+++ src/maasserver/forms.py	2013-11-27 17:08:38 +0000
@@ -199,6 +199,16 @@
     'power_parameters'.
     """
 
+    cpu_count = forms.IntegerField(
+        required=False, initial=0,
+        label="CPU Count")
+    memory = forms.IntegerField(
+        required=False, initial=0,
+        label="Memory")
+    storage = forms.IntegerField(
+        required=False, initial=0,
+        label="Disk space")
+
     class Meta:
         model = Node
 
@@ -207,6 +217,9 @@
         fields = NodeForm.Meta.fields + (
             'power_type',
             'power_parameters',
+            'cpu_count',
+            'memory',
+            'storage',
             )
 
     def __init__(self, data=None, files=None, instance=None, initial=None):

=== modified file 'src/maasserver/tests/test_api_node.py'
--- src/maasserver/tests/test_api_node.py	2013-10-18 16:35:07 +0000
+++ src/maasserver/tests/test_api_node.py	2013-11-27 17:08:38 +0000
@@ -537,6 +537,20 @@
             {'mac_address': new_power_address},
             reload_object(node).power_parameters)
 
+    def test_PUT_updates_cpu_memory_storage(self):
+        self.become_admin()
+        node = factory.make_node(
+            owner=self.logged_in_user,
+            power_type=POWER_TYPE.WAKE_ON_LAN)
+        response = self.client_put(
+            self.get_node_uri(node),
+            {'cpu_count': 1, 'memory': 1024, 'storage': 2048})
+        self.assertEqual(httplib.OK, response.status_code)
+        node = reload_object(node)
+        self.assertEqual(1, node.cpu_count)
+        self.assertEqual(1024, node.memory)
+        self.assertEqual(2048, node.storage)
+
     def test_PUT_updates_power_parameters_accepts_only_mac_for_wol(self):
         self.become_admin()
         node = factory.make_node(

=== modified file 'src/maasserver/tests/test_forms.py'
--- src/maasserver/tests/test_forms.py	2013-10-20 19:50:56 +0000
+++ src/maasserver/tests/test_forms.py	2013-11-27 17:08:38 +0000
@@ -336,6 +336,9 @@
                 'distro_series',
                 'power_type',
                 'power_parameters',
+                'cpu_count',
+                'memory',
+                'storage',
             ],
             list(form.fields))