initramfs network configuration ignored if only ip6= on kernel command line

Bug #1639930 reported by Scott Moser on 2016-11-07
12
This bug affects 1 person
Affects Status Importance Assigned to Milestone
MAAS
Undecided
Unassigned
cloud-init
Medium
Unassigned
cloud-init (Ubuntu)
Medium
Unassigned
Xenial
Medium
Scott Moser
Yakkety
Medium
Unassigned

Bug Description

=== Begin SRU Template ===
[Impact]
On a system booted with both ip6= and ip= on the kernel command line
cloud-init will raise an exception and fail to process user-data and
have its normal affect on boot.

That is because cloud-init previously raised an exception when more
than one file in /run/net*.conf declared the same DEVICE. Changes to
isc-dhcp and initramfs-tools have changed their behavior and cloud-init
has to adjust to allow DEVICE6= and DEVICE= in separate files.

[Test Case]
Boot a system on a network with both ipv4 and ipv6 dhcp servers,
and pass kernel command line with:
  ip=dhcp ip6=dhcp

[Regression Potential]
Regression seems unlikely as this is relaxing a check. Where previously
an exception would have been raised, cloud-init will now go on.

So it seems most likely, something that didn't work before (due to raised
exception) would now still not work, but with failures. That is not
expected, but that would likely be where regressions were found.

=== End SRU Template ===

In changes made under bug 1621615 (specifically a1cdebdea), we now expect that there may be a 'ip6=' argument on the kernel command line. The changes made did not test the case where there is 'ip6=' and no 'ip='.

The code currently will return with no network configuration found if there is only ip6=...

Related bugs:
 * bug 1621615: network not configured when ipv6 netbooted into cloud-init
 * bug 1621507: initramfs-tools configure_networking() fails to dhcp IPv6 addresses
 * bug 1635716: Can't bring up a machine on a dual network (ipv4 and ipv6)

Related branches

Revision history for this message
Scott Moser (smoser) wrote :

The initial fix is easy, but bug 1621615 changes sneaked in without a unit test.

I'd really like a unit test of read_kernel_cmdline_config to accompany this change.
Also, a change to 'DHCP6_CONTENT_1' in tests/unittests/test_net.py to contain what we're currently expecting.

diff --git a/cloudinit/net/cmdline.py b/cloudinit/net/cmdline.py
index 4075a27..a077730 100644
--- a/cloudinit/net/cmdline.py
+++ b/cloudinit/net/cmdline.py
@@ -199,7 +199,7 @@ def read_kernel_cmdline_config(files=None, mac_addrs=None, cmdline=None):
         if data64:
             return util.load_yaml(_b64dgz(data64))

- if 'ip=' not in cmdline:
+ if 'ip=' not in cmdline and 'ip6=' not in cmdline:
         return None

Changed in cloud-init:
status: New → Confirmed
importance: Undecided → Medium
summary: - initramfs kernel configuration ignored if only ip6= on kernel command
+ initramfs network configuration ignored if only ip6= on kernel command
line
Changed in cloud-init (Ubuntu):
importance: Undecided → Medium
status: New → Confirmed
LaMont Jones (lamont) on 2016-11-07
tags: added: maas-ipv6
Changed in maas:
milestone: none → 2.1.2
Revision history for this message
Scott Moser (smoser) wrote :

Working branch at https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/310390

Still needs another test case or two. (would like both ip6 and ip4 at once and also only ip=)

Changed in maas:
milestone: 2.1.2 → 2.1.3
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package cloud-init - 0.7.8-61-g2d2ec70-0ubuntu1

