Code review comment for ~smoser/cloud-init:fix/1788915-vlan-sysconfig-rendering

Revision history for this message
Ryan Harper (raharper) wrote :

Sorry, I'm still a little confused. From your test case, previously we rendered an "infra0" file like so:

% cat test_vlan/etc/sysconfig/network-scripts/ifcfg-infra0
# Created by cloud-init on instance boot automatically, do not edit.
#
BOOTPROTO=none
DEVICE=infra0
IPADDR=10.0.1.2
NETMASK=255.255.0.0
NM_CONTROLLED=no
ONBOOT=yes
PHYSDEV=infra
STARTMODE=auto
TYPE=Ethernet
USERCTL=no
VLAN=yes

And the line "PHYSDEV=infra" is the bug, right? Instead it should say

"PHYSDEV=eth0"; and omit "TYPE=Ethernet".

And this fails with an error in Centos/RHEL system on boot? Yes, I see
as captured in

https://code.launchpad.net/bugs/1788915

OK, now, instead of just changing the PHYSDEV line to use the "link" value
from the config, you're also suggesting that we allow (but warn) when
the DEVICE name is not in required format?

You said that you tested in a container and it didn't work; I'm taking
that to mean the DEVICE=infra0 case; to which I agree that it is going
to fail due to lack of VLAN_ID being present in what's rendered.

The config you're providing as an example which has a name "infra0" but
both the link and the port number means that in sysconfig renderer we
could warn but *fix* since we have what we need to render a supported
configuration.

Shouldn't we take that path?

review: Needs Information

« Back to merge proposal