Bug#1028212: prometheus-node-exporter-collectors: APT update deadlock - prevents unattended security upgrades

2023-10-16 Thread Antoine Beaupré
Control: tags -1 +pending

On 2023-10-13 09:17:49, Kyle Fazzari wrote:
> I don't entirely agree, but disagreement is okay. I do at least 
> recommend accompanying this with a cache age statistic, as we discussed 
> earlier.

Both of those have been done upstream, and I've uploaded a new snapshot
taking those into account.

I'm also planning on doing a stable update once this reaches
testing. Testers and feedback welcome!

-- 
Je viens d'un pays où engagé veut dire que tu t'es trouvé une job.
- Patrice Desbiens



Processed: Re: Bug#1028212: prometheus-node-exporter-collectors: APT update deadlock - prevents unattended security upgrades

2023-10-16 Thread Debian Bug Tracking System
Processing control commands:

> tags -1 +pending
Bug #1028212 [prometheus-node-exporter-collectors] 
prometheus-node-exporter-collectors: APT update deadlock - prevents unattended 
security upgrades
Added tag(s) pending.

-- 
1028212: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1028212
Debian Bug Tracking System
Contact ow...@bugs.debian.org with problems



Bug#1028212: prometheus-node-exporter-collectors: APT update deadlock - prevents unattended security upgrades

2023-10-13 Thread Antoine Beaupré
On 2023-10-13 11:40:17, Antoine Beaupré wrote:

[...]

> What's the magic setting to make apt check those updates on its own? I
> often get confused between unattended-upgrades and apt there...

Answering my own question, again, on my Debian bookworm machine, there's
a `/etc/cron.daily/apt-compat` script (for systemd-less systems) and a
`apt-daily.service` service. The latter does
`/usr/lib/apt/apt.systemd.daily update` which, if
`APT::Periodic::Update-Package-Lists` is set in apt-config(8), will
periodically update the package list.

I believe that is the canonical and normal way of doing this.



Bug#1028212: prometheus-node-exporter-collectors: APT update deadlock - prevents unattended security upgrades

2023-10-13 Thread Antoine Beaupré
On 2023-10-13 11:59:23, Antoine Beaupré wrote:
> severity 1028212 serious
> tags 1028212 +patch

[...]

> From 3b17a4dcb8caa56191c5be523c874a7f470bd04a Mon Sep 17 00:00:00 2001

[...]

> diff --git a/apt_info.py b/apt_info.py
> index eb1a642..9b1b675 100755
> --- a/apt_info.py
> +++ b/apt_info.py

[...]

>  registry = CollectorRegistry()
>  _write_pending_upgrades(registry, cache)
>  _write_held_upgrades(registry, cache)

That patch doesn't actually apply cleanly on bookworm because upstream
did some refactoring, the attached patch should work better.

A.
-- 
C'est avec les pierres de la loi qu'on a bâti les prisons,
et avec les briques de la religion, les bordels.
- William Blake
>From 28c179ddfd3d7e0f5bc49b93f924f0dffba5b71d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Antoine=20Beaupr=C3=A9?= 
Date: Fri, 13 Oct 2023 12:29:48 -0400
Subject: [PATCH] do not run apt update or simulate apt dist-upgrade
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This is causing all sorts of problems. The first of which is that
we're hitting our poor mirrors every time the script is ran, which, in
the Debian package configuration, is *every 15 minutes* (!!).

The second is that this locks the cache and makes this script
needlessly stumble upon a possible regression in APT from Debian
bookworm and Ubuntu 22.06:

https://bugs.launchpad.net/ubuntu/+source/apt/+bug/2003851

That still has to be confirmed: it's possible that `apt update` can
hang for a long time, but that shouldn't concern us if we delegate
this work out of band.

I also do not believe actually performing the `dist-upgrade`
calculations is necessary to compute the pending upgrades at all. I've
done work with python-apt for other projects and haven't found that to
be required: the cache has the necessary information about pending
upgrades.

Closes: #179

Signed-off-by: Antoine Beaupré 
---
 apt_info.py | 9 -
 1 file changed, 9 deletions(-)

diff --git a/apt_info.py b/apt_info.py
index 59f3ad7..81a276b 100755
--- a/apt_info.py
+++ b/apt_info.py
@@ -9,7 +9,6 @@
 
 import apt
 import collections
-import contextlib
 import os
 
 _UpgradeInfo = collections.namedtuple("_UpgradeInfo", ["labels", "count"])
@@ -90,14 +89,6 @@ def _write_reboot_required():
 def _main():
 cache = apt.cache.Cache()
 
-# First of all, attempt to update the index. If we don't have permission
-# to do so (or it fails for some reason), it's not the end of the world,
-# we'll operate on the old index.
-with contextlib.suppress(apt.cache.LockFailedException, apt.cache.FetchFailedException):
-cache.update()
-
-cache.open()
-cache.upgrade(True)
 _write_pending_upgrades(cache)
 _write_held_upgrades(cache)
 _write_autoremove_pending(cache)
-- 
2.39.2



Bug#1028212: prometheus-node-exporter-collectors: APT update deadlock - prevents unattended security upgrades

