Merge ~mpontillo/maas:remote-script-runner-reliability into maas:master

[Mike Pontillo]

Make remote script runner more reliable

 * Use a monotonic clock instead of relative time.
 * Ensure stdin is redirected from /dev/null.

[Lee Trager (ltrager)]

I didn't think about NTP resetting the clock when implementing this. Using time.monotic() should fix any time outs due to NTP changing the system time. I don't see any issue with closing STDIN either.

[Mike Pontillo (mpontillo)]

After looking at `lsof` output, I don't think it's necessary to redirect stdin to DEVNULL; this already seems to happen by default due to the way cloud-init is running and/or how it runs the scripts. But I think we should do it anyway for good measure.

The other change I think we should make to the runner related to bug #1718517 - we should surface any HTTP errors that occur on submit, and we should retry in the case of HTTP 408 or 5xx errors (such as service unavailable, like when Apache is up and running but MAAS is restarting) or timeouts.

diff --git a/src/metadataserver/user_data/templates/snippets/maas_run_remote_scripts.py b/src/metadataserver/user_data/templates/snippets/maas_run_remote_scripts.py
index 9dc5c24..5ff7ced 100644
--- a/src/metadataserver/user_data/templates/snippets/maas_run_remote_scripts.py
+++ b/src/metadataserver/user_data/templates/snippets/maas_run_remote_scripts.py
@@ -29,6 +29,7 @@ import re
 import shlex
 from subprocess import (
     check_output,
+    DEVNULL,
     PIPE,
     Popen,
     TimeoutExpired,
@@ -91,7 +92,7 @@ def download_and_extract_tar(url, creds, scripts_dir):
 def run_and_check(
         cmd, combined_path, stdout_path, stderr_path, script_name, args,
         ignore_error=False):
-    proc = Popen(cmd, stdout=PIPE, stderr=PIPE)
+    proc = Popen(cmd, stdin=DEVNULL, stdout=PIPE, stderr=PIPE)
     capture_script_output(proc, combined_path, stdout_path, stderr_path)
     if proc.returncode != 0 and not ignore_error:
         args['exit_status'] = proc.returncode
diff --git a/src/provisioningserver/refresh/__init__.py b/src/provisioningserver/refresh/__init__.py
index bf05f10..cd52816 100644
--- a/src/provisioningserver/refresh/__init__.py
+++ b/src/provisioningserver/refresh/__init__.py
@@ -7,6 +7,7 @@ import os
 import socket
 import stat
 from subprocess import (
+    DEVNULL,
     PIPE,
     Popen,
     TimeoutExpired,
@@ -164,7 +165,7 @@ def runscripts(scripts, url, creds, tmpdir):
             timeout_seconds = timeout.seconds
 
         try:
-            proc = Popen(script_path, stdout=PIPE, stderr=PIPE)
+            proc = Popen(script_path, stdin=DEVNULL, stdout=PIPE, stderr=PIPE)
             capture_script_output(
                 proc, combined_path, stdout_path, stderr_path, timeout_seconds)
         except OSError as e:
diff --git a/src/provisioningserver/refresh/maas_api_helper.py b/src/provisioningserver/refresh/maas_api_helper.py
index d4dd138..eac7d49 100644
--- a/src/provisioningserver/refresh/maas_api_helper.py
+++ b/src/provisioningserver/refresh/maas_api_helper.py
@@ -295,7 +295,7 @@ def capture_script_output(
         timeout = None
     else:
         # Pad the timeout by 60 seconds to allow for cleanup.
-        timeout = datetime.now() + timedelta(seconds=(timeout_seconds + 60))
+        timeout = time.monotonic() + timeout_seconds + 60
 
     # Create the file and then open it in read write mode for terminal
     # emulation.
@@ -309,14 +309,14 @@ def capture_script_output(
                 while selector.get_map() and proc.poll() is None:
                     # Select with a short timeout so that we don't tight loop.
                     _select_script_output(selector, combined, 0.1)
-                    if timeout is not None and datetime.now() > timeout:
+                    if timeout is not None and time.monotonic() > timeout:
                         break
                 else:
                     # Process has finished or has closed stdout and stderr.
                     # Process anything still sitting in the latter's buffers.
                     _select_script_output(selector, combined, 0.0)
 
-    now = datetime.now()
+    now = time.monotonic()
     # Wait for the process to finish.
     if timeout is None:
         # No timeout just wait until the process finishes.
@@ -329,7 +329,7 @@ def capture_script_output(
         # stdout and stderr have been closed but the timeout has not been
         # exceeded. Run with the remaining amount of time.
         try:
-            return proc.wait(timeout=(timeout - now).seconds)
+            return proc.wait(timeout=(timeout - now))
         except TimeoutExpired:
             # Make sure the process was killed
             proc.kill()