Bug#871833: conntrack-tools: Fix autopkgtests for compatibility with Ubuntu kernel, containers

2017-08-14 Thread Steve Langasek 
Hi Arturo,

On Sun, Aug 13, 2017 at 03:13:50PM +0200, Arturo Borrero Gonzalez wrote:
> On 12 August 2017 at 06:15, Steve Langasek  
> wrote:

> > The conntrack-tools 1.4.4+snapshot20161117 update was blocked from reaching
> > Ubuntu's 17.04 release, because it regresses its autopkgtests in Ubuntu
> > compared to 1.4.3-3.

> Hi Steve,

> thanks for your work, comments below.

> > I have so far identified two problems with the tests when running on Ubuntu:

> >  - the tests modprobe a bunch of nf_conntrack_* modules; if some of these
> >modules don't load because they are built into the kernel (as is the case
> >for some of them on Ubuntu), these tests fail.

> This is good to have. I can upstream the patch.

> >  - several tests are marked 'isolation-container', but provide a
> >configuration to conntrackd that requires host-level privileges, so
> >conntrackd fails at startup when run in a container.

> The scheduling & bufsize things seems a bit hackish for what we need:
> a simple test run of conntrackd.
> I would simplify the logic by just ignoring these configuration
> options and using conntrackd/system defaults.
> I mean: in this test case, we are interested in checking that
> conntrackd can run with a basic configuration. The actual
> configuration is less important.

Ok, I wasn't sure so I went with a solution that made only minimal changes
to the config when testing on a machine / VM.  But if this is not part of
the behavior that's important to test, I agree that it's better to drop so
we get the same test behavior in VMs and containers.

> Please note, that scheduling defaults have changed in conntrackd [0].
> If we don't specify a scheduling configuration, conntrackd will try to
> use SCHED_RR by default. Which may require further privileges? I don't
> have a full autopkgtest environment to test different setups (i.e.,
> LXC, virtualization, etc).

I see that the upstream patch does:


+   if (sched_setscheduler(0, sched_type, ) < 0)
+   dlog(LOG_WARNING, "scheduler configuration failed: %s. "
+"Likely a bug in conntrackd, please report it. "
+"Continuing with system default scheduler.",
+strerror(errno));

So that's fine, this will just fall back gracefully in a container.

> So, please, I would ask you to:

> 1) send a (separate) patch for the modules things

> 2) make changes to the test suite according to the simplifications I
> mentioned (separate patch)

Ok, attached the split out patches.

> I believe that by following this approach Ubuntu will benefit as well.
> 
> [0] 
> http://git.netfilter.org/conntrack-tools/commit/?id=210f5429678dba06f361b1f37bcb946f27e2e20b

Cheers,
-- 
Steve Langasek   Give me a lever long enough and a Free OS
Debian Developer   to set it on, and I can move the world.
Ubuntu Developerhttp://www.debian.org/
slanga...@ubuntu.com vor...@debian.org
diff -Nru conntrack-tools-1.4.4+snapshot20161117/debian/patches/series conntrack-tools-1.4.4+snapshot20161117/debian/patches/series
--- conntrack-tools-1.4.4+snapshot20161117/debian/patches/series	2016-12-05 03:52:20.0 -0800
+++ conntrack-tools-1.4.4+snapshot20161117/debian/patches/series	2017-08-11 14:50:43.0 -0700
@@ -1 +1,2 @@
 missing-include.patch
+skip-already-loaded-modules.patch
diff -Nru conntrack-tools-1.4.4+snapshot20161117/debian/patches/skip-already-loaded-modules.patch conntrack-tools-1.4.4+snapshot20161117/debian/patches/skip-already-loaded-modules.patch
--- conntrack-tools-1.4.4+snapshot20161117/debian/patches/skip-already-loaded-modules.patch	1969-12-31 16:00:00.0 -0800
+++ conntrack-tools-1.4.4+snapshot20161117/debian/patches/skip-already-loaded-modules.patch	2017-08-11 20:53:24.0 -0700
@@ -0,0 +1,56 @@
+Description: Don't fail on modprobe since the driver might be built-in
+ Any of these nf drivers could be built-ins instead of modules; don't cause
+ the testsuite to fail on modprobe, instead let it proceed and succeed/fail
+ later based on actual test results.
+ .
+ Ideally we would check up front if the driver is loaded rather than trying
+ to modprobe and ignoring failures, but there doesn't seem to be a reliable
+ place to check this in the kernel filesystem
+Author: Steve Langasek 
+
+Index: conntrack-tools-1.4.4+snapshot20161117/tests/conntrack/run-test.sh
+===
+--- conntrack-tools-1.4.4+snapshot20161117.orig/tests/conntrack/run-test.sh
 conntrack-tools-1.4.4+snapshot20161117/tests/conntrack/run-test.sh
