Merge master #2598

Merged
merged 131 commits into from Jun 22, 2015

Conversation

Projects
None yet

Merge master into devices-api-maas feature branch. Only change from master is a minor conflict resolution in lxc-broker_test.go.

(Review request: http://reviews.vapour.ws/r/1970/)

davecheney and others added some commits Mar 31, 2015

all: fix declared by not used error in tests
gccgo is stricter than gc in this respect and discovered some unused
variables which were obviously supposed to be hooked up to tests.

This approach isn't actually thread safe, but as the rest of the tests
in this suite also commit the same crimes, it feels like the lesser evil
to fix the test and unblock gccgo.

Change-Id: Id49cc6d1c289a1cbf170f03acd0488774f70a702
provider/openstack: fix #1450737
Make openstack.createVolume return an error if the persistent
attribute is set to false. Add test which checks that the correct
error is thrown.
Merge pull request #2496 from davecheney/100-fix-lp-1461385
apiserver/instancepoller: fix data race in test

Fixes LP 1461385

(Review request: http://reviews.vapour.ws/r/1863/)
Merge pull request #2513 from axw/lp1461871-state-struct-assertions
state: fix block-device/address update assertions

Change block-device and machine address updates to not
care about concurrent updates.

Block-device and address updates are currently asserting
that there are no concurrent changes. There are several
problems with this:
 - to do this, they assert that the current field value
   is equal to the in-memory struct representation. This
   means that removing or reordering fields in the struct
   will cause assertion failures
 - even without changing the above, there is an issue in
   mgo that can cause reordering of map elements in
   assertions
 - the assertions are pointless

The assertions are pointless for two main reasons:
 - for each of block-devices and addresses, there is a single
   worker responsible for updating. Any concurrent update
   issues are thus purely academic
 - even if we prevent a change between observing the current
   value and updating, we still loop and then update. The net
   effect is that we always update, except in the case of
   "state changing too fast".

Fixes https://bugs.launchpad.net/bugs/1461871

(Review request: http://reviews.vapour.ws/r/1879/)
Merge pull request #2490 from axw/storage-reattachment
storage: reattach storage between restarts

Volume and filesystem attachments may not be persistent.
For example, loop devices must be reattached after restart,
and filesystems must be remounted. We update the storage
provisioner to request recreation of existing attachments
that were not created by the same storage provisioner worker.

This was tested with tmpfs, managedfs, ebs, loop; various
fixes (some existing TODOs) have been made to those providers
as a result. I will test on OpenStack before landing.

(Review request: http://reviews.vapour.ws/r/1857/)
Merge pull request #2525 from axw/lp1461871-state-struct-assertions-m…
…aster

state: fix block-device/address update assertions

Forward port of #2513 to master.

Change block-device and machine address updates to not
care about concurrent updates.

Block-device and address updates are currently asserting
that there are no concurrent changes. There are several
problems with this:
 - to do this, they assert that the current field value
   is equal to the in-memory struct representation. This
   means that removing or reordering fields in the struct
   will cause assertion failures
 - even without changing the above, there is an issue in
   mgo that can cause reordering of map elements in
   assertions
 - the assertions are pointless. Even if we prevent a change
   between observing the current value and updating, we still
   loop and then update. The net effect is that we always update,
   except in the case of "state changing too fast".

Fixes https://bugs.launchpad.net/bugs/1461871

(Review request: http://reviews.vapour.ws/r/1891/)
Merge pull request #2526 from axw/storage-reattachment-master
storage: reattach storage between restarts

Forward port of #2490 to master.

Volume and filesystem attachments may not be persistent.
For example, loop devices must be reattached after restart,
and filesystems must be remounted. We update the storage
provisioner to request recreation of existing attachments
that were not created by the same storage provisioner worker.

This was tested with tmpfs, managedfs, ebs, loop; various
fixes (some existing TODOs) have been made to those providers
as a result. I will test on OpenStack before landing.

(Review request: http://reviews.vapour.ws/r/1892/)
Revert "jujuc: send stdin to server"
This reverts commit f35a8c6.

f35a8c6 fixed bug #1454678 but introduced bug #1463117, which is more
serious. The juju/utils dependency is not reverted as this did not
cause the bug.
Merge pull request #2521 from ericsnowcurrently/hook-context-cleanup
Split up jujuc.Context into sub-interfaces.

Splitting up the interface provides maintenance and on-boarding benefits.  It also helps explicitly delineate how new features fit into hook contexts.

Since the patch uses composition of the sub-interfaces, jujuc.Context keeps all the same methods, which means that no other code in Juju needs to change.  The only adjustment in the patch is renaming "ContextStorage" to "ContextStorageAttachment", which was necessary to make room for the "ContextStorage" sub-interface.

(Review request: http://reviews.vapour.ws/r/1887/)
Merge pull request #2535 from sinzui/txn-licence-1.25
Update deps to use txn with fixed licence.

Update github.com/juju/txn in dependencies.tsv to use the revision with the
fixed licence. This change pulls in just 1 commit; this branch was using the
previous tip.

Fixes https://bugs.launchpad.net/juju-core/+bug/1463455

(Review request: http://reviews.vapour.ws/r/1902/)
lease: fix data race in tests (again)
Fixes LP 1463641
Fixes LP 1458721 (again)
Merge pull request #2539 from davecheney/fixedbugs/1463641
lease: fix data race in tests (again)

Fixes LP 1463641
Fixes LP 1458721 (again)

(Review request: http://reviews.vapour.ws/r/1906/)
Merge pull request #2541 from axw/storage-lifecycle-constraints
state: storage entity relationship fixes

Ensure that storage cannot be added to units
unless they're alive. Also, storage instances
owned by units will now be removed once the
attachment is removed.

When a storage instance is removed from state,
we will now unassign any previously assigned
volumes and filesystems from it. The volumes
and filesystems are not destroyed.

Finally, we ensure prerequisites are provisioned
in SetFilesystemInfo, and
Set{Volume,Filesystem}AttachmentInfo, and update
several tests to go through the proper sequence
of operations.

(Review request: http://reviews.vapour.ws/r/1908/)
Switch to forked version of sys package
The version from golang.org/x/sys requires version 1.4.x of Go to build. This branch switches to a forked version of that that builds on 1.2.1. This change will be reverted when we upgrade to Go 1.4.X.
Merge pull request #2514 from waigani/fix-1450737-cinder-volume
provider/openstack: fix #1450737

Make openstack.createVolume return an error if the persistent
attribute is set to false. Add test which checks that the correct
error is thrown.

(Review request: http://reviews.vapour.ws/r/1880/)
Merge pull request #2530 from waigani/fix-1463117-debug-hook-cmd-hang…
…s-125

Revert "jujuc: send stdin to server"

This reverts commit f35a8c6.

f35a8c6 fixed bug #1454678 but introduced bug #1463117, which is more
serious. The juju/utils dependency is not reverted as this did not
cause the bug.

(Review request: http://reviews.vapour.ws/r/1896/)
Merge pull request #2538 from ericsnowcurrently/hook-context-test-dou…
…bles

Expose the hook context test doubles.

The existing test doubles for juju.Context are hidden away in the util_test.go file.  They also aren't well organized.  This patch cleans them up and factors them out into a separate testing sub-packages under jujuc.  This is a prerequisite for factoring some of the hook context machinery out of the jujuc package.

(Review request: http://reviews.vapour.ws/r/1905/)
cmd/juju: fix #1447899 upgrade juju fails
When upgrading an agent from a client that is more than one minor
version ahead, the agent's minor version is incremented. This branch
resets the agent's original patch version to zero. If this is not
done, juju will not find tools if the new version has no patch larger
than the old version's patch.

For example, upgrading a 1.20.14 agent using a 1.22.1 client will
result in a desired version of 1.21.14 - but if the newest available
1.21 tools is 1.21.3, juju will complain that no compatible tools are
available. This branch corrects the bug by returning, in this
scenario, a desired version of 1.21.0 and thus finding the 1.21.3
tools.
Merge pull request #2547 from waigani/fix-1447895-action-panic-125
Fix #1447895 Panic if jujud restarts while action is running

Check that opState.Hook != nil to avoid panic as shown in logs.

This is a forward port. Original fix targeted 1.24.

(Review request: http://reviews.vapour.ws/r/1915/)
Merge pull request #2546 from waigani/fix-1447899-upgrade-fails
cmd/juju: fix #1447899 upgrade juju fails

When upgrading an agent from a client that is more than one minor
version ahead, the agent's minor version is incremented. This branch
resets the agent's original patch version to zero. If this is not
done, juju will not find tools if the new version has no patch larger
than the old version's patch.

For example, upgrading a 1.20.14 agent using a 1.22.1 client will
result in a desired version of 1.21.14 - but if the newest available
1.21 tools is 1.21.3, juju will complain that no compatible tools are
available. This branch corrects the bug by returning, in this
scenario, a desired version of 1.21.0 and thus finding the 1.21.3
tools.

(Review request: http://reviews.vapour.ws/r/1914/)
Test fixes
 - don't care about the order that the workers are listed in.
 - check that all workers have started, not just the last on the list.
Merge pull request #2531 from gabriel-samfira/switch-to-forked-sys-pa…
…ckage

Switch to forked version of sys package

The version from golang.org/x/sys requires version 1.4.x of Go to build. This branch switches to a forked version of that that builds on 1.2.1. This change will be reverted when we upgrade to Go 1.4.X.

(Review request: http://reviews.vapour.ws/r/1897/)
Merge pull request #2543 from dooferlad/cleaner-uses-api
Cleaner now uses API rather than running inside the state server.



(Review request: http://reviews.vapour.ws/r/1910/)
state: remove machine storage with machine
When a machine is removed from state, remove
any remaining non-persistent volumes and
filesystems with it.
Merge pull request #2549 from dimitern/lp-1348663-1.25-dhcp-release
Fixed lp:1348663 - (port to 1.25) ensure NICs are brought down to trigger DHCPRELEASE

Straightforward port of #2548 to master, no other changes.

Added a new "juju-clean-shutdown" init job/service to all machines at
initial boot (as an upstart job or systemd service, as needed). This job
ensures that on reboot, shutdown, or halt, "ifdown -a" is called to
bring down (and consequently issue a DHCPRELEASE packet) all machine
network interfaces.

A drive-by fix in the MAAS provider is included. I've discovered when
you bootstrap (or add) a vivid machine on MAAS the script creating
juju-br0 (calling ifdown PRIMARY_NIC ; ifup juju-br0) is somehow causing
a duplicate route (e.g. 10.14.0.0/24 proto kernel scope link dev et0 src
10.14.0.100 and 10.14.0.0/24 proto kernel scope link dev juju-br0 src
10.14.0.100) which effectively makes the bootstrap node inaccessible.
The drive-by fix just flushes any routes on the primary NIC before
bringing up juju-br0 - verified to work by live testing.

Live tested extensively on MAAS and EC2 using different combinations of
series (precise, trusty, vivid) and workloads in containers (LXC, KVM).
Each machine or container was manually stopped or rebooted and
DHCPRELEASE packets are observed as this happens on the MAAS DHCP server
side using dhcpdump.

(Review request: http://reviews.vapour.ws/r/1919/)

jujubot and others added some commits Jun 15, 2015

Merge pull request #1988 from davecheney/100-fix-unused-vars
all: fix declared by not used error in tests

gccgo is stricter than gc in this respect and discovered some unused
variables which were obviously supposed to be hooked up to tests.

This approach isn't actually thread safe, but as the rest of the tests
in this suite also commit the same crimes, it feels like the lesser evil
to fix the test and unblock gccgo.

Change-Id: Id49cc6d1c289a1cbf170f03acd0488774f70a702

(Review request: http://reviews.vapour.ws/r/1337/)
Merge pull request #2557 from anastasiamac/bug1462146-storage-add
Merge pull request #2515 from axw/storage-add-bug1462146

This is a forward port from 1.24 (340a95e)
Please review as there were conflicts.


Create machine storage on storage-add

When storage is added to a unit, we must create corresponding
machine storage if the unit is already assigned to a machine.

Supersedes http://reviews.vapour.ws/r/1870/

Bug https://bugs.launchpad.net/juju-core/+bug/1462146

(Review request: http://reviews.vapour.ws/r/1881/)
Conflicts:
	state/storage.go
	state/storage_dynamicadd_test.go

(Review request: http://reviews.vapour.ws/r/1927/)
LP-1464392 :: Close mgo.Iter used in FindActionTagsByPrefix
 We should probably modify the signature of FindActionTagsByPrefix to
 return an error, but I don't want to change the API for this.

 Hopefully logging an error is good enough.
Merge pull request #2554 from johnweldon/lp-1464392
LP-1464392 :: Close mgo.Iter used in FindActionTagsByPrefix

 We should probably modify the signature of FindActionTagsByPrefix to
 return an error, but I don't want to change the API for this.

 Hopefully logging an error is good enough.

(Review request: http://reviews.vapour.ws/r/1924/)
Fixes for race conditions in LXC and KVM tests.
GetContainerInterfaceInfo not called when address allocation is disabled.
MaintainInstance implemented for KVM broker
Adding a route that already exists no longer results in an error.
Merge pull request #2487 from rogpeppe/027-environs-config-table
environs/config: derive schema from table of all attributes

This is the first step on the way to having environment config information
that can be introspected. In this PR we just make the table and leave
all other semantics unchanged, leaving that for a subsequent PR.

Note that some of the error messages have changed because the new
scheme makes it natural to specify sets of possible values in the schema.


(Review request: http://reviews.vapour.ws/r/1853/)
Merge pull request #2559 from davecheney/fix-vet-errors
All: fix various issues detected by go vet

apiserver/apiserver.go:148: unreachable code
cmd/juju/package_test.go:9: +build comment must appear before package clause and be followed by a blank line
juju/paths/paths.go:50: unreachable code
provider/ec2/provider.go:88: result of fmt.Errorf call not used
provider/maas/util.go:48: struct field tag `yaml:,omitempty` not compatible with reflect.StructTag.Get: bad syntax for struct tag value
state/service.go:40: struct field tag `bson:forcecharm"` not compatible with reflect.StructTag.Get: bad syntax for struct tag value
state/storage.go:237: unreachable code
state/backups/storage_test.go:77: comparison of function Stored != nil is always true
state/presence/presence.go:713: struct field tag "retval" not compatible with reflect.StructTag.Get: bad syntax for struct tag pair
state/presence/presence.go:716: struct field tag "localTime" not compatible with reflect.StructTag.Get: bad syntax for struct tag pair
state/watcher/watcher_test.go:143: struct field tag "txn-revno" not compatible with reflect.StructTag.Get: bad syntax for struct tag pair
testing/factory/factory.go:122: direct assignment to atomic value
worker/txnpruner/txnpruner.go:40: unreachable code

(Review request: http://reviews.vapour.ws/r/1929/)
Merge pull request #2562 from axw/worker-storageprovisioner-detach
worker/storageprovisioner: detach storage

Detach volumes and filesystems when attachments are seen to be Dying. If an
attachment is Dying but has never been created, remove it immediately.

Also, we add the volume and filesystem IDs to volume and filesystem
attachment parameters passed over the API. These are used in detachment,
and passing them over the API means we can avoid dependencies between
watchers in the storage provisioner for the purpose of detaching.

(Review request: http://reviews.vapour.ws/r/1932/)
apiserver: refactor apiserver.Server.Addr method to return a *net.TCP…
…Addr

Driveby refactoring removing a bunch of ugly string juggling.
Merge pull request #2558 from davecheney/apiserver-server-addr
apiserver: refactor apiserver.Server.Addr method to return a *net.TCPAddr

Drive by refactoring removing a bunch of ugly string juggling.

(Review request: http://reviews.vapour.ws/r/1928/)
Merge pull request #2567 from wallyworld/bootstrap-auto-upgrade-fix
Allow agent version to be pinned when bootstrapping

Fixes: https://bugs.launchpad.net/bugs/1455260

juju bootstrap gains a new "agent-version" argument, used to ensure that only the explicitly specified version of agent tools is used to run the agents.

The agent-verson arg can be a numeric version (eg 1.2.3) or the full binary version (eg 1.2.3-trusty-amd64). In the later case just the number is extracted and used. This is to allow `juju version` to be used to pin the tools version.

An alias for "match the client version" is --no-auto-upgrade.

The agent-version number specified must match the client major and minor version numbers to help better ensure compatibility. 

(Review request: http://reviews.vapour.ws/r/1937/)
Fix 1459093: Upgrade fails if there's an unknown series in streams
Allow tools metadata with an unknown OS to be merged into
simplestreams. Such tools will not have a known series version.
Introduce an "unknown" series version. Use this to construct the
metadata's productId under which these tools are catalogued in
the simplestreams JSON output.

In addition to testing the above, test that upgrade-juju succeeds with
an unknown series in simplestreams.
Merge pull request #2569 from waigani/version-parse-no-panic
Fix 1459093: Upgrade fails if there's an unknown series in streams

Allow tools metadata with an unknown OS to be merged into
simplestreams. Such tools will not have a known series version.
Introduce an "unknown" series version. Use this to construct the
metadata's productId under which these tools are catalogued in 
the simplestreams JSON output.

In addition to testing the above, test that upgrade-juju succeeds with
an unknown series in simplestreams.

(Review request: http://reviews.vapour.ws/r/1939/)
state: Fix 1465873 Environment.Users mismatched UUID
Make Environment.Users return an error if the environment's UUID does
not match that of the backing state's environment.
worker/storageprovisioner: react to Dead vo/fs
When a volume or filesystem becomes Dying, we
now simply clear out any pending provisioning.
When a volume or filesystem becomes Dead, then
we destroy (if provisioned) and remove from
state. The entity being Dead is predicated on
it having no dependents; this will be taken
care of in state.

Note: currently we don't have a DestroyFilesystems
method in storage.FilesystemSource.
Fixed up TestStartInstanceLoopMountsDisallowed.
ErrLoopMountNotAllowed removed (obsolete).
storage: add FilesystemSource.DestroyFilesystems
All existing filesystem implementations have no-op
implementations, since they don't actually create
anything (their Attach/DetachFilesystems create
filesystems).
storage/provider: implement DetachFilesystems
All of the existing filesystem storage providers
use "unmount" to detach. We introduce some common
code to unmount if mounted, and common test code
for each provider.
storage/provider: implement loop DetachVolumes
This diff implements the DetachVolumes method of
the loop storage provider, which is mostly just
extracted from the existing DestroyVolumes method.
The latter is changed to just remove the backing
file.
Merge pull request #2566 from dooferlad/addr-containers-fixes
Various LXC and KVM fixes

Fixes for race conditions in LXC and KVM tests.
GetContainerInterfaceInfo not called when address allocation is disabled.
MaintainInstance implemented for KVM broker
Adding a route that already exists no longer results in an error.

(Review request: http://reviews.vapour.ws/r/1936/)
Merge pull request #2522 from tasdomas/split-up-context-test
Refactor test into smaller ones.

The test should probably be a collection of smaller tests.

(Review request: http://reviews.vapour.ws/r/1888/)
Merge pull request #2573 from waigani/fix-1465873-env-users-125
state: Fix 1465873 Environment.Users mismatched UUID

Make Environment.Users return an error if the environment's UUID does
not match that of the backing state's environment.

(Review request: http://reviews.vapour.ws/r/1946/)
environs: Ignore unknown tools metadata
When no OS or series version can be found for tools metadata,
ignore the record.
Merge pull request #2582 from waigani/ignore-unknown-series
environs: Ignore unknown tools metadata

When no OS or series version can be found for tools metadata,
ignore the record.

(Review request: http://reviews.vapour.ws/r/1955/)
apiserver/storageprovisioner: implement Remove
Add the Remove method, to remove destroyed volumes
and filesystems from state.
Merge pull request #2578 from axw/storageprovisioner-destroy
worker/storageprovisioner: react to Dead vo/fs

When a volume or filesystem becomes Dying, we now simply clear out any
pending provisioning. When a volume or filesystem becomes Dead, then we
destroy (if provisioned) and remove from state. The entity being Dead is
predicated on it having no dependents; this will be taken care of in state.

The VolumeParams and FilesystemParams methods will now return results
for already-provisioned entities. This is so that we can construct the
storage source from pool config, to destroy the IaaS resources.

EnsureDead has been removed from the API, since we will not be using it.

Note: currently we don't have a DestroyFilesystems method in
storage.FilesystemSource. Also, the Remove method on the apiserver side
has not been implemented. Neither of these poses a problem, since we don't
yet set volumes or filesystems to Dead in state.

(Review request: http://reviews.vapour.ws/r/1951/)
apiserver: fix LP 1466011
Fixes LP 1466011

http://reviews.vapour.ws/r/1928/ removed a bunch of string munging logic related to the reported address of the api server.

This change exposed an expectation by the apiserver test cases that the apiserver would _always_ report it was listening on "localhost", even though the test suite never bound the socket to localhost in the first place. Fix this by explicitly binding to localhost as the test expects, and in the process remove a bunch of code in the api client which had grown to expect a set of 'special' addresses in the api.Info.Addrs slice.

As _real_ clients always use the address specified in their jenv file, that is they never are asked to connect to "0.0.0.0" or "::" or "localhost", the behaviour of the test now better mirrors how the code is used.
Merge pull request #2583 from axw/storage-provider-loop-detach
storage/provider: implement loop DetachVolumes

This diff implements the DetachVolumes method of
the loop storage provider, which is mostly just
extracted from the existing DestroyVolumes method.
The latter is changed to just remove the backing
file.

(Review request: http://reviews.vapour.ws/r/1956/)
Merge pull request #2581 from axw/storage-provider-detachfilesystems
storage/provider: implement DetachFilesystems

All of the existing filesystem storage providers
use "unmount" to detach. We introduce some common
code to unmount if mounted, and common test code
for each provider.

(Review request: http://reviews.vapour.ws/r/1954/)
Merge pull request #2579 from axw/apiserver-storageprovisioner-remove
apiserver/storageprovisioner: implement Remove

Add the Remove method, to remove destroyed volumes
and filesystems from state.

(Review request: http://reviews.vapour.ws/r/1952/)
Merge pull request #2580 from axw/storage-filesystemsource-destroyfil…
…esystems

storage: add FilesystemSource.DestroyFilesystems

All existing filesystem implementations have no-op
implementations, since they don't actually create
anything (their Attach/DetachFilesystems create
filesystems).

(Review request: http://reviews.vapour.ws/r/1953/)
Merge pull request #2590 from davecheney/fixedbugs/1466011
apiserver: fix LP 1466011

Fixes LP 1466011

http://reviews.vapour.ws/r/1928/ removed a bunch of string munging logic related to the reported address of the api server.

This change exposed an expectation by the apiserver test cases that the apiserver would _always_ report it was listening on "localhost", even though the test suite never bound the socket to localhost in the first place. Fix this by explicitly binding to 127.0.0.1 as the test expects, and in the process remove a bunch of code in the api client which had grown to expect a set of 'special' addresses in the api.Info.Addrs slice.

As _real_ clients always use the address specified in their jenv file, that is they never are asked to connect to "0.0.0.0" or "::" or "localhost", the behaviour of the test now better mirrors how the code is used.

$$merge$$

Contributor

jujubot commented Jun 19, 2015

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

Contributor

jujubot commented Jun 19, 2015

Build failed: Tests failed
build url: http://juju-ci.vapour.ws:8080/job/github-merge-juju/3725

Passes on my machine! Trying again.

$$merge$$

Contributor

jujubot commented Jun 19, 2015

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

Contributor

jujubot commented Jun 19, 2015

Build failed: Tests failed
build url: http://juju-ci.vapour.ws:8080/job/github-merge-juju/3730

$$merge$$

Contributor

jujubot commented Jun 22, 2015

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

jujubot added a commit that referenced this pull request Jun 22, 2015

Merge pull request #2598 from voidspace/devices-master-merge-2
Merge master

Merge master into devices-api-maas feature branch. Only change from master is a minor conflict resolution in lxc-broker_test.go.

(Review request: http://reviews.vapour.ws/r/1970/)

@jujubot jujubot merged commit a5c14a0 into juju:devices-api-maas Jun 22, 2015

@voidspace voidspace deleted the voidspace:devices-master-merge-2 branch Oct 6, 2016

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