Merge lp:~gocept/landscape-client/py3-manager-customgraph into lp:~landscape/landscape-client/trunk

[Steffen Allner]

This MP considers the landscape.manager.customgraph module, where the main change was the use of bytes as hashes, which is required by the message schema. md5().hexdigest() returns bytes in Py2 and unicode in Py3, so we have to encode on time with "ascii".

Other changes are minor and consider opening the script_file handler in binary mode as already changed in l.manager.scriptexecution. Furthermore, the generator result of range() does not support .pop() in Python 3.

[Steffen Allner (sallner)]

We have to wait for other branches to land, as it is dependend on py3-broker-ping _and_ py3-manager-scriptexecution.

[🤖 Landscape Builder (landscape-builder)]

review: Abstain (executing tests)

[🤖 Landscape Builder (landscape-builder)]

Command: TRIAL_ARGS=-j4 make ci-check
Result: Success
Revno: 985
Branch: lp:~gocept/landscape-client/py3-manager-customgraph
Jenkins: https://ci.lscape.net/job/latch-test-xenial/3803/

review: Approve (test results)

Unmerged revisions

=== modified file 'landscape/lib/scriptcontent.py'
--- landscape/lib/scriptcontent.py	2017-03-13 07:48:18 +0000
+++ landscape/lib/scriptcontent.py	2017-03-29 07:05:08 +0000
@@ -12,5 +12,7 @@
     """
     Return a hash for a given script.
     """
-    encoded_script = script.encode('utf-8')
-    return md5(encoded_script).hexdigest()
+    encoded_script = script.encode("utf-8")
+    # As this hash is sent in message which requires bytes in the schema, we
+    # have to encode here.
+    return md5(encoded_script).hexdigest().encode("ascii")

=== modified file 'landscape/lib/tests/test_scriptcontent.py'
--- landscape/lib/tests/test_scriptcontent.py	2011-07-05 05:09:11 +0000
+++ landscape/lib/tests/test_scriptcontent.py	2017-03-29 07:05:08 +0000
@@ -1,7 +1,6 @@
 import unittest
 
-from landscape.lib.scriptcontent import (build_script,
-    generate_script_hash)
+from landscape.lib.scriptcontent import (build_script, generate_script_hash)
 
 
 class ScriptContentTest(unittest.TestCase):
@@ -21,4 +20,4 @@
 
         self.assertEqual(hash1, hash2)
         self.assertNotEqual(hash1, hash3)
-        self.assertTrue(isinstance(hash1, str))
+        self.assertTrue(isinstance(hash1, bytes))

=== modified file 'landscape/manager/customgraph.py'
--- landscape/manager/customgraph.py	2017-01-23 12:14:33 +0000
+++ landscape/manager/customgraph.py	2017-03-29 07:05:08 +0000
@@ -131,7 +131,7 @@
             logging.error(u"Attempt to add graph with unknown user %s" %
                           user)
         else:
-            script_file = open(filename, "w")
+            script_file = open(filename, "wb")
             # file is closed in write_script_file
             self.write_script_file(
                 script_file, filename, shell, code, uid, gid)
@@ -229,7 +229,7 @@
             if os.path.isfile(filename):
                 script_hash = self._get_script_hash(filename)
             else:
-                script_hash = ""
+                script_hash = b""
             if graph_id not in self._data:
                 self._data[graph_id] = {
                     "values": [], "error": u"", "script-hash": script_hash}

=== modified file 'landscape/manager/tests/test_customgraph.py'
--- landscape/manager/tests/test_customgraph.py	2017-01-10 14:33:40 +0000
+++ landscape/manager/tests/test_customgraph.py	2017-03-29 07:05:08 +0000
@@ -29,7 +29,7 @@
         os.makedirs(os.path.join(self.data_path, "custom-graph-scripts"))
         self.manager.config.script_users = "ALL"
         self.graph_manager = CustomGraphPlugin(
-            create_time=range(1500, 0, -300).pop)
+            create_time=list(range(1500, 0, -300)).pop)
         self.manager.add(self.graph_manager)
 
     def _exit_process_protocol(self, protocol, stdout):
