2.1 into develop #7044

Merged
merged 27 commits into from Mar 1, 2017

Conversation

Projects
None yet
3 participants
Owner

jameinel commented Feb 27, 2017

Merge 2.1 into develop including

jameinel and others added some commits Feb 22, 2017

Add a network.Route type and network.Route.Validate()
This provides some infrastructure so that we can pass routes to /e/n/i
at creation time. Unfortunately not dynamic at all, but neither is the
host machine's /e/n/i.
Propagate static routes into the container network configuration.
Use ip route add and del in post-up and pre-down commands.
Include at least a direct test of the Provisioner client side code.
This tests that we are converting the response of a NetworkConfig into a network.InterfaceInfo
correctly. Ultimately we probably want a broader test, but this is a good start towards it.
Handle mutex.ErrCancelled coming from worker.Stop
This happens very occasionally when stopping the uniter after a
successful test. It happens when the waitHooks step (which acquires and
releases the hook lock) blocks the uniter code from acquiring the lock -
the mutex code loops and delays for 250ms in that case, and it's still
in that delay select when the test closes the abort channel. This makes
the mutex return ErrCancelled which bubbles back up to the test.
Merge pull request #7037 from babbageclunk/uniter-lock-test-fix
Handle mutex.ErrCancelled coming from worker.Stop

## Description of change

This occurs very occasionally when stopping the uniter after a
successful run of `UniterSuite.TestUniterSteadyStateUpgradeRelations`. 
It happens when the waitHooks step (which acquires and
releases the hook lock) blocks the uniter code from acquiring the lock -
the mutex code loops and delays for 250ms in that case, and it's still
in that delay select when the test closes the abort channel. This makes
the mutex return ErrCancelled which bubbles back up to the test.

## QA steps

Running the test under `stress` would fail after about 20 mins. I left the test running for a couple of hours and didn't see it again. I also changed the `resolver.Loop `code to always return `ErrCancelled` and confirmed that the test would log the error but pass.

## Bug reference

Fixes https://bugs.launchpad.net/juju/+bug/1635664
Make DebugHook test more reliable on busy machines
TestRunHook was regularly failing on test hosts with an error indicating
that the flock process was being killed by a timer before we managed to
get the debug directory. It passes reliably on a (relatively unloaded)
dev machine, but I could simulate a heavier load by adding a delay in
the goroutine the test starts and get similarly flaky behaviour.

Increase the timeout of the flock command to match the timeout of the
select loop below it - this should make the failures much less likely.
Merge pull request #7038 from babbageclunk/debug-hook-test-fix
Make DebugHook test more reliable on busy machines

## Description of change

TestRunHook was regularly failing on test hosts with an error indicating
that the flock process was being killed by a timer before we managed to
get the debug directory. It passes reliably on a (relatively unloaded)
dev machine, but I could simulate a heavier load by adding a delay in
the goroutine the test starts and get similarly flaky behaviour.

Increase the timeout of the flock command to match the timeout of the
select loop below it - this should make the failures much less likely.

## Bug reference

Hopefullly fixes https://bugs.launchpad.net/juju/+bug/1612747
Merge pull request #7034 from jameinel/2.1.1-static-routes-1653708
2.1.1 static routes for containers

## Description of change

This change allows Juju to propagate Static Route information that MAAS declares into containers. It depends on this patch to gomaasapi: juju/gomaasapi#63. (I updated dependencies.tsv for my version of the code, but we should update dependencies to match the final merge.) The other changes to dependencies.tsv are just reordering because of how the file was generated.

We use the Static Routes API https://docs.ubuntu.com/maas/2.0/en/api#static-route to find what routes are needed, and apply them based on their Source subnet.
We add a post-up and a pre-down rule that uses 'ip route add'. This differs slightly from MAAS and Curtin that use "route add", but it means we can use CIDR instead of netmask.
http://bazaar.launchpad.net/~curtin-dev/curtin/trunk/view/head:/curtin/net/__init__.py#L380

