launchpad-reviewers team mailing list archive

[John A Meinel <john@xxxxxxxxxxxxxxxxx>]

John A Meinel has proposed merging lp:~jameinel/maas/1.2-expose-kernel-opts-on-tag into lp:maas/1.2.

Commit message:
Expose kernel_opts as part of the Tag api's

This lets you set the options and query them in the API, and document how they work.

Requested reviews:
  MAAS Maintainers (maas-maintainers)

For more details, see:
https://code.launchpad.net/~jameinel/maas/1.2-expose-kernel-opts-on-tag/+merge/133239

I imagine there is more documentation that needs tweaking, since we'll now need a whole section about setting kernel options, and how that will interact with tags, etc. But at least this should be a reasonable start.

-- 
https://code.launchpad.net/~jameinel/maas/1.2-expose-kernel-opts-on-tag/+merge/133239
Your team MAAS Maintainers is requested to review the proposed merge of lp:~jameinel/maas/1.2-expose-kernel-opts-on-tag into lp:maas/1.2.

=== modified file 'src/maasserver/api.py'
--- src/maasserver/api.py	2012-11-02 09:48:02 +0000
+++ src/maasserver/api.py	2012-11-07 13:34:33 +0000
@@ -1420,6 +1420,7 @@
         'name',
         'definition',
         'comment',
+        'kernel_opts',
         )
 
     def read(self, request, name):
@@ -1540,6 +1541,11 @@
             It is meant as a human readable description of the tag.
         :param definition: An XPATH query that will be evaluated against the
             hardware_details stored for all nodes (output of `lshw -xml`).
+        :param kernel_opts: Can be None. If set, nodes associated with this tag
+            will add this string to their kernel options when booting. The
+            value overrides the global 'kernel_opts' setting. If more than one
+            tag is associated with a node, the one with the lowest alphabetical
+            name will be picked (eg 01-my-tag will be taken over 99-tag-name).
         """
         if not request.user.is_superuser:
             raise PermissionDenied()

=== modified file 'src/maasserver/forms.py'
--- src/maasserver/forms.py	2012-11-06 18:19:54 +0000
+++ src/maasserver/forms.py	2012-11-07 13:34:33 +0000
@@ -855,6 +855,7 @@
             'name',
             'comment',
             'definition',
+            'kernel_opts',
             )
 
     def clean_definition(self):

=== modified file 'src/maasserver/models/node.py'
--- src/maasserver/models/node.py	2012-11-07 10:16:45 +0000
+++ src/maasserver/models/node.py	2012-11-07 13:34:33 +0000
@@ -67,7 +67,10 @@
     get_db_state,
     strip_domain,
     )
-from maasserver.utils.orm import get_first
+from maasserver.utils.orm import (
+    get_first,
+    get_one,
+    )
 from piston.models import Token
 from provisioningserver.enum import (
     POWER_TYPE,
@@ -694,6 +697,7 @@
         else:
             return None
 
+<<<<<<< TREE
     def get_effective_kernel_options(self):
         """Determine any special kernel parameters for this node.
 
@@ -706,6 +710,28 @@
         global_value = Config.objects.get_config('kernel_opts')
         return None, global_value
 
+=======
+    def get_effective_kernel_options(self):
+        """Determine any special kernel parameters for this node.
+
+        :return: (tag, kernel_options)
+            tag is a Tag object or None. If None, the kernel_options came from
+            the global setting.
+            kernel_options, a string indicating extra kernel_options that
+            should be used when booting this node. May be None if no tags match
+            and no global setting has been configured.
+        """
+        # First, see if there are any tags associated with this node that has a
+        # custom kernel parameter
+        tags = self.tags.filter(kernel_opts__isnull=False)
+        tags = tags.order_by('name')[:1]
+        tag = get_one(tags)
+        if tag is not None:
+            return tag, tag.kernel_opts
+        global_value = Config.objects.get_config('kernel_opts')
+        return None, global_value
+
+>>>>>>> MERGE-SOURCE
     @property
     def work_queue(self):
         """The name of the queue for tasks specific to this node."""

=== modified file 'src/maasserver/tests/test_api.py'
--- src/maasserver/tests/test_api.py	2012-11-06 18:19:54 +0000
+++ src/maasserver/tests/test_api.py	2012-11-07 13:34:33 +0000
@@ -2999,6 +2999,30 @@
             % (invalid,))
         self.assertFalse(Tag.objects.filter(name=invalid).exists())
 