2023-10-13 Thread Antoine Beaupré
On 2023-10-13 09:17:49, Kyle Fazzari wrote:

[...]

> I don't entirely agree, but disagreement is okay. I do at least 
> recommend accompanying this with a cache age statistic, as we discussed 
> earlier.

Right, that would be a better way of going around doing that. I have a
separate upstream issue about this, and we can track that problem
there:

https://github.com/prometheus-community/node-exporter-textfile-collector-scripts/issues/180

We might be able to squeeze that metric along with the hotfix here, that
said, we just need to make sure of the better way of reporting that
metric.

a.
-- 
I've got to design so you can put it together out of garbage cans. In
part because that's what I started from, but mostly because I don’t
trust the industrial structure—they might decide to suppress us
weirdos and try to deny us the parts we need.
   - Lee Felsenstein



Bug#1028212: prometheus-node-exporter-collectors: APT update deadlock - prevents unattended security upgrades

2023-10-13 Thread Kyle Fazzari




On 10/13/23 09:14, Antoine Beaupré wrote:

On 2023-10-13 09:05:35, Kyle Fazzari wrote:

On 10/13/23 08:26, Julian Andres Klode wrote:

Also please do not run apt update in the background or try to
calculate dist upgrades, that is evil and you're breaking stuff.
If you want to check for updates, make sure the periodic apt service
is configured to run. You are entitled to one run per day. If you
do not operate the mirror infrastructure please do not run your own
updates out of band.


A fair critique, although as I mentioned in an earlier email, this
collector cannot do its job if it's running on a significantly
out-of-date cache. At the same time, if feels out of scope for this
Debian package to ensure the periodic apt service is configured to run.
Feels like a rock and a hard place. Thoughts?


I think this is a deployment issue. People who provision this package
should *not* expect it to run `apt update`: I certainly didn't, and the
previous (shell) implementation didn't either.

We have other tools that continuously pull mirrors, and as jak stated,
APT can be configured to do so as well (although I'm not sure what the
canonical way of doing so).

So let's not do this here.


I don't entirely agree, but disagreement is okay. I do at least 
recommend accompanying this with a cache age statistic, as we discussed 
earlier.


Kyle



Bug#1028212: prometheus-node-exporter-collectors: APT update deadlock - prevents unattended security upgrades

2023-10-13 Thread Antoine Beaupré
On 2023-10-13 09:05:35, Kyle Fazzari wrote:
> On 10/13/23 08:26, Julian Andres Klode wrote:
>> Also please do not run apt update in the background or try to
>> calculate dist upgrades, that is evil and you're breaking stuff.
>> If you want to check for updates, make sure the periodic apt service
>> is configured to run. You are entitled to one run per day. If you
>> do not operate the mirror infrastructure please do not run your own
>> updates out of band.
>
> A fair critique, although as I mentioned in an earlier email, this 
> collector cannot do its job if it's running on a significantly 
> out-of-date cache. At the same time, if feels out of scope for this 
> Debian package to ensure the periodic apt service is configured to run. 
> Feels like a rock and a hard place. Thoughts?

I think this is a deployment issue. People who provision this package
should *not* expect it to run `apt update`: I certainly didn't, and the
previous (shell) implementation didn't either.

We have other tools that continuously pull mirrors, and as jak stated,
APT can be configured to do so as well (although I'm not sure what the
canonical way of doing so).

So let's not do this here.

-- 
A developed country is not a place where the poor have cars. It's
where the rich use public transportation.
- Gustavo Petro 



Bug#1028212: prometheus-node-exporter-collectors: APT update deadlock - prevents unattended security upgrades

2023-10-13 Thread Kyle Fazzari




On 10/13/23 08:26, Julian Andres Klode wrote:

Also please do not run apt update in the background or try to
calculate dist upgrades, that is evil and you're breaking stuff.
If you want to check for updates, make sure the periodic apt service
is configured to run. You are entitled to one run per day. If you
do not operate the mirror infrastructure please do not run your own
updates out of band.


A fair critique, although as I mentioned in an earlier email, this 
collector cannot do its job if it's running on a significantly 
out-of-date cache. At the same time, if feels out of scope for this 
Debian package to ensure the periodic apt service is configured to run. 
Feels like a rock and a hard place. Thoughts?


Kyle



Processed: Re: Bug#1028212: prometheus-node-exporter-collectors: APT update deadlock - prevents unattended security upgrades

2023-10-13 Thread Debian Bug Tracking System
Processing commands for cont...@bugs.debian.org:

> severity 1028212 serious
Bug #1028212 [prometheus-node-exporter-collectors] 
prometheus-node-exporter-collectors: APT update deadlock - prevents unattended 
security upgrades
Severity set to 'serious' from 'important'
> tags 1028212 +patch
Bug #1028212 [prometheus-node-exporter-collectors] 
prometheus-node-exporter-collectors: APT update deadlock - prevents unattended 
security upgrades
Added tag(s) patch.
> thanks
Stopping processing here.

Please contact me if you need assistance.
-- 
1028212: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1028212
Debian Bug Tracking System
Contact ow...@bugs.debian.org with problems