Query Azure storage account key from Azure.

Query Azure storage account key from Azure.

As per the planned phasing of the Azure provider, the environment's config
must currently still specify a key for the environment's Azure storage
account.  The key is available from Azure itself, and here we replace the
configuration item with an automated query.

The query is performed on demand, and the result cached in a new field on
azureEnviron called storageAccountKey.  I had wanted to query it immediately
when an azureEnviron is created, but that introduced an unexpected Azure
request in dozens of tests — some of which already routed such requests to a
fake http service but now needed an exta canned response inserted in the right
place (not always easy), and others needed extra code to create such a fake in
the first place.  Not pleasant, so instead I went with on-demand querying, and
an update to makeEnviron() which ensures that this query is not needed by
default.

There were some complications with locking.  Go's execution model does not
enable the conventional rule where a lock's current owner can grab the lock
again without deadlocking.  This made it hard to get a handle on the Azure
management API from within a critical section, because locking is involved on
the lowest level.  I used a well-known optimistic technique to get around
that, and it also lets us avoid a long-held lock while the Azure query is
underway.

https://code.launchpad.net/~jtv/juju-core/az-get-storage-key/+merge/178131

(do not edit description out of merge proposal)

[jtv.canonical, 2013-08-01 17:17:37]

Please take a look.

[mue, 2013-08-02 08:54:07]

LGTM, only some minor comments.

https://codereview.appspot.com/12270043/diff/1/environs/azure/environ.go
File environs/azure/environ.go (right):

https://codereview.appspot.com/12270043/diff/1/environs/azure/environ.go#newc...
environs/azure/environ.go:117: return "", fmt.Errorf("could not obtain storage
account keys: %v", err)
Mostly used the term "cannot" in our error messages so far.

https://codereview.appspot.com/12270043/diff/1/environs/azure/environ.go#newc...
environs/azure/environ.go:833: // The caller can use env.getSnapshot().
Very detailed explanation, but I like it.

https://codereview.appspot.com/12270043/diff/1/environs/azure/environ_test.go
File environs/azure/environ_test.go (right):

https://codereview.appspot.com/12270043/diff/1/environs/azure/environ_test.go...
environs/azure/environ_test.go:16: . "launchpad.net/gocheck"
We started to separate the 3rd party packages too and to also alias gocheck with
gc and the juju-core checkers with jc. Good opportunity to do this here too.

[jtv.canonical, 2013-08-05 11:36:38]

Thanks for the review!  Sorry I couldn't follow up right away.


>
https://codereview.appspot.com/12270043/diff/1/environs/azure/environ.go#newc...
> environs/azure/environ.go:117: return "", fmt.Errorf("could not obtain storage
> account keys: %v", err)
> Mostly used the term "cannot" in our error messages so far.

OK.  I changed it.


>
https://codereview.appspot.com/12270043/diff/1/environs/azure/environ_test.go...
> environs/azure/environ_test.go:16: . "launchpad.net/gocheck"
> We started to separate the 3rd party packages too and to also alias gocheck
with
> gc and the juju-core checkers with jc. Good opportunity to do this here too.

Fixed the separation of imports.  I'll leave the "gc" change for another branch
-- it'd be impolite to my next reviewer to mix all those changes into the diff.

[jtv.canonical, 2013-08-05 11:47:03]

Please take a look.

[rvb, 2013-08-05 12:02:12]

On 2013/08/05 11:47:03, jtv.canonical wrote:
> Please take a look.

LGTM.

[0]

121	+func extractStorageKey(keys *gwacl.StorageAccountKeys) string {
122	+	if keys.Primary != "" {
123	+		return keys.Primary
124	+	}
125	+	return keys.Secondary

Maybe I'm wrong but I don't see that any of the auto-generated keys can be the
empty string.  Couldn't we just use the primary key and simplify the code a bit?

[jtv.canonical, 2013-08-05 12:32:41]

On 2013/08/05 12:02:12, rvb wrote:

> Maybe I'm wrong but I don't see that any of the auto-generated keys can be the
> empty string.  Couldn't we just use the primary key and simplify the code a
bit?

Thanks.  From just trying it I would say yes.  This is input from Azure,
however, so I'm being a little bit paranoid about it.  Very little is actually
documented about what's in that struct.