Merge lp:~axwalk/juju-core/manual-bootstrap into lp:~go-bot/juju-core/trunk
- manual-bootstrap
- Merge into trunk
| Status: | Merged |
|---|---|
| Approved by: | Andrew Wilkins |
| Approved revision: | no longer in the source branch. |
| Merged at revision: | 1850 |
| Proposed branch: | lp:~axwalk/juju-core/manual-bootstrap |
| Merge into: | lp:~go-bot/juju-core/trunk |
| Diff against target: |
1104 lines (+616/-155) 16 files modified
cmd/juju/bootstrap.go (+19/-0) cmd/jujud/machine.go (+1/-1) environs/interface.go (+11/-0) environs/manual/agent.go (+28/-40) environs/manual/bootstrap.go (+135/-0) environs/manual/bootstrap_test.go (+164/-0) environs/manual/detection.go (+14/-3) environs/manual/detection_test.go (+7/-63) environs/manual/fakessh_test.go (+112/-0) environs/manual/provisioner.go (+17/-10) environs/manual/provisioner_test.go (+28/-23) environs/manual/tools.go (+24/-0) testing/testbase/patch.go (+12/-0) testing/testbase/patch_test.go (+9/-0) worker/localstorage/config.go (+14/-0) worker/localstorage/worker.go (+21/-15) |
| To merge this branch: | bzr merge lp:~axwalk/juju-core/manual-bootstrap |
| Related bugs: |
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Juju Engineering | Pending | ||
|
Review via email:
|
|||
Commit message
Implement manual bootstrapping
environs/bootstrap:
- Added BootstrapStorage interface. If an Environ
implements this interface, its BootstrapStorage
will be used by cmd/juju bootstrap.
cmd/juju:
- Updated bootstrap subcommand to use
environs/
as described above.
provider/
- Added LocalStorageConfig interface, which an
Environ may implement to describe the configuration
of a "localstorage".
environs/
- Added NewFileStorage (i.e. implement StorageWrite)
for testing in environs/manual.
environs/manual:
- Refactored existing code to support bootstrap machines.
- Added "Bootstrap" function to manually bootstrap an
existing machine.
To manually bootstrap, you must provide an environment that
implements the new LocalStorageConfig and BootstrapStorage
interfaces. In other words, you can (currently) only manually
bootstrap a machine which is expected to manage the storage
for its environment. This may be relaxed later if needed.
Next up is the null provider, which uses this new Bootstrap
function. I need to address one last issue with it first:
the null provider mustn't narrow the tools to default-series.
Description of the change
Implement manual bootstrapping
environs/bootstrap:
- Added BootstrapStorage interface. If an Environ
implements this interface, its BootstrapStorage
will be used by cmd/juju bootstrap.
cmd/juju:
- Updated bootstrap subcommand to use
environs/
as described above.
provider/
- Added LocalStorageConfig interface, which an
Environ may implement to describe the configuration
of a "localstorage".
environs/
- Added NewFileStorage (i.e. implement StorageWrite)
for testing in environs/manual.
environs/manual:
- Refactored existing code to support bootstrap machines.
- Added "Bootstrap" function to manually bootstrap an
existing machine.
To manually bootstrap, you must provide an environment that
implements the new LocalStorageConfig and BootstrapStorage
interfaces. In other words, you can (currently) only manually
bootstrap a machine which is expected to manage the storage
for its environment. This may be relaxed later if needed.
Next up is the null provider, which uses this new Bootstrap
function. I need to address one last issue with it first:
the null provider mustn't narrow the tools to default-series.
| Andrew Wilkins (axwalk) wrote : | # |
| Andrew Wilkins (axwalk) wrote : | # |
Please take a look.
| William Reade (fwereade) wrote : | # |
Looking very nice; just a few questions, and a suggestion that you
consider moving the local storage worker into the top-level worker
package, rather than hiding it away under local.
https:/
File cmd/juju/
https:/
cmd/juju/
environ.
Would that maybe be ".(BootstrapSto
experimenting with things like:
type HasBootstrapStorage interface {
BootstrapS
}
...which IMO leads at least to more-readable casts. YMMV.
https:/
File environs/
https:/
environs/
length)
As suggested in the previous review, please copy the whole lot somewhere
out of the way and move atomically into place on success; in this case I
think we've definitely vulnerable to errors with longer-term
consequences.
https:/
File environs/
https:/
environs/
This is really nice; can we produce some output while we're doing so? I
guess it'll have to be straight to the log for now, but a few INFO
messages would be helpful.
https:/
environs/
Don't think so...
https:/
File environs/
https:/
environs/
gc.ErrorMatches, "Environ argument is nil")
FWIW, I'm kinda ok with panics for unexpected nils.
https:/
environs/
Eyebrow slightly raised here... I'm not sure that's actually valid. Do
we initialize the machine id counter in state such that all subsequent
numbers are higher?
https:/
environs/
checkProvisione
This code block is looking awfully familiar now. Can we tidy it up a
little?
https:/
File environs/
https:/
environs/
Why would we be getting multiple arches out of FindInstanceTools? Aren't
we passing one in?
| Andrew Wilkins (axwalk) wrote : | # |
Thank you for the review. I have moved provider/
worker/
https:/
File cmd/juju/
https:/
cmd/juju/
environ.
On 2013/09/11 13:51:21, fwereade wrote:
> Would that maybe be ".(BootstrapSto
Yes, that's a bit better, thanks.
> Elsewhere, we're experimenting with
> things like:
> type HasBootstrapStorage interface {
> BootstrapStorage() (Storage, error)
> }
> ...which IMO leads at least to more-readable casts. YMMV.
I'm not a big fan of the "HasX" style. IMO, the name should represent
the behaviour or role, not something an implementer has.
https:/
File environs/
https:/
environs/
length)
On 2013/09/11 13:51:21, fwereade wrote:
> As suggested in the previous review, please copy the whole lot
somewhere out of
> the way and move atomically into place on success; in this case I
think we've
> definitely vulnerable to errors with longer-term consequences.
Done. It's a bit simpler here :)
https:/
File environs/
https:/
environs/
On 2013/09/11 13:51:21, fwereade wrote:
> This is really nice; can we produce some output while we're doing so?
I guess
> it'll have to be straight to the log for now, but a few INFO messages
would be
> helpful.
Sure. I've added some logger.Infofs to checkProvisioned,
detectSeriesAnd
purposely not added Errorfs, as error results will be logged upstream.
https:/
environs/
On 2013/09/11 13:51:21, fwereade wrote:
> Don't think so...
Oops! Nor do I. I had that in there from early on, and forgot to take it
out :)
https:/
File environs/
https:/
environs/
gc.ErrorMatches, "Environ argument is nil")
On 2013/09/11 13:51:21, fwereade wrote:
> FWIW, I'm kinda ok with panics for unexpected nils.
Noted.
https:/
environs/
On 2013/09/11 13:51:21, fwereade wrote:
> Eyebrow slightly raised here... I'm not sure that's actually valid. Do
we
> initialize the machine id counter in state such that all subsequent
numbers are
> higher?
The reason for t...
| Andrew Wilkins (axwalk) wrote : | # |
Please take a look.
| William Reade (fwereade) wrote : | # |
This is great, thanks. Modulo the /tmp-filesystem issue, this LGTM, on
the understanding that the local storage is not yet fit for purpose and
needs some concept of authentication [0] before we can actually let
people use this feature.
Let me know if I've completely misunderstood the overlap between
SSHStorage and LocalStorage, which is not a blocker for this CL but must
be resolved in a followup (if it's how I understood it to be).
[0] or to not be exposed, which we *might* be able to get away with if
we always used SSH storage from outside..?
https:/
File cmd/juju/
https:/
cmd/juju/
environ.
On 2013/09/12 07:59:00, axw1 wrote:
> On 2013/09/11 13:51:21, fwereade wrote:
> > Would that maybe be ".(BootstrapSto
> Yes, that's a bit better, thanks.
> > Elsewhere, we're experimenting with
> > things like:
> >
> > type HasBootstrapStorage interface {
> > BootstrapStorage() (Storage, error)
> > }
> >
> > ...which IMO leads at least to more-readable casts. YMMV.
> I'm not a big fan of the "HasX" style. IMO, the name should represent
the
> behaviour or role, not something an implementer has.
Your mileage may vary :). I'm not really sold on either way tbh.
https:/
File environs/
https:/
environs/
On 2013/09/12 07:59:00, axw1 wrote:
> On 2013/09/11 13:51:21, fwereade wrote:
> > Eyebrow slightly raised here... I'm not sure that's actually valid.
Do we
> > initialize the machine id counter in state such that all subsequent
numbers
> are
> > higher?
> The reason for this test is that Environ.Bootstrap is provided with a
machine ID
> to use. manual.Bootstrap is invoked via the null provider's Bootstrap
method.
> Thus, the test exists so as not to make assumptions about being on
machine "0".
> Whether or not the counter is incremented... I don't think that can be
its
> concern, if it's been provided with the ID.
Yeah, this isn't your problem. I think it still is a problem that we can
allow bootstrap with arbitrary ids, but that other bits of the system
assign special meaning to "0" (it somehow seems that every time we
eliminate a special case for 0 we end up with a new one somewhere else).
https:/
File cmd/jujud/
https:/
cmd/jujud/
"launchpad.
drop the explicit name?
https:/
File environs/
https:/
environs/
| Andrew Wilkins (axwalk) wrote : | # |
Haven't pushed changes yet, need to sort out the storage issues. Here's
my answers anyway.
https:/
File cmd/jujud/
https:/
cmd/jujud/
"launchpad.
On 2013/09/13 09:24:59, fwereade wrote:
> drop the explicit name?
Done.
https:/
File environs/
https:/
environs/
(environs.Storage, error) {
On 2013/09/13 09:24:59, fwereade wrote:
> High-level question while I think of it: does the SSHStorage on
bootstrap get
> reused but backed with a FileStorage? I sort of imagine so, in which
case, cool
> ...but is it possible for there to be overlap? ie should we be sharing
the
> flocking everywhere?
> or am I just fantasizing?
filestorage is only used for testing at the moment.
environs/
storage directory. However, there is no overlap in use;
environs/sshstorage is only used to write the tools and bootstrap state
file. *If* there's a failure bootstrapping, the bootstrap storage may
also be used to remove the state file.
Since there's no overlap, there's no need to share the locking. I did
just realise that I totally broke the manual bootstrap code though, when
I changed sshstorage's directory structure. I'll have to revisit that
before merging.
https:/
environs/
ioutil.TempFile("", "juju-filestora
On 2013/09/13 09:24:59, fwereade wrote:
> Same different-
This is only for testing, but I will fix it anyway. I think localstorage
ought to be renamed httpstorage, and have the backend contain one of
these.
https:/
File environs/
https:/
environs/
On 2013/09/13 09:24:59, fwereade wrote:
> OK, I think my problem here is maybe that it's not *just* about
bootstrap
> storage, right? It's the same storage we always use, but this is the
path to it
> that's accessible from a client machine. Yes?
> So, at least in the short term, we *will* need SSHStorage and
LocalStorage to
> coexist.
They're pointing to the same location, but they're never used
concurrently.
https:/
File worker/
https:/
worker/
On 2013/09/13 09:24:59, fwereade wrote:
> ah, I see. Can we call this localstorage please?
Sorry, fixed.
https:/
| Roger Peppe (rogpeppe) wrote : | # |
Looks good. Some minor comments and suggestions below.
https:/
File cmd/juju/
https:/
cmd/juju/
bootstrapStorage}
nice
https:/
File environs/
https:/
environs/
that returns a environs.Storage that may
I think I'd define this inside environs/
This package doesn't define or use it, and that file
is a useful reference for people implementing providers.
https:/
File environs/
https:/
environs/
(environs.Storage, error) {
Doc comment please.
Also, wouldn't it make more sense to define NewFileStorageR
terms of NewFileStorage? That way you wouldn't need the dynamic type
conversion, as Storage can be assigned to StorageReader.
For future consideration, I think filestorage.New and
filestorage.
filestorage.
https:/
File environs/
https:/
environs/
s/machineenv/
https:/
File environs/
https:/
environs/
argument is empty")
Errors don't generally start with a capital letter.
Also, if it's only used once, I wouldn't bother defining a variable for
it.
https:/
environs/
argument is nil")
ditto
https:/
environs/
s/will/will be/ ?
https:/
environs/
provisioned: %v", err)
"failed to check provisioned status: %v" ?
https:/
environs/
Why not defer this after the call to SaveState and thereby lose the need
for the savedState variable?
https:/
| Tim Penhey (thumper) wrote : | # |
Still some changes needed here I think
https:/
File cmd/juju/
https:/
cmd/juju/
bootstrapStorage}
If all we are caring about is the storage, why are we changing the
environ?
Does it not make sense to change the things that depend on the storage
to just depend on the storage and not the entire environment, then you
don't need to mock out the Storage method.
Obviously here the important thing is that we want to use a particular
storage for bootstrap, so I'd expect to be assigning the
bootstrapStorage to a storage variable, not mocking out the method with
a different structure.
https:/
File environs/
https:/
environs/
that returns a environs.Storage that may
On 2013/09/14 07:28:03, rog wrote:
> I think I'd define this inside environs/
> This package doesn't define or use it, and that file
> is a useful reference for people implementing providers.
Agreed. It is something that is used by the environment, so it makes
more sense to live there.
https:/
File environs/
https:/
environs/
ioutil.TempFile("", "juju-filestora
On 2013/09/13 09:41:26, axw1 wrote:
> On 2013/09/13 09:24:59, fwereade wrote:
> > Same different-
> This is only for testing, but I will fix it anyway. I think
localstorage ought
> to be renamed httpstorage, and have the backend contain one of these.
That sounds like a good idea IMO
https:/
File environs/
https:/
environs/
args.envcfg, constraints.
I fear that using names like "envcfg" means that we have more and more
assumed knowledge encoded into our variable names. Here I like some
parts of the Zen of Python "Explicit is better than implicit", yes it is
python, but I think the principle holds.
https:/
File environs/
https:/
environs/
Arg, here I know that I'm going to clash on names with Rog, but IMO
BootstrapSto
is a terrible name.
HasBootstrap
makes a lot more sense to me. Firstly you can look at the name and know
what is going on, and we aren't inventing words to go by the weird...
| Andrew Wilkins (axwalk) wrote : | # |
https:/
File cmd/juju/
https:/
cmd/juju/
bootstrapStorage}
On 2013/09/16 03:02:08, thumper wrote:
> If all we are caring about is the storage, why are we changing the
environ?
> Does it not make sense to change the things that depend on the storage
to just
> depend on the storage and not the entire environment, then you don't
need to
> mock out the Storage method.
The storage must be passed down to environs/bootstrap Bootstrap, and
then to environs/tools FindTools. Possibly more places. Those things
also want an Environ (in the case of FindTools, it's hidden in an
interface conversion for the legacy fallback codepath).
Bootstrap storage must be used for *everything* until the environment is
bootstrapped. Leaving a method for acquiring broken/unusable storage
seems like a mistake to me.
> Obviously here the important thing is that we want to use a particular
storage
> for bootstrap, so I'd expect to be assigning the bootstrapStorage to a
storage
> variable, not mocking out the method with a different structure.
https:/
File environs/
https:/
environs/
that returns a environs.Storage that may
On 2013/09/14 07:28:03, rog wrote:
> I think I'd define this inside environs/
> This package doesn't define or use it, and that file
> is a useful reference for people implementing providers.
Fair point, done.
https:/
File environs/
https:/
environs/
(environs.Storage, error) {
On 2013/09/14 07:28:03, rog wrote:
> Doc comment please.
Done. Seems someone else came along and wrote a writer constructor in
the mean time. It lacks a doc comment, so I'll add to that :)
> Also, wouldn't it make more sense to define NewFileStorageR
terms of
> NewFileStorage? That way you wouldn't need the dynamic type
conversion, as
> Storage can be assigned to StorageReader.
That would simplify the code here. My rationale for doing it this way is
that it limits exposure.
IMO, you shouldn't be able to convert a StorageReader to a Storage,
unless you originally created it with NewStorage and converted it to
StorageReader.
> For future consideration, I think filestorage.New and
filestorage.
> would be more idiomatic names than filestorage.
> filestorage.
I do agree, but sadly it seems there's no consensus.
https:/
environs/
ioutil.TempFile...
| Andrew Wilkins (axwalk) wrote : | # |
Please take a look.
| Roger Peppe (rogpeppe) wrote : | # |
https:/
File environs/
https:/
environs/
On 2013/09/16 04:56:48, axw1 wrote:
> On 2013/09/16 03:02:08, thumper wrote:
> > Arg, here I know that I'm going to clash on names with Rog, but IMO
> > BootstrapStorager
> > is a terrible name.
> > HasBootstrapStorage
> > makes a lot more sense to me. Firstly you can look at the name and
know what
> is
> > going on, and we aren't inventing words to go by the weird idea that
> interfaces
> > should end with "er".
> >
> > bootstrapStorage, ok := sometype.
> >
> > reads ok to me (FSVO ok given the syntax of the type assertion).
> Copy and paste of my reply to William: I'm not a big fan of the "HasX"
style.
> IMO, the name should represent the behaviour or role, not something an
> implementer has.
+lots.
I'm not keen on "BootstrapStorager" as a name either,
but I think it's important that interface names are nouns
not predicates - an interface name should represent what the
object *is*, not what it *can be*.
| Tim Penhey (thumper) wrote : | # |
On 2013/09/16 16:51:34, rog wrote:
https:/
> File environs/
https:/
> environs/
> On 2013/09/16 04:56:48, axw1 wrote:
> > On 2013/09/16 03:02:08, thumper wrote:
> > > Arg, here I know that I'm going to clash on names with Rog, but
IMO
> > > BootstrapStorager
> > > is a terrible name.
> > > HasBootstrapStorage
> > > makes a lot more sense to me. Firstly you can look at the name
and know
> what
> > is
> > > going on, and we aren't inventing words to go by the weird idea
that
> > interfaces
> > > should end with "er".
> > >
> > > bootstrapStorage, ok := sometype.
> > >
> > > reads ok to me (FSVO ok given the syntax of the type assertion).
> >
> > Copy and paste of my reply to William: I'm not a big fan of the
"HasX" style.
> > IMO, the name should represent the behaviour or role, not something
an
> > implementer has.
> +lots.
> I'm not keen on "BootstrapStorager" as a name either,
> but I think it's important that interface names are nouns
> not predicates - an interface name should represent what the
> object *is*, not what it *can be*.
Why not then just call the interface "BootstrapStorage".
| Roger Peppe (rogpeppe) wrote : | # |
On 16 September 2013 22:01, <email address hidden> wrote:
> Why not then just call the interface "BootstrapStorage".
That would be appropriate for the storage itself, but this
interface represents an object that can be asked *for* the storage,
which doesn't seem quite the same thing to me.
| William Reade (fwereade) wrote : | # |
If the only remaining contentious point is (Has)BootstrapS
then I call dealer's choice and LGTM whatever he picks; I don't think
any of the names are great, but I also don't think any of them are so
bad as to unacceptably reduce the quality of the codebase ;).
If there are other problems, carry on as usual, ofc, but let's not block
this on naming arguments.
| Go Bot (go-bot) wrote : | # |
The attempt to merge lp:~axwalk/juju-core/manual-bootstrap into lp:juju-core failed. Below is the output from the failed tests.
environs/
# launchpad.
cmd/juju/
cmd/juju/
Preview Diff
| 1 | === modified file 'cmd/juju/bootstrap.go' |
| 2 | --- cmd/juju/bootstrap.go 2013-09-20 00:52:13 +0000 |
| 3 | +++ cmd/juju/bootstrap.go 2013-09-20 05:05:14 +0000 |
| 4 | @@ -17,6 +17,7 @@ |
| 5 | "launchpad.net/juju-core/environs" |
| 6 | "launchpad.net/juju-core/environs/bootstrap" |
| 7 | "launchpad.net/juju-core/environs/config" |
| 8 | + "launchpad.net/juju-core/environs/storage" |
| 9 | "launchpad.net/juju-core/environs/sync" |
| 10 | envtools "launchpad.net/juju-core/environs/tools" |
| 11 | "launchpad.net/juju-core/errors" |
| 12 | @@ -68,6 +69,15 @@ |
| 13 | if err != nil { |
| 14 | return err |
| 15 | } |
| 16 | + // If the environment has a special bootstrap Storage, use it wherever |
| 17 | + // we'd otherwise use environ.Storage. |
| 18 | + if bs, ok := environ.(environs.BootstrapStorager); ok { |
| 19 | + bootstrapStorage, err := bs.BootstrapStorage() |
| 20 | + if err != nil { |
| 21 | + return fmt.Errorf("failed to acquire bootstrap storage: %v", err) |
| 22 | + } |
| 23 | + environ = &bootstrapStorageEnviron{environ, bootstrapStorage} |
| 24 | + } |
| 25 | // TODO: if in verbose mode, write out to Stdout if a new cert was created. |
| 26 | _, err = environs.EnsureCertificate(environ, environs.WriteCertAndKey) |
| 27 | if err != nil { |
| 28 | @@ -222,3 +232,12 @@ |
| 29 | } |
| 30 | return unique.Values() |
| 31 | } |
| 32 | + |
| 33 | +type bootstrapStorageEnviron struct { |
| 34 | + environs.Environ |
| 35 | + bootstrapStorage storage.Storage |
| 36 | +} |
| 37 | + |
| 38 | +func (b *bootstrapStorageEnviron) Storage() storage.Storage { |
| 39 | + return b.bootstrapStorage |
| 40 | +} |
| 41 | |
| 42 | === modified file 'cmd/jujud/machine.go' |
| 43 | --- cmd/jujud/machine.go 2013-09-18 06:27:52 +0000 |
| 44 | +++ cmd/jujud/machine.go 2013-09-20 05:05:14 +0000 |
| 45 | @@ -20,7 +20,6 @@ |
| 46 | "launchpad.net/juju-core/log" |
| 47 | "launchpad.net/juju-core/names" |
| 48 | "launchpad.net/juju-core/provider" |
| 49 | - localstorage "launchpad.net/juju-core/provider/local/storage" |
| 50 | "launchpad.net/juju-core/state" |
| 51 | "launchpad.net/juju-core/state/api/params" |
| 52 | "launchpad.net/juju-core/state/apiserver" |
| 53 | @@ -29,6 +28,7 @@ |
| 54 | "launchpad.net/juju-core/worker/cleaner" |
| 55 | "launchpad.net/juju-core/worker/deployer" |
| 56 | "launchpad.net/juju-core/worker/firewaller" |
| 57 | + "launchpad.net/juju-core/worker/localstorage" |
| 58 | "launchpad.net/juju-core/worker/logger" |
| 59 | "launchpad.net/juju-core/worker/machiner" |
| 60 | "launchpad.net/juju-core/worker/minunitsworker" |
| 61 | |
| 62 | === modified file 'environs/interface.go' |
| 63 | --- environs/interface.go 2013-09-18 07:13:09 +0000 |
| 64 | +++ environs/interface.go 2013-09-20 05:05:14 +0000 |
| 65 | @@ -68,6 +68,17 @@ |
| 66 | PublicStorage() storage.StorageReader |
| 67 | } |
| 68 | |
| 69 | +// BootstrapStorager is an interface that returns a environs.Storage that may |
| 70 | +// be used before the bootstrap machine agent has been provisioned. |
| 71 | +// |
| 72 | +// This is useful for environments where the storage is managed by the machine |
| 73 | +// agent once bootstrapped. |
| 74 | +type BootstrapStorager interface { |
| 75 | + // BootstrapStorager returns an environs.Storage that may be used while |
| 76 | + // bootstrapping a machine. |
| 77 | + BootstrapStorage() (storage.Storage, error) |
| 78 | +} |
| 79 | + |
| 80 | // ConfigGetter implements access to an environments configuration. |
| 81 | type ConfigGetter interface { |
| 82 | // Config returns the configuration data with which the Environ was created. |
| 83 | |
| 84 | === modified file 'environs/manual/agent.go' |
| 85 | --- environs/manual/agent.go 2013-09-13 08:39:36 +0000 |
| 86 | +++ environs/manual/agent.go 2013-09-20 05:05:14 +0000 |
| 87 | @@ -14,7 +14,7 @@ |
| 88 | "launchpad.net/juju-core/constraints" |
| 89 | "launchpad.net/juju-core/environs" |
| 90 | "launchpad.net/juju-core/environs/cloudinit" |
| 91 | - envtools "launchpad.net/juju-core/environs/tools" |
| 92 | + "launchpad.net/juju-core/environs/config" |
| 93 | "launchpad.net/juju-core/provider" |
| 94 | "launchpad.net/juju-core/state" |
| 95 | "launchpad.net/juju-core/state/api" |
| 96 | @@ -23,21 +23,27 @@ |
| 97 | ) |
| 98 | |
| 99 | type provisionMachineAgentArgs struct { |
| 100 | - host string |
| 101 | - dataDir string |
| 102 | - env environs.Environ |
| 103 | - machine *state.Machine |
| 104 | - nonce string |
| 105 | - stateInfo *state.Info |
| 106 | - apiInfo *api.Info |
| 107 | - series string |
| 108 | - arch string |
| 109 | - tools *tools.Tools |
| 110 | + host string |
| 111 | + dataDir string |
| 112 | + bootstrap bool |
| 113 | + environConfig *config.Config |
| 114 | + machineId string |
| 115 | + nonce string |
| 116 | + stateFileURL string |
| 117 | + stateInfo *state.Info |
| 118 | + apiInfo *api.Info |
| 119 | + tools *tools.Tools |
| 120 | + |
| 121 | + // agentEnv is an optional map of |
| 122 | + // arbitrary key/value pairs to pass |
| 123 | + // into the machine agent. |
| 124 | + agentEnv map[string]string |
| 125 | } |
| 126 | |
| 127 | // provisionMachineAgent connects to a machine over SSH, |
| 128 | // copies across the tools, and installs a machine agent. |
| 129 | func provisionMachineAgent(args provisionMachineAgentArgs) error { |
| 130 | + logger.Infof("Provisioning machine agent on %s", args.host) |
| 131 | script, err := provisionMachineAgentScript(args) |
| 132 | if err != nil { |
| 133 | return err |
| 134 | @@ -58,28 +64,27 @@ |
| 135 | // provisionMachineAgentScript generates the script necessary |
| 136 | // to install a machine agent on the specified host. |
| 137 | func provisionMachineAgentScript(args provisionMachineAgentArgs) (string, error) { |
| 138 | - tools := args.tools |
| 139 | - if tools == nil { |
| 140 | - var err error |
| 141 | - tools, err = findMachineAgentTools(args.env, args.series, args.arch) |
| 142 | - if err != nil { |
| 143 | - return "", err |
| 144 | - } |
| 145 | - } |
| 146 | - |
| 147 | // We generate a cloud-init config, which we'll then pull the runcmds |
| 148 | // and prerequisite packages out of. Rather than generating a cloud-config, |
| 149 | // we'll just generate a shell script. |
| 150 | - mcfg := environs.NewMachineConfig(args.machine.Id(), args.nonce, args.stateInfo, args.apiInfo) |
| 151 | + var mcfg *cloudinit.MachineConfig |
| 152 | + if args.bootstrap { |
| 153 | + mcfg = environs.NewBootstrapMachineConfig(args.machineId, args.nonce) |
| 154 | + } else { |
| 155 | + mcfg = environs.NewMachineConfig(args.machineId, args.nonce, args.stateInfo, args.apiInfo) |
| 156 | + } |
| 157 | if args.dataDir != "" { |
| 158 | mcfg.DataDir = args.dataDir |
| 159 | } |
| 160 | - mcfg.Tools = tools |
| 161 | - err := environs.FinishMachineConfig(mcfg, args.env.Config(), constraints.Value{}) |
| 162 | + mcfg.Tools = args.tools |
| 163 | + err := environs.FinishMachineConfig(mcfg, args.environConfig, constraints.Value{}) |
| 164 | if err != nil { |
| 165 | return "", err |
| 166 | } |
| 167 | mcfg.AgentEnvironment[agent.ProviderType] = provider.Null |
| 168 | + for k, v := range args.agentEnv { |
| 169 | + mcfg.AgentEnvironment[k] = v |
| 170 | + } |
| 171 | cloudcfg := corecloudinit.New() |
| 172 | if cloudcfg, err = cloudinit.Configure(mcfg, cloudcfg); err != nil { |
| 173 | return "", err |
| 174 | @@ -116,20 +121,3 @@ |
| 175 | script = append(head, tail...) |
| 176 | return strings.Join(script, "\n"), nil |
| 177 | } |
| 178 | - |
| 179 | -func findMachineAgentTools(env environs.Environ, series, arch string) (*tools.Tools, error) { |
| 180 | - agentVersion, ok := env.Config().AgentVersion() |
| 181 | - if !ok { |
| 182 | - return nil, fmt.Errorf("no agent version set in environment configuration") |
| 183 | - } |
| 184 | - possibleTools, err := envtools.FindInstanceTools(env, agentVersion, series, &arch) |
| 185 | - if err != nil { |
| 186 | - return nil, err |
| 187 | - } |
| 188 | - arches := possibleTools.Arches() |
| 189 | - possibleTools, err = possibleTools.Match(tools.Filter{Arch: arch}) |
| 190 | - if err != nil { |
| 191 | - return nil, fmt.Errorf("chosen architecture %v not present in %v", arch, arches) |
| 192 | - } |
| 193 | - return possibleTools[0], nil |
| 194 | -} |
| 195 | |
| 196 | === added file 'environs/manual/bootstrap.go' |
| 197 | --- environs/manual/bootstrap.go 1970-01-01 00:00:00 +0000 |
| 198 | +++ environs/manual/bootstrap.go 2013-09-20 05:05:14 +0000 |
| 199 | @@ -0,0 +1,135 @@ |
| 200 | +// Copyright 2013 Canonical Ltd. |
| 201 | +// Licensed under the AGPLv3, see LICENCE file for details. |
| 202 | + |
| 203 | +package manual |
| 204 | + |
| 205 | +import ( |
| 206 | + "errors" |
| 207 | + "fmt" |
| 208 | + |
| 209 | + "launchpad.net/juju-core/agent" |
| 210 | + "launchpad.net/juju-core/environs" |
| 211 | + envtools "launchpad.net/juju-core/environs/tools" |
| 212 | + "launchpad.net/juju-core/instance" |
| 213 | + "launchpad.net/juju-core/names" |
| 214 | + "launchpad.net/juju-core/provider" |
| 215 | + "launchpad.net/juju-core/state" |
| 216 | + "launchpad.net/juju-core/tools" |
| 217 | + "launchpad.net/juju-core/worker/localstorage" |
| 218 | +) |
| 219 | + |
| 220 | +const BootstrapInstanceId = instance.Id(manualInstancePrefix) |
| 221 | + |
| 222 | +// LocalStorageEnviron is an Environ where the bootstrap node |
| 223 | +// manages its own local storage. |
| 224 | +type LocalStorageEnviron interface { |
| 225 | + environs.Environ |
| 226 | + environs.BootstrapStorager |
| 227 | + localstorage.LocalStorageConfig |
| 228 | +} |
| 229 | + |
| 230 | +type BootstrapArgs struct { |
| 231 | + Host string |
| 232 | + Environ LocalStorageEnviron |
| 233 | + MachineId string |
| 234 | + PossibleTools tools.List |
| 235 | +} |
| 236 | + |
| 237 | +// TODO(axw) make this configurable? |
| 238 | +const dataDir = "/var/lib/juju" |
| 239 | + |
| 240 | +func errMachineIdInvalid(machineId string) error { |
| 241 | + return fmt.Errorf("%q is not a valid machine ID", machineId) |
| 242 | +} |
| 243 | + |
| 244 | +// NewManualBootstrapEnviron wraps a LocalStorageEnviron with another which |
| 245 | +// overrides the Bootstrap method; when Bootstrap is invoked, the specified |
| 246 | +// host will be manually bootstrapped. |
| 247 | +func Bootstrap(args BootstrapArgs) (err error) { |
| 248 | + if args.Host == "" { |
| 249 | + return errors.New("host argument is empty") |
| 250 | + } |
| 251 | + if args.Environ == nil { |
| 252 | + return errors.New("environ argument is nil") |
| 253 | + } |
| 254 | + if !names.IsMachine(args.MachineId) { |
| 255 | + return errMachineIdInvalid(args.MachineId) |
| 256 | + } |
| 257 | + |
| 258 | + provisioned, err := checkProvisioned(args.Host) |
| 259 | + if err != nil { |
| 260 | + return fmt.Errorf("failed to check provisioned status: %v", err) |
| 261 | + } |
| 262 | + if provisioned { |
| 263 | + return ErrProvisioned |
| 264 | + } |
| 265 | + |
| 266 | + bootstrapStorage, err := args.Environ.BootstrapStorage() |
| 267 | + if err != nil { |
| 268 | + return err |
| 269 | + } |
| 270 | + |
| 271 | + hc, series, err := detectSeriesAndHardwareCharacteristics(args.Host) |
| 272 | + if err != nil { |
| 273 | + return fmt.Errorf("error detecting hardware characteristics: %v", err) |
| 274 | + } |
| 275 | + |
| 276 | + // Filter tools based on detected series/arch. |
| 277 | + logger.Infof("Filtering possible tools: %v", args.PossibleTools) |
| 278 | + possibleTools, err := args.PossibleTools.Match(tools.Filter{ |
| 279 | + Arch: *hc.Arch, |
| 280 | + Series: series, |
| 281 | + }) |
| 282 | + if err != nil { |
| 283 | + return err |
| 284 | + } |
| 285 | + |
| 286 | + // Store the state file. If provisioning fails, we'll remove the file. |
| 287 | + logger.Infof("Saving bootstrap state file to bootstrap storage") |
| 288 | + err = provider.SaveState( |
| 289 | + bootstrapStorage, |
| 290 | + &provider.BootstrapState{ |
| 291 | + StateInstances: []instance.Id{BootstrapInstanceId}, |
| 292 | + Characteristics: []instance.HardwareCharacteristics{hc}, |
| 293 | + }, |
| 294 | + ) |
| 295 | + if err != nil { |
| 296 | + return err |
| 297 | + } |
| 298 | + defer func() { |
| 299 | + if err != nil { |
| 300 | + logger.Errorf("bootstrapping failed, removing state file: %v", err) |
| 301 | + bootstrapStorage.Remove(provider.StateFile) |
| 302 | + } |
| 303 | + }() |
| 304 | + |
| 305 | + // Get a file:// scheme tools URL for the tools, which will have been |
| 306 | + // copied to the remote machine's storage directory. |
| 307 | + tools := *possibleTools[0] |
| 308 | + storageDir := args.Environ.StorageDir() |
| 309 | + toolsStorageName := envtools.StorageName(tools.Version) |
| 310 | + tools.URL = fmt.Sprintf("file://%s/%s", storageDir, toolsStorageName) |
| 311 | + |
| 312 | + // Add the local storage configuration. |
| 313 | + agentEnv := map[string]string{ |
| 314 | + agent.StorageAddr: args.Environ.StorageAddr(), |
| 315 | + agent.StorageDir: storageDir, |
| 316 | + agent.SharedStorageAddr: args.Environ.SharedStorageAddr(), |
| 317 | + agent.SharedStorageDir: args.Environ.SharedStorageDir(), |
| 318 | + } |
| 319 | + |
| 320 | + // Finally, provision the machine agent. |
| 321 | + stateFileURL := fmt.Sprintf("file://%s/%s", storageDir, provider.StateFile) |
| 322 | + err = provisionMachineAgent(provisionMachineAgentArgs{ |
| 323 | + host: args.Host, |
| 324 | + dataDir: dataDir, |
| 325 | + environConfig: args.Environ.Config(), |
| 326 | + stateFileURL: stateFileURL, |
| 327 | + machineId: args.MachineId, |
| 328 | + bootstrap: true, |
| 329 | + nonce: state.BootstrapNonce, |
| 330 | + tools: &tools, |
| 331 | + agentEnv: agentEnv, |
| 332 | + }) |
| 333 | + return err |
| 334 | +} |
| 335 | |
| 336 | === added file 'environs/manual/bootstrap_test.go' |
| 337 | --- environs/manual/bootstrap_test.go 1970-01-01 00:00:00 +0000 |
| 338 | +++ environs/manual/bootstrap_test.go 2013-09-20 05:05:14 +0000 |
| 339 | @@ -0,0 +1,164 @@ |
| 340 | +// Copyright 2013 Canonical Ltd. |
| 341 | +// Licensed under the AGPLv3, see LICENCE file for details. |
| 342 | + |
| 343 | +package manual |
| 344 | + |
| 345 | +import ( |
| 346 | + "os" |
| 347 | + |
| 348 | + gc "launchpad.net/gocheck" |
| 349 | + |
| 350 | + "launchpad.net/juju-core/environs" |
| 351 | + "launchpad.net/juju-core/environs/filestorage" |
| 352 | + "launchpad.net/juju-core/environs/storage" |
| 353 | + "launchpad.net/juju-core/environs/tools" |
| 354 | + coreerrors "launchpad.net/juju-core/errors" |
| 355 | + "launchpad.net/juju-core/instance" |
| 356 | + "launchpad.net/juju-core/juju/testing" |
| 357 | + "launchpad.net/juju-core/provider" |
| 358 | + jc "launchpad.net/juju-core/testing/checkers" |
| 359 | +) |
| 360 | + |
| 361 | +type bootstrapSuite struct { |
| 362 | + testing.JujuConnSuite |
| 363 | + env *localStorageEnviron |
| 364 | +} |
| 365 | + |
| 366 | +var _ = gc.Suite(&bootstrapSuite{}) |
| 367 | + |
| 368 | +type localStorageEnviron struct { |
| 369 | + environs.Environ |
| 370 | + storage storage.Storage |
| 371 | + storageAddr string |
| 372 | + storageDir string |
| 373 | + sharedStorageAddr string |
| 374 | + sharedStorageDir string |
| 375 | +} |
| 376 | + |
| 377 | +func (e *localStorageEnviron) BootstrapStorage() (storage.Storage, error) { |
| 378 | + return e.storage, nil |
| 379 | +} |
| 380 | + |
| 381 | +func (e *localStorageEnviron) StorageAddr() string { |
| 382 | + return e.storageAddr |
| 383 | +} |
| 384 | + |
| 385 | +func (e *localStorageEnviron) StorageDir() string { |
| 386 | + return e.storageDir |
| 387 | +} |
| 388 | + |
| 389 | +func (e *localStorageEnviron) SharedStorageAddr() string { |
| 390 | + return e.sharedStorageAddr |
| 391 | +} |
| 392 | + |
| 393 | +func (e *localStorageEnviron) SharedStorageDir() string { |
| 394 | + return e.sharedStorageDir |
| 395 | +} |
| 396 | + |
| 397 | +func (s *bootstrapSuite) SetUpTest(c *gc.C) { |
| 398 | + s.JujuConnSuite.SetUpTest(c) |
| 399 | + s.env = &localStorageEnviron{ |
| 400 | + Environ: s.Conn.Environ, |
| 401 | + storageDir: c.MkDir(), |
| 402 | + } |
| 403 | + storage, err := filestorage.NewFileStorageWriter(s.env.storageDir, filestorage.UseDefaultTmpDir) |
| 404 | + c.Assert(err, gc.IsNil) |
| 405 | + s.env.storage = storage |
| 406 | +} |
| 407 | + |
| 408 | +func (s *bootstrapSuite) getArgs(c *gc.C) BootstrapArgs { |
| 409 | + hostname, err := os.Hostname() |
| 410 | + c.Assert(err, gc.IsNil) |
| 411 | + toolsList, err := tools.FindBootstrapTools(s.Conn.Environ, tools.BootstrapToolsParams{}) |
| 412 | + c.Assert(err, gc.IsNil) |
| 413 | + return BootstrapArgs{ |
| 414 | + Host: hostname, |
| 415 | + Environ: s.env, |
| 416 | + MachineId: "0", |
| 417 | + PossibleTools: toolsList, |
| 418 | + } |
| 419 | +} |
| 420 | + |
| 421 | +func (s *bootstrapSuite) TestBootstrap(c *gc.C) { |
| 422 | + args := s.getArgs(c) |
| 423 | + args.Host = "ubuntu@" + args.Host |
| 424 | + |
| 425 | + defer fakeSSH{series: s.Conn.Environ.Config().DefaultSeries()}.install(c).Restore() |
| 426 | + err := Bootstrap(args) |
| 427 | + c.Assert(err, gc.IsNil) |
| 428 | + |
| 429 | + bootstrapState, err := provider.LoadState(s.env.storage) |
| 430 | + c.Assert(err, gc.IsNil) |
| 431 | + c.Assert( |
| 432 | + bootstrapState.StateInstances, |
| 433 | + gc.DeepEquals, |
| 434 | + []instance.Id{BootstrapInstanceId}, |
| 435 | + ) |
| 436 | + |
| 437 | + // Do it all again; this should work, despite the fact that |
| 438 | + // there's a bootstrap state file. Existence for that is |
| 439 | + // checked in general bootstrap code (environs/bootstrap). |
| 440 | + defer fakeSSH{series: s.Conn.Environ.Config().DefaultSeries()}.install(c).Restore() |
| 441 | + err = Bootstrap(args) |
| 442 | + c.Assert(err, gc.IsNil) |
| 443 | + |
| 444 | + // We *do* check that the machine has no juju* upstart jobs, though. |
| 445 | + defer installFakeSSH(c, "", "/etc/init/jujud-machine-0.conf", 0).Restore() |
| 446 | + err = Bootstrap(args) |
| 447 | + c.Assert(err, gc.Equals, ErrProvisioned) |
| 448 | +} |
| 449 | + |
| 450 | +func (s *bootstrapSuite) TestBootstrapScriptFailure(c *gc.C) { |
| 451 | + args := s.getArgs(c) |
| 452 | + args.Host = "ubuntu@" + args.Host |
| 453 | + series := s.Conn.Environ.Config().DefaultSeries() |
| 454 | + defer fakeSSH{series: series, provisionAgentExitCode: 1}.install(c).Restore() |
| 455 | + err := Bootstrap(args) |
| 456 | + c.Assert(err, gc.NotNil) |
| 457 | + |
| 458 | + // Since the script failed, the state file should have been |
| 459 | + // removed from storage. |
| 460 | + _, err = provider.LoadState(s.env.storage) |
| 461 | + c.Assert(err, jc.Satisfies, coreerrors.IsNotBootstrapped) |
| 462 | +} |
| 463 | + |
| 464 | +func (s *bootstrapSuite) TestBootstrapEmptyHost(c *gc.C) { |
| 465 | + args := s.getArgs(c) |
| 466 | + args.Host = "" |
| 467 | + c.Assert(Bootstrap(args), gc.ErrorMatches, "host argument is empty") |
| 468 | +} |
| 469 | + |
| 470 | +func (s *bootstrapSuite) TestBootstrapNilEnviron(c *gc.C) { |
| 471 | + args := s.getArgs(c) |
| 472 | + args.Environ = nil |
| 473 | + c.Assert(Bootstrap(args), gc.ErrorMatches, "environ argument is nil") |
| 474 | +} |
| 475 | + |
| 476 | +func (s *bootstrapSuite) TestBootstrapInvalidMachineId(c *gc.C) { |
| 477 | + args := s.getArgs(c) |
| 478 | + args.MachineId = "" |
| 479 | + c.Assert(Bootstrap(args), gc.ErrorMatches, `"" is not a valid machine ID`) |
| 480 | + args.MachineId = "bahhumbug" |
| 481 | + c.Assert(Bootstrap(args), gc.ErrorMatches, `"bahhumbug" is not a valid machine ID`) |
| 482 | +} |
| 483 | + |
| 484 | +func (s *bootstrapSuite) TestBootstrapAlternativeMachineId(c *gc.C) { |
| 485 | + args := s.getArgs(c) |
| 486 | + args.MachineId = "1" |
| 487 | + defer fakeSSH{series: s.Conn.Environ.Config().DefaultSeries()}.install(c).Restore() |
| 488 | + c.Assert(Bootstrap(args), gc.IsNil) |
| 489 | +} |
| 490 | + |
| 491 | +func (s *bootstrapSuite) TestBootstrapNoMatchingTools(c *gc.C) { |
| 492 | + // Empty tools list. |
| 493 | + args := s.getArgs(c) |
| 494 | + args.PossibleTools = nil |
| 495 | + series := s.Conn.Environ.Config().DefaultSeries() |
| 496 | + defer fakeSSH{series: series, skipProvisionAgent: true}.install(c).Restore() |
| 497 | + c.Assert(Bootstrap(args), gc.ErrorMatches, "no matching tools available") |
| 498 | + |
| 499 | + // Non-empty list, but none that match the series/arch. |
| 500 | + defer fakeSSH{series: "edgy", skipProvisionAgent: true}.install(c).Restore() |
| 501 | + args = s.getArgs(c) |
| 502 | + c.Assert(Bootstrap(args), gc.ErrorMatches, "no matching tools available") |
| 503 | +} |
| 504 | |
| 505 | === modified file 'environs/manual/detection.go' |
| 506 | --- environs/manual/detection.go 2013-09-12 02:04:05 +0000 |
| 507 | +++ environs/manual/detection.go 2013-09-20 05:05:14 +0000 |
| 508 | @@ -11,6 +11,7 @@ |
| 509 | "strings" |
| 510 | |
| 511 | "launchpad.net/juju-core/instance" |
| 512 | + "launchpad.net/juju-core/utils" |
| 513 | ) |
| 514 | |
| 515 | // checkProvisionedScript is the script to run on the remote machine |
| 516 | @@ -23,9 +24,9 @@ |
| 517 | // checkProvisioned checks if any juju upstart jobs already |
| 518 | // exist on the host machine. |
| 519 | func checkProvisioned(sshHost string) (bool, error) { |
| 520 | - cmd := sshCommand(sshHost, "bash") |
| 521 | + logger.Infof("Checking if %s is already provisioned", sshHost) |
| 522 | + cmd := sshCommand(sshHost, fmt.Sprintf("bash -c %s", utils.ShQuote(checkProvisionedScript))) |
| 523 | var stdout, stderr bytes.Buffer |
| 524 | - cmd.Stdin = bytes.NewBufferString(checkProvisionedScript) |
| 525 | cmd.Stdout = &stdout |
| 526 | cmd.Stderr = &stderr |
| 527 | if err := cmd.Run(); err != nil { |
| 528 | @@ -34,13 +35,21 @@ |
| 529 | } |
| 530 | return false, err |
| 531 | } |
| 532 | - return len(strings.TrimSpace(stdout.String())) > 0, nil |
| 533 | + output := strings.TrimSpace(stdout.String()) |
| 534 | + provisioned := len(output) > 0 |
| 535 | + if provisioned { |
| 536 | + logger.Infof("%s is already provisioned [%q]", sshHost, output) |
| 537 | + } else { |
| 538 | + logger.Infof("%s is not provisioned", sshHost) |
| 539 | + } |
| 540 | + return provisioned, nil |
| 541 | } |
| 542 | |
| 543 | // detectSeriesHardwareCharacteristics detects the OS series |
| 544 | // and hardware characteristics of the remote machine by |
| 545 | // connecting to the machine and executing a bash script. |
| 546 | func detectSeriesAndHardwareCharacteristics(sshHost string) (hc instance.HardwareCharacteristics, series string, err error) { |
| 547 | + logger.Infof("Detecting series and characteristics on %s", sshHost) |
| 548 | cmd := sshCommand(sshHost, "bash") |
| 549 | cmd.Stdin = bytes.NewBufferString(detectionScript) |
| 550 | out, err := cmd.CombinedOutput() |
| 551 | @@ -101,6 +110,7 @@ |
| 552 | } |
| 553 | |
| 554 | // TODO(axw) calculate CpuPower. What algorithm do we use? |
| 555 | + logger.Infof("series: %s, characteristics: %s", series, hc) |
| 556 | return hc, series, nil |
| 557 | } |
| 558 | |
| 559 | @@ -116,6 +126,7 @@ |
| 560 | } |
| 561 | |
| 562 | const detectionScript = `#!/bin/bash |
| 563 | +set -e |
| 564 | lsb_release -cs |
| 565 | uname -m |
| 566 | grep MemTotal /proc/meminfo |
| 567 | |
| 568 | === modified file 'environs/manual/detection_test.go' |
| 569 | --- environs/manual/detection_test.go 2013-09-20 02:53:59 +0000 |
| 570 | +++ environs/manual/detection_test.go 2013-09-20 05:05:14 +0000 |
| 571 | @@ -4,10 +4,6 @@ |
| 572 | package manual |
| 573 | |
| 574 | import ( |
| 575 | - "fmt" |
| 576 | - "io/ioutil" |
| 577 | - "os" |
| 578 | - "path/filepath" |
| 579 | "strings" |
| 580 | |
| 581 | gc "launchpad.net/gocheck" |
| 582 | @@ -22,58 +18,6 @@ |
| 583 | |
| 584 | var _ = gc.Suite(&detectionSuite{}) |
| 585 | |
| 586 | -// sshscript should only print the result on the first execution, |
| 587 | -// to handle the case where it's called multiple times. On |
| 588 | -// subsequent executions, it should find the next 'ssh' in $PATH |
| 589 | -// and exec that. |
| 590 | -var sshscript = `#!/bin/bash |
| 591 | -if [ ! -e "$0.run" ]; then |
| 592 | - touch "$0.run" |
| 593 | - diff "$0.expected-input" - |
| 594 | - exitcode=$? |
| 595 | - if [ $exitcode -ne 0 ]; then |
| 596 | - echo "ERROR: did not match expected input" >&2 |
| 597 | - exit $exitcode |
| 598 | - fi |
| 599 | - # stdout |
| 600 | - %s |
| 601 | - # stderr |
| 602 | - %s |
| 603 | - exit %d |
| 604 | -else |
| 605 | - export PATH=${PATH#*:} |
| 606 | - exec ssh $* |
| 607 | -fi` |
| 608 | - |
| 609 | -// sshresponse creates a fake "ssh" command in a new $PATH, |
| 610 | -// updates $PATH, and returns a function to reset $PATH to |
| 611 | -// its original value when called. |
| 612 | -// |
| 613 | -// output may be: |
| 614 | -// - nil (no output) |
| 615 | -// - a string (stdout) |
| 616 | -// - a slice of strings, of length two (stdout, stderr) |
| 617 | -func sshresponse(c *gc.C, input string, output interface{}, rc int) func() { |
| 618 | - fakebin := c.MkDir() |
| 619 | - ssh := filepath.Join(fakebin, "ssh") |
| 620 | - sshexpectedinput := ssh + ".expected-input" |
| 621 | - var stdout, stderr string |
| 622 | - switch output := output.(type) { |
| 623 | - case nil: |
| 624 | - case string: |
| 625 | - stdout = fmt.Sprintf("cat<<EOF\n%s\nEOF", output) |
| 626 | - case []string: |
| 627 | - stdout = fmt.Sprintf("cat<<EOF\n%s\nEOF", output[0]) |
| 628 | - stderr = fmt.Sprintf("cat>&2<<EOF\n%s\nEOF", output[1]) |
| 629 | - } |
| 630 | - script := fmt.Sprintf(sshscript, stdout, stderr, rc) |
| 631 | - err := ioutil.WriteFile(ssh, []byte(script), 0777) |
| 632 | - c.Assert(err, gc.IsNil) |
| 633 | - err = ioutil.WriteFile(sshexpectedinput, []byte(input), 0644) |
| 634 | - c.Assert(err, gc.IsNil) |
| 635 | - return testbase.PatchEnvironment("PATH", fakebin+":"+os.Getenv("PATH")) |
| 636 | -} |
| 637 | - |
| 638 | func (s *detectionSuite) TestDetectSeries(c *gc.C) { |
| 639 | response := strings.Join([]string{ |
| 640 | "edgy", |
| 641 | @@ -81,14 +25,14 @@ |
| 642 | "MemTotal: 4096 kB", |
| 643 | "processor: 0", |
| 644 | }, "\n") |
| 645 | - defer sshresponse(c, detectionScript, response, 0)() |
| 646 | + defer installFakeSSH(c, detectionScript, response, 0)() |
| 647 | _, series, err := detectSeriesAndHardwareCharacteristics("whatever") |
| 648 | c.Assert(err, gc.IsNil) |
| 649 | c.Assert(series, gc.Equals, "edgy") |
| 650 | } |
| 651 | |
| 652 | func (s *detectionSuite) TestDetectionError(c *gc.C) { |
| 653 | - defer sshresponse(c, detectionScript, "oh noes", 33)() |
| 654 | + defer installFakeSSH(c, detectionScript, "oh noes", 33)() |
| 655 | _, _, err := detectSeriesAndHardwareCharacteristics("whatever") |
| 656 | c.Assert(err, gc.ErrorMatches, "exit status 33 \\(oh noes\\)") |
| 657 | } |
| 658 | @@ -148,7 +92,7 @@ |
| 659 | for i, test := range tests { |
| 660 | c.Logf("test %d: %s", i, test.summary) |
| 661 | scriptResponse := strings.Join(test.scriptResponse, "\n") |
| 662 | - defer sshresponse(c, detectionScript, scriptResponse, 0)() |
| 663 | + defer installFakeSSH(c, detectionScript, scriptResponse, 0)() |
| 664 | hc, _, err := detectSeriesAndHardwareCharacteristics("hostname") |
| 665 | c.Assert(err, gc.IsNil) |
| 666 | c.Assert(hc.String(), gc.Equals, test.expectedHc) |
| 667 | @@ -156,25 +100,25 @@ |
| 668 | } |
| 669 | |
| 670 | func (s *detectionSuite) TestCheckProvisioned(c *gc.C) { |
| 671 | - defer sshresponse(c, checkProvisionedScript, "", 0)() |
| 672 | + defer installFakeSSH(c, "", "", 0)() |
| 673 | provisioned, err := checkProvisioned("example.com") |
| 674 | c.Assert(err, gc.IsNil) |
| 675 | c.Assert(provisioned, jc.IsFalse) |
| 676 | |
| 677 | - defer sshresponse(c, checkProvisionedScript, "non-empty", 0)() |
| 678 | + defer installFakeSSH(c, "", "non-empty", 0)() |
| 679 | provisioned, err = checkProvisioned("example.com") |
| 680 | c.Assert(err, gc.IsNil) |
| 681 | c.Assert(provisioned, jc.IsTrue) |
| 682 | |
| 683 | // stderr should not affect result. |
| 684 | - defer sshresponse(c, checkProvisionedScript, []string{"", "non-empty-stderr"}, 0)() |
| 685 | + defer installFakeSSH(c, "", []string{"", "non-empty-stderr"}, 0)() |
| 686 | provisioned, err = checkProvisioned("example.com") |
| 687 | c.Assert(err, gc.IsNil) |
| 688 | c.Assert(provisioned, jc.IsFalse) |
| 689 | |
| 690 | // if the script fails for whatever reason, then checkProvisioned |
| 691 | // will return an error. stderr will be included in the error message. |
| 692 | - defer sshresponse(c, checkProvisionedScript, []string{"non-empty-stdout", "non-empty-stderr"}, 255)() |
| 693 | + defer installFakeSSH(c, "", []string{"non-empty-stdout", "non-empty-stderr"}, 255)() |
| 694 | _, err = checkProvisioned("example.com") |
| 695 | c.Assert(err, gc.ErrorMatches, "exit status 255 \\(non-empty-stderr\\)") |
| 696 | } |
| 697 | |
| 698 | === added file 'environs/manual/fakessh_test.go' |
| 699 | --- environs/manual/fakessh_test.go 1970-01-01 00:00:00 +0000 |
| 700 | +++ environs/manual/fakessh_test.go 2013-09-20 05:05:14 +0000 |
| 701 | @@ -0,0 +1,112 @@ |
| 702 | +// Copyright 2013 Canonical Ltd. |
| 703 | +// Licensed under the AGPLv3, see LICENCE file for details. |
| 704 | + |
| 705 | +package manual |
| 706 | + |
| 707 | +import ( |
| 708 | + "fmt" |
| 709 | + "io/ioutil" |
| 710 | + "os" |
| 711 | + "path/filepath" |
| 712 | + "strings" |
| 713 | + |
| 714 | + gc "launchpad.net/gocheck" |
| 715 | + |
| 716 | + "launchpad.net/juju-core/testing/testbase" |
| 717 | +) |
| 718 | + |
| 719 | +// sshscript should only print the result on the first execution, |
| 720 | +// to handle the case where it's called multiple times. On |
| 721 | +// subsequent executions, it should find the next 'ssh' in $PATH |
| 722 | +// and exec that. |
| 723 | +var sshscript = `#!/bin/bash |
| 724 | +if [ ! -e "$0.run" ]; then |
| 725 | + touch "$0.run" |
| 726 | + diff "$0.expected-input" - |
| 727 | + exitcode=$? |
| 728 | + if [ $exitcode -ne 0 ]; then |
| 729 | + echo "ERROR: did not match expected input" >&2 |
| 730 | + exit $exitcode |
| 731 | + fi |
| 732 | + # stdout |
| 733 | + %s |
| 734 | + # stderr |
| 735 | + %s |
| 736 | + exit %d |
| 737 | +else |
| 738 | + export PATH=${PATH#*:} |
| 739 | + exec ssh $* |
| 740 | +fi` |
| 741 | + |
| 742 | +// installFakeSSH creates a fake "ssh" command in a new $PATH, |
| 743 | +// updates $PATH, and returns a function to reset $PATH to |
| 744 | +// its original value when called. |
| 745 | +// |
| 746 | +// output may be: |
| 747 | +// - nil (no output) |
| 748 | +// - a string (stdout) |
| 749 | +// - a slice of strings, of length two (stdout, stderr) |
| 750 | +func installFakeSSH(c *gc.C, input string, output interface{}, rc int) testbase.Restorer { |
| 751 | + fakebin := c.MkDir() |
| 752 | + ssh := filepath.Join(fakebin, "ssh") |
| 753 | + sshexpectedinput := ssh + ".expected-input" |
| 754 | + var stdout, stderr string |
| 755 | + switch output := output.(type) { |
| 756 | + case nil: |
| 757 | + case string: |
| 758 | + stdout = fmt.Sprintf("cat<<EOF\n%s\nEOF", output) |
| 759 | + case []string: |
| 760 | + stdout = fmt.Sprintf("cat<<EOF\n%s\nEOF", output[0]) |
| 761 | + stderr = fmt.Sprintf("cat>&2<<EOF\n%s\nEOF", output[1]) |
| 762 | + } |
| 763 | + script := fmt.Sprintf(sshscript, stdout, stderr, rc) |
| 764 | + err := ioutil.WriteFile(ssh, []byte(script), 0777) |
| 765 | + c.Assert(err, gc.IsNil) |
| 766 | + err = ioutil.WriteFile(sshexpectedinput, []byte(input), 0644) |
| 767 | + c.Assert(err, gc.IsNil) |
| 768 | + return testbase.PatchEnvironment("PATH", fakebin+":"+os.Getenv("PATH")) |
| 769 | +} |
| 770 | + |
| 771 | +// fakeSSH wraps the invocation of installFakeSSH based on the parameters. |
| 772 | +type fakeSSH struct { |
| 773 | + series string |
| 774 | + arch string |
| 775 | + |
| 776 | + // exit code for the machine agent provisioning script. |
| 777 | + provisionAgentExitCode int |
| 778 | + |
| 779 | + // there are conditions other than error in the above |
| 780 | + // that might cause provisioning to not go ahead, such |
| 781 | + // as tools being missing. |
| 782 | + skipProvisionAgent bool |
| 783 | +} |
| 784 | + |
| 785 | +// install installs fake SSH commands, which will respond to |
| 786 | +// manual provisioning/bootstrapping commands with the specified |
| 787 | +// output and exit codes. |
| 788 | +func (r fakeSSH) install(c *gc.C) testbase.Restorer { |
| 789 | + series := r.series |
| 790 | + if series == "" { |
| 791 | + series = "precise" |
| 792 | + } |
| 793 | + arch := r.arch |
| 794 | + if arch == "" { |
| 795 | + arch = "amd64" |
| 796 | + } |
| 797 | + detectionoutput := strings.Join([]string{ |
| 798 | + series, |
| 799 | + arch, |
| 800 | + "MemTotal: 4096 kB", |
| 801 | + "processor: 0", |
| 802 | + }, "\n") |
| 803 | + var restore testbase.Restorer |
| 804 | + add := func(input string, output interface{}, rc int) { |
| 805 | + restore = restore.Add(installFakeSSH(c, input, output, rc)) |
| 806 | + } |
| 807 | + if !r.skipProvisionAgent { |
| 808 | + add("", nil, r.provisionAgentExitCode) |
| 809 | + } |
| 810 | + add(detectionScript, detectionoutput, 0) |
| 811 | + add("", nil, 0) // checkProvisioned |
| 812 | + return restore |
| 813 | +} |
| 814 | |
| 815 | === modified file 'environs/manual/provisioner.go' |
| 816 | --- environs/manual/provisioner.go 2013-09-18 04:43:04 +0000 |
| 817 | +++ environs/manual/provisioner.go 2013-09-20 05:05:14 +0000 |
| 818 | @@ -104,6 +104,14 @@ |
| 819 | return nil, err |
| 820 | } |
| 821 | |
| 822 | + tools := args.Tools |
| 823 | + if tools == nil { |
| 824 | + tools, err = findInstanceTools(env, series, *hc.Arch) |
| 825 | + if err != nil { |
| 826 | + return nil, err |
| 827 | + } |
| 828 | + } |
| 829 | + |
| 830 | // Generate a unique nonce for the machine. |
| 831 | uuid, err := utils.NewUUID() |
| 832 | if err != nil { |
| 833 | @@ -137,16 +145,15 @@ |
| 834 | |
| 835 | // Finally, provision the machine agent. |
| 836 | err = provisionMachineAgent(provisionMachineAgentArgs{ |
| 837 | - host: args.Host, |
| 838 | - dataDir: args.DataDir, |
| 839 | - env: env, |
| 840 | - machine: m, |
| 841 | - nonce: nonce, |
| 842 | - stateInfo: stateInfo, |
| 843 | - apiInfo: apiInfo, |
| 844 | - series: series, |
| 845 | - arch: *hc.Arch, |
| 846 | - tools: args.Tools, |
| 847 | + host: args.Host, |
| 848 | + dataDir: args.DataDir, |
| 849 | + environConfig: env.Config(), |
| 850 | + machineId: m.Id(), |
| 851 | + bootstrap: false, |
| 852 | + nonce: nonce, |
| 853 | + stateInfo: stateInfo, |
| 854 | + apiInfo: apiInfo, |
| 855 | + tools: tools, |
| 856 | }) |
| 857 | if err != nil { |
| 858 | return m, err |
| 859 | |
| 860 | === modified file 'environs/manual/provisioner_test.go' |
| 861 | --- environs/manual/provisioner_test.go 2013-09-20 00:33:38 +0000 |
| 862 | +++ environs/manual/provisioner_test.go 2013-09-20 05:05:14 +0000 |
| 863 | @@ -6,13 +6,15 @@ |
| 864 | import ( |
| 865 | "fmt" |
| 866 | "os" |
| 867 | - "strings" |
| 868 | |
| 869 | gc "launchpad.net/gocheck" |
| 870 | |
| 871 | - "launchpad.net/juju-core/environs/tools" |
| 872 | + "launchpad.net/juju-core/environs/storage" |
| 873 | + envtesting "launchpad.net/juju-core/environs/testing" |
| 874 | "launchpad.net/juju-core/instance" |
| 875 | "launchpad.net/juju-core/juju/testing" |
| 876 | + jc "launchpad.net/juju-core/testing/checkers" |
| 877 | + "launchpad.net/juju-core/version" |
| 878 | ) |
| 879 | |
| 880 | type provisionerSuite struct { |
| 881 | @@ -31,32 +33,35 @@ |
| 882 | } |
| 883 | |
| 884 | func (s *provisionerSuite) TestProvisionMachine(c *gc.C) { |
| 885 | - // Prepare a mock ssh response for the detection phase. |
| 886 | - detectionoutput := strings.Join([]string{ |
| 887 | - "edgy", |
| 888 | - "armv4", |
| 889 | - "MemTotal: 4096 kB", |
| 890 | - "processor: 0", |
| 891 | - }, "\n") |
| 892 | + const series = "precise" |
| 893 | + const arch = "amd64" |
| 894 | |
| 895 | args := s.getArgs(c) |
| 896 | hostname := args.Host |
| 897 | args.Host = "ubuntu@" + args.Host |
| 898 | |
| 899 | - defer sshresponse(c, detectionScript, detectionoutput, 0)() |
| 900 | - defer sshresponse(c, checkProvisionedScript, "", 0)() |
| 901 | + envtesting.RemoveTools(c, s.Conn.Environ.Storage()) |
| 902 | + envtesting.RemoveTools(c, s.Conn.Environ.PublicStorage().(storage.Storage)) |
| 903 | + defer fakeSSH{ |
| 904 | + series: series, arch: arch, skipProvisionAgent: true, |
| 905 | + }.install(c).Restore() |
| 906 | m, err := ProvisionMachine(args) |
| 907 | - c.Assert(err, gc.ErrorMatches, "no matching tools available") |
| 908 | + c.Assert(err, gc.ErrorMatches, "no tools available") |
| 909 | c.Assert(m, gc.IsNil) |
| 910 | |
| 911 | - toolsList, err := tools.FindBootstrapTools(s.Conn.Environ, tools.BootstrapToolsParams{}) |
| 912 | - c.Assert(err, gc.IsNil) |
| 913 | - args.Tools = toolsList[0] |
| 914 | + cfg := s.Conn.Environ.Config() |
| 915 | + number, ok := cfg.AgentVersion() |
| 916 | + c.Assert(ok, jc.IsTrue) |
| 917 | + binVersion := version.Binary{number, series, arch} |
| 918 | + envtesting.UploadFakeToolsVersion(c, s.Conn.Environ.Storage(), binVersion) |
| 919 | |
| 920 | - for _, errorCode := range []int{255, 0} { |
| 921 | - defer sshresponse(c, "", "", errorCode)() // executing script |
| 922 | - defer sshresponse(c, detectionScript, detectionoutput, 0)() |
| 923 | - defer sshresponse(c, checkProvisionedScript, "", 0)() |
| 924 | + for i, errorCode := range []int{255, 0} { |
| 925 | + c.Logf("test %d: code %d", i, errorCode) |
| 926 | + defer fakeSSH{ |
| 927 | + series: series, |
| 928 | + arch: arch, |
| 929 | + provisionAgentExitCode: errorCode, |
| 930 | + }.install(c).Restore() |
| 931 | m, err = ProvisionMachine(args) |
| 932 | if errorCode != 0 { |
| 933 | c.Assert(err, gc.ErrorMatches, fmt.Sprintf("exit status %d", errorCode)) |
| 934 | @@ -64,9 +69,9 @@ |
| 935 | } else { |
| 936 | c.Assert(err, gc.IsNil) |
| 937 | c.Assert(m, gc.NotNil) |
| 938 | - // machine ID will be 2, not 1. Even though we failed and the |
| 939 | + // machine ID will be incremented. Even though we failed and the |
| 940 | // machine is removed, the ID is not reused. |
| 941 | - c.Assert(m.Id(), gc.Equals, "2") |
| 942 | + c.Assert(m.Id(), gc.Equals, fmt.Sprint(i)) |
| 943 | instanceId, err := m.InstanceId() |
| 944 | c.Assert(err, gc.IsNil) |
| 945 | c.Assert(instanceId, gc.Equals, instance.Id("manual:"+hostname)) |
| 946 | @@ -75,10 +80,10 @@ |
| 947 | |
| 948 | // Attempting to provision a machine twice should fail. We effect |
| 949 | // this by checking for existing juju upstart configurations. |
| 950 | - defer sshresponse(c, checkProvisionedScript, "/etc/init/jujud-machine-0.conf", 0)() |
| 951 | + defer installFakeSSH(c, "", "/etc/init/jujud-machine-0.conf", 0)() |
| 952 | _, err = ProvisionMachine(args) |
| 953 | c.Assert(err, gc.Equals, ErrProvisioned) |
| 954 | - defer sshresponse(c, checkProvisionedScript, "/etc/init/jujud-machine-0.conf", 255)() |
| 955 | + defer installFakeSSH(c, "", "/etc/init/jujud-machine-0.conf", 255)() |
| 956 | _, err = ProvisionMachine(args) |
| 957 | c.Assert(err, gc.ErrorMatches, "error checking if provisioned: exit status 255") |
| 958 | } |
| 959 | |
| 960 | === added file 'environs/manual/tools.go' |
| 961 | --- environs/manual/tools.go 1970-01-01 00:00:00 +0000 |
| 962 | +++ environs/manual/tools.go 2013-09-20 05:05:14 +0000 |
| 963 | @@ -0,0 +1,24 @@ |
| 964 | +// Copyright 2013 Canonical Ltd. |
| 965 | +// Licensed under the AGPLv3, see LICENCE file for details. |
| 966 | + |
| 967 | +package manual |
| 968 | + |
| 969 | +import ( |
| 970 | + "fmt" |
| 971 | + |
| 972 | + "launchpad.net/juju-core/environs" |
| 973 | + envtools "launchpad.net/juju-core/environs/tools" |
| 974 | + "launchpad.net/juju-core/tools" |
| 975 | +) |
| 976 | + |
| 977 | +func findInstanceTools(env environs.Environ, series, arch string) (*tools.Tools, error) { |
| 978 | + agentVersion, ok := env.Config().AgentVersion() |
| 979 | + if !ok { |
| 980 | + return nil, fmt.Errorf("no agent version set in environment configuration") |
| 981 | + } |
| 982 | + possibleTools, err := envtools.FindInstanceTools(env, agentVersion, series, &arch) |
| 983 | + if err != nil { |
| 984 | + return nil, err |
| 985 | + } |
| 986 | + return possibleTools[0], nil |
| 987 | +} |
| 988 | |
| 989 | === modified file 'testing/testbase/patch.go' |
| 990 | --- testing/testbase/patch.go 2013-09-20 02:10:46 +0000 |
| 991 | +++ testing/testbase/patch.go 2013-09-20 05:05:14 +0000 |
| 992 | @@ -12,6 +12,18 @@ |
| 993 | // to restore some previous state. |
| 994 | type Restorer func() |
| 995 | |
| 996 | +// Add returns a Restorer that restores first f1 |
| 997 | +// and then f. It is valid to call this on a nil |
| 998 | +// Restorer. |
| 999 | +func (f Restorer) Add(f1 Restorer) Restorer { |
| 1000 | + return func() { |
| 1001 | + f1.Restore() |
| 1002 | + if f != nil { |
| 1003 | + f.Restore() |
| 1004 | + } |
| 1005 | + } |
| 1006 | +} |
| 1007 | + |
| 1008 | // Restore restores some previous state. |
| 1009 | func (r Restorer) Restore() { |
| 1010 | r() |
| 1011 | |
| 1012 | === modified file 'testing/testbase/patch_test.go' |
| 1013 | --- testing/testbase/patch_test.go 2013-09-20 02:10:46 +0000 |
| 1014 | +++ testing/testbase/patch_test.go 2013-09-20 05:05:14 +0000 |
| 1015 | @@ -74,3 +74,12 @@ |
| 1016 | c.Check(os.Getenv(envName), gc.Equals, "initial") |
| 1017 | os.Setenv(envName, oldValue) |
| 1018 | } |
| 1019 | + |
| 1020 | +func (*PatchEnvironmentSuite) TestRestorerAdd(c *gc.C) { |
| 1021 | + var order []string |
| 1022 | + first := testbase.Restorer(func() { order = append(order, "first") }) |
| 1023 | + second := testbase.Restorer(func() { order = append(order, "second") }) |
| 1024 | + restore := first.Add(second) |
| 1025 | + restore() |
| 1026 | + c.Assert(order, gc.DeepEquals, []string{"second", "first"}) |
| 1027 | +} |
| 1028 | |
| 1029 | === renamed directory 'provider/local/storage' => 'worker/localstorage' |
| 1030 | === added file 'worker/localstorage/config.go' |
| 1031 | --- worker/localstorage/config.go 1970-01-01 00:00:00 +0000 |
| 1032 | +++ worker/localstorage/config.go 2013-09-20 05:05:14 +0000 |
| 1033 | @@ -0,0 +1,14 @@ |
| 1034 | +// Copyright 2013 Canonical Ltd. |
| 1035 | +// Licensed under the AGPLv3, see LICENCE file for details. |
| 1036 | + |
| 1037 | +package localstorage |
| 1038 | + |
| 1039 | +// LocalStorageConfig is an interface that, if implemented, may be used |
| 1040 | +// to configure a machine agent for use with the localstorage worker in |
| 1041 | +// this package. |
| 1042 | +type LocalStorageConfig interface { |
| 1043 | + StorageDir() string |
| 1044 | + StorageAddr() string |
| 1045 | + SharedStorageDir() string |
| 1046 | + SharedStorageAddr() string |
| 1047 | +} |
| 1048 | |
| 1049 | === modified file 'worker/localstorage/worker.go' |
| 1050 | --- provider/local/storage/worker.go 2013-09-19 03:14:56 +0000 |
| 1051 | +++ worker/localstorage/worker.go 2013-09-20 05:05:14 +0000 |
| 1052 | @@ -1,4 +1,7 @@ |
| 1053 | -package storage |
| 1054 | +// Copyright 2013 Canonical Ltd. |
| 1055 | +// Licensed under the AGPLv3, see LICENCE file for details. |
| 1056 | + |
| 1057 | +package localstorage |
| 1058 | |
| 1059 | import ( |
| 1060 | "launchpad.net/loggo" |
| 1061 | @@ -10,7 +13,7 @@ |
| 1062 | "launchpad.net/juju-core/worker" |
| 1063 | ) |
| 1064 | |
| 1065 | -var logger = loggo.GetLogger("juju.local.storage") |
| 1066 | +var logger = loggo.GetLogger("juju.worker.localstorage") |
| 1067 | |
| 1068 | type storageWorker struct { |
| 1069 | config agent.Config |
| 1070 | @@ -55,19 +58,22 @@ |
| 1071 | |
| 1072 | sharedStorageDir := s.config.Value(agent.SharedStorageDir) |
| 1073 | sharedStorageAddr := s.config.Value(agent.SharedStorageAddr) |
| 1074 | - logger.Infof("serving %s on %s", sharedStorageDir, sharedStorageAddr) |
| 1075 | - |
| 1076 | - sharedStorage, err := filestorage.NewFileStorageWriter(sharedStorageDir, filestorage.UseDefaultTmpDir) |
| 1077 | - if err != nil { |
| 1078 | - logger.Errorf("error with local storage: %v", err) |
| 1079 | - return err |
| 1080 | - } |
| 1081 | - sharedStorageListener, err := httpstorage.Serve(sharedStorageAddr, sharedStorage) |
| 1082 | - if err != nil { |
| 1083 | - logger.Errorf("error with local storage: %v", err) |
| 1084 | - return err |
| 1085 | - } |
| 1086 | - defer sharedStorageListener.Close() |
| 1087 | + if sharedStorageAddr != "" && sharedStorageDir != "" { |
| 1088 | + logger.Infof("serving %s on %s", sharedStorageDir, sharedStorageAddr) |
| 1089 | + sharedStorage, err := filestorage.NewFileStorageWriter(sharedStorageDir, filestorage.UseDefaultTmpDir) |
| 1090 | + if err != nil { |
| 1091 | + logger.Errorf("error with local storage: %v", err) |
| 1092 | + return err |
| 1093 | + } |
| 1094 | + sharedStorageListener, err := httpstorage.Serve(sharedStorageAddr, sharedStorage) |
| 1095 | + if err != nil { |
| 1096 | + logger.Errorf("error with local storage: %v", err) |
| 1097 | + return err |
| 1098 | + } |
| 1099 | + defer sharedStorageListener.Close() |
| 1100 | + } else { |
| 1101 | + logger.Infof("no shared storage: dir=%q addr=%q", sharedStorageDir, sharedStorageAddr) |
| 1102 | + } |
| 1103 | |
| 1104 | logger.Infof("storage routines started, awaiting death") |
| 1105 |
Reviewers: mp+184714_
Message:
Please take a look.
Description:
Implement manual bootstrapping
environs/bootstrap:
- Added BootstrapStorage interface. If an Environ
implements this interface, its BootstrapStorage
will be used by cmd/juju bootstrap.
cmd/juju:
- Updated bootstrap subcommand to use
environs/
as described above.
provider/
- Added LocalStorageConfig interface, which an
Environ may implement to describe the configuration
of a "localstorage".
environs/
- Added NewFileStorage (i.e. implement StorageWrite)
for testing in environs/manual.
environs/manual:
- Refactored existing code to support bootstrap machines.
- Added "Bootstrap" function to manually bootstrap an
existing machine.
To manually bootstrap, you must provide an environment that
implements the new LocalStorageConfig and BootstrapStorage
interfaces. In other words, you can (currently) only manually
bootstrap a machine which is expected to manage the storage
for its environment. This may be relaxed later if needed.
Next up is the null provider, which uses this new Bootstrap
function. I need to address one last issue with it first:
the null provider mustn't narrow the tools to default-series.
https:/
(do not edit description out of merge proposal)
Please review this at https:/
Affected files (+525, -53 lines):
A [revision details]
M cmd/juju/
M environs/
M environs/
M environs/
A environs/
A environs/
M environs/
M environs/
A environs/
A provider/
M provider/