← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~jtv/maas/lshw-as-commissioning-script into lp:maas

 

Jeroen T. Vermeulen has proposed merging lp:~jtv/maas/lshw-as-commissioning-script into lp:maas.

Commit message:
Turn 01-lshw into a built-in commissioning script.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~jtv/maas/lshw-as-commissioning-script/+merge/139641

This is part of a two-step development discussed with Julian.  The goal is to get the main commissioning script downloading any custom commissioning scripts from the metadata service (as a tarball -- API for this already exists) and executing them in runparts fashion.

Code for executing the scripts is already in place, but currently the main commissioning script just writes its own built-in commissioning script, 01-lshw, directly into the node's filesystem.  The branch you're looking at includes that lshw script into the tarball that the metadata service provides, renamed to 00-maas-01-lshw to match our documented naming scheme.  This also involves a data migration: all commissioning results named 01-lshw.out now become 00-maas-01-lshw.out.

The next step will be to remove the "add_script" function and the creation of the lshw script from the main commissioning script, and replace it with the download and unpacking of the commissioning tarball.  This will then include the new lshw script as provided by the server.


Jeroen
-- 
https://code.launchpad.net/~jtv/maas/lshw-as-commissioning-script/+merge/139641
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/maas/lshw-as-commissioning-script into lp:maas.
=== modified file 'etc/maas/commissioning-user-data'
--- etc/maas/commissioning-user-data	2012-11-30 17:35:04 +0000
+++ etc/maas/commissioning-user-data	2012-12-13 09:37:21 +0000
@@ -193,7 +193,7 @@
 }
 
 ### begin writing files ###
-add_script "01-lshw" <<"END_LSHW"
+add_script "00-maas-01-lshw" <<"END_LSHW"
 #!/bin/sh
 lshw -xml
 END_LSHW

=== modified file 'src/metadataserver/api.py'
--- src/metadataserver/api.py	2012-11-30 04:27:00 +0000
+++ src/metadataserver/api.py	2012-12-13 09:37:21 +0000
@@ -191,7 +191,7 @@
         """Store commissioning result files for `node`."""
         for name, uploaded_file in request.FILES.items():
             raw_content = uploaded_file.read()
-            if name == "01-lshw.out":
+            if name == "00-maas-01-lshw.out":
                 node.set_hardware_details(raw_content)
             contents = raw_content.decode('utf-8')
             NodeCommissionResult.objects.store_data(node, name, contents)

