Merge lp:~therve/landscape-client/juju-env-info into lp:~landscape/landscape-client/trunk

[Thomas Herve]

The branch adds a new registration fields populated with the content of a file in the data path. I'm not super happy about it yet, but it feels it does the job.

[Chris Glass (tribaal)]

Looks good, +1!

I hope goju will provide an API instead, it would feel "cleaner", but this will do nicely until then :)

review: Approve

[Björn Tillenius (bjornt)]

I think it's good enough, so I'm giving it a +1. As mentioned on IRC, I'd rather
see each key and value go in the message itself, rather than sending the JSON
blob. I'm not sure how easy it would be to extend our schema to be more flexible,
though. This is one way of allowing a schema to contain any number of optional
keys, but not a very pretty one.

review: Approve

=== modified file 'landscape/broker/registration.py'
--- landscape/broker/registration.py	2012-05-17 20:59:23 +0000
+++ landscape/broker/registration.py	2013-01-16 08:32:22 +0000
@@ -1,3 +1,4 @@
+import os
 import time
 import logging
 import socket
@@ -97,6 +98,7 @@
         self._fetch_async = fetch_async
         self._otp = None
         self._ec2_data = None
+        self.extra_data_path = os.path.join(self._config.data_path, "extra")
 
     def should_register(self):
         id = self._identity
@@ -205,8 +207,8 @@
                     self._config.url = exchange_url
                     self._config.ping_url = ping_url
                     if "ssl-ca-certificate" in instance_data:
-                        from landscape.configuration import \
-                            store_public_key_data
+                        from landscape.configuration import (
+                            store_public_key_data)
                         public_key_file = store_public_key_data(
                             self._config, instance_data["ssl-ca-certificate"])
                         self._config.ssl_public_key = public_key_file
@@ -253,16 +255,16 @@
                 tags = None
                 logging.error("Invalid tags provided for cloud "
                               "registration.")
+            message = {"hostname": get_fqdn(),
+                       "vm-info": get_vm_info(),
+                       "tags": tags}
             if self._config.cloud and self._ec2_data is not None:
                 if self._otp:
                     logging.info("Queueing message to register with OTP")
-                    message = {"type": "register-cloud-vm",
-                               "otp": self._otp,
-                               "hostname": get_fqdn(),
-                               "account_name": None,
-                               "registration_password": None,
-                               "tags": tags,
-                               "vm-info": get_vm_info()}
+                    message.update({"type": "register-cloud-vm",
+                                    "otp": self._otp,
+                                    "account_name": None,
+                                    "registration_password": None})
                     message.update(self._ec2_data)
                     self._exchange.send(message)
                 elif id.account_name:
@@ -270,31 +272,30 @@
                     logging.info(
                         u"Queueing message to register with account %r %s"
                         u"as an EC2 instance." % (id.account_name, with_tags))
-                    message = {"type": "register-cloud-vm",
-                               "otp": None,
-                               "hostname": get_fqdn(),
-                               "account_name": id.account_name,
-                               "registration_password": \
-                                   id.registration_key,
-                               "tags": tags,
-                               "vm-info": get_vm_info()}
+                    message.update({
+                        "type": "register-cloud-vm",
+                        "otp": None,
+                        "account_name": id.account_name,
+                        "registration_password": id.registration_key})
                     message.update(self._ec2_data)
                     self._exchange.send(message)
                 else:
                     self._reactor.fire("registration-failed")
             elif id.account_name:
+                if os.path.exists(self.extra_data_path):
+                    fd = file(self.extra_data_path)
+                    extra = fd.read()
+                    fd.close()
+                    message["extra"] = extra
                 with_word = ["without", "with"][bool(id.registration_key)]
                 with_tags = ["", u"and tags %s " % tags][bool(tags)]
                 logging.info(u"Queueing message to register with account %r %s"
                              "%s a password." % (id.account_name, with_tags,
                                                  with_word))
