diff mbox series

[v1] isar-bootstrap: Run gpg-agent before starting apt-key

Message ID 20201216155330.28348-1-ibr@radix50.net
State Superseded, archived
Headers show
Series [v1] isar-bootstrap: Run gpg-agent before starting apt-key | expand

Commit Message

Baurzhan Ismagulov Dec. 16, 2020, 5:53 a.m. UTC 
From: Yuri Adamov <yadamov@ilbers.de>

Building rpi-stretch natively (under qemu) sometimes fails with:

gpg: can't connect to the agent: IPC connect call failed

gpg starts gpg-agent and times out after 5 s. This value is hard-coded.

Besides, leaving running gpg-agent processes is not clean and prevents
unmounting of filesystems.

This patch starts and stops the agent manually.

Signed-off-by: Yuri Adamov <yadamov@ilbers.de>
---

Notes:
    * Submitting WIP for preview, as cleaning up will require testing time.
    * Remove sleeping.
    * Remove -9 in kill.
    * Maybe check if starting the agent is necessary.
    * Remove OVERRIDES_append and get_distro_needs_gpg_support() if unused.

 .../recipes-core/isar-bootstrap/isar-bootstrap.inc | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

Comments

Jan Kiszka Dec. 16, 2020, 6:41 a.m. UTC | #1 
On 16.12.20 16:53, Baurzhan Ismagulov wrote:
> From: Yuri Adamov <yadamov@ilbers.de>
> 
> Building rpi-stretch natively (under qemu) sometimes fails with:
> 
> gpg: can't connect to the agent: IPC connect call failed
> 
> gpg starts gpg-agent and times out after 5 s. This value is hard-coded.
> 

This is not limited to stretch or rpi. We were seeing this with buster
builds on our CI systems as well - likely when they were overloaded.

> Besides, leaving running gpg-agent processes is not clean and prevents
> unmounting of filesystems.
> 
> This patch starts and stops the agent manually.
> 
> Signed-off-by: Yuri Adamov <yadamov@ilbers.de>
> ---
> 
> Notes:
>     * Submitting WIP for preview, as cleaning up will require testing time.
>     * Remove sleeping.

Yep, that would be good.

>     * Remove -9 in kill.
>     * Maybe check if starting the agent is necessary.
>     * Remove OVERRIDES_append and get_distro_needs_gpg_support() if unused.

That last two points I was wondering as well: Why do we need to make it
unconditionally now? That should at least be explain - or fixed.