## QA steps

Using MAAS create a static route for one of your subnets (you can do so via the web ui by looking at the source subnet). You'll need to create destination subnets for it to route to. Afterwards, deploy a node, and you should see that "ip route show" lists that static route. You should then be able to deploy a container onto that machine using the same space. It should have the same route. Without this patch (stock juju 2.1.0) it should not.

## Documentation changes

Potentially we should document static routes with containers, but it feels a bit like people using static routes just expect it to work.

## Bug reference

[lp:1653708](https://bugs.launchpad.net/juju/+bug/1653708)
Owner

jameinel commented Feb 27, 2017

$$merge$$ self-approving merge up of 2.1 into develop

Contributor

jujubot commented Feb 27, 2017

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

jameinel and others added some commits Feb 27, 2017

MaaS 2.0 issues a 404 when requesting static-routes.
It seems static-routes is only in 2.1+ but isn't reported in the API version,
nor in the 'capabilities' list.
Merge pull request #7045 from jameinel/2.1.1-no-maas-2.0-static-route…
…s-1668359

MaaS 2.0 issues a 404 when requesting static-routes.

It seems static-routes is only in 2.1+ but isn't reported in the API version,
nor in the 'capabilities' list.

## Description of change

MAAS 2.1 implemented a new API (static-routes), that we wanted to propagate into containers. However, MAAS 2.0 does not have that API, but we don't have a way to tell the API isn't available based on the API version, nor on the capabilities tags. So instead we try the API, and handle getting a 404 back.

## QA steps

We have a functional test that tests we can create containers and get access to them on MAAS 2.0.

To test directly "juju bootstrap" onto a MAAS 2.0 machine, and "juju add-machine -m controller lxd:0". That container should be on the host's network, instead of on lxdbr0, and there shouldn't be this error in the log file:
2017-02-27 17:55:22 WARNING juju.provisioner lxd-broker.go:82 failed to prepare container "0/lxd/1" network config: unable to look up static-routes: unexpected: ServerError: 404 NOT FOUND (Unknown API endpoint: /MAAS/api/2.0/static-routes/.)

## Documentation changes

No.

## Bug reference

[lp:1668359](https://bugs.launchpad.net/juju/+bug/1668359)
Concrete testing shows that its actually UnexpectedError.
The content of the error is a ServerError with the values we were
checking, but it has been wrapped in an UnexpectedError which doesn't
seem to make the original error available.
Merge pull request #7048 from jameinel/2.1.1-no-maas-2.0-static-route…
…s-1668359

Concrete testing shows that its actually UnexpectedError.

The content of the error is a ServerError with the values we were
checking, but it has been wrapped in an UnexpectedError which doesn't
seem to make the original error available.

## Description of change

MAAS 2.0 doesn't support static-routes, and I misunderstood the error we were getting. We get a ServerError *wrapped* by a UnexpectedError, and errors.Wrap doesn't let you get to the original error.

## QA steps

Against a MAAS 2.0 controller:
```
  $ juju bootstrap
  $ juju switch controller
  $ juju add-machine lxd:0
  $ juju status
  $ juju ssh 0/lxd/0
```
Without this patch we get an error and the 'fallback to lxdbr0' code kicks in and you get an unreachable container.

## Documentation changes

No, as 2.1 didn't ship with this bug.

## Bug reference

[lp:1668359](https://pad.lv/1668359)
Owner

jameinel commented Mar 1, 2017

$$merge$$ because now we concretely have the static-routes fix in place.

Owner

jameinel commented Mar 1, 2017

Build failed: Generating tarball failed
build url:

Owner

jameinel commented Mar 1, 2017

$$merge$$ (trying to fake the bot into noticing again)

Contributor

jujubot commented Mar 1, 2017

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

@jujubot jujubot merged commit 4bc3a3a into juju:develop Mar 1, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment