Merge lp:~paelzer/curtin/curtin-bug-1645680-gpgagent into lp:~curtin-dev/curtin/trunk

[Christian Ehrhardt ]

Workaround gpg-agent/dirmngr spawned daemons by add-apt-repository.

Kill dirmngr/gpg-agent after add-apt-repository as an interim workaround.
The failure occurs only when gpg2 is used, so only on yakkety or newer.

Add a Yakkety test to excercise this path.

A fix - or better interim workaround - for bug 1645680.

Looks good in make check and works on my system when tested with the apt feature to add ppa's on Zesty.

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

PASSED: Continuous integration, rev:433
https://jenkins.ubuntu.com/server/job/curtin-ci/275/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=amd64/275
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=i386/275
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=ppc64el/275
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=s390x/275

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

review: Approve (continuous-integration)

[Scott Moser (smoser)]

My primary comment is that I dont know that I like this being in ChrootableTarget.
Putting it there, as opposed to a wrapper around that makes it seem more legitimate than the ugly hack that it is.

other comments in line.

[Christian Ehrhardt  (paelzer)]

Hi Scott,
it is a hack as intended :-)

The main reason to make it part of ChrootableTarget is that it has to happen before the exit path of ChrootableTarget which does the umount.

We could try to make it part of the "with ChrootableTarget" section.
That would get rid of the flag as well and much more - rest of feedback ok and commented inline.

Testing new version and pushing soon.

[Christian Ehrhardt  (paelzer)]

Test with new version http://paste.ubuntu.com/23558651/
See line 1198 for the "effect" of the fix.

Looks much nicer as overall diff (located very close to the issue)
... pushing

[lp:~paelzer/curtin/curtin-bug-1645680-gpgagent]

434. By Christian Ehrhardt  on 2016-11-30

make fix local to the add-apt-repository call

Along that integrate some minor review feedback.

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

PASSED: Continuous integration, rev:434
https://jenkins.ubuntu.com/server/job/curtin-ci/277/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=amd64/277
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=i386/277
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=ppc64el/277
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=s390x/277

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

review: Approve (continuous-integration)

[lp:~paelzer/curtin/curtin-bug-1645680-gpgagent]

435. By Christian Ehrhardt  on 2016-11-30

move cleanup in a finally statement

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

PASSED: Continuous integration, rev:435
https://jenkins.ubuntu.com/server/job/curtin-ci/278/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=amd64/278
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=i386/278
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=ppc64el/278
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=s390x/278

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

review: Approve (continuous-integration)

[Christian Ehrhardt  (paelzer)]

Adapted some of the suggestions (and explained at the comments why not all), thanks Ryan for looking into it.

Tested and pushing ...

[lp:~paelzer/curtin/curtin-bug-1645680-gpgagent]

436. By Christian Ehrhardt  on 2016-12-01

use regexp to have only one killall

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

PASSED: Continuous integration, rev:436
https://jenkins.ubuntu.com/server/job/curtin-ci/279/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=amd64/279
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=i386/279
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=ppc64el/279
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=s390x/279

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

review: Approve (continuous-integration)

[Christian Ehrhardt  (paelzer)]

Thanks for merging.
Sorry on the chars, actually my git commits are autocolored and 80 char
blocked - need to get that for my bzr as well.
Or might we move curtin to git as well?

=== modified file 'curtin/commands/apt_config.py'
--- curtin/commands/apt_config.py	2016-11-14 22:55:12 +0000
+++ curtin/commands/apt_config.py	2016-12-01 07:12:43 +0000
@@ -24,6 +24,7 @@
 import os
 import re
 import sys
+import time
 import yaml
 
 from curtin.log import LOG
@@ -402,13 +403,21 @@
             ent['filename'] += ".list"
 
         if aa_repo_match(source):
-            try:
-                with util.ChrootableTarget(
-                        target, sys_resolvconf=True) as in_chroot:
+            with util.ChrootableTarget(
+                    target, sys_resolvconf=True) as in_chroot:
+                time_entered = time.time()
+                try:
                     in_chroot.subp(["add-apt-repository", source])
-            except util.ProcessExecutionError:
-                LOG.exception("add-apt-repository failed.")
-                raise
+                except util.ProcessExecutionError:
+                    LOG.exception("add-apt-repository failed.")
+                    raise
+                finally:
+                    # workaround to gnupg >=2.x spawning daemons (LP: #1645680)
+                    seconds_since = time.time() - time_entered + 1
+                    in_chroot.subp(['killall', '--wait', '--quiet',
+                                    '--younger-than', '%ds' % seconds_since,
+                                    '--regexp', '(dirmngr|gpg-agent)'],
+                                   rcs=[0, 1])
             continue
 
         sourcefn = util.target_path(target, ent['filename'])