=== added file 'src/metadataserver/migrations/0005_rename_lshw_commissioning_output.py'
--- src/metadataserver/migrations/0005_rename_lshw_commissioning_output.py	1970-01-01 00:00:00 +0000
+++ src/metadataserver/migrations/0005_rename_lshw_commissioning_output.py	2012-12-13 09:37:21 +0000
@@ -0,0 +1,167 @@
+# -*- coding: utf-8 -*-
+import datetime
+
+from django.db import models
+from south.db import db
+from south.v2 import DataMigration
+
+
+def rename_commissioning_results(orm, old_name, new_name):
+    """Rename any `NodeCommissionResult` called `old_name` to `new_name`."""
+    ncrs = orm['metadataserver.NodeCommissionResult'].objects
+    ncrs.filter(name=old_name).update(name=new_name)
+
+
+class Migration(DataMigration):
+    """Rename lshw output in accordance with new naming convention.
+
+    The commissioning results for "lshw" were written as 01-lshw.out: the
+    output of 01-lshw.  But our naming convention reserves such names for
+    user-provided commissioning scripts.  MAAS-internal commissioning
+    scripts have names starting with 00-maas-*.
+    """
+
+    def forwards(self, orm):
+        rename_commissioning_results(orm, '01-lshw.out', '00-maas-01-lshw.out')
+
+    def backwards(self, orm):
+        rename_commissioning_results(orm, '00-maas-01-lshw.out', '01-lshw.out')
+
+    models = {
+        'auth.group': {
+            'Meta': {'object_name': 'Group'},
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'name': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '80'}),
+            'permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'})
+        },
+        'auth.permission': {
+            'Meta': {'ordering': "('content_type__app_label', 'content_type__model', 'codename')", 'unique_together': "(('content_type', 'codename'),)", 'object_name': 'Permission'},
+            'codename': ('django.db.models.fields.CharField', [], {'max_length': '100'}),
+            'content_type': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['contenttypes.ContentType']"}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'name': ('django.db.models.fields.CharField', [], {'max_length': '50'})
+        },
+        'auth.user': {
+            'Meta': {'object_name': 'User'},
+            'date_joined': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}),
+            'email': ('django.db.models.fields.EmailField', [], {'unique': 'True', 'max_length': '75', 'blank': 'True'}),
+            'first_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}),
+            'groups': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Group']", 'symmetrical': 'False', 'blank': 'True'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'is_active': ('django.db.models.fields.BooleanField', [], {'default': 'True'}),
+            'is_staff': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+            'is_superuser': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+            'last_login': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}),
+            'last_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}),
+            'password': ('django.db.models.fields.CharField', [], {'max_length': '128'}),
+            'user_permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'}),
+            'username': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '30'})
+        },
+        'contenttypes.contenttype': {
+            'Meta': {'ordering': "('name',)", 'unique_together': "(('app_label', 'model'),)", 'object_name': 'ContentType', 'db_table': "'django_content_type'"},
+            'app_label': ('django.db.models.fields.CharField', [], {'max_length': '100'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'model': ('django.db.models.fields.CharField', [], {'max_length': '100'}),
+            'name': ('django.db.models.fields.CharField', [], {'max_length': '100'})
+        },
+        u'maasserver.node': {
+            'Meta': {'object_name': 'Node'},
+            'after_commissioning_action': ('django.db.models.fields.IntegerField', [], {'default': '0'}),
+            'architecture': ('django.db.models.fields.CharField', [], {'default': "u'i386/generic'", 'max_length': '31'}),
+            'cpu_count': ('django.db.models.fields.IntegerField', [], {'default': '0'}),
+            'created': ('django.db.models.fields.DateTimeField', [], {}),
+            'distro_series': ('django.db.models.fields.CharField', [], {'default': 'None', 'max_length': '10', 'null': 'True', 'blank': 'True'}),
+            'error': ('django.db.models.fields.CharField', [], {'default': "u''", 'max_length': '255', 'blank': 'True'}),
+            'hardware_details': ('maasserver.fields.XMLField', [], {'default': 'None', 'null': 'True', 'blank': 'True'}),
+            'hostname': ('django.db.models.fields.CharField', [], {'default': "u''", 'unique': 'True', 'max_length': '255', 'blank': 'True'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'memory': ('django.db.models.fields.IntegerField', [], {'default': '0'}),
+            'netboot': ('django.db.models.fields.BooleanField', [], {'default': 'True'}),
+            'nodegroup': ('django.db.models.fields.related.ForeignKey', [], {'to': u"orm['maasserver.NodeGroup']", 'null': 'True'}),
+            'owner': ('django.db.models.fields.related.ForeignKey', [], {'default': 'None', 'to': "orm['auth.User']", 'null': 'True', 'blank': 'True'}),
+            'power_parameters': ('maasserver.fields.JSONObjectField', [], {'default': "u''", 'blank': 'True'}),
+            'power_type': ('django.db.models.fields.CharField', [], {'default': "u''", 'max_length': '10', 'blank': 'True'}),
+            'status': ('django.db.models.fields.IntegerField', [], {'default': '0', 'max_length': '10'}),
+            'system_id': ('django.db.models.fields.CharField', [], {'default': "u'node-7aa50e22-44f0-11e2-8a6a-fa163e5288ba'", 'unique': 'True', 'max_length': '41'}),
+            'tags': ('django.db.models.fields.related.ManyToManyField', [], {'to': u"orm['maasserver.Tag']", 'symmetrical': 'False'}),
+            'token': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['piston.Token']", 'null': 'True'}),
+            'updated': ('django.db.models.fields.DateTimeField', [], {})
+        },
+        u'maasserver.nodegroup': {
+            'Meta': {'object_name': 'NodeGroup'},
+            'api_key': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '18'}),
+            'api_token': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['piston.Token']", 'unique': 'True'}),
+            'cluster_name': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '100', 'blank': 'True'}),
+            'created': ('django.db.models.fields.DateTimeField', [], {}),
+            'dhcp_key': ('django.db.models.fields.CharField', [], {'default': "u''", 'max_length': '255', 'blank': 'True'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'maas_url': ('django.db.models.fields.CharField', [], {'default': "u''", 'max_length': '255', 'blank': 'True'}),
+            'name': ('django.db.models.fields.CharField', [], {'max_length': '80', 'blank': 'True'}),
+            'status': ('django.db.models.fields.IntegerField', [], {'default': '0'}),
+            'updated': ('django.db.models.fields.DateTimeField', [], {}),
+            'uuid': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '36'})
+        },
+        u'maasserver.tag': {
+            'Meta': {'object_name': 'Tag'},
+            'comment': ('django.db.models.fields.TextField', [], {'blank': 'True'}),
+            'created': ('django.db.models.fields.DateTimeField', [], {}),
+            'definition': ('django.db.models.fields.TextField', [], {'blank': 'True'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'kernel_opts': ('django.db.models.fields.TextField', [], {'null': 'True', 'blank': 'True'}),
+            'name': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '256'}),
+            'updated': ('django.db.models.fields.DateTimeField', [], {})
+        },
+        u'metadataserver.commissioningscript': {
+            'Meta': {'object_name': 'CommissioningScript'},
+            'content': ('metadataserver.fields.BinaryField', [], {}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'name': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '255'})
+        },
+        u'metadataserver.nodecommissionresult': {
+            'Meta': {'unique_together': "((u'node', u'name'),)", 'object_name': 'NodeCommissionResult'},
+            'data': ('django.db.models.fields.CharField', [], {'max_length': '1048576'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'name': ('django.db.models.fields.CharField', [], {'max_length': '100'}),
+            'node': ('django.db.models.fields.related.ForeignKey', [], {'to': u"orm['maasserver.Node']"})
+        },
+        u'metadataserver.nodekey': {
+            'Meta': {'object_name': 'NodeKey'},
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'key': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '18'}),
+            'node': ('django.db.models.fields.related.ForeignKey', [], {'to': u"orm['maasserver.Node']", 'unique': 'True'}),
+            'token': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['piston.Token']", 'unique': 'True'})
+        },
+        u'metadataserver.nodeuserdata': {
+            'Meta': {'object_name': 'NodeUserData'},
+            'data': ('metadataserver.fields.BinaryField', [], {}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'node': ('django.db.models.fields.related.ForeignKey', [], {'to': u"orm['maasserver.Node']", 'unique': 'True'})
+        },
+        'piston.consumer': {
+            'Meta': {'object_name': 'Consumer'},
+            'description': ('django.db.models.fields.TextField', [], {}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'key': ('django.db.models.fields.CharField', [], {'max_length': '18'}),
+            'name': ('django.db.models.fields.CharField', [], {'max_length': '255'}),
+            'secret': ('django.db.models.fields.CharField', [], {'max_length': '32'}),
+            'status': ('django.db.models.fields.CharField', [], {'default': "'pending'", 'max_length': '16'}),
+            'user': ('django.db.models.fields.related.ForeignKey', [], {'blank': 'True', 'related_name': "'consumers'", 'null': 'True', 'to': "orm['auth.User']"})
+        },
+        'piston.token': {
+            'Meta': {'object_name': 'Token'},
+            'callback': ('django.db.models.fields.CharField', [], {'max_length': '255', 'null': 'True', 'blank': 'True'}),
+            'callback_confirmed': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+            'consumer': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['piston.Consumer']"}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'is_approved': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+            'key': ('django.db.models.fields.CharField', [], {'max_length': '18'}),
+            'secret': ('django.db.models.fields.CharField', [], {'max_length': '32'}),
+            'timestamp': ('django.db.models.fields.IntegerField', [], {'default': '1355381041L'}),
+            'token_type': ('django.db.models.fields.IntegerField', [], {}),
+            'user': ('django.db.models.fields.related.ForeignKey', [], {'blank': 'True', 'related_name': "'tokens'", 'null': 'True', 'to': "orm['auth.User']"}),
+            'verifier': ('django.db.models.fields.CharField', [], {'max_length': '10'})
+        }
+    }
+
+    complete_apps = ['metadataserver']
+    symmetrical = True

=== modified file 'src/metadataserver/models/commissioningscript.py'
--- src/metadataserver/models/commissioningscript.py	2012-11-30 17:21:28 +0000
+++ src/metadataserver/models/commissioningscript.py	2012-12-13 09:37:21 +0000
@@ -18,6 +18,7 @@
 from io import BytesIO
 import os.path
 import tarfile
+from textwrap import dedent
 
 from django.db.models import (
     CharField,
@@ -31,6 +32,28 @@
 # extracted into this directory.
 ARCHIVE_PREFIX = "commissioning.d"
 
+# Built-in script to run lshw.
+LSHW_SCRIPT = dedent("""\
+    #!/bin/sh
+    lshw -xml
+    """)
+
+# Built-in commissioning scripts.  These go into the commissioning
+# tarball together with user-provided commissioning scripts.
+# To keep namespaces separated, names of the built-in scripts must be
+# prefixed with "00-maas-" or "99-maas-".
+BUILTIN_COMMISSIONING_SCRIPTS = {
+    '00-maas-01-lshw': LSHW_SCRIPT.encode('ascii'),
+}
+
+
+def add_script_to_archive(tarball, name, content):
+    """Add a commissioning script to an archive of commissioning scripts."""
+    assert isinstance(content, bytes), "Script content must be binary."
+    tarinfo = tarfile.TarInfo(name=os.path.join(ARCHIVE_PREFIX, name))
+    tarinfo.size = len(content)
+    tarball.addfile(tarinfo, BytesIO(content))
+
 
 class CommissioningScriptManager(Manager):
     """Utility for the collection of `CommissioningScript`s."""
@@ -42,11 +65,11 @@
         """
         binary = BytesIO()
         tarball = tarfile.open(mode='w', fileobj=binary)
-        for script in self.all().order_by('name'):
-            path = os.path.join(ARCHIVE_PREFIX, script.name)
-            tarinfo = tarfile.TarInfo(name=path)
-            tarinfo.size = len(script.content)
-            tarball.addfile(tarinfo, BytesIO(script.content))
+        scripts = sorted(
+            BUILTIN_COMMISSIONING_SCRIPTS.items() +
+            [(script.name, script.content) for script in self.all()])
+        for name, content in scripts:
+            add_script_to_archive(tarball, name, content)
         tarball.close()
         binary.seek(0)
         return binary.read()

=== modified file 'src/metadataserver/tests/test_api.py'
--- src/metadataserver/tests/test_api.py	2012-11-30 08:20:44 +0000
+++ src/metadataserver/tests/test_api.py	2012-12-13 09:37:21 +0000
@@ -351,8 +351,8 @@
                 'application/x-tgz',
             })
         archive = tarfile.open(fileobj=BytesIO(response.content))
-        self.assertItemsEqual(
-            [os.path.join(ARCHIVE_PREFIX, script.name)],
+        self.assertIn(
+            os.path.join(ARCHIVE_PREFIX, script.name),
             archive.getnames())
 
     def test_other_user_than_node_cannot_signal_commissioning_result(self):
@@ -548,7 +548,8 @@
         node = factory.make_node(status=NODE_STATUS.COMMISSIONING, memory=512)
         client = self.make_node_client(node=node)
         xmlbytes = "<t\xe9st/>".encode("utf-8")
-        response = self.call_signal(client, files={'01-lshw.out': xmlbytes})
+        response = self.call_signal(
+            client, files={'00-maas-01-lshw.out': xmlbytes})
         self.assertEqual(httplib.OK, response.status_code)
         node = reload_object(node)
         self.assertEqual(xmlbytes, node.hardware_details)

=== modified file 'src/metadataserver/tests/test_commissioningscript.py'
--- src/metadataserver/tests/test_commissioningscript.py	2012-11-30 03:54:09 +0000
+++ src/metadataserver/tests/test_commissioningscript.py	2012-12-13 09:37:21 +0000
@@ -19,9 +19,13 @@
 
 from maasserver.testing.factory import factory
 from maasserver.testing.testcase import TestCase
+from maastesting.matchers import ContainsAll
 from maastesting.utils import sample_binary_data
 from metadataserver.fields import Bin
-from metadataserver.models import CommissioningScript
+from metadataserver.models import (
+    CommissioningScript,
+    commissioningscript as cs_module,
+    )
 from metadataserver.models.commissioningscript import ARCHIVE_PREFIX
 
 
@@ -44,35 +48,35 @@
 
     def test_get_archive_wraps_scripts_in_tar(self):
         script = factory.make_commissioning_script()
+        path = os.path.join(ARCHIVE_PREFIX, script.name)
         archive = open_tarfile(CommissioningScript.objects.get_archive())
-        archived_script = archive.next()
-        self.assertTrue(archived_script.isfile())
-        self.assertEqual(
-            os.path.join(ARCHIVE_PREFIX, script.name),
-            archived_script.name)
-        self.assertEqual(
-            script.content,
-            archive.extractfile(archived_script).read())
+        self.assertTrue(archive.getmember(path).isfile())
+        self.assertEqual(script.content, archive.extractfile(path).read())
 
     def test_get_archive_wraps_all_scripts(self):
         scripts = {factory.make_commissioning_script() for counter in range(3)}
         archive = open_tarfile(CommissioningScript.objects.get_archive())
-        self.assertItemsEqual(
-            {os.path.join(ARCHIVE_PREFIX, script.name) for script in scripts},
-            archive.getnames())
+        self.assertThat(
+            archive.getnames(),
+            ContainsAll({
+                os.path.join(ARCHIVE_PREFIX, script.name)
+                for script in scripts
+                }))
 
     def test_get_archive_supports_binary_scripts(self):
         script = factory.make_commissioning_script(content=sample_binary_data)
+        path = os.path.join(ARCHIVE_PREFIX, script.name)
         archive = open_tarfile(CommissioningScript.objects.get_archive())
-        archived_script = archive.next()
-        self.assertEqual(
-            script.content,
-            archive.extractfile(archived_script).read())
+        self.assertEqual(script.content, archive.extractfile(path).read())
 
-    def test_get_archive_returns_empty_tarball_if_no_scripts(self):
-        CommissioningScript.objects.all().delete()
+    def test_get_archive_includes_builtin_scripts(self):
+        name = factory.make_name('00-maas')
+        path = os.path.join(ARCHIVE_PREFIX, name)
+        content = factory.getRandomString().encode('ascii')
+        self.patch(cs_module, 'BUILTIN_COMMISSIONING_SCRIPTS', {name: content})
         archive = open_tarfile(CommissioningScript.objects.get_archive())
-        self.assertItemsEqual([], archive.getnames())
+        self.assertIn(path, archive.getnames())
+        self.assertEqual(content, archive.extractfile(path).read())
 
 
 class TestCommissioningScript(TestCase):