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

[Ryan McCabe]

sysconfig: Correctly render dns and dns search info.

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 McCabe (rmccabe)]

Updated per your last comments for merge after your fixes.

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

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

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

review: Needs Fixing (continuous-integration)

[Ryan McCabe (rmccabe)]

The unit tests will fail on this one until the patch you suggested is applied, btw, because of the dns search info being passed as a list.

[Ryan Harper (raharper)]

I was suggesting you pick up that patch and include it with this fix. If you'd prefer, I can MP that separately.

[Ryan McCabe (rmccabe)]

Sure, no problem. I can stick it in there and resubmit.

[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

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

FAILED: Continuous integration, rev:f85901e5bab02d47c824a945f4ee969a5c6afda3
https://jenkins.ubuntu.com/server/job/cloud-init-ci/504/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    FAILED: MAAS Compatability Testing

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

review: Needs Fixing (continuous-integration)

[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.

[Ryan McCabe (rmccabe)]

Yeah, some combination of misclicks landed this in "needs review" from superseded, and I didn't see a way to put it back to there.

[Scott Moser (smoser)]

Ryan,
One question inline.

Also, rebase on master and push and the bot will re-test which will hopefully be happy (trunk was busted, causing the failure that it reported).

Last, i think the "listify" that you're doing better fits in 'normalize_subnets'

  http://paste.ubuntu.com/26007995/

Try that diff on top of yours and see what you think.

take it and re-push and we should be good.

review: Needs Fixing

[Ryan McCabe (rmccabe)]

Responses inline. Your diff looks good. re-pushed with that and one change per the inline comments.

Unmerged commits

f85901e... by Ryan McCabe on 2017-11-16

Render DNS and DOMAIN lines for sysconfig

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/network_state.py b/cloudinit/net/network_state.py
index 0e830ee..0af0f54 100644
--- a/cloudinit/net/network_state.py
+++ b/cloudinit/net/network_state.py
@@ -325,13 +325,24 @@ class NetworkStateInterpreter(object):
         # convert subnet ipv6 netmask to cidr as needed
         subnets = _normalize_subnets(command.get('subnets'))
 
+        # listify per-subnet dns and
         # automatically set 'use_ipv6' if any addresses are ipv6
-        if not self.use_ipv6:
-            for subnet in subnets:
+        for subnet in subnets:
+            if not self.use_ipv6:
                 if (subnet.get('type').endswith('6') or
                         is_ipv6_addr(subnet.get('address'))):
                     self.use_ipv6 = True
                     break
+            if 'dns_search' in subnet:
+                paths = subnet['dns_search']
+                if not isinstance(paths, list):
+                    paths = paths.split()
+                    subnet['dns_search'] = paths
+            if 'dns_nameservers' in subnet:
+                addrs = subnet['dns_nameservers']
+                if not type(addrs) == list:
+                    addrs = addrs.split()
+                    subnet['dns_nameservers'] = addrs
 
         iface.update({
             'name': command.get('name'),
diff --git a/cloudinit/net/sysconfig.py b/cloudinit/net/sysconfig.py
index f572796..6b0b007 100644
--- a/cloudinit/net/sysconfig.py
+++ b/cloudinit/net/sysconfig.py
@@ -347,6 +347,13 @@ class Renderer(renderer.Renderer):
                     else:
                         iface_cfg['GATEWAY'] = subnet['gateway']
 
+                if 'dns_search' in subnet:
+                    iface_cfg['DOMAIN'] = ' '.join(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