Merge lp://qastaging/~raharper/curtin/trunk.vmtest-remove-bug-timers into lp://qastaging/~curtin-dev/curtin/trunk

PASSED: Continuous integration, rev:532
https://jenkins.ubuntu.com/server/job/curtin-ci/644/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/644
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/644
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/644
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/644

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/644/rebuild

For testing, generally this affects the following path:

./tools/jenkins-runner -vv --nologcapture tests/vmtests/test_network*

Note, that while we're specifically looking for all of the ArtfulTest* tests to now pass (or skip)
we need to ensure that we don't regress non-artful releases.

Looks pretty good, couple of nits and questions.

Thanks for the review, I'll fix up several things you've mentioned.

PASSED: Continuous integration, rev:536
https://jenkins.ubuntu.com/server/job/curtin-ci/645/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/645
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/645
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/645
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/645

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/645/rebuild

On Thu, Oct 5, 2017 at 11:21 AM, Scott Moser <email address hidden> wrote:

>
>
> Diff comments:
>
> > === modified file 'curtin/commands/curthooks.py'
> > --- curtin/commands/curthooks.py 2017-08-04 19:53:29 +0000
> > +++ curtin/commands/curthooks.py 2017-10-04 20:53:13 +0000
> > @@ -687,6 +689,19 @@
> > if pkg not in needed_packages:
> > needed_packages.append(pkg)
> >
> > + # do not install ifenslave if target release is artful as it
> > + # triggers an install of ifupdown which will break network rendering
> > + if 'ifenslave' in needed_packages:
> > + codename, _ = util.subp(['lsb_release', '--codename',
> '--short'],
>
> util.lsb_release(target=target)
>

I didn't want the dict =); will fix.

>
> > + capture=True, target=target)
> > + # drop the newline
> > + codename = codename.strip()
> > + LOG.debug('Target release codename: %s', codename)
> > + if codename == 'artful':
> > + LOG.debug("Skipping install of package 'ifenslave' to
> prevent"
> > + " network configutation errors on release %s",
> codename)
> > + needed_packages.remove('ifenslave')
>
> what about bridge-utils and vlan ?
>

bridge-utils recommends ifupdown but does not install
vlan does not bring in ifupdown either.

>
> it looks like we will install those in some scenarios too.
> maybe they dont break stuff, but ... fix ?
>
> > +
> > if needed_packages:
> > state = util.load_command_environment()
> > with events.ReportEventStack(
>
>
> --
> 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.
>

but vlan and bridge-utils are not needed or desired in artful, right?

that means best case we install some stuff that odesnt get used and slow down the install.

PASSED: Continuous integration, rev:537
https://jenkins.ubuntu.com/server/job/curtin-ci/646/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/646
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/646
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/646
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/646

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/646/rebuild

vlan appears to be installed already,
bridge-utils while not needed directly to create bridges, is highly useful
for modifying/displaying bridge config. I don't think it's bad to install
it.

On Thu, Oct 5, 2017 at 11:44 AM, Scott Moser <email address hidden> wrote:

> but vlan and bridge-utils are not needed or desired in artful, right?
>
> --
> 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.
>

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

started vmtest run at https://jenkins.ubuntu.com/server/view/curtin/job/curtin-vmtest-devel-debug/5/

i'm good with this. lets see it pass the test run above and then pull.

PASSED: Continuous integration, rev:539
https://jenkins.ubuntu.com/server/job/curtin-ci/647/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-amd64/647
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-arm64/647
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-ppc64el/647
    SUCCESS: https://jenkins.ubuntu.com/server/job/curtin-ci/nodes=metal-s390x/647

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/curtin-ci/647/rebuild

Computer says yes!

Ran 2167 tests in 9300.984s

OK (SKIP=161)
Thu, 05 Oct 2017 23:51:46 +0000: vmtest end [0] in 9302s
+ RET=0