+    def test_POST_new_kernel_opts(self):
+        self.become_admin()
+        name = factory.getRandomString()
+        definition = '//node'
+        comment = factory.getRandomString()
+        kernel_opts = factory.getRandomString()
+        response = self.client.post(
+            self.get_uri('tags/'),
+            {
+                'op': 'new',
+                'name': name,
+                'comment': comment,
+                'definition': definition,
+                'kernel_opts': kernel_opts,
+            })
+        self.assertEqual(httplib.OK, response.status_code)
+        parsed_result = json.loads(response.content)
+        self.assertEqual(name, parsed_result['name'])
+        self.assertEqual(comment, parsed_result['comment'])
+        self.assertEqual(definition, parsed_result['definition'])
+        self.assertEqual(kernel_opts, parsed_result['kernel_opts'])
+        self.assertEqual(
+            kernel_opts, Tag.objects.filter(name=name)[0].kernel_opts)
+
     def test_POST_new_populates_nodes(self):
         self.become_admin()
         node1 = factory.make_node()

=== modified file 'src/maasserver/tests/test_node.py'
--- src/maasserver/tests/test_node.py	2012-11-07 10:16:45 +0000
+++ src/maasserver/tests/test_node.py	2012-11-07 13:34:33 +0000
@@ -303,17 +303,76 @@
         successful_types = [node_power_types[node] for node in started_nodes]
         self.assertItemsEqual(configless_power_types, successful_types)
 
-    def test_get_effective_kernel_options_with_nothing_set(self):
-        node = factory.make_node()
-        self.assertEqual((None, None), node.get_effective_kernel_options())
-
-    def test_get_effective_kernel_options_sees_global_config(self):
-        node = factory.make_node()
-        kernel_opts = factory.getRandomString()
-        Config.objects.set_config('kernel_opts', kernel_opts)
-        self.assertEqual(
-            (None, kernel_opts), node.get_effective_kernel_options())
-
+<<<<<<< TREE
+    def test_get_effective_kernel_options_with_nothing_set(self):
+        node = factory.make_node()
+        self.assertEqual((None, None), node.get_effective_kernel_options())
+
+    def test_get_effective_kernel_options_sees_global_config(self):
+        node = factory.make_node()
+        kernel_opts = factory.getRandomString()
+        Config.objects.set_config('kernel_opts', kernel_opts)
+        self.assertEqual(
+            (None, kernel_opts), node.get_effective_kernel_options())
+
+=======
+    def test_get_effective_kernel_options_with_nothing_set(self):
+        node = factory.make_node()
+        self.assertEqual((None, None), node.get_effective_kernel_options())
+
+    def test_get_effective_kernel_options_sees_global_config(self):
+        node = factory.make_node()
+        kernel_opts = factory.getRandomString()
+        Config.objects.set_config('kernel_opts', kernel_opts)
+        self.assertEqual(
+            (None, kernel_opts), node.get_effective_kernel_options())
+
+    def test_get_effective_kernel_options_not_confused_by_empty_tag(self):
+        node = factory.make_node()
+        tag = factory.make_tag()
+        node.tags.add(tag)
+        kernel_opts = factory.getRandomString()
+        Config.objects.set_config('kernel_opts', kernel_opts)
+        self.assertEqual(
+            (None, kernel_opts), node.get_effective_kernel_options())
+
+    def test_get_effective_kernel_options_ignores_unassociated_tag_value(self):
+        node = factory.make_node()
+        tag = factory.make_tag(kernel_opts=factory.getRandomString())
+        self.assertEqual((None, None), node.get_effective_kernel_options())
+
+    def test_get_effective_kernel_options_uses_tag_value(self):
+        node = factory.make_node()
+        tag = factory.make_tag(kernel_opts=factory.getRandomString())
+        node.tags.add(tag)
+        self.assertEqual(
+            (tag, tag.kernel_opts), node.get_effective_kernel_options())
+
+    def test_get_effective_kernel_options_tag_overrides_global(self):
+        node = factory.make_node()
+        global_opts = factory.getRandomString()
+        Config.objects.set_config('kernel_opts', global_opts)
+        tag = factory.make_tag(kernel_opts=factory.getRandomString())
+        node.tags.add(tag)
+        self.assertEqual(
+            (tag, tag.kernel_opts), node.get_effective_kernel_options())
+
+    def test_get_effective_kernel_options_uses_first_real_tag_value(self):
+        node = factory.make_node()
+        # Intentionally create them in reverse order, so the default 'db' order
+        # doesn't work, and we have asserted that we sort them.
+        tag3 = factory.make_tag(factory.make_name('tag-03-'),
+                                kernel_opts=factory.getRandomString())
+        tag2 = factory.make_tag(factory.make_name('tag-02-'),
+                                kernel_opts=factory.getRandomString())
+        tag1 = factory.make_tag(factory.make_name('tag-01-'), kernel_opts=None)
+        self.assertTrue(tag1.name < tag2.name)
+        self.assertTrue(tag2.name < tag3.name)
+        node.tags.add(tag1, tag2, tag3)
+        self.assertEqual(
+            (tag2, tag2.kernel_opts), node.get_effective_kernel_options())
+
+>>>>>>> MERGE-SOURCE
     def test_acquire(self):
         node = factory.make_node(status=NODE_STATUS.READY)
         user = factory.make_user()