> 
>  .../recipes-core/isar-bootstrap/isar-bootstrap.inc | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc b/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc
> index 4925a45d..74569e5d 100644
> --- a/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc
> +++ b/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc
> @@ -24,7 +24,7 @@ DISTRO_BOOTSTRAP_KEYFILES = ""
>  THIRD_PARTY_APT_KEYFILES = ""
>  DEPLOY_ISAR_BOOTSTRAP ?= ""
>  DISTRO_BOOTSTRAP_BASE_PACKAGES = "locales"
> -DISTRO_BOOTSTRAP_BASE_PACKAGES_append_gnupg = ",gnupg"
> +DISTRO_BOOTSTRAP_BASE_PACKAGES_append = ",gnupg"
>  DISTRO_BOOTSTRAP_BASE_PACKAGES_append_https-support = "${@https_support(d)}"
>  
>  inherit deb-dl-dir
> @@ -307,14 +307,24 @@ isar_bootstrap() {
>              mkdir -p "${ROOTFSDIR}/etc/apt/apt.conf.d"
>              install -v -m644 "${WORKDIR}/isar-apt.conf" \
>                               "${ROOTFSDIR}/etc/apt/apt.conf.d/50isar.conf"
> +            MY_GPGHOME=$(chroot "${ROOTFSDIR}" mktemp -d /tmp/gpghomeXXXXXXXXXX)
> +            echo "Created temporary directory ${MY_GPGHOME} for gpg-agent"
> +            chroot "${ROOTFSDIR}" gpg-agent --homedir "${MY_GPGHOME}" --daemon
>              find ${APT_KEYS_DIR}/ -type f | while read keyfile
>              do
>                  kfn="$(basename $keyfile)"
>                  cp $keyfile "${ROOTFSDIR}/tmp/$kfn"
>                  chroot "${ROOTFSDIR}" /usr/bin/apt-key \
> -                   --keyring ${THIRD_PARTY_APT_KEYRING} add "/tmp/$kfn"
> +                    --keyring ${THIRD_PARTY_APT_KEYRING} \
> +                    --homedir ${MY_GPGHOME} add "/tmp/$kfn"
>                  rm "${ROOTFSDIR}/tmp/$kfn"
>              done
> +            sleep 4
> +            GPG_AGENT_PID=$(ps -aux | grep "gpg-agent.*${MY_GPGHOME}" | grep -v grep | awk  '{print $2}')
> +            echo "Killing gpg-agent with pid $GPG_AGENT_PID"
> +            /bin/kill -9 ${GPG_AGENT_PID}
> +            sleep 4
> +            chroot "${ROOTFSDIR}" /bin/rm -rf "${MY_GPGHOME}"
>  
>              if [ "${@get_distro_suite(d, True)}" = "stretch" ] && [ "${@get_host_release().split('.')[0]}" -lt "4" ]; then
>                  install -v -m644 "${WORKDIR}/isar-apt-fallback.conf" \
> 

I do like the approach of controlling gpg's lifecycle. As you said, some
cleanup is needed, but I'm all for going this direction.

Jan
Jan Kiszka Jan. 18, 2021, 7:30 a.m. UTC | #2 
On 16.12.20 17:41, [ext] Jan Kiszka wrote:
> On 16.12.20 16:53, Baurzhan Ismagulov wrote:
>> From: Yuri Adamov <yadamov@ilbers.de>
>>
>> Building rpi-stretch natively (under qemu) sometimes fails with:
>>
>> gpg: can't connect to the agent: IPC connect call failed
>>
>> gpg starts gpg-agent and times out after 5 s. This value is hard-coded.
>>
> 
> This is not limited to stretch or rpi. We were seeing this with buster
> builds on our CI systems as well - likely when they were overloaded.
> 
>> Besides, leaving running gpg-agent processes is not clean and prevents
>> unmounting of filesystems.
>>
>> This patch starts and stops the agent manually.
>>
>> Signed-off-by: Yuri Adamov <yadamov@ilbers.de>
>> ---
>>
>> Notes:
>>     * Submitting WIP for preview, as cleaning up will require testing time.
>>     * Remove sleeping.
> 
> Yep, that would be good.
> 
>>     * Remove -9 in kill.
>>     * Maybe check if starting the agent is necessary.
>>     * Remove OVERRIDES_append and get_distro_needs_gpg_support() if unused.
> 
> That last two points I was wondering as well: Why do we need to make it
> unconditionally now? That should at least be explain - or fixed.
> 
>>
>>  .../recipes-core/isar-bootstrap/isar-bootstrap.inc | 14 ++++++++++++--
>>  1 file changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc b/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc
>> index 4925a45d..74569e5d 100644
>> --- a/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc
>> +++ b/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc
>> @@ -24,7 +24,7 @@ DISTRO_BOOTSTRAP_KEYFILES = ""
>>  THIRD_PARTY_APT_KEYFILES = ""
>>  DEPLOY_ISAR_BOOTSTRAP ?= ""
>>  DISTRO_BOOTSTRAP_BASE_PACKAGES = "locales"
>> -DISTRO_BOOTSTRAP_BASE_PACKAGES_append_gnupg = ",gnupg"
>> +DISTRO_BOOTSTRAP_BASE_PACKAGES_append = ",gnupg"
>>  DISTRO_BOOTSTRAP_BASE_PACKAGES_append_https-support = "${@https_support(d)}"
>>  
>>  inherit deb-dl-dir
>> @@ -307,14 +307,24 @@ isar_bootstrap() {
>>              mkdir -p "${ROOTFSDIR}/etc/apt/apt.conf.d"
>>              install -v -m644 "${WORKDIR}/isar-apt.conf" \
>>                               "${ROOTFSDIR}/etc/apt/apt.conf.d/50isar.conf"
>> +            MY_GPGHOME=$(chroot "${ROOTFSDIR}" mktemp -d /tmp/gpghomeXXXXXXXXXX)
>> +            echo "Created temporary directory ${MY_GPGHOME} for gpg-agent"
>> +            chroot "${ROOTFSDIR}" gpg-agent --homedir "${MY_GPGHOME}" --daemon
>>              find ${APT_KEYS_DIR}/ -type f | while read keyfile
>>              do
>>                  kfn="$(basename $keyfile)"
>>                  cp $keyfile "${ROOTFSDIR}/tmp/$kfn"
>>                  chroot "${ROOTFSDIR}" /usr/bin/apt-key \
>> -                   --keyring ${THIRD_PARTY_APT_KEYRING} add "/tmp/$kfn"
>> +                    --keyring ${THIRD_PARTY_APT_KEYRING} \
>> +                    --homedir ${MY_GPGHOME} add "/tmp/$kfn"
>>                  rm "${ROOTFSDIR}/tmp/$kfn"
>>              done
>> +            sleep 4
>> +            GPG_AGENT_PID=$(ps -aux | grep "gpg-agent.*${MY_GPGHOME}" | grep -v grep | awk  '{print $2}')
>> +            echo "Killing gpg-agent with pid $GPG_AGENT_PID"
>> +            /bin/kill -9 ${GPG_AGENT_PID}
>> +            sleep 4
>> +            chroot "${ROOTFSDIR}" /bin/rm -rf "${MY_GPGHOME}"
>>  
>>              if [ "${@get_distro_suite(d, True)}" = "stretch" ] && [ "${@get_host_release().split('.')[0]}" -lt "4" ]; then
>>                  install -v -m644 "${WORKDIR}/isar-apt-fallback.conf" \
>>
> 
> I do like the approach of controlling gpg's lifecycle. As you said, some
> cleanup is needed, but I'm all for going this direction.
> 
> Jan
> 

Any news on this?

Jan
diff mbox series

Patch

diff --git a/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc b/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc
index 4925a45d..74569e5d 100644
--- a/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc
+++ b/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc
@@ -24,7 +24,7 @@  DISTRO_BOOTSTRAP_KEYFILES = ""
 THIRD_PARTY_APT_KEYFILES = ""
 DEPLOY_ISAR_BOOTSTRAP ?= ""
 DISTRO_BOOTSTRAP_BASE_PACKAGES = "locales"
-DISTRO_BOOTSTRAP_BASE_PACKAGES_append_gnupg = ",gnupg"
+DISTRO_BOOTSTRAP_BASE_PACKAGES_append = ",gnupg"
 DISTRO_BOOTSTRAP_BASE_PACKAGES_append_https-support = "${@https_support(d)}"
 
 inherit deb-dl-dir
@@ -307,14 +307,24 @@  isar_bootstrap() {
             mkdir -p "${ROOTFSDIR}/etc/apt/apt.conf.d"
             install -v -m644 "${WORKDIR}/isar-apt.conf" \
                              "${ROOTFSDIR}/etc/apt/apt.conf.d/50isar.conf"
+            MY_GPGHOME=$(chroot "${ROOTFSDIR}" mktemp -d /tmp/gpghomeXXXXXXXXXX)
+            echo "Created temporary directory ${MY_GPGHOME} for gpg-agent"
+            chroot "${ROOTFSDIR}" gpg-agent --homedir "${MY_GPGHOME}" --daemon
             find ${APT_KEYS_DIR}/ -type f | while read keyfile
             do
                 kfn="$(basename $keyfile)"
                 cp $keyfile "${ROOTFSDIR}/tmp/$kfn"
                 chroot "${ROOTFSDIR}" /usr/bin/apt-key \
-                   --keyring ${THIRD_PARTY_APT_KEYRING} add "/tmp/$kfn"
+                    --keyring ${THIRD_PARTY_APT_KEYRING} \
+                    --homedir ${MY_GPGHOME} add "/tmp/$kfn"
                 rm "${ROOTFSDIR}/tmp/$kfn"
             done
+            sleep 4
+            GPG_AGENT_PID=$(ps -aux | grep "gpg-agent.*${MY_GPGHOME}" | grep -v grep | awk  '{print $2}')
+            echo "Killing gpg-agent with pid $GPG_AGENT_PID"
+            /bin/kill -9 ${GPG_AGENT_PID}
+            sleep 4
+            chroot "${ROOTFSDIR}" /bin/rm -rf "${MY_GPGHOME}"
 
             if [ "${@get_distro_suite(d, True)}" = "stretch" ] && [ "${@get_host_release().split('.')[0]}" -lt "4" ]; then
                 install -v -m644 "${WORKDIR}/isar-apt-fallback.conf" \