launchpad-reviewers team mailing list archive

[Tom Wardill <tom.wardill@xxxxxxxxxxxxx>]

Tom Wardill has proposed merging ~twom/lpbuildbot-worker:use-lxc-exec into lpbuildbot-worker:main.

Commit message:
Use lxc exec rather than pylxd

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~twom/lpbuildbot-worker/+git/lpbuildbot-worker/+merge/390846
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~twom/lpbuildbot-worker:use-lxc-exec into lpbuildbot-worker:main.

diff --git a/create-lp-tests-lxd b/create-lp-tests-lxd
index 8564bcb..5af4d6e 100755
--- a/create-lp-tests-lxd
+++ b/create-lp-tests-lxd
@@ -1,11 +1,13 @@
 #!/usr/bin/env python3
 
 import argparse
+from datetime import datetime
 import os
 from os.path import expanduser
 from pathlib import Path
 from pwd import getpwnam
 import shlex
+import subprocess
 import sys
 
 from pylxd import Client
@@ -55,21 +57,31 @@ def _put_file(container, source, destination):
         container.files.put(destination, source_file.read())
 
 
-def _exec(container, command, env={}, return_stdout=False, user=None, cwd=None):
-    # The version of pylxd in xenial (2.0.7) is missing a lot of options
-    # for execute (return code, running-as-user).
+def _exec(
+    container, command, return_stdout=False, user=None, cwd=None, exit_on_error=True
+):
+    container_name = container.name
     command = " ".join(shlex.quote(arg) for arg in command)
     if cwd:
         command = "cd {} && {}".format(shlex.quote(cwd), command)
     if user:
-        stdout, stderr = container.execute(["sudo", "su", user, "-c", command], env)
-    else:
-        stdout, stderr = container.execute(["sh", "-c", command], env)
+        command = "sudo su {} -c {}".format(user, shlex.quote(command))
+    print("{} Running: {}".format(datetime.utcnow().isoformat(), command))
 
-    print(stdout)
-    print(stderr, file=sys.stderr)
+    # This should use `capture_output`, but it's not available in py3.5
     if return_stdout:
-        return stdout
+        result = subprocess.run(
+            ["lxc", "exec", container_name, "--", "bash", "-c", command],
+            check=exit_on_error,
+            stdout=subprocess.PIPE,
+        )
+    else:
+        result = subprocess.run(
+            ["lxc", "exec", container_name, "--", "bash", "-c", command],
+            check=exit_on_error,
+        )
+    stdout = result.stdout.decode() if result.stdout else None
+    return result.returncode, stdout
 
 
 def delete_old_image(client, image_name):
@@ -144,9 +156,7 @@ def install_code(container, series, directory):
 
     print("Installing Launchpad apt dependencies")
     _exec(
-        container,
-        ["apt-get", "install", "-y"] + LP_DEB_DEPENDENCIES,
-        env={"DEBIAN_FRONTEND": "noninteractive", "LANG": "C"},
+        container, ["apt-get", "install", "-y"] + LP_DEB_DEPENDENCIES,
     )
 
     print("User configuration")
@@ -163,12 +173,14 @@ def install_code(container, series, directory):
         ],
     )
     _exec(container, ["adduser", "buildbot", "sudo"])
