Merge ~nacc/git-ubuntu:lp1741949-unset-snap-in-lxc-env into git-ubuntu:master

[Nish Aravamudan]

Make Jenkins happy.

Make Jenkins happy.

[Robie Basak (racb)]

Why are we unsetting SNAP for some calls to lxc, but not all of them? Is there some criteria we have to use to decide this? Or are you just working around the calls that happen to break for now?

As long as we have to work around some other project's bug, I'd like to have a bug reference in a comment in our code which shows us why the workaround is present and what its status is for us to get rid of it. So when I asked for a bug against this, I meant I wanted an open bug task against the project which we are working around (whether that is snapd or lxd I'm not sure).

review: Needs Information

[Server Team CI bot (server-team-bot)]

FAILED: Continuous integration, rev:c21e653eddde8b9ad7d4d89ee56ede9a4d504dc1
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~racb/usd-importer/+git/usd-importer/+merge/336297/+edit-commit-message

https://jenkins.ubuntu.com/server/job/git-ubuntu-ci/246/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Style Check
    SUCCESS: Unit Tests
    SUCCESS: Integration Tests
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/git-ubuntu-ci/246/rebuild

review: Needs Fixing (continuous-integration)

[Server Team CI bot (server-team-bot)]

FAILED: Continuous integration, rev:d16589b9f2835f9cf1cae6daf228447b343e7330
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~racb/usd-importer/+git/usd-importer/+merge/336297/+edit-commit-message

https://jenkins.ubuntu.com/server/job/git-ubuntu-ci/250/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Style Check
    SUCCESS: Unit Tests
    SUCCESS: Integration Tests
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/git-ubuntu-ci/250/rebuild

review: Needs Fixing (continuous-integration)

[Server Team CI bot (server-team-bot)]

FAILED: Continuous integration, rev:3e1087eb7c21e31b2f07bf3c44591102914ab0f1
https://jenkins.ubuntu.com/server/job/git-ubuntu-ci/254/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Style Check
    SUCCESS: Unit Tests
    FAILED: Integration Tests

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/git-ubuntu-ci/254/rebuild

review: Needs Fixing (continuous-integration)

[Nish Aravamudan (nacc)]

There is a typo in my last run.py line, I'll try and fix it to pass CI this
weekend.

On Jan 19, 2018 17:52, "Server Team CI bot" <
<email address hidden>> wrote:

> Review: Needs Fixing continuous-integration
>
> FAILED: Continuous integration, rev:3e1087eb7c21e31b2f07bf3c445911
> 02914ab0f1
> https://jenkins.ubuntu.com/server/job/git-ubuntu-ci/254/
> Executed test runs:
> SUCCESS: Checkout
> SUCCESS: Style Check
> SUCCESS: Unit Tests
> FAILED: Integration Tests
>
> Click here to trigger a rebuild:
> https://jenkins.ubuntu.com/server/job/git-ubuntu-ci/254/rebuild
>
> --
> https://code.launchpad.net/~nacc/usd-importer/+git/usd-
> importer/+merge/336391
> You are the owner of ~nacc/usd-importer:lp1741949-unset-snap-in-lxc-env.
>

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:467d0f591f7fcd79b7fb7a99ca6be12585ffc3ef
https://jenkins.ubuntu.com/server/job/git-ubuntu-ci/255/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Style Check
    SUCCESS: Unit Tests
    SUCCESS: Integration Tests
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/git-ubuntu-ci/255/rebuild

review: Approve (continuous-integration)

[Andreas Hasenack (ahasenack)]

Nice to see it applied to all lxc commands, and even nicer to see a green CI run :)

+1

review: Approve

[Robie Basak (racb)]

Thanks! I'll merge this now.

review: Approve

diff --git a/gitubuntu/build.py b/gitubuntu/build.py
index 072d3e7..22b95a9 100644
--- a/gitubuntu/build.py
+++ b/gitubuntu/build.py
@@ -35,7 +35,13 @@ from gitubuntu.cache import CACHE_PATH
 from gitubuntu.dsc import GitUbuntuDsc
 import gitubuntu.lint
 import gitubuntu.git_repository
