Hello Cyril,
thank you for your report!
Am Tue, 17 Mar 2020 04:00:18 +0100
schrieb Cyril Brulebois <k...@debian.org>:
> 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 = <STATE>) {
> 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 <de...@sumpfralle.de>
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 <de...@sumpfralle.de>
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) is sufficient to mach these packages.
+ $release_name =~ s|/updates$||;
+ $release_names{$release_name} = 1;
}
}
return sort keys %release_names;
commit 9a792a331b9d652e7e48355bfdf5c2817f0c7845
Author: mafri <47006694+mafr...@users.noreply.github.com>
Date: Sat Aug 10 15:15:10 2019 +0200
Fixed iteration at apt_all
last was triggered at each iteration of the @releases array. So the print was only executed if current line of state file is the first entry of @releases.
diff --git a/plugins/node.d.linux/apt_all.in b/plugins/node.d.linux/apt_all.in
index e0f63f69..38baad58 100644
--- a/plugins/node.d.linux/apt_all.in
+++ b/plugins/node.d.linux/apt_all.in
@@ -108,8 +108,10 @@ sub print_state() {
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;
+ if ($line =~ /^(hold|pending)_$release_cleaned\.(value|extinfo)/) {
+ print $line ;
+ last;
+ }
}
}
close STATE;