-    gid = _exec(
+    _, gid = _exec(
         container,
         ["python", "-c", "import pwd; print pwd.getpwnam('buildbot').pw_gid"],
         return_stdout=True,
     )
-    _exec(container, ["addgroup", "--gid", gid, "buildbot"])
+    _exec(
+        container, ["addgroup", "--gid", gid.strip(), "buildbot"], exit_on_error=False
+    )
     _exec(container, ["chown", "-R", "buildbot:buildbot", directory])
 
     print("Configuring ssh")
@@ -242,6 +254,7 @@ if __name__ == "__main__":
     parser = argparse.ArgumentParser(description="Build a LXD image for Buildbot")
     parser.add_argument("series", type=str, help="Ubuntu series to base the image on.")
     parser.add_argument("directory", type=str, help="Directory to mount code from.")
+
     args = parser.parse_args()
     image_name = "lptests-{}".format(args.series)
 
@@ -251,6 +264,8 @@ if __name__ == "__main__":
 
     # Work around xenial's pylxd not understanding the lxd snap.
     os.environ.setdefault("LXD_DIR", "/var/snap/lxd/common/lxd")
+    os.environ.setdefault("DEBIAN_FRONTEND", "noninteractive")
+    os.environ.setdefault("LANG", "C")
 
     client = Client()
 
diff --git a/lp-setup-lxd-build b/lp-setup-lxd-build
index ba2d386..233e1bf 100755
--- a/lp-setup-lxd-build
+++ b/lp-setup-lxd-build
@@ -1,6 +1,7 @@
 #!/usr/bin/env python3
 
 import argparse
+from datetime import datetime
 import os
 from os.path import expanduser
 import pathlib
@@ -19,21 +20,31 @@ BUILD_STEPS = [
 ]
 
 
-def _exec(container, command, env={}, return_stdout=False, user=None, cwd=None):
-    # The version of pylxd in xenial (2.0.7) is missing a lot of options
-    # for execute (return code, running-as-user).
+def _exec(
+    container, command, return_stdout=False, user=None, cwd=None, exit_on_error=True
+):
+    container_name = container.name
     command = " ".join(shlex.quote(arg) for arg in command)
     if cwd:
         command = "cd {} && {}".format(shlex.quote(cwd), command)
     if user:
-        stdout, stderr = container.execute(["sudo", "su", user, "-c", command], env)
-    else:
-        stdout, stderr = container.execute(["sh", "-c", command], env)
+        command = "sudo su {} -c {}".format(user, shlex.quote(command))
+    print("{} Running: {}".format(datetime.utcnow().isoformat(), command))
 
-    print(stdout)
-    print(stderr, file=sys.stderr)
+    # This should use `capture_output`, but it's not available in py3.5
     if return_stdout:
-        return stdout
+        result = subprocess.run(
+            ["lxc", "exec", container_name, "--", "bash", "-c", command],
+            check=exit_on_error,
+            stdout=subprocess.PIPE,
+        )
+    else:
+        result = subprocess.run(
+            ["lxc", "exec", container_name, "--", "bash", "-c", command],
+            check=exit_on_error,
+        )
+    stdout = result.stdout.decode() if result.stdout else None
+    return result.returncode, stdout
 
 
 def start_new_container(client, image_name, work_dir):
diff --git a/lp-setup-lxd-test b/lp-setup-lxd-test
index c428ee6..8444810 100755
--- a/lp-setup-lxd-test
+++ b/lp-setup-lxd-test
@@ -1,6 +1,7 @@
 #!/usr/bin/env python3
 
 import argparse
+from datetime import datetime
 import os
 from os.path import expanduser
 import random
@@ -12,21 +13,31 @@ import sys
 from pylxd import Client
 
 
-def _exec(container, command, env={}, return_stdout=False, user=None, cwd=None):
-    # The version of pylxd in xenial (2.0.7) is missing a lot of options
-    # for execute (return code, running-as-user).
+def _exec(
+    container, command, return_stdout=False, user=None, cwd=None, exit_on_error=True
+):
+    container_name = container.name
     command = " ".join(shlex.quote(arg) for arg in command)
     if cwd:
         command = "cd {} && {}".format(shlex.quote(cwd), command)
     if user:
-        stdout, stderr = container.execute(["sudo", "su", user, "-c", command], env)
-    else:
-        stdout, stderr = container.execute(["sh", "-c", command], env)
+        command = "sudo su {} -c {}".format(user, shlex.quote(command))
+    print("{} Running: {}".format(datetime.utcnow().isoformat(), command))
 
-    print(stdout)
-    print(stderr, file=sys.stderr)
+    # This should use `capture_output`, but it's not available in py3.5
     if return_stdout:
-        return stdout
+        result = subprocess.run(
+            ["lxc", "exec", container_name, "--", "bash", "-c", command],
+            check=exit_on_error,
+            stdout=subprocess.PIPE,
+        )
+    else:
+        result = subprocess.run(
+            ["lxc", "exec", container_name, "--", "bash", "-c", command],
+            check=exit_on_error,
+        )
+    stdout = result.stdout.decode() if result.stdout else None
+    return result.returncode, stdout
 
 
 def create_ephemeral_container(