-from gitubuntu.run import decode_binary, run, runq
+from gitubuntu.run import (
+    decode_binary,
+    get_cmd_in_origpath,
+    run,
+    runq,
+    run_lxc,
+)
 from gitubuntu.source_information import (
     GitUbuntuSourceInformation,
     NoPublicationHistoryException,
@@ -889,15 +895,6 @@ def do_build_host_exitstack(
 
     return built_pardir_contents
 
-def get_cmd_in_origpath(cmd):
-    return shutil.which(
-        cmd,
-        path=os.getenv(
-            'ORIG_PATH',
-            os.getenv('PATH', os.defpath),
-        ),
-    )
-
 @functools.lru_cache()
 def _gpg_abspath():
     orig_gpg = get_cmd_in_origpath('gpg')
@@ -905,18 +902,11 @@ def _gpg_abspath():
         return orig_gpg
     raise RuntimeError("Unable to find a gpg binary, is it installed?")
 
[e-mail]_cache()
-def _lxc_abspath():
-    orig_lxc = get_cmd_in_origpath('lxc')
-    if orig_lxc:
-        return orig_lxc
-    raise RuntimeError("Unable to find a lxc binary, is it installed?")
-
 def _cleanup_lxd(container_name):
-    run([_lxc_abspath(), 'stop', '--force', container_name])
+    run_lxc(['stop', '--force', container_name])
 
 def _run_in_lxd(container_name, args, user=None, **kwargs):
-    cmd = [_lxc_abspath(), 'exec', container_name, '--',]
+    _args = ['exec', container_name, '--',]
     # user == None means run as root
     if user:
         try:
@@ -944,9 +934,9 @@ def _run_in_lxd(container_name, args, user=None, **kwargs):
                 )
             else:
                 raise
-        cmd.extend(['sudo', '-s', '-H', '-u', user,])
-    cmd.extend(args)
-    return run(cmd, **kwargs)
+        _args.extend(['sudo', '-s', '-H', '-u', user,])
+    _args.extend(args)
+    return run_lxc(_args, **kwargs)
 
 def do_build_lxd_exitstack(
     changelog,
@@ -960,19 +950,18 @@ def do_build_lxd_exitstack(
     retry_backoffs,
     stack,
 ):
-    lxc = _lxc_abspath()
     try:
-        run([lxc, 'list'])
+        run_lxc(['list'])
     except Exception as e:
         raise RuntimeError("LXD running?") from e
 
     container_name = petname.Generate(2, '-')
 
-    cmd = [lxc, 'launch', '-e']
+    args = ['launch', '-e']
     if profile:
-        cmd.extend(['-p', profile])
+        args.extend(['-p', profile])
     if image:
-        cmd.extend([image,])
+        args.extend([image,])
     else:
         try:
             source = derive_source_from_changelog(changelog)
@@ -985,12 +974,12 @@ def do_build_lxd_exitstack(
             raise
         if source == 'ubuntu':
             # devel is properly handled for the daily remote
-            cmd.extend(['ubuntu-daily:%s' % series,])
+            args.extend(['ubuntu-daily:%s' % series,])
         else:
-            cmd.extend(['images:debian/%s' % series,])
-    cmd.extend([container_name,])
+            args.extend(['images:debian/%s' % series,])
+    args.extend([container_name,])
     try:
-        run(cmd)
+        run_lxc(args)
     except Exception as e:
         raise RuntimeError("Failed to launch ephemeral build container") from e
 
@@ -1030,23 +1019,24 @@ def do_build_lxd_exitstack(
         )
 
     try:
-        run([
-            lxc,
-            'file',
-            'push',
-            archive_tarball_name,
-            '%s/tmp/' % container_name,
-        ])
+        run_lxc(
+            [
+                'file',
+                'push',
+                archive_tarball_name,
+                '%s/tmp/' % container_name,
+            ],
+        )
     except Exception as e:
         raise RuntimeError(
             "Failed to push archive tarball to ephemeral build container"
         ) from e
 
-    cmd = [lxc, 'file', 'push']
-    cmd.extend(tarballs)
-    cmd.extend(['%s/tmp/' % container_name,])
+    args = ['file', 'push']
+    args.extend(tarballs)
+    args.extend(['%s/tmp/' % container_name,])
     try:
-        run(cmd)
+        run_lxc(args)
     except Exception as e:
         raise RuntimeError("Failed to push orig tarballs to container") from e
 
@@ -1142,14 +1132,14 @@ def do_build_lxd_exitstack(
     )
 
     built_temp_contents = new_temp_contents - orig_temp_contents
-    cmd = [lxc, 'file', 'pull']
-    cmd.extend([
+    args = ['file', 'pull']
+    args.extend([
             '%s%s' % (container_name, os.path.join('/tmp', f))
             for f in built_temp_contents
         ]
     )
-    cmd.extend([os.pardir,])
-    run(cmd)
+    args.extend([os.pardir,])
+    run_lxc(args)
 
     return [os.path.join(os.pardir, f) for f in built_temp_contents]
 
diff --git a/gitubuntu/run.py b/gitubuntu/run.py
index 4b1a86c..8fb4d85 100644
--- a/gitubuntu/run.py
+++ b/gitubuntu/run.py
@@ -1,4 +1,7 @@
+import functools
 import logging
+import os
+import shutil
 import subprocess
 
 
@@ -102,3 +105,39 @@ def decode_binary(binary, verbose=True):
             logging.warning("Failed to decode blob: %s", e)
             logging.warning("blob=%s", binary.decode(errors='replace'))
         return binary.decode('utf-8', errors='replace')
+
+def get_cmd_in_origpath(cmd):
+    return shutil.which(
+        cmd,
+        path=os.getenv(
+            'ORIG_PATH',
+            os.getenv('PATH', os.defpath),
+        ),
+    )
+
[e-mail]_cache()
+def _lxc_abspath():
+    orig_lxc = get_cmd_in_origpath('lxc')
+    if orig_lxc:
+        return orig_lxc
+    raise RuntimeError("Unable to find a lxc binary, is it installed?")
+
+
+def run_lxc(args, env=None, **kwargs):
+    cmd = [_lxc_abspath()] + args
+    # lxd assumes that if SNAP is set in the environment, that it is
+    # running as a snap. That is not necessarily true when git-ubuntu is
+    # the snapped application. So unset SNAP before we exec.
+    # LP: #1741949
+    # This can be dropped if https://github.com/lxc/lxd/issues/4183 is
+    # resolved.
+    if env:
+        env_unset_SNAP = env.copy()
+    else:
+        env_unset_SNAP = os.environ.copy()
+    try:
+        del env_unset_SNAP['SNAP']
+    except KeyError:
+        pass
+
+    return run(cmd, env=env_unset_SNAP, **kwargs)