Merge ~rmccabe/cloud-init:bug1705804-2 into cloud-init:master

[Ryan McCabe]

Currently when dns and dns search info is provided, it is not rendered
when outputting to sysconfig format.

This patch causes the DNS and DOMAIN lines to be written out rendering
sysconfig.

LP: #1705804

[Ryan McCabe (rmccabe)]

Updated to handle dns_search when received as either a string or a list
Updated per Ryan Harper's other comments on the previous iteration

[Server Team CI bot (server-team-bot)]

FAILED: Continuous integration, rev:cc2c67acb04881e38c6d8b5e2f7a68e7c5e99164
https://jenkins.ubuntu.com/server/job/cloud-init-ci/494/
Executed test runs:
    SUCCESS: Checkout
    FAILED: Unit & Style Tests

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/494/rebuild

review: Needs Fixing (continuous-integration)

[Ryan Harper (raharper)]

On Thu, Nov 16, 2017 at 12:35 PM, Ryan McCabe <email address hidden> wrote:

>
>
> Diff comments:
>
> > diff --git a/cloudinit/net/sysconfig.py b/cloudinit/net/sysconfig.py
> > index f572796..6e11247 100644
> > --- a/cloudinit/net/sysconfig.py
> > +++ b/cloudinit/net/sysconfig.py
> > @@ -347,6 +347,16 @@ class Renderer(renderer.Renderer):
> > else:
> > iface_cfg['GATEWAY'] = subnet['gateway']
> >
> > + if 'dns_search' in subnet:
> > + if isinstance(subnet['dns_search'], (list, tuple)):
>
> Is adding the dns search and nameservers info into the network_state dict
> the right thing to do here? It looks like dns info from all interfaces, not
> just subnets of the current one, is being added, and it produces bad output
> e.g.,:
>
>
> iface lo inet loopback
> dns-nameservers 69.9.160.191 69.9.191.4 10.0.0.1
> dns-search foo.com

dns-nameservers is valid and accepts space separated list

>
>
> when it should be
>
> iface lo inet loopback
> dns-nameservers 10.0.0.1
> dns-search foo.com
>
> Doing only the listify and not adding the info to the network_state dict
> there seems to do the right thing.
>

Yes; that may be the best bet here. That keeps with the more general idea
that dns-settings should be
scoped per-interface.

>
> > + iface_cfg['DOMAIN'] = '
> '.join(subnet['dns_search'][:3])
> > + else:
> > + iface_cfg['DOMAIN'] = subnet['dns_search']
> > +
> > + if 'dns_nameservers' in subnet:
> > + for i, k in enumerate(subnet['dns_nameservers'][:3],
> 1):
> > + iface_cfg['DNS' + str(i)] = k
> > +
> > @classmethod
> > def _render_subnet_routes(cls, iface_cfg, route_cfg, subnets):
> > for i, subnet in enumerate(subnets,
> start=len(iface_cfg.children)):
>
>
> --
> https://code.launchpad.net/~rmccabe/cloud-init/+git/cloud-
> init/+merge/333722
> Your team cloud-init commiters is requested to review the proposed merge
> of ~rmccabe/cloud-init:bug1705804-2 into cloud-init:master.
>
> _______________________________________________
> Mailing list: https://launchpad.net/~cloud-init-dev
> Post to : <email address hidden>
> Unsubscribe : https://launchpad.net/~cloud-init-dev
> More help : https://help.launchpad.net/ListHelp
>

[Scott Moser (smoser)]

one comment on ryan's patch inline

do not use .split(" "), here is why:
 > "a b c".split()
 ['a', 'b', 'c']

 > "a b c".split(" ")
 ['a', 'b', '', '', 'c']

[Ryan McCabe (rmccabe)]

Sorry, what I meant re: "dns-nameservers 69.9.160.191 69.9.191.4 10.0.0.1" is that the first two IPs were pulled from an ethX interface and don't belong there, given the yaml, not that there was a problem with the syntax.

ACK, re: Scott's comment

I'll resubmit a proposal that does only the listify and cleans up the split

[Ryan Harper (raharper)]

> one comment on ryan's patch inline
>
> do not use .split(" "), here is why:
> > "a b c".split()
> ['a', 'b', 'c']
>
> > "a b c".split(" ")
> ['a', 'b', '', '', 'c']

I had to hit reply to see how those two strings looked different

Even cut and pasting, something is removing the additional spaces you added:

"a b c".split(" ")

Crazy, but point taken,

>>> "a...b.c".split(".")
['a', '', '', 'b', 'c']

[Scott Moser (smoser)]

I'm going to mark this as 'rejected' as i'm 99% sure you have fixed with
https://code.launchpad.net/~rmccabe/cloud-init/+git/cloud-init/+merge/333844

re-open if that is not the case.

Unmerged commits

cc2c67a... by Ryan McCabe on 2017-11-15

Currently when dns and dns search info is provided, it is not rendered
when outputting to sysconfig format.

This patch causes the DNS and DOMAIN lines to be written out rendering
sysconfig.

LP: #1705804

diff --git a/cloudinit/net/sysconfig.py b/cloudinit/net/sysconfig.py
index f572796..6e11247 100644
--- a/cloudinit/net/sysconfig.py
+++ b/cloudinit/net/sysconfig.py
@@ -347,6 +347,16 @@ class Renderer(renderer.Renderer):
                     else:
                         iface_cfg['GATEWAY'] = subnet['gateway']
 
+                if 'dns_search' in subnet:
+                    if isinstance(subnet['dns_search'], (list, tuple)):
+                      iface_cfg['DOMAIN'] = ' '.join(subnet['dns_search'][:3])
+                    else:
+                      iface_cfg['DOMAIN'] = subnet['dns_search']
+
+                if 'dns_nameservers' in subnet:
+                    for i, k in enumerate(subnet['dns_nameservers'][:3], 1):
+                        iface_cfg['DNS' + str(i)] = k
+
     @classmethod
     def _render_subnet_routes(cls, iface_cfg, route_cfg, subnets):
         for i, subnet in enumerate(subnets, start=len(iface_cfg.children)):
diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py
index bbb63cb..f3fa2a3 100644
--- a/tests/unittests/test_net.py
+++ b/tests/unittests/test_net.py
@@ -436,6 +436,9 @@ NETWORK_CONFIGS = {
                 BOOTPROTO=dhcp
                 DEFROUTE=yes
                 DEVICE=eth99
+                DNS1=8.8.8.8
+                DNS2=8.8.4.4
+                DOMAIN="barley.maas sach.maas"
                 GATEWAY=65.61.151.37
                 HWADDR=c0:d6:9f:2c:e8:80
                 IPADDR=192.168.21.3
@@ -836,6 +839,9 @@ pre-down route del -net 10.0.0.0 netmask 255.0.0.0 gw 11.0.0.1 metric 3 || true
                 BOOTPROTO=none
                 DEFROUTE=yes
                 DEVICE=eth0.101
+                DNS1=192.168.0.10
+                DNS2=10.23.23.134
+                DOMAIN="barley.maas sacchromyces.maas brettanomyces.maas"
                 GATEWAY=192.168.0.1
                 IPADDR=192.168.0.2
                 IPADDR1=192.168.2.10