Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

2.1 into develop #6953

Merged
merged 29 commits into from Feb 9, 2017
Merged

2.1 into develop #6953

merged 29 commits into from Feb 9, 2017

Conversation

jameinel
Copy link
Member

@jameinel jameinel commented Feb 9, 2017

Merge the latest 2.1 work into develop.

Including:

  • Track uses of StatePool to help find memory leaks
  • More work on AdoptResources
  • Update host bridging changes to use the machine lock while making changes, so as to not do network changes while a hook is executing.
  • Allow for clock skew for lease manager
  • Add interactive auth for 'lxd' provider

babbageclunk and others added 29 commits February 7, 2017 14:00
Needs to rename security group yet.
Track the locations of reference sources for the state pool.

In the continuing saga of tracking memory leaks this branch adds introspection into the state pool that is used by the apiserver.

The state pool now records the stack whenever the Get method is called, and clears that stack when the releaser is called.

## QA steps

- bootstrap a lxd provider
- juju ssh -m controller 0
- juju-statepool-report
No testing of security group renaming yet.
And change group updating to deal with the fact that the UUIDs have
dashes in them - my naive implementation ignored that.
…nstack

migrations: Implement AdoptResources for the openstack provider

## Description of change
The AdoptResources environ method will be used at the end of a model migration to update the controller tag for all of the resources used by the model, so that if the source controller is subsequently destroyed then these resources won't be cleaned up with it.

In the openstack provider we tag volumes and instances with the controller UUID. Metadata for these can be updated using the API - I've added the SetVolumeMetadata method to the goose Cinder client (dependency updated here). Security groups don't have tags, so the controller is included as a component of the group name. The group names for the current model are all rewritten with the new controller UUID.

## QA steps
None yet - this method isn't called from the migration master yet.

## Documentation changes
N/A

## Bug reference
This is part of the fix for https://bugs.launchpad.net/juju/+bug/1648063
SetHostMachineNetworkConfig should be taking a names.MachineTag not an arbitrary string,
especially when the string is labeled 'ID' (eg '0') but actually contains a 'Tag' ('machine-0').
Start working on changing the lxd broker to take a 'prepareHost' instead of passing in lots of
context just so it can hand it back again.
Working towards having prepareHost grab the machine-lock while bridging.
…56326"

This reverts commit 7fc3938, reversing
changes made to 49316b2.
…2947

Revert "Merge pull request juju#6933 from jameinel/2.1-no-fallback-1656326"

This reverts commit 7fc3938, reversing
changes made to 49316b2.

CI found that not all clouds act like my expectation, so just reverting to unblock next 2.1 release. They still are succeeding for the 'wrong' reasons, but that's life.

https://pad.lv/1662947 to revert the "fix" for https://pad.lv/1656326
2.1 network uses lock 1662586

## Description of change

Change the Bridging code to use the machine-lock while executing.

## QA steps

I tried to create a charm that would exercise the network while another charm was deployed, but I wasn't successful in reproducing this. The associated bug does have some examples of it happening in the wild. There are a few hypothetical issues:
- You have to be doing something significant over the network that takes enough time during the install hook.
- Your *first* container does wait for the lock, because it needs the lock to 'apt install lxd' etc. So either the container initialization has to happen first, and then release the lock, or somehow the ordering has to be just right.
- My test was trying to do something with a local service that I could ensure would take a long time, but its possible that even though I was connecting to the IP address that was being brought down and back up again, sockets are smart enough to not die if the downtime is short enough.

It might be possible to "juju deploy $SOMETHING --to lxd:0" and then "juju deploy $ELSE --to 0", and have the lxd initialization trigger, but while it is downloading the container image, the ELSE starts doing its thing and ends up racing with the container image finally downloading and triggering us wanting to change networking with the install hook downloading something. (I'm not entirely sure that we wait for the image to be downloaded before we reconfigure bridging, though, I'm not 100% sure when the image is downloaded vs when the bridging is done.)

## Documentation changes

No specific CLI/API changes.

## Bug reference

https://pad.lv/1662586
WatchAllModels now gets passed a state pool

Currently each Juju GUI that connects to the controller will ask for something to watch all the models. Each of these watchers was creating its own state pool. So in a controller with many models with many GUI connections, we'd have many extra state.State instances being created for very little use.