---------------
cloud-init (0.7.8-61-g2d2ec70-0ubuntu1) zesty; urgency=medium

  * debian/grub-legacy-ec2.install: install post(inst|rm) files correctly.
    [Simon Deziel] (LP: #1581416)
  * New upstream snapshot.
    - OpenStack: extend physical types to include hyperv, hw_veb, vhost_user. [Scott Moser] (LP: #1642679)
    - tests: fix assumptions that expected no eth0 in system. [Scott Moser] (LP: #1644043)
    - net/cmdline: Consider ip= or ip6= on command line not only ip= [Scott Moser] (LP: #1639930)
    - Just use file logging by default [Joshua Harlow] (LP: #1643990)
    - Improve formatting for ProcessExecutionError [Wesley Wiedenmeier]
    - flake8: fix trailing white space [Scott Moser]
    - Doc: various documentation fixes [Sean Bright]
    - cloudinit/config/cc_rh_subscription.py: Remove repos before adding [Brent Baude]
    - packages/redhat: fix rpm spec file. [Scott Moser]
    - main: set TZ in environment if not already set. [Ryan Harper]
    - Azure: No longer rely on walinux agent. [Scott Moser] (LP: #1538522)
    - disk_setup: Use sectors as unit when formatting MBR disks with sfdisk. [Daniel Watkins] (LP: #1460715)

 -- Scott Moser <email address hidden> Mon, 28 Nov 2016 16:08:09 -0500

Changed in cloud-init (Ubuntu):
status: Confirmed → Fix Released
Scott Moser (smoser) on 2016-12-02
Changed in cloud-init (Ubuntu Xenial):
status: New → In Progress
Changed in cloud-init (Ubuntu Yakkety):
status: New → Confirmed
Changed in cloud-init (Ubuntu Xenial):
importance: Undecided → Medium
Changed in cloud-init (Ubuntu Yakkety):
importance: Undecided → Medium
Changed in cloud-init (Ubuntu Xenial):
assignee: nobody → Scott Moser (smoser)
Scott Moser (smoser) on 2016-12-02
description: updated
description: updated
Scott Moser (smoser) on 2016-12-02
Changed in cloud-init (Ubuntu Yakkety):
status: Confirmed → In Progress
Revision history for this message
Robie Basak (racb) wrote :

This is tangled up into the MAAS IPv6 SRU happening in bug 1621507. Please check that bug for verification-done before accepting cloud-init into -updates.

Revision history for this message
Robie Basak (racb) wrote : Please test proposed package

Hello Scott, or anyone else affected,

Accepted cloud-init into xenial-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/cloud-init/0.7.8-49-g9e904bb-0ubuntu1~16.04.2 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed.Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

Changed in cloud-init (Ubuntu Xenial):
status: In Progress → Fix Committed
tags: added: verification-needed
Scott Moser (smoser) on 2016-12-12
Changed in cloud-init:
status: Confirmed → Fix Committed
Revision history for this message
Scott Moser (smoser) wrote :

The test cases described in bug 1621507 cover the code path that this change
was done to fix. Additionally, unit tests (which run during build) were added
to verify the failing code path.

Specifically, this bug would be seen when the system was booted with
both ip= and ip6= on the kernel command line.
 [+]/[+] MAAS booting IPv4 on dual-stack network (with and without dhcp6)
 [+]/[+] MAAS booting IPv6 on dual-stack network (with and without dhcp4)

As a result of the tests mentioned above having been done, I am marking
this bug as verification-done.

tags: added: verification-done
removed: verification-needed
Revision history for this message
Scott Moser (smoser) wrote :

Well, further investigation showed to me that maas boots with:
   ip=off ip6=dhcp
   ip=dhcp ip6=off
but will never boot with either
   a.) ip=dhcp ip6=dhcp
   b.) ip6=dhcp

An oddity in the initramfs implementation is that 'ip6=dhcp' will enable ip=dhcp. Which seems arcane, but since ip=off is available, it does at least allow the user to imply sane behavior.

Because maas will never do both ipv6 and ipv4, then in their code path the initramfs will never creates a /run/net-eth0.conf and /run/net6-eth0.conf in their environment. That was the case that the changes here fixed. So, my statement in comment 6 was incorrect.

However, I have verified that this actually does work via some manual testing using qemu and some work i'd done for open-isci testing.

We boot a qemu system like this:
qemu-system-x86_64 -enable-kvm -device virtio-scsi-pci,id=virtio-scsi-xkvm -device virtio-net-pci,netdev=net00 -netdev type=tap,id=net00,script=no,downscript=no,ifname=tapvm0 -echr 0x5 -m 1024 -nographic -kernel /tmp/tgt-boot-test.UT2p0T/kernel -initrd /tmp/tgt-boot-test.UT2p0T/initrd -append "nomodeset iscsi_initiator=maas-enlist iscsi_target_name=tgt-boot-test-UT2p0T iscsi_target_ip=10.5.219.1iscsi_target_port=3260 iscsi_initiator=maas-enlist ip6=dhcp ro net.ifnames=0 BOOTIF_DEFAULT=eth0 root=/dev/disk/by-path/ip-10.5.219.1:3260-iscsi-tgt-boot-test-UT2p0T-lun-1-part1 overlayroot=tmpfs console=ttyS0 ds=nocloud-net;seedfrom=http://10.5.219.1:32600/"

Revision history for this message
Scott Moser (smoser) wrote :
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package cloud-init - 0.7.8-49-g9e904bb-0ubuntu1~16.04.2

---------------
cloud-init (0.7.8-49-g9e904bb-0ubuntu1~16.04.2) xenial-proposed; urgency=medium

  * cherry-pick 18203bf: disk_setup: Use sectors as unit when formatting
    MBR disks with sfdisk. (LP: #1460715)
  * cherry-pick 6e92c5f: net/cmdline: Consider ip= or ip6= on command
    line not only ip= (LP: #1639930)
  * cherry-pick 8c6878a: tests: fix assumptions that expected no eth0 in
    system. (LP: #1644043)
  * cherry-pick 2d2ec70: OpenStack: extend physical types to include
    hyperv, hw_veb, vhost_user. (LP: #1642679)

 -- Scott Moser <email address hidden> Thu, 01 Dec 2016 16:57:39 -0500

Changed in cloud-init (Ubuntu Xenial):
status: Fix Committed → Fix Released
Revision history for this message
Robie Basak (racb) wrote : Update Released

The verification of the Stable Release Update for cloud-init has completed successfully and the package has now been released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

Revision history for this message
Scott Moser (smoser) wrote :

This is fixed in cloud-init 0.7.9.

Changed in cloud-init:
status: Fix Committed → Fix Released
Revision history for this message
LaMont Jones (lamont) wrote :

No maas changes were required here, since it always specifies both ip= and ip6=

Changed in maas:
status: New → Fix Released
Revision history for this message
Brian Murray (brian-murray) wrote : Please test proposed package

Hello Scott, or anyone else affected,

Accepted cloud-init into yakkety-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/cloud-init/0.7.8-68-gca3ae67-0ubuntu1~16.10.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed.Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, and change the tag from verification-needed to verification-done. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed. In either case, details of your testing will help us make a better decision.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance!

Changed in cloud-init (Ubuntu Yakkety):
status: In Progress → Fix Committed
tags: removed: verification-done
tags: added: verification-needed
Revision history for this message
Scott Moser (smoser) wrote :
tags: added: verification-done
removed: verification-needed
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package cloud-init - 0.7.8-68-gca3ae67-0ubuntu1~16.10.1

---------------
cloud-init (0.7.8-68-gca3ae67-0ubuntu1~16.10.1) yakkety; urgency=medium

  * debian/cherry-pick: add utility for cherry picking commits from upstream
    into patches in debian/patches.
  * New upstream snapshot.
    - mounts: use mount -a again to accomplish mounts (LP: #1647708)
    - CloudSigma: Fix bug where datasource was not loaded in local search.
      (LP: #1648380)
    - when adding a user, strip whitespace from group list
      [Lars Kellogg-Stedman] (LP: #1354694)
    - fix decoding of utf-8 chars in yaml test
    - Replace usage of sys_netdev_info with read_sys_net (LP: #1625766)
    - fix problems found in python2.6 test.
    - OpenStack: extend physical types to include hyperv, hw_veb, vhost_user.
      (LP: #1642679)
    - tests: fix assumptions that expected no eth0 in system. (LP: #1644043)
    - net/cmdline: Consider ip= or ip6= on command line not only ip=
      (LP: #1639930)
    - Just use file logging by default [Joshua Harlow] (LP: #1643990)
    - Improve formatting for ProcessExecutionError [Wesley Wiedenmeier]
    - flake8: fix trailing white space
    - Doc: various documentation fixes [Sean Bright]
    - cloudinit/config/cc_rh_subscription.py: Remove repos before adding
      [Brent Baude]
    - packages/redhat: fix rpm spec file.
    - main: set TZ in environment if not already set. [Ryan Harper]
    - disk_setup: Use sectors as unit when formatting MBR disks with sfdisk.
      [Daniel Watkins] (LP: #1460715)

 -- Scott Moser <email address hidden> Mon, 19 Dec 2016 15:07:12 -0500

Changed in cloud-init (Ubuntu Yakkety):
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers