|
|
|
Machine agent for local provider bootstrap
If you are using a local provider, alway upload tools.
This uses the standard mechanism for getting tools into
the local provider storage.
From there the tools are unpacked the same way as an
agent would normally do it, although we do cheat and go
directly to the file on disk so we don't have to pretend
to download it somewhere else.
I changed the error message for the not implemented errors
so I could see which thing was failing and needed to be
implemented to get the next step working.
https://code.launchpad.net/~thumper/juju-core/local-provider-machine-0/+merge/174921
Requires: https://code.launchpad.net/~thumper/juju-core/find-jujud/+merge/174918
(do not edit description out of merge proposal)
Patch Set 1 #
Total comments: 6
Total messages: 5
|
thumper
Please take a look.
|
12 years, 6 months ago (2013年07月16日 05:09:57 UTC) #1 | ||||||||||||||||||||||||||||||||||||||||||||||||||
Please take a look.
LGTM. I wonder if uploading tools really is required for the local provider, seems synctools in some form should be extendable to work with the local provider as well (download to well-known location or similar). https://codereview.appspot.com/11327043/diff/1/environs/local/environ.go File environs/local/environ.go (right): https://codereview.appspot.com/11327043/diff/1/environs/local/environ.go#newc... environs/local/environ.go:370: tools.Binary.Series = version.CurrentSeries() Rather than doing this, can't we take the user's machine series when finding tools, and retry if that finds nothing, using the configured series?
https://codereview.appspot.com/11327043/diff/1/environs/local/environ.go File environs/local/environ.go (right): https://codereview.appspot.com/11327043/diff/1/environs/local/environ.go#newc... environs/local/environ.go:370: tools.Binary.Series = version.CurrentSeries() On 2013年07月16日 10:06:57, gz wrote: > Rather than doing this, can't we take the user's machine series when finding > tools, and retry if that finds nothing, using the configured series? Unfortunately that is kinda hard-coded further down the path in FindBootstrapTools, and somewhat out of the scope of this change.
The encapsulation breaks and tools version issues make me sad but I appreciate the need to get this landed. So long as bugs and TODOs are raised to ensure stuff gets fixed. LGTM https://codereview.appspot.com/11327043/diff/1/environs/local/environ.go File environs/local/environ.go (right): https://codereview.appspot.com/11327043/diff/1/environs/local/environ.go#newc... environs/local/environ.go:363: // Again, brutally abuse our knowledge here. These breaks in encapsulation - can we raise a bug and add a TODO to fix them. Or is it expected that the sync-tools work can be used. Either way, a TODO is warranted as we don't want to leave this as is long term. https://codereview.appspot.com/11327043/diff/1/environs/local/environ.go#newc... environs/local/environ.go:370: tools.Binary.Series = version.CurrentSeries() On 2013年07月17日 01:45:47, thumper wrote: > On 2013年07月16日 10:06:57, gz wrote: > > Rather than doing this, can't we take the user's machine series when finding > > tools, and retry if that finds nothing, using the configured series? > > Unfortunately that is kinda hard-coded further down the path in > FindBootstrapTools, and somewhat out of the scope of this change. So if I am on Saucy, precise tools might still be used locally. Can we please raise a bug and add a TODO to ensure this gets fixed.
https://codereview.appspot.com/11327043/diff/1/environs/local/environ.go File environs/local/environ.go (right): https://codereview.appspot.com/11327043/diff/1/environs/local/environ.go#newc... environs/local/environ.go:363: // Again, brutally abuse our knowledge here. On 2013年07月17日 02:33:41, wallyworld wrote: > These breaks in encapsulation - can we raise a bug and add a TODO to fix them. > Or is it expected that the sync-tools work can be used. Either way, a TODO is > warranted as we don't want to leave this as is long term. Actually I'm not entirely convinced that we have to. We are in the special case of bringing up a machine agent. It is ok to use knowledge here. https://codereview.appspot.com/11327043/diff/1/environs/local/environ.go#newc... environs/local/environ.go:370: tools.Binary.Series = version.CurrentSeries() On 2013年07月17日 02:33:41, wallyworld wrote: > On 2013年07月17日 01:45:47, thumper wrote: > > On 2013年07月16日 10:06:57, gz wrote: > > > Rather than doing this, can't we take the user's machine series when finding > > > tools, and retry if that finds nothing, using the configured series? > > > > Unfortunately that is kinda hard-coded further down the path in > > FindBootstrapTools, and somewhat out of the scope of this change. > > So if I am on Saucy, precise tools might still be used locally. Can we please > raise a bug and add a TODO to ensure this gets fixed. No. What is used is the jujud on your machine, which is either built for you locally, or part of the package. Either way, they are fine. Also, these same tools are uploaded for the machines. Since Go makes stand alone executables, this too works.