@@ -128,7 +128,7 @@
 
         def check(ignore):
             self.graph_manager.exchange()
-            script_hash = "483f2304b49063680c75e3c9e09cf6d0"
+            script_hash = b"483f2304b49063680c75e3c9e09cf6d0"
             self.assertMessages(
                 self.broker_service.message_store.get_pending_messages(),
                 [{"data": {123: {"error": u"",
@@ -153,11 +153,11 @@
                 [{"data":
                       {123: {"error": u"",
                              "values": [(300, 1.0)],
-                             "script-hash": "483f2304b49063680c75e3c9e09cf6d0"
+                             "script-hash": b"483f2304b49063680c75e3c9e09cf6d0"
                             },
                        124: {"error": u"",
                              "values": [(300, 2.0)],
-                             "script-hash": "73a74b1530b2256db7edacb9b9cc385e"
+                             "script-hash": b"73a74b1530b2256db7edacb9b9cc385e"
                             }
                       },
                   "type": "custom-graph"}])
@@ -175,7 +175,7 @@
                 [{"data":
                       {123: {"error": u" (process exited with code 1)",
                              "values": [],
-                             "script-hash": "eaca3ba1a3bf1948876eba320148c5e9"
+                             "script-hash": b"eaca3ba1a3bf1948876eba320148c5e9"
                             }
                       },
                   "type": "custom-graph"}])
@@ -192,7 +192,7 @@
         spawn = factory.spawns[0]
         self.assertEqual(spawn[1], filename)
 
-        self._exit_process_protocol(spawn[0], "foobar")
+        self._exit_process_protocol(spawn[0], b"foobar")
 
         def check(ignore):
             self.graph_manager.exchange()
@@ -203,7 +203,7 @@
                              u"InvalidFormatError: Failed to convert to "
                               "number: 'foobar'",
                              "values": [], "script-hash":
-                                 "baab6c16d9143523b7865d46896e4596"}},
+                                 b"baab6c16d9143523b7865d46896e4596"}},
                   "type": "custom-graph"}])
         return result.addCallback(check)
 
@@ -218,7 +218,7 @@
         spawn = factory.spawns[0]
         self.assertEqual(spawn[1], filename)
 
-        self._exit_process_protocol(spawn[0], "")
+        self._exit_process_protocol(spawn[0], b"")
 
         def check(ignore):
             self.graph_manager.exchange()
@@ -228,7 +228,7 @@
                       {123: {"error": u"NoOutputError: Script did not output "
                                        "any value",
                              "values": [], "script-hash":
-                                 "baab6c16d9143523b7865d46896e4596"}},
+                                 b"baab6c16d9143523b7865d46896e4596"}},
                   "type": "custom-graph"}])
         return result.addCallback(check)
 
@@ -243,9 +243,9 @@
 
         self.assertEqual(len(factory.spawns), 2)
         spawn = factory.spawns[0]
-        self._exit_process_protocol(spawn[0], "")
+        self._exit_process_protocol(spawn[0], b"")
         spawn = factory.spawns[1]
-        self._exit_process_protocol(spawn[0], "0.5")
+        self._exit_process_protocol(spawn[0], b"0.5")
 
         def check(ignore):
             self.graph_manager.exchange()
@@ -254,10 +254,12 @@
                 [{"data":
                       {123: {"error": u"NoOutputError: Script did not output "
                                        "any value",
-                             "script-hash": "baab6c16d9143523b7865d46896e4596",
+                             "script-hash":
+                                 b"baab6c16d9143523b7865d46896e4596",
                              "values": []},
                        124: {"error": u"",
-                             "script-hash": "baab6c16d9143523b7865d46896e4596",
+                             "script-hash":
+                                 b"baab6c16d9143523b7865d46896e4596",
                              "values": [(300, 0.5)]}},
                   "type": "custom-graph"}])
         return result.addCallback(check)
@@ -273,9 +275,9 @@
 
         self.assertEqual(len(factory.spawns), 2)
         spawn = factory.spawns[0]
