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

[Thomas Herve]

The branch makes the client reports *all* results from smart update, allowing
to correctly know when the client is correctly working or not.

[Sidnei da Silva (sidnei)]

LGTM. +1!

review: Approve

[Free Ekanayaka (free.ekanayaka)]

Great, +1!

[1]

+ "out": out,

The output could be biggish, not sure that we really want to include it in the message, also it's not clear what use the server would make of it. Maybe it could be skipped initially (till we find a use case)? IIRC adding a field to a message shouldn't be painful. This is not blocking if you think that the extra payload is negligible.

review: Approve

[lp:~therve/landscape-client/package-reporter-result]

331. By Thomas Herve on 2011-05-12

Remove output from the package reporter result

[Andreas Hasenack (ahasenack)]

Verified in staging, with a staging client.

=== modified file 'landscape/message_schemas.py'
--- landscape/message_schemas.py	2011-04-29 16:23:16 +0000
+++ landscape/message_schemas.py	2011-05-12 10:22:26 +0000
@@ -297,9 +297,9 @@
     "request-id": Int(),
     })
 
-PACKAGE_REPORTER_ERROR = Message("package-reporter-error", {
-    "error-code": Int(),
-    "error-text": utf8})
+PACKAGE_REPORTER_RESULT = Message("package-reporter-result", {
+    "code": Int(),
+    "err": utf8})
 
 ADD_PACKAGES = Message("add-packages", {
     "packages": List(KeyDict({"name": utf8,
@@ -390,7 +390,7 @@
                REGISTER, REGISTER_CLOUD_VM, TEMPERATURE, PROCESSOR_INFO,
                USERS, PACKAGES, PACKAGE_LOCKS,
                CHANGE_PACKAGES_RESULT, UNKNOWN_PACKAGE_HASHES,
-               ADD_PACKAGES, PACKAGE_REPORTER_ERROR, TEXT_MESSAGE, TEST,
+               ADD_PACKAGES, PACKAGE_REPORTER_RESULT, TEXT_MESSAGE, TEST,
                CUSTOM_GRAPH, REBOOT_REQUIRED, APT_PREFERENCES, EUCALYPTUS_INFO,
                EUCALYPTUS_INFO_ERROR, NETWORK_DEVICE, NETWORK_ACTIVITY,
                REBOOT_REQUIRED_INFO]:
 
=== modified file 'landscape/package/reporter.py'
--- landscape/package/reporter.py	2011-04-29 16:23:16 +0000
+++ landscape/package/reporter.py	2011-05-12 10:22:26 +0000
@@ -201,26 +201,22 @@
             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)
-            if smart_failed:
-                deferred = self._broker.call_if_accepted(
-                    "package-reporter-error", self.send_error, code, err)
-            else:
-                deferred = succeed(None)
+            deferred = self._broker.call_if_accepted(
+                "package-reporter-result", self.send_result, code, err)
             deferred.addCallback(lambda ignore: (out, err, code))
             return deferred
 
         result.addCallback(callback)
         return result
 
-    def send_error(self, code, err):
+    def send_result(self, code, err):
         """
-        If an error happened in smart update, reports it to the server in a
-        message.
+        Report the package reporter result to the server in a message.
         """
         message = {
-            "type": "package-reporter-error",
-            "error-code": code,
-            "error-text": err}
+            "type": "package-reporter-result",
+            "code": code,
+            "err": err}
         return self._broker.send_message(message, True)
 
     def handle_task(self, task):
 
=== modified file 'landscape/package/tests/test_reporter.py'
--- landscape/package/tests/test_reporter.py	2011-04-29 16:23:16 +0000
+++ landscape/package/tests/test_reporter.py	2011-05-12 10:22:26 +0000
@@ -668,7 +668,7 @@
         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"])
+        message_store.set_accepted_types(["package-reporter-result"])
         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)
@@ -679,8 +679,33 @@
 
             def callback(ignore):
                 self.assertMessages(message_store.get_pending_messages(),
-                    [{"type": "package-reporter-error",
-                      "error-code": 2, "error-text": "error"}])
+                    [{"type": "package-reporter-result",
+                      "code": 2, "err": u"error"}])
+            result.addCallback(callback)
+            result.chainDeferred(deferred)
+
+        reactor.callWhenRunning(do_test)
+        return deferred
+
+    def test_run_smart_update_report_success(self):
+        """
+        L{PackageReporter.run_smart_update} also reports success to be able to
+        know the proper state of the client.
+        """
+        message_store = self.broker_service.message_store
+        message_store.set_accepted_types(["package-reporter-result"])
+        self.reporter.smart_update_filename = self.makeFile(
+            "#!/bin/sh\necho -n error >&2\necho -n output\nexit 0")
+        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-result",
+                      "code": 0, "err": u"error"}])
             result.addCallback(callback)
             result.chainDeferred(deferred)