launchpad-reviewers team mailing list archive

[Raphaël Badin <raphael.badin@xxxxxxxxxxxxx>]

Raphaël Badin has proposed merging lp:~rvb/maas/empty-tag-bug-131232 into lp:maas.

Commit message:
Ignore tags without a definition in node.update_hardware_details.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1131418 in MAAS: "Nodes don't go to ready, after commissioning they get a 500 error when reporting back to maas"
  https://bugs.launchpad.net/maas/+bug/1131418

For more details, see:
https://code.launchpad.net/~rvb/maas/empty-tag-bug-131232/+merge/160672

Support for tags without a definition (aka empty tags) was added here: https://code.launchpad.net/~gz/maas/manual_tags/+merge/131232. A special case was missing: node.update_hardware_details() needs to skip empty tags.
-- 
https://code.launchpad.net/~rvb/maas/empty-tag-bug-131232/+merge/160672
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/maas/empty-tag-bug-131232 into lp:maas.

=== modified file 'src/maasserver/models/node.py'
--- src/maasserver/models/node.py	2013-02-28 02:37:57 +0000
+++ src/maasserver/models/node.py	2013-04-24 14:46:33 +0000
@@ -396,6 +396,8 @@
     node.cpu_count = cpu_count or 0
     node.memory = memory
     for tag in tag_manager.all():
+        if not tag.definition:
+            continue
         has_tag = evaluator(tag.definition)
         if has_tag:
             node.tags.add(tag)

=== modified file 'src/maasserver/tests/test_node.py'
--- src/maasserver/tests/test_node.py	2013-02-28 02:37:57 +0000
+++ src/maasserver/tests/test_node.py	2013-04-24 14:46:33 +0000
@@ -641,6 +641,19 @@
         node = reload_object(node)
         self.assertEqual([], list(node.tags.all()))
 
+    def test_hardware_updates_ignores_empty_tags(self):
+        # Tags with empty definitions are ignored when
+        # node.set_hardware_details gets called.
+        factory.make_tag(definition='')
+        node = factory.make_node()
+        node.save()
+        xmlbytes = '<node/>'
+        node.set_hardware_details(xmlbytes)
+        node = reload_object(node)
+        # The real test is that node.set_hardware_details does not blow
+        # up, see bug 1131418.
+        self.assertEqual([], list(node.tags.all()))
+
     def test_fqdn_returns_hostname_if_dns_not_managed(self):
         nodegroup = factory.make_node_group(
             name=factory.getRandomString(),