-        self._exit_process_protocol(spawn[0], "foo")
+        self._exit_process_protocol(spawn[0], b"foo")
         spawn = factory.spawns[1]
-        self._exit_process_protocol(spawn[0], "")
+        self._exit_process_protocol(spawn[0], b"")
 
         def check(ignore):
             self.graph_manager.exchange()
@@ -284,11 +286,13 @@
                 [{"data":
                       {123: {"error": u"InvalidFormatError: Failed to convert "
                                        "to number: 'foo'",
-                             "script-hash": "baab6c16d9143523b7865d46896e4596",
+                             "script-hash":
+                                 b"baab6c16d9143523b7865d46896e4596",
                              "values": []},
                        124: {"error": u"NoOutputError: Script did not output "
                                        "any value",
-                             "script-hash": "baab6c16d9143523b7865d46896e4596",
+                             "script-hash":
+                                 b"baab6c16d9143523b7865d46896e4596",
                              "values": []}},
                   "type": "custom-graph"}])
         return result.addCallback(check)
@@ -319,7 +323,7 @@
         self.assertEqual(spawn[6], 5678)
         mock_getpwnam.assert_called_with("bar")
 
-        self._exit_process_protocol(spawn[0], "spam")
+        self._exit_process_protocol(spawn[0], b"spam")
 
         return result
 
@@ -344,7 +348,7 @@
                       {"error":
                        u"ProhibitedUserError: Custom graph cannot be run as "
                         "user %s" % (username,),
-                       "script-hash": "",
+                       "script-hash": b"",
                        "values": []}},
                   "type": "custom-graph"}])
 
@@ -369,7 +373,7 @@
                 self.broker_service.message_store.get_pending_messages(),
                 [{"data": {123:
                       {"error": u"UnknownUserError: Unknown user 'foo'",
-                       "script-hash": "",
+                       "script-hash": b"",
                        "values": []}},
                   "type": "custom-graph"}])
             mock_getpwnam.assert_called_with("foo")
@@ -399,7 +403,7 @@
                 [{"data": {123: {"error":
                                     u"Process exceeded the 10 seconds limit",
                                 "script-hash":
-                                    "9893532233caff98cd083a116b013c0b",
+                                    b"9893532233caff98cd083a116b013c0b",
                                 "values": []}},
                   "type": "custom-graph"}])
 
