On Thu, Oct 5, 2017 at 1:38 PM, Scott Moser <email address hidden> wrote:

>
>
> Diff comments:
>
> >
> > === modified file 'examples/tests/network_alias.yaml'
> > --- examples/tests/network_alias.yaml 2017-06-13 22:10:10 +0000
> > +++ examples/tests/network_alias.yaml 2017-10-05 16:35:19 +0000
> > @@ -9,11 +9,9 @@
> >            subnets:
> >                - type: static
> >                  address: 10.47.98.1/24
> > -                mtu: 1501
> >                - type: static
> >                  address: 2001:4800:78ff:1b:be76:4eff:fe06:ffac
> >                  netmask: 'ffff:ffff:ffff:ffff::'
> > -                mtu: 1480
>
> you say "not needed" in your commit message, but why?
> and isnt it concerning that this diddnt have fallout in tests ?
>

It does have fallout for netplan which doesn;'t support ipv6 mtu;
however, the *scope* of this test is multiple ip address per interface
The mtu itself has no affect on assigning multiple addresses to a single
interface.

The failure to apply an ipv6 MTU in this test is not germane; we're
only testing if we can set multiple ips.


>
> >          # multi_v4_alias: multiple v4 addrs on same interface
> >          - type: physical
> >            name: interface1
> >
> > === modified file 'tests/vmtests/test_network.py'
> > --- tests/vmtests/test_network.py     2017-08-02 15:46:35 +0000
> > +++ tests/vmtests/test_network.py     2017-10-05 16:35:19 +0000
> > @@ -285,14 +285,19 @@
> >          ip_route_show = self.load_collect_file("ip_route_show")
> >          logger.debug("ip route show:\n{}".format(ip_route_show))
> >          for line in [line for line in ip_route_show.split('\n')
> > -                     if 'src' in line]:
> > +                     if 'src' in line and not
> line.startswith('default')]:
> > +            print('ip_route_show: line: %s' % line)
> >              m = re.search(r'^(?P<network>\S+)\sdev\s' +
> >                            r'(?P<devname>\S+)\s+' +
> > -                          r'proto kernel\s+scope link' +
> > -                          r'\s+src\s(?P<src_ip>\S+)',
> > +                          r'proto\s(?P<proto>\S+)\s+' +
> > +                          r'scope\s(?P<scope>\S+)\s+' +
> > +                          r'src\s(?P<src_ip>\S+)',
> >                            line)
> > -            route_info = m.groupdict('')
> > -            logger.debug(route_info)
> > +            if m:
> > +                route_info = m.groupdict('')
> > +                logger.debug(route_info)
> > +            else:
> > +                raise ValueError('Failed match ip_route_show line: %s'
> % line)
>
> bikeshed: this is probably more of a RuntimeException
> kind of "got stuff i didnt expect".
> doesnt matter. take input or not.
>

Sure


>
> >
> >          routes = {
> >              '4': route_n,
> > @@ -390,7 +395,8 @@
> >                      gateways.append(subnet.get('gateway'))
> >                  for route in subnet.get('routes', []):
> >                      gateways += __find_gw_config(route)
> > -                return gateways
> > +                # drop duplicate gateways (static routes)
>
> this does have the side affect of sorting your list.
>

That's ok, we iterate through the whole list, each item is
checked if it's found in the entire output.


> > +                return list(set(gateways))
> >
> >              # handle gateways by looking at routing table
> >              configured_gws = __find_gw_config(subnet)
>
>
> --
> https://code.launchpad.net/~raharper/curtin/trunk.vmtest-
> remove-bug-timers/+merge/331757
> You are the owner of lp:~raharper/curtin/trunk.vmtest-remove-bug-timers.
>