This change also allows an additional facade creation signature to be used with the RegisterStandardFacade function.

## QA steps

Bootstrap and log into the GUI.

## Documentation changes

There are no user facing changes with this at all.

## Bug reference

Part of the fix for https://bugs.launchpad.net/juju/+bug/1649719
Use monotonic clock in lease manager skew calculations

## Description of change

A monotonic clock is introduced to the lease manager so that when it calculates skew, there is no possibility of time going backwards and causing an error.

## QA steps

bootstrap and run list-models to smoke test

## Bug reference

https://bugs.launchpad.net/juju/+bug/1662202
bump verison to 2.1.0

## Please provide the following details to expedite Pull Request review:

----

## Description of change
Version bump.

Why is this change needed?
So we can release.

## QA steps
Look at the diff and make sure there's no typos.
Add a global state tracker to help find leaks.

We have a strong suspicion that the key memory leaks are due to excessive and unnecessary *state.State instances. This branch adds a global reference map that stores an item for each new state object along with the stack trace. The state close method also lets the tracker know that at least the object thinks it is closed.

This will hopefully shed light on why we have so many state instances leaking.

## QA steps

bootstrap a controller
juju ssh -m controller 0
juju-statetracker-report
This updates the provider tags for all cloud resources to indicate the
new controller uuid, so that they won't be cleaned up if the old
controller is destroyed. It will be called from the migrationmaster
worker when the migration is complete.
Add the "interactive" auth-type for LXD, which is
used in add-credential for interactively adding a
credential for a LXD cloud. Currently we only
support generating credentials for local LXD;
later we will extend this to support generating
credentials for remote, untrusted LXD by prompting
the user to verify the certificate fingerprint and
enter a trust password.
…server

migrations: Add AdoptResources to the migrationtarget facade

## Description of change
This updates the provider tags for all cloud resources to indicate the
new controller uuid, so that they won't be cleaned up if the old
controller is destroyed. It will be called from the migrationmaster
worker when the migration is complete.

## QA steps

No QA steps yet - the code to call this during migration will be in a 
subsequent PR.

## Bug reference
Part of the fix for https://bugs.launchpad.net/juju/+bug/1648063
…teractive

provider/lxd: add interactive auth-type

## Description of change

Add the "interactive" auth-type for LXD, which is
used in add-credential for interactively adding a
credential for a LXD cloud. Currently we only
support generating credentials for local LXD;
later we will extend this to support generating
credentials for remote, untrusted LXD by prompting
the user to verify the certificate fingerprint and
enter a trust password.

## QA steps

1. juju add-credential localhost
```
Enter credential name: foo
Auth Types
interactive*
certificate

Select auth-type: 
Loaded client cert/key from "/home/andrew/.config/lxc"
Credentials added for cloud localhost.
```
2. juju bootstrap localhost
3. juju add-user bob
4. juju grant bob add-model
5. lxc launch ubuntu-xenial x
6. lxc file push \`which juju\` x/tmp/juju
7. lxc exec x /tmp/juju register ...
8. lxc exec x /tmp/juju add-model foo
```
ERROR cannot auto-generate credential for remote LXD

Until support is added for verifying and authenticating to remote LXD hosts,
you must generate the credential by hand, adding the certificate to LXD using
the "lxc config trust" command.
```
9. juju credentials --format=yaml localhost > /tmp/localhost-credentials.yaml
10. lxc file push /tmp/localhost-credentials.yaml x/root/.local/share/juju/credentials.yaml
11. lxc exec x /tmp/juju add-model foo
```
Uploading credential 'localhost/bob/foo' to controller
Added 'foo' model on localhost/localhost with credential 'foo' for user 'bob'
```

## Documentation changes

There is a change in workflow, but probably unusual enough that it doesn't need documenting?

## Bug reference

Does not fix any bugs, but hopefully alleviates some of the pain caused by https://bugs.launchpad.net/juju/+bug/1662587.
There were a few conflicts, but should be resolved.
@jameinel
Copy link
Member Author

jameinel commented Feb 9, 2017

$$merge$$ self approving a merge of 2.1 into develop

@jujubot
Copy link
Collaborator

jujubot commented Feb 9, 2017

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

@jujubot jujubot merged commit 8cf2481 into juju:develop Feb 9, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
7 participants