Bug#954118: munin-plugins-core: apt_all is all broken

[devel]

Hello Cyril,


On Mon, 30 Mar 2020 20:34:41 +0200 Cyril Brulebois  wrote:
> Beware, starting with bullseye (Debian 11, currently in “testing” mode),
> the security suite will be named -updates rather than
> /updates.

Thanks for the hint. This is now handled properly since Munin 2.0.65.


> I'll see what happens in the upcoming days/weeks; I already anticipate
> that I'll have to tweak the limits for some suites: “apt-get
> dist-upgrade” would only upgrade packages from the “-backports”
> suite *if they are currently installed from there*, while it seems the
> new apt_all plugin reports all packages that could be upgraded there,
> even if the current version is that of the base … I can expand
> with some example if you'd like.
> 
> 
> FWIW: That's the usual behaviour for suites configured with:
> 
> NotAutomatic: yes
> ButAutomaticUpgrades: yes
> 
> which is the case for *-backports suites (experimental is a little
> different).

Maybe you could take a look at the current state of your apt_all tracking?

I would be glad to fix remaining details for Bullseye, if possible.

Thank you!

Cheers,
Lars

Bug#954118: munin-plugins-core: apt_all is all broken

[Cyril Brulebois]

Hallo Lars!

de...@sumpfralle.de  (2020-03-20):
> thank you for your report!
[…]
> Thank you for your analysis and recommendations.

You're very welcome!

> I verified, that the packages available via "buster/updates" are
> assigned to the "buster" distribution. Thus the issue can be solved by
> removing "/updates" from distribution names.

Beware, starting with bullseye (Debian 11, currently in “testing” mode),
the security suite will be named -updates rather than
/updates.

This can be seen here already:
  http://security.debian.org/debian-security/dists/

Anyway, I didn't check the actual apt-related changes in this particular
commit…

> I just pushed the corrsponding changes upstream.
> 
> Attached you find the three relevant upstream commits (including the
> fix for the "last" condition above). I would be happy, if you could
> test the attached patches and report back, whether these work for you.

… instead, I've deployed the most recent version of the plugin, taken
from the stable-2.0 branch (2.0.59 tag). The idea is to give it a bit
of “black box testing”, as if I didn't know what's supposed to happen
there. :)

I'll see what happens in the upcoming days/weeks; I already anticipate
that I'll have to tweak the limits for some suites: “apt-get
dist-upgrade” would only upgrade packages from the “-backports”
suite *if they are currently installed from there*, while it seems the
new apt_all plugin reports all packages that could be upgraded there,
even if the current version is that of the base … I can expand
with some example if you'd like.


FWIW: That's the usual behaviour for suites configured with:

NotAutomatic: yes
ButAutomaticUpgrades: yes

which is the case for *-backports suites (experimental is a little
different).


Tschüß!
-- 
Cyril Brulebois (k...@debian.org)
D-I release manager -- Release team member -- Freelance Consultant


signature.asc
Description: PGP signature

Bug#954118: munin-plugins-core: apt_all is all broken

[devel]

Hello Cyril,

thank you for your report!


Am Tue, 17 Mar 2020 04:00:18 +0100
schrieb Cyril Brulebois :

> Problem 1: This is not reproducible.
> 
> sub guess_releases() {
> …
> return keys %release_names;
> }
> 
> as the various known suites will not be returned in the same order.
> Switching to “sort keys” fixes this issue.

Good catch! I will do this.


> Problem 2: This doesn't do what it should.
> 
> sub print_state() {
> …
> while (my $line = ) {
> foreach my $release (@releases) {
> my $release_cleaned = clean_fieldname($release);
> # print only lines that are exected for the currently 
> requested releases
> print $line if ($line =~ 
> /^(hold|pending)_$release_cleaned\.(value|extinfo)/);
> last;
> }
> }
> close STATE;
> }
> 
> One could see the “last;” as an optimization: if a line matches, don't
> bother checking the other release. But it's not inside the matching
> conditional! Meaning whatever result the first line gets (match or no
> match), one gets out of the loop…
> 
> Dropping the “last;” fixes this issue. (One could optimize a little by
> using a regular if() form, and putting the “last;” inside.)

Indeed this was fixed and released by upstream meanwhile (commit 9a792a331b).


