Merge lp:~raharper/curtin/trunk.vmtest-remove-bug-timers into lp:~curtin-dev/curtin/trunk
Status: | Merged | ||||||||
---|---|---|---|---|---|---|---|---|---|
Merged at revision: | 532 | ||||||||
Proposed branch: | lp:~raharper/curtin/trunk.vmtest-remove-bug-timers | ||||||||
Merge into: | lp:~curtin-dev/curtin/trunk | ||||||||
Diff against target: |
544 lines (+128/-134) 13 files modified
curtin/commands/curthooks.py (+19/-5) examples/tests/network_alias.yaml (+0/-2) examples/tests/network_static_routes.yaml (+10/-15) tests/vmtests/__init__.py (+20/-1) tests/vmtests/test_network.py (+15/-11) tests/vmtests/test_network_bonding.py (+16/-17) tests/vmtests/test_network_bridging.py (+15/-9) tests/vmtests/test_network_enisource.py (+2/-4) tests/vmtests/test_network_ipv6.py (+0/-11) tests/vmtests/test_network_ipv6_enisource.py (+3/-17) tests/vmtests/test_network_ipv6_vlan.py (+0/-6) tests/vmtests/test_network_mtu.py (+26/-24) tests/vmtests/test_network_vlan.py (+2/-12) |
||||||||
To merge this branch: | bzr merge lp:~raharper/curtin/trunk.vmtest-remove-bug-timers | ||||||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Server Team CI bot | continuous-integration | Approve on 2017-10-05 | |
Scott Moser | Approve on 2017-10-05 | ||
Chad Smith | 2017-10-04 | Pending | |
Review via email:
|
Description of the change
vmtest: fix artful networking
- Remove skip_by_date calls in vmtests that are no longer valid
- ifenslave brings in ifupdown, so filter out ifenslave package if target
release is artful
- Adjust artful bonding test to skip checking for ifslave and instead check
that ifenslave is *not* installed
- Drop mtu settings in network-
- Move static routes under an interface subnet to be compatible with
netplan format which requires routes under an interface
- Refactor ip_route_show parsing, ignoring default route and fetching
variable settings like 'proto'.
- Skiptest bridging in artful, need a cloud-init fix for stp in netplan
- Skiptest for mtu in artful, need networkd to support mtu6 support
Ryan Harper (raharper) wrote : | # |
For testing, generally this affects the following path:
./tools/
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.
Chad Smith (chad.smith) wrote : | # |
Looks pretty good, couple of nits and questions.
Ryan Harper (raharper) wrote : | # |
Thanks for the review, I'll fix up several things you've mentioned.
- 533. By Ryan Harper on 2017-10-04
-
Fix spelling of 'additional' in curthooks log message
- 534. By Ryan Harper on 2017-10-04
-
Remove unneeded .lower(), switch to an equality check
- 535. By Ryan Harper on 2017-10-04
-
Raise a ValueError in ip_route_show regex failure to match
- 536. By Ryan Harper on 2017-10-04
-
Just check for empty dpkg-query status in Artful Bonding tests
PASSED: Continuous integration, rev:536
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
Click here to trigger a rebuild:
https:/
Ryan Harper (raharper) wrote : | # |
On Thu, Oct 5, 2017 at 11:21 AM, Scott Moser <email address hidden> wrote:
>
>
> Diff comments:
>
> > === modified file 'curtin/
> > --- curtin/
> > +++ curtin/
> > @@ -687,6 +689,19 @@
> > if pkg not in needed_packages:
> > needed_
> >
> > + # 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(
> '--short'],
>
> util.lsb_
>
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_
>
> 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_
> > with events.
>
>
> --
> https:/
> remove-
> You are the owner of lp:~raharper/curtin/trunk.vmtest-remove-bug-timers.
>
- 537. By Ryan Harper on 2017-10-05
-
Use util.lsb_release()
Scott Moser (smoser) wrote : | # |
but vlan and bridge-utils are not needed or desired in artful, right?
Scott Moser (smoser) wrote : | # |
that means best case we install some stuff that odesnt get used and slow down the install.
PASSED: Continuous integration, rev:537
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
Click here to trigger a rebuild:
https:/
Ryan Harper (raharper) wrote : | # |
vlan appears to be installed already,
bridge-utils while not needed directly to create bridges, is highly useful
for modifying/
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:/
> remove-
> You are the owner of lp:~raharper/curtin/trunk.vmtest-remove-bug-timers.
>
Ryan Harper (raharper) wrote : | # |
On Thu, Oct 5, 2017 at 1:38 PM, Scott Moser <email address hidden> wrote:
>
>
> Diff comments:
>
> >
> > === modified file 'examples/
> > --- examples/
> > +++ examples/
> > @@ -9,11 +9,9 @@
> > subnets:
> > - type: static
> > address: 10.47.98.1/24
> > - mtu: 1501
> > - type: static
> > address: 2001:4800:
> > netmask: '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/
> > --- tests/vmtests/
> > +++ tests/vmtests/
> > @@ -285,14 +285,19 @@
> > ip_route_show = self.load_
> > logger.debug("ip route show:\n{
> > for line in [line for line in ip_route_
> > - if 'src' in line]:
> > + if 'src' in line and not
> line.startswith
> > + print('
> > m = re.search(
> > r'(?P<devname>
> > - r'proto kernel\s+scope link' +
> > - r'\s+src\
> > + r'proto\
> > + r'scope\
> > + r'src\s(
> > line)
> > - route_info = m.groupdict('')
> > - logger.
> > + if m:
> > + route_info = m.groupdict('')
> > + logger.
> > + 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.
> > for route in subnet.
> > gateways += __find_
> > - return gateways
> > + # drop duplicate gateways (static routes)
>
> this does have the side affect of sorting your list.
>
That's ok, we iterate ...
- 538. By Ryan Harper on 2017-10-05
-
Merge from smoser
- 539. By Ryan Harper on 2017-10-05
-
Fix up merge issues
Scott Moser (smoser) wrote : | # |
started vmtest run at https:/
Scott Moser (smoser) wrote : | # |
i'm good with this. lets see it pass the test run above and then pull.
PASSED: Continuous integration, rev:539
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
Click here to trigger a rebuild:
https:/
Ryan Harper (raharper) wrote : | # |
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
PASSED: Continuous integration, rev:532 /jenkins. ubuntu. com/server/ job/curtin- ci/644/ /jenkins. ubuntu. com/server/ job/curtin- ci/nodes= metal-amd64/ 644 /jenkins. ubuntu. com/server/ job/curtin- ci/nodes= metal-arm64/ 644 /jenkins. ubuntu. com/server/ job/curtin- ci/nodes= metal-ppc64el/ 644 /jenkins. ubuntu. com/server/ job/curtin- ci/nodes= metal-s390x/ 644
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
Click here to trigger a rebuild: /jenkins. ubuntu. com/server/ job/curtin- ci/644/ rebuild
https:/