+@@ -10,10 +10,12 @@
+ #
+ # XXX: module auto-load not support by nfnetlink_cttimeout yet :-(
+ #
+-modprobe nf_conntrack_ipv4
+-modprobe nf_conntrack_ipv6
+-modprobe nf_conntrack_proto_udplite
+-modprobe nf_conntrack_proto_sctp
+-modprobe

Bug#871833: conntrack-tools: Fix autopkgtests for compatibility with Ubuntu kernel, containers

2017-08-13 Thread Arturo Borrero Gonzalez 
On 12 August 2017 at 06:15, Steve Langasek  wrote:
>
> The conntrack-tools 1.4.4+snapshot20161117 update was blocked from reaching
> Ubuntu's 17.04 release, because it regresses its autopkgtests in Ubuntu
> compared to 1.4.3-3.

Hi Steve,

thanks for your work, comments below.

>
> I have so far identified two problems with the tests when running on Ubuntu:
>
>  - the tests modprobe a bunch of nf_conntrack_* modules; if some of these
>modules don't load because they are built into the kernel (as is the case
>for some of them on Ubuntu), these tests fail.

This is good to have. I can upstream the patch.

>  - several tests are marked 'isolation-container', but provide a
>configuration to conntrackd that requires host-level privileges, so
>conntrackd fails at startup when run in a container.
>

The scheduling & bufsize things seems a bit hackish for what we need:
a simple test run of conntrackd.
I would simplify the logic by just ignoring these configuration
options and using conntrackd/system defaults.
I mean: in this test case, we are interested in checking that
conntrackd can run with a basic configuration. The actual
configuration is less important.

Please note, that scheduling defaults have changed in conntrackd [0].
If we don't specify a scheduling configuration, conntrackd will try to
use SCHED_RR by default. Which may require further privileges? I don't
have a full autopkgtest environment to test different setups (i.e.,
LXC, virtualization, etc).

So, please, I would ask you to:

1) send a (separate) patch for the modules things
2) make changes to the test suite according to the simplifications I
mentioned (separate patch)

I believe that by following this approach Ubuntu will benefit as well.

[0] 
http://git.netfilter.org/conntrack-tools/commit/?id=210f5429678dba06f361b1f37bcb946f27e2e20b

Bug#871833: conntrack-tools: Fix autopkgtests for compatibility with Ubuntu kernel, containers

2017-08-11 Thread Steve Langasek 
Package: conntrack-tools
User: ubuntu-de...@lists.ubuntu.com
Usertags: origin-ubuntu artful ubuntu-patch autopkgtest
Version: 1:1.4.4+snapshot20161117-5
Severity: normal
Tags: patch

Dear maintainers,

The conntrack-tools 1.4.4+snapshot20161117 update was blocked from reaching
Ubuntu's 17.04 release, because it regresses its autopkgtests in Ubuntu
compared to 1.4.3-3.

I have so far identified two problems with the tests when running on Ubuntu:

 - the tests modprobe a bunch of nf_conntrack_* modules; if some of these
   modules don't load because they are built into the kernel (as is the case
   for some of them on Ubuntu), these tests fail.
 - several tests are marked 'isolation-container', but provide a
   configuration to conntrackd that requires host-level privileges, so
   conntrackd fails at startup when run in a container.

I have uploaded the attached patch to Ubuntu to address both of these
problems.  Please consider applying in Debian as well.

I am still seeing one test failure on the Ubuntu autopkgtest infrastructure
that I haven't yet reproduced locally.  I'll send another patch if and when
I manage to address this.