> Problem 3: This plugin doesn't report security updates!
> 
> # try to determine the currently available distributions by inspecting 
> the repository URLs
> sub guess_releases() {
> open(my $fh, "-|", "apt-get update --print-uris")
> or die("Failed to determine distribution releases via 'apt-get 
> update --print-uris");
> my %release_names;
> my $line;
> while ( ! eof($fh) ) {
> defined( $line = readline $fh ) or die "Failed to read line from 
> output of 'apt-get': $!";
> # example line:
> # 'http://ftp.debian.org/debian/dists/stable/InRelease' 
> ftp.debian.org_debian_dists_stable_InRelease 0
> if ($line =~ m'^.*/dists/([^/]+)/.*$') {
> $release_names{$1} = 1;
> }
> }
> return keys %release_names;
> }
> 
> The m'^.*/dists/([^/]+)/.*$' pattern doesn't allow for distribution
> names with slashes, meaning no luck for buster/updates.
> 
> Switching to m'^.*/dists/(.+)/(?:In)?Release.*$' would fix the suite
> detection, but then the plugin wouldn't work properly anyway:
> 
> E: The value 'buster/updates' is invalid for APT::Default-Release as such 
> a release is not available in the sources

Thank you for your analysis and recommendations.

I verified, that the packages available via "buster/updates" are assigned to
the "buster" distribution. Thus the issue can be solved by removing "/updates"
from distribution names.

I just pushed the corrsponding changes upstream.

Attached you find the three relevant upstream commits (including the fix for
the "last" condition above).
I would be happy, if you could test the attached patches and report back,
whether these work for you.

Thank you!

Cheers,
Lars
commit 435d2c6ca24b253d49323c0b122ad5f7f9869e18
Author: Lars Kruse 
Date:   Fri Mar 20 14:35:31 2020 +0100

Plugin apt_all: enforce stable order of fields

Previously the order of releases (and thus: fields) was random.

Thanks, Cyril Brulebois.
See https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=954118

diff --git a/plugins/node.d.linux/apt_all.in b/plugins/node.d.linux/apt_all.in
index fd3c373d..cdfc21fe 100644
--- a/plugins/node.d.linux/apt_all.in
+++ b/plugins/node.d.linux/apt_all.in
@@ -83,7 +83,7 @@ sub guess_releases() {
 $release_names{$1} = 1;
 }
 }
-return keys %release_names;
+return sort keys %release_names;
 }
 
 
commit b8d366dc7c0a89b63a0bc5f288dc46b9c6e1c763
Author: Lars Kruse 
Date:   Fri Mar 20 14:41:30 2020 +0100

Plugin apt_all: include release names with slashes (e.g. buster/updates)

Previously security updates were ignored due to the additional slash in
the release name.

Thanks, Cyril Brulebois.
See https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=954118

diff --git a/plugins/node.d.linux/apt_all.in b/plugins/node.d.linux/apt_all.in
index cdfc21fe..fddcfacf 100644
--- a/plugins/node.d.linux/apt_all.in
+++ b/plugins/node.d.linux/apt_all.in
@@ -79,8 +79,12 @@ sub guess_releases() {
 defined( $line = readline $fh ) or die "Failed to read line from output of 'apt-get': $!";
 # example line:
 # 'http://ftp.debian.org/debian/dists/stable/InRelease' ftp.debian.org_debian_dists_stable_InRelease 0
-if ($line =~ m'^.*/dists/([^/]+)/.*$') {
-$release_names{$1} = 1;
+if ($line =~ m#^.*/dists/([^']+)/[^/]+/[^/]+/Packages#) {
+my $release_name = $1;
+# The security updates are named like "buster/updates". The first part (before the
+# slash) 

Bug#954118: munin-plugins-core: apt_all is all broken

[Cyril Brulebois]

Package: munin-plugins-core
Version: 2.0.49-1
Severity: important

Hi,

I've noticed strange behaviour with the apt_all plugin, which alternates
between the OK (0 pending) and UNKNOWN status for e.g. buster and
buster-proposed-updates, leading to many notification mails.

Looking into it, it seems there are a few serious problems for something
that one might be tempted to trust to reflect the need for updating a
system…


Problem 1: This is not reproducible.

sub guess_releases() {
…
return keys %release_names;
}

as the various known suites will not be returned in the same order.
Switching to “sort keys” fixes this issue.


Problem 2: This doesn't do what it should.

sub print_state() {
…
while (my $line = ) {
foreach my $release (@releases) {
my $release_cleaned = clean_fieldname($release);
# print only lines that are exected for the currently requested 
releases
print $line if ($line =~ 
/^(hold|pending)_$release_cleaned\.(value|extinfo)/);
last;
}
}
close STATE;
}

One could see the “last;” as an optimization: if a line matches, don't
bother checking the other release. But it's not inside the matching
conditional! Meaning whatever result the first line gets (match or no
match), one gets out of the loop…

Dropping the “last;” fixes this issue. (One could optimize a little by
using a regular if() form, and putting the “last;” inside.)


Problem 3: This plugin doesn't report security updates!

# try to determine the currently available distributions by inspecting the 
repository URLs
sub guess_releases() {
open(my $fh, "-|", "apt-get update --print-uris")
or die("Failed to determine distribution releases via 'apt-get 
update --print-uris");
my %release_names;
my $line;
while ( ! eof($fh) ) {
defined( $line = readline $fh ) or die "Failed to read line from 
output of 'apt-get': $!";
# example line:
# 'http://ftp.debian.org/debian/dists/stable/InRelease' 
ftp.debian.org_debian_dists_stable_InRelease 0
if ($line =~ m'^.*/dists/([^/]+)/.*$') {
$release_names{$1} = 1;
}
}
return keys %release_names;
}

The m'^.*/dists/([^/]+)/.*$' pattern doesn't allow for distribution
names with slashes, meaning no luck for buster/updates.

Switching to m'^.*/dists/(.+)/(?:In)?Release.*$' would fix the suite
detection, but then the plugin wouldn't work properly anyway:

E: The value 'buster/updates' is invalid for APT::Default-Release as such a 
release is not available in the sources


I'm not sure how to best approach a possible fix for this third problem,
so I'll try and check what the “apt” plugin does (it seems to lump all
updates in a single value). If “apt_all” is not fixed in this regard…
well it seems to me it's actively harmful to our users as they are *not*
trackingsecurity updates?


Cheers,
-- 
Cyril Brulebois (k...@debian.org)
D-I release manager -- Release team member -- Freelance Consultant