-                message = {"type": "register",
-                           "computer_title": id.computer_title,
-                           "account_name": id.account_name,
-                           "registration_password": id.registration_key,
-                           "hostname": get_fqdn(),
-                           "tags": tags,
-                           "vm-info": get_vm_info()}
+                message.update({"type": "register",
+                                "computer_title": id.computer_title,
+                                "account_name": id.account_name,
+                                "registration_password": id.registration_key})
                 self._exchange.send(message)
             elif self._config.provisioning_otp:
                 logging.info(u"Queueing message to register with OTP as a"

=== modified file 'landscape/broker/tests/test_registration.py'
--- landscape/broker/tests/test_registration.py	2012-05-17 20:59:23 +0000
+++ landscape/broker/tests/test_registration.py	2013-01-16 08:32:22 +0000
@@ -297,6 +297,35 @@
         self.assertEqual(len(messages), 1)
         self.assertEqual(messages[0]["type"], "register")
 
+    def test_registration_with_extra_data(self):
+        """
+        The broker sends the content of the extra data file if present during
+        registration.
+        """
+        extra_file = os.path.join(self.config.data_path, "extra")
+        fd = file(extra_file, "wb")
+        fd.write("data!\n")
+        fd.close()
+
+        self.mstore.set_accepted_types(["register"])
+        self.config.computer_title = "Computer Title"
+        self.config.account_name = "account_name"
+        self.config.registration_key = "SEKRET"
+        self.reactor.fire("pre-exchange")
+        self.assertMessages(
+            self.mstore.get_pending_messages(),
+            [{"type": "register",
+              "computer_title": "Computer Title",
+              "account_name": "account_name",
+              "registration_password": "SEKRET",
+              "hostname": "ooga.local",
+              "tags": None,
+              "extra": "data!\n",
+              "vm-info": get_vm_info()}])
+        self.assertEqual(self.logfile.getvalue().strip(),
+                         "INFO: Queueing message to register with account "
+                         "'account_name' with a password.")
+
     def test_no_message_when_should_register_is_false(self):
         """If we already have a secure id, do not queue a register message.
         """
@@ -499,11 +528,11 @@
         return user_data
 
     def prepare_query_results(
-        self, user_data=None, instance_key="key1", launch_index=0,
-        local_hostname="ooga.local", public_hostname="ooga.amazon.com",
-        reservation_key=u"res1", ramdisk_key=u"ram1", kernel_key=u"kernel1",
-        image_key=u"image1", local_ip="10.0.0.1", public_ip="10.0.0.2",
-        ssl_ca_certificate=None):
+            self, user_data=None, instance_key="key1", launch_index=0,
+            local_hostname="ooga.local", public_hostname="ooga.amazon.com",
+            reservation_key=u"res1", ramdisk_key=u"ram1",
+            kernel_key=u"kernel1", image_key=u"image1", local_ip="10.0.0.1",
+            public_ip="10.0.0.2", ssl_ca_certificate=None):
         if user_data is None:
             user_data = self.get_user_data(
                 ssl_ca_certificate=ssl_ca_certificate)
