Merge lp:~tealeg/landscape-client/notify-on-script-output-truncation into lp:~landscape/landscape-client/trunk

[Geoff Teale]

Add an indication of truncation to process output that has been truncated prior to delivery to the server.

Add an indication of truncation to process output that has been truncated prior to delivery to the server.

Testing instructions:

Run the test suite ;-)

[🤖 Landscape Builder (landscape-builder)]

review: Abstain (executing tests)

[🤖 Landscape Builder (landscape-builder)]

Command: TRIAL_ARGS=-j4 make check
Result: Fail
Revno: 923
Branch: lp:~tealeg/landscape-client/notify-on-script-output-truncation
Jenkins: https://ci.lscape.net/job/latch-test-precise/126/

review: Needs Fixing (test results)

[🤖 Landscape Builder (landscape-builder)]

review: Abstain (executing tests)

[🤖 Landscape Builder (landscape-builder)]

Command: TRIAL_ARGS=-j4 make check
Result: Success
Revno: 924
Branch: lp:~tealeg/landscape-client/notify-on-script-output-truncation
Jenkins: https://ci.lscape.net/job/latch-test-precise/127/

review: Approve (test results)

[Alberto Donato (ack)]

+1 looks good.

Just a nit inline.

review: Approve

[Adam Collard (adam-collard)]

review: Needs Fixing

[Geoff Teale (tealeg)]

All points addressed.

[Adam Collard (adam-collard)]

Nice, thanks for that. +1

review: Approve

[🤖 Landscape Builder (landscape-builder)]

review: Abstain (executing tests)

[🤖 Landscape Builder (landscape-builder)]

Command: TRIAL_ARGS=-j4 make check
Result: Success
Revno: 926
Branch: lp:~tealeg/landscape-client/notify-on-script-output-truncation
Jenkins: https://ci.lscape.net/job/latch-test-precise/158/

review: Approve (test results)

[lp:~tealeg/landscape-client/notify-on-script-output-truncation]

927. By Geoff Teale on 2016-10-06

lint

=== modified file 'landscape/manager/scriptexecution.py'
--- landscape/manager/scriptexecution.py	2016-08-17 20:31:48 +0000
+++ landscape/manager/scriptexecution.py	2016-10-06 11:03:00 +0000
@@ -6,7 +6,6 @@
 import os
 import pwd
 import tempfile
-import operator
 import shutil
 
 from twisted.internet.protocol import ProcessProtocol
@@ -95,6 +94,8 @@
         process with.
     """
 
+    truncation_indicator = "\n**OUTPUT TRUNCATED**"
+
     def __init__(self, process_factory=None):
         if process_factory is None:
             from twisted.internet import reactor as process_factory
@@ -124,7 +125,7 @@
             gid = None
 
         pp = ProcessAccumulationProtocol(
-            self.registry.reactor, self.size_limit)
+            self.registry.reactor, self.size_limit, self.truncation_indicator)
         self.process_factory.spawnProcess(
             pp, filename, uid=uid, gid=gid, path=path, env=env)
         if time_limit is not None:
@@ -309,11 +310,15 @@
     @ivar size_limit: The number of bytes at which to truncate output.
     """
 
-    def __init__(self, reactor, size_limit):
+    def __init__(self, reactor, size_limit, truncation_indicator=""):
         self.data = []
+        self._size = 0
         self.result_deferred = Deferred()
         self._cancelled = False
         self.size_limit = size_limit
+        self._truncation_indicator = truncation_indicator
+        self._truncation_offset = len(self._truncation_indicator)
+        self._truncated_size_limit = self.size_limit - self._truncation_offset
         self.reactor = reactor
         self._scheduled_cancel = None
 
@@ -327,8 +332,15 @@
         Add it to our buffer, as long as it doesn't go over L{size_limit}
         bytes.
         """
-        current_size = reduce(operator.add, map(len, self.data), 0)
-        self.data.append(data[:self.size_limit - current_size])
+        if self._size < self.size_limit:
+            data_length = len(data)
+            if (self._size + data_length) >= self._truncated_size_limit:
+                extent = (self._truncated_size_limit - self._size)
+                self.data.append(data[:extent] + self._truncation_indicator)
+                self._size = self.size_limit
+            else:
+                self.data.append(data)
+                self._size += data_length
 
     def processEnded(self, reason):
         """Fire back the deferred.
@@ -351,8 +363,8 @@
             if reason.check(ProcessDone):
                 self.result_deferred.callback(data)
             else:
-                self.result_deferred.errback(ProcessFailedError(data,
-                                                                exit_code))
+                self.result_deferred.errback(
+                    ProcessFailedError(data, exit_code))
 
     def _cancel(self):
         """

=== modified file 'landscape/manager/tests/test_scriptexecution.py'
--- landscape/manager/tests/test_scriptexecution.py	2016-06-15 23:20:02 +0000
+++ landscape/manager/tests/test_scriptexecution.py	2016-10-06 11:03:00 +0000
@@ -437,10 +437,41 @@
         self.plugin.process_factory = factory
         self.plugin.size_limit = 100
         result = self.plugin.run_script("/bin/sh", "")
-        result.addCallback(self.assertEqual, "x" * 100)
-
-        protocol = factory.spawns[0][0]
-        protocol.childDataReceived(1, "x" * 200)
+
+        # Ultimately we assert that the resulting output is limited to
+        # 100 bytes and indicates its truncation.
+        result.addCallback(self.assertEqual,
+                           ("x" * 79) + "\n**OUTPUT TRUNCATED**")
+
+        protocol = factory.spawns[0][0]
+
+        # Push 200 bytes of output, so we trigger truncation.
+        protocol.childDataReceived(1, "x" * 200)
+
+        for fd in (0, 1, 2):
+            protocol.childConnectionLost(fd)
+        protocol.processEnded(Failure(ProcessDone(0)))
+
+        return result
+
+    def test_command_output_ends_with_truncation(self):
+        """After truncation, no further output is recorded."""
+        factory = StubProcessFactory()
+        self.plugin.process_factory = factory
+        self.plugin.size_limit = 100
+        result = self.plugin.run_script("/bin/sh", "")
+
+        # Ultimately we assert that the resulting output is limited to
+        # 100 bytes and indicates its truncation.
+        result.addCallback(self.assertEqual,
+                           ("x" * 79) + "\n**OUTPUT TRUNCATED**")
+        protocol = factory.spawns[0][0]
+
+        # Push 200 bytes of output, so we trigger truncation.
+        protocol.childDataReceived(1, "x" * 200)
+        # Push 200 bytes more
+        protocol.childDataReceived(1, "x" * 200)
+
         for fd in (0, 1, 2):
             protocol.childConnectionLost(fd)
         protocol.processEnded(Failure(ProcessDone(0)))