Thanks,
-- 
Steve Langasek   Give me a lever long enough and a Free OS
Debian Developer   to set it on, and I can move the world.
Ubuntu Developerhttp://www.debian.org/
slanga...@ubuntu.com vor...@debian.org
diff -Nru conntrack-tools-1.4.4+snapshot20161117/debian/patches/series 
conntrack-tools-1.4.4+snapshot20161117/debian/patches/series
--- conntrack-tools-1.4.4+snapshot20161117/debian/patches/series
2016-12-05 03:52:20.0 -0800
+++ conntrack-tools-1.4.4+snapshot20161117/debian/patches/series
2017-08-11 14:50:43.0 -0700
@@ -1 +1,2 @@
 missing-include.patch
+skip-already-loaded-modules.patch
diff -Nru 
conntrack-tools-1.4.4+snapshot20161117/debian/patches/skip-already-loaded-modules.patch
 
conntrack-tools-1.4.4+snapshot20161117/debian/patches/skip-already-loaded-modules.patch
--- 
conntrack-tools-1.4.4+snapshot20161117/debian/patches/skip-already-loaded-modules.patch
 1969-12-31 16:00:00.0 -0800
+++ 
conntrack-tools-1.4.4+snapshot20161117/debian/patches/skip-already-loaded-modules.patch
 2017-08-11 20:53:24.0 -0700
@@ -0,0 +1,56 @@
+Description: Don't fail on modprobe since the driver might be built-in
+ Any of these nf drivers could be built-ins instead of modules; don't cause
+ the testsuite to fail on modprobe, instead let it proceed and succeed/fail
+ later based on actual test results.
+ .
+ Ideally we would check up front if the driver is loaded rather than trying
+ to modprobe and ignoring failures, but there doesn't seem to be a reliable
+ place to check this in the kernel filesystem
+Author: Steve Langasek 
+
+Index: conntrack-tools-1.4.4+snapshot20161117/tests/conntrack/run-test.sh
+===
+--- conntrack-tools-1.4.4+snapshot20161117.orig/tests/conntrack/run-test.sh
 conntrack-tools-1.4.4+snapshot20161117/tests/conntrack/run-test.sh
+@@ -10,10 +10,12 @@
+ #
+ # XXX: module auto-load not support by nfnetlink_cttimeout yet :-(
+ #
+-modprobe nf_conntrack_ipv4
+-modprobe nf_conntrack_ipv6
+-modprobe nf_conntrack_proto_udplite
+-modprobe nf_conntrack_proto_sctp
+-modprobe nf_conntrack_proto_dccp
+-modprobe nf_conntrack_proto_gre
++# any or all of these might be built-ins rather than modules, so don't error
++# out on failure from modprobe
++modprobe nf_conntrack_ipv4 || true
++modprobe nf_conntrack_ipv6 || true
++modprobe nf_conntrack_proto_udplite || true
++modprobe nf_conntrack_proto_sctp || true
++modprobe nf_conntrack_proto_dccp || true
++modprobe nf_conntrack_proto_gre || true
+ ./test testcases
+Index: conntrack-tools-1.4.4+snapshot20161117/tests/nfct/run-test.sh
+===
+--- conntrack-tools-1.4.4+snapshot20161117.orig/tests/nfct/run-test.sh
 conntrack-tools-1.4.4+snapshot20161117/tests/nfct/run-test.sh
+@@ -11,10 +11,12 @@
+ #
+ # XXX: module auto-load not support by nfnetlink_cttimeout yet :-(
+ #
+-modprobe nf_conntrack_ipv4
+-modprobe nf_conntrack_ipv6
+-modprobe nf_conntrack_proto_udplite
+-modprobe nf_conntrack_proto_sctp
+-modprobe nf_conntrack_proto_dccp
+-modprobe nf_conntrack_proto_gre
++# any or all of these might be built-ins rather than modules, so don't error
++# out on failure from modprobe
++modprobe nf_conntrack_ipv4 || true
++modprobe nf_conntrack_ipv6 || true
++modprobe nf_conntrack_proto_udplite || true
++modprobe nf_conntrack_proto_sctp || true
++modprobe nf_conntrack_proto_dccp || true
++modprobe nf_conntrack_proto_gre || true
+ ./test timeout
diff -Nru 
conntrack-tools-1.4.4+snapshot20161117/debian/tests/basic-daemon-test.sh 
conntrack-tools-1.4.4+snapshot20161117/debian/tests/basic-daemon-test.sh
---