Merge lp:~therve/landscape-client/package-reporter-errors into lp:~landscape/landscape-client/trunk

[Thomas Herve]

That part is of course too easy. The reporter just sends a new message when a smart update fails. We need to do something on the server side to handle it. I was thinking about adding an alert?

[Free Ekanayaka (free.ekanayaka)]

Looks good, +1.

An alert sounds fine, though sometimes the errors might be transient, like an HTTP timeout.

review: Approve

[Björn Tillenius (bjornt)]

Looks good to me, +1

review: Approve

=== modified file 'landscape/message_schemas.py'
--- landscape/message_schemas.py	2011-04-25 17:56:33 +0000
+++ landscape/message_schemas.py	2011-04-29 16:27:30 +0000
@@ -46,7 +46,7 @@
     "client-uptime",
     {"period": Tuple(Float(), Float()),
      "components": List(Int())},
-    optional=["components"]) # just for backwards compatibility
+    optional=["components"])  # just for backwards compatibility
 
 OPERATION_RESULT = Message(
     "operation-result",
@@ -90,10 +90,10 @@
 HARDWARE_INVENTORY = Message("hardware-inventory", {
     "devices": List(Any(Tuple(Constant("create"), hal_data),
                         Tuple(Constant("update"),
-                              Unicode(), # udi,
-                              hal_data, # creates,
-                              hal_data, # updates,
-                              hal_data), # deletes
+                              Unicode(),  # udi,
+                              hal_data,  # creates,
+                              hal_data,  # updates,
+                              hal_data),  # deletes
                         Tuple(Constant("delete"),
                               Unicode()),
                         ),
@@ -297,6 +297,10 @@
     "request-id": Int(),
     })
 
+PACKAGE_REPORTER_ERROR = Message("package-reporter-error", {
+    "error-code": Int(),
+    "error-text": utf8})
+
 ADD_PACKAGES = Message("add-packages", {
     "packages": List(KeyDict({"name": utf8,
                               "description": Unicode(),
@@ -386,8 +390,8 @@
                REGISTER, REGISTER_CLOUD_VM, TEMPERATURE, PROCESSOR_INFO,
                USERS, PACKAGES, PACKAGE_LOCKS,
                CHANGE_PACKAGES_RESULT, UNKNOWN_PACKAGE_HASHES,
-               ADD_PACKAGES, TEXT_MESSAGE, TEST, CUSTOM_GRAPH,
-               REBOOT_REQUIRED, APT_PREFERENCES, EUCALYPTUS_INFO,
+               ADD_PACKAGES, PACKAGE_REPORTER_ERROR, TEXT_MESSAGE, TEST,
+               CUSTOM_GRAPH, REBOOT_REQUIRED, APT_PREFERENCES, EUCALYPTUS_INFO,
                EUCALYPTUS_INFO_ERROR, NETWORK_DEVICE, NETWORK_ACTIVITY,
                REBOOT_REQUIRED_INFO]:
     message_schemas[schema.type] = schema
 
=== modified file 'landscape/package/reporter.py'
--- landscape/package/reporter.py	2010-04-01 08:28:22 +0000
+++ landscape/package/reporter.py	2011-04-29 16:27:30 +0000
@@ -201,11 +201,28 @@
             logging.debug("'%s' exited with status %d (out='%s', err='%s'" % (
                 self.smart_update_filename, code, out, err))
             touch_file(self._config.smart_update_stamp_filename)
-            return (out, err, code)
+            if smart_failed:
+                deferred = self._broker.call_if_accepted(
+                    "package-reporter-error", self.send_error, code, err)
+            else:
+                deferred = succeed(None)
+            deferred.addCallback(lambda ignore: (out, err, code))
+            return deferred
 
         result.addCallback(callback)
         return result
 
+    def send_error(self, code, err):
+        """
+        If an error happened in smart update, reports it to the server in a
+        message.
+        """
+        message = {
+            "type": "package-reporter-error",
+            "error-code": code,
+            "error-text": err}
+        return self._broker.send_message(message, True)
+
     def handle_task(self, task):
         message = task.data
         if message["type"] == "package-ids":
 
=== modified file 'landscape/package/tests/test_reporter.py'
--- landscape/package/tests/test_reporter.py	2010-04-16 10:44:51 +0000
+++ landscape/package/tests/test_reporter.py	2011-04-29 16:27:30 +0000
@@ -60,7 +60,7 @@
             previous(self)
             pkg2 = self.get_packages_by_name("name2")[0]
             pkg2.upgrades += (DebUpgrades("name1", "=", "version1-release1"),)
-            self.reload_cache() # Relink relations.
+            self.reload_cache()  # Relink relations.
         self.Facade.channels_reloaded = callback
 
     def set_pkg1_installed(self):
@@ -662,6 +662,31 @@
         reactor.callWhenRunning(do_test)
         return deferred
 
+    def test_run_smart_update_report_failures(self):
+        """
+        If L{PackageReporter.run_smart_update} fails, a message is sent to the
+        server reporting the error, to be able to fix the problem centrally.
+        """
+        message_store = self.broker_service.message_store
+        message_store.set_accepted_types(["package-reporter-error"])
+        self.reporter.smart_update_filename = self.makeFile(
+            "#!/bin/sh\necho -n error >&2\necho -n output\nexit 2")
+        os.chmod(self.reporter.smart_update_filename, 0755)
+        deferred = Deferred()
+
+        def do_test():
+            result = self.reporter.run_smart_update()
+
+            def callback(ignore):
+                self.assertMessages(message_store.get_pending_messages(),
+                    [{"type": "package-reporter-error",
+                      "error-code": 2, "error-text": "error"}])
+            result.addCallback(callback)
+            result.chainDeferred(deferred)
+
+        reactor.callWhenRunning(do_test)
+        return deferred
+
     def test_run_smart_update_warns_exit_code_1_and_non_empty_stderr(self):
         """
         The L{PackageReporter.run_smart_update} method should log a warning