Code review comment for lp:~axwalk/juju-core/manual-bootstrap
Revision history for this message
| William Reade (fwereade) wrote | # |
© 2004
Canonical Ltd.
•
Terms of use
•
Data privacy
•
Contact Launchpad Support
•
Blog
•
Careers
•
System status
•
bbfa235
(Get the code!)
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/
(environs.Storage, error) {
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?
https:/
environs/
ioutil.TempFile("", "juju-filestora
Same different-
https:/
File environs/
https:/
environs/
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.
https:/
File worker/
https:/
worker/
ah, I see. Can we call this localstorage please?
https:/