@@ -512,18 +541,17 @@
         api_base = "http://169.254.169.254/latest"
         self.query_results.clear()
         for url_suffix, value in [
-            ("/user-data", user_data),
-            ("/meta-data/instance-id", instance_key),
-            ("/meta-data/reservation-id", reservation_key),
-            ("/meta-data/local-hostname", local_hostname),
-            ("/meta-data/public-hostname", public_hostname),
-            ("/meta-data/ami-launch-index", str(launch_index)),
-            ("/meta-data/kernel-id", kernel_key),
-            ("/meta-data/ramdisk-id", ramdisk_key),
-            ("/meta-data/ami-id", image_key),
-            ("/meta-data/local-ipv4", local_ip),
-            ("/meta-data/public-ipv4", public_ip),
-            ]:
+                ("/user-data", user_data),
+                ("/meta-data/instance-id", instance_key),
+                ("/meta-data/reservation-id", reservation_key),
+                ("/meta-data/local-hostname", local_hostname),
+                ("/meta-data/public-hostname", public_hostname),
+                ("/meta-data/ami-launch-index", str(launch_index)),
+                ("/meta-data/kernel-id", kernel_key),
+                ("/meta-data/ramdisk-id", ramdisk_key),
+                ("/meta-data/ami-id", image_key),
+                ("/meta-data/local-ipv4", local_ip),
+                ("/meta-data/public-ipv4", public_ip)]:
             self.query_results[api_base + url_suffix] = value
 
     def prepare_cloud_registration(self, account_name=None,
@@ -664,7 +692,8 @@
         If we have an SSL certificate CA included in the user-data, this should
         be written out, and the configuration updated to reflect this.
         """
-        key_filename = os.path.join(self.config.data_path,
+        key_filename = os.path.join(
+            self.config.data_path,
             "%s.ssl_public_key" % os.path.basename(self.config_filename))
 
         print_text_mock = self.mocker.replace(print_text)
@@ -758,11 +787,12 @@
                                 account_name=u"onward",
                                 registration_password=u"password",
                                 tags=u"london,server")])
-        self.assertEqual(self.logfile.getvalue().strip()[:-7],
-           "INFO: Queueing message to register with account u'onward' and "
-           "tags london,server as an EC2 instance.\n    "
-           "INFO: Starting message exchange with http://localhost:91919.\n    "
-           "INFO: Message exchange completed in")
+        self.assertEqual(
+            self.logfile.getvalue().strip()[:-7],
+            "INFO: Queueing message to register with account u'onward' and "
+            "tags london,server as an EC2 instance.\n    "
+            "INFO: Starting message exchange with http://localhost:91919.\n"
+            "    INFO: Message exchange completed in")
 
     def test_queueing_cloud_registration_message_resets_message_store(self):
         """

=== modified file 'landscape/message_schemas.py'
--- landscape/message_schemas.py	2013-01-15 09:15:51 +0000
+++ landscape/message_schemas.py	2013-01-16 08:32:22 +0000
@@ -104,21 +104,17 @@
 
 
 LOAD_AVERAGE = Message("load-average", {
-    "load-averages": List(Tuple(Int(), Float())),
-    })
+    "load-averages": List(Tuple(Int(), Float()))})
 
 CPU_USAGE = Message("cpu-usage", {
-    "cpu-usages": List(Tuple(Int(), Float())),
-    })
+    "cpu-usages": List(Tuple(Int(), Float()))})
 
 CEPH_USAGE = Message("ceph-usage", {
     "ceph-usages": List(Tuple(Int(), Float())),
-    "ring-id": utf8,
-    })
+    "ring-id": utf8})
 
 MEMORY_INFO = Message("memory-info", {
-    "memory-info": List(Tuple(Float(), Int(), Int())),
-    })
+    "memory-info": List(Tuple(Float(), Int(), Int()))})
 
 RESYNCHRONIZE = Message(
     "resynchronize",
@@ -137,8 +133,7 @@
                                       "device": utf8,
                                       "filesystem": utf8,
                                       "total-space": Int()}),
-                             )),
-    })
+                             ))})
 
 FREE_SPACE = Message("free-space", {
     "free-space": List(Tuple(Float(), utf8, Int()))})
@@ -154,9 +149,10 @@
      "hostname": utf8,
      "account_name": utf8,
      "tags": Any(utf8, Constant(None)),
-     "vm-info": String()},
+     "vm-info": String(),
+     "extra": Any(utf8, None)},
     # hostname wasn't around in old versions
-    optional=["registration_password", "hostname", "tags", "vm-info"])
+    optional=["registration_password", "hostname", "tags", "vm-info", "extra"])
 
 
 REGISTER_PROVISIONED_MACHINE = Message(
@@ -182,12 +178,11 @@
      "vm-info": String(),
      "public_ipv4": Unicode(),
      "local_ipv4": Unicode()},
-     optional=["tags", "vm-info", "public_ipv4", "local_ipv4"])
+    optional=["tags", "vm-info", "public_ipv4", "local_ipv4"])
 
 TEMPERATURE = Message("temperature", {
     "thermal-zone": utf8,
-    "temperatures": List(Tuple(Int(), Float())),
-    })
+    "temperatures": List(Tuple(Int(), Float()))})
 
 PROCESSOR_INFO = Message(
     "processor-info",
@@ -196,8 +191,7 @@
                                  "model": utf8,
                                  "cache-size": Int(),
                                  },
-                                optional=["vendor", "cache-size"])),
-    })
+                                optional=["vendor", "cache-size"]))})
 
 user_data = KeyDict({
     "uid": Int(),
@@ -324,8 +318,7 @@
 
 UNKNOWN_PACKAGE_HASHES = Message("unknown-package-hashes", {
     "hashes": List(String()),
-    "request-id": Int(),
-    })
+    "request-id": Int()})
 
 PACKAGE_REPORTER_RESULT = Message("package-reporter-result", {
     "code": Int(),
@@ -342,8 +335,7 @@
                               "version": utf8,
                               "type": Int(),
                               })),
-    "request-id": Int(),
-    })
+    "request-id": Int()})
 
 TEXT_MESSAGE = Message("text-message", {
     "message": Unicode()})