@@ -420,7 +424,7 @@
         self.graph_manager.exchange()
         self.assertMessages(
             self.broker_service.message_store.get_pending_messages(),
-            [{"data": {123: {"error": u"", "script-hash": "", "values": []}},
+            [{"data": {123: {"error": u"", "script-hash": b"", "values": []}},
               "type": "custom-graph"}])
 
     def test_send_message_add_stored_graph(self):
@@ -440,9 +444,10 @@
         self.graph_manager.exchange()
         self.assertMessages(
             self.broker_service.message_store.get_pending_messages(),
-            [{"api": "3.2",
+            [{"api": b"3.2",
               "data": {123: {"error": u"",
-                             "script-hash": "e00a2f44dbc7b6710ce32af2348aec9b",
+                             "script-hash":
+                                 b"e00a2f44dbc7b6710ce32af2348aec9b",
                              "values": []}},
               "timestamp": 0,
               "type": "custom-graph"}])
@@ -463,7 +468,7 @@
         self.graph_manager.exchange()
         self.assertMessages(
             self.broker_service.message_store.get_pending_messages(),
-            [{"api": "3.2",
+            [{"api": b"3.2",
               "data": {},
               "timestamp": 0,
               "type": "custom-graph"}])
@@ -488,15 +493,17 @@
         self.graph_manager.exchange()
         self.assertMessages(
             self.broker_service.message_store.get_pending_messages(),
-            [{"api": "3.2",
+            [{"api": b"3.2",
               "data": {123: {"error": u"",
-                             "script-hash": "e00a2f44dbc7b6710ce32af2348aec9b",
+                             "script-hash":
+                                 b"e00a2f44dbc7b6710ce32af2348aec9b",
                              "values": []}},
               "timestamp": 0,
               "type": "custom-graph"},
-             {"api": "3.2",
+             {"api": b"3.2",
               "data": {123: {"error": u"",
-                             "script-hash": "e00a2f44dbc7b6710ce32af2348aec9b",
+                             "script-hash":
+                                 b"e00a2f44dbc7b6710ce32af2348aec9b",
                              "values": []}},
               "timestamp": 0,
               "type": "custom-graph"}])
@@ -522,15 +529,17 @@
         self.graph_manager.exchange()
         self.assertMessages(
             self.broker_service.message_store.get_pending_messages(),
-            [{"api": "3.2",
+            [{"api": b"3.2",
               "data": {123: {"error": u"",
-                             "script-hash": "e00a2f44dbc7b6710ce32af2348aec9b",
+                             "script-hash":
+                                 b"e00a2f44dbc7b6710ce32af2348aec9b",
                              "values": []}},
               "timestamp": 0,
               "type": "custom-graph"},
-             {"api": "3.2",
+             {"api": b"3.2",
               "data": {123: {"error": u"",
-                             "script-hash": "d483816dc0fbb51ede42502a709b0e2a",
+                             "script-hash":
+                                 b"d483816dc0fbb51ede42502a709b0e2a",
                              "values": []}},
               "timestamp": 0,
               "type": "custom-graph"}])
@@ -565,16 +574,16 @@
                      "username": username,
                      "graph-id": 123})
 
-        self._exit_process_protocol(spawn[0], "1.0")
+        self._exit_process_protocol(spawn[0], b"1.0")
 
         def check(ignore):
             self.graph_manager.exchange()
             self.assertMessages(
                 self.broker_service.message_store.get_pending_messages(),
-                [{"api": "3.2",
+                [{"api": b"3.2",
                   "data": {123: {"error": u"",
                                  "script-hash":
-                                    "991e15a81929c79fe1d243b2afd99c62",
+                                     b"991e15a81929c79fe1d243b2afd99c62",
                                  "values": []}},
                   "timestamp": 0,
                   "type": "custom-graph"}])
@@ -607,13 +616,13 @@
             {"type": "custom-graph-remove",
                      "graph-id": 123})
 
-        self._exit_process_protocol(spawn[0], "1.0")
+        self._exit_process_protocol(spawn[0], b"1.0")
 
         def check(ignore):
             self.graph_manager.exchange()
             self.assertMessages(
                 self.broker_service.message_store.get_pending_messages(),
-                [{"api": "3.2", "data": {}, "timestamp": 0, "type":
+                [{"api": b"3.2", "data": {}, "timestamp": 0, "type":
                   "custom-graph"}])
         return result.addCallback(check)
 
@@ -682,7 +691,7 @@
                 [{"data": {123:
                       {"error":
                            u"UnknownUserError: Unknown user '%s'" % username,
-                       "script-hash": "9893532233caff98cd083a116b013c0b",
+                       "script-hash": b"9893532233caff98cd083a116b013c0b",
                        "values": []}},
                   "type": "custom-graph"}])
 

=== modified file 'py3_ready_tests'
--- py3_ready_tests	2017-03-28 22:58:59 +0000
+++ py3_ready_tests	2017-03-29 07:05:08 +0000
@@ -1,20 +1,15 @@
 landscape.lib.tests
 landscape.sysinfo.tests
 landscape.user.tests
+landscape.broker.tests
+landscape.package.tests
 landscape.tests.test_configuration
 landscape.tests.test_watchdog
-landscape.broker.tests.test_registration
-landscape.broker.tests.test_amp
-landscape.broker.tests.test_server
-landscape.package.tests
-
-
 landscape.tests.test_reactor
 landscape.tests.test_schema
+
 landscape.manager.tests.test_scriptexecution
-
-
-landscape.broker.tests.test_ping
-
-landscape.broker.tests.test_exchange
-landscape.broker.tests.test_store
+landscape.manager.tests.test_customgraph
+
+
+