Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(1454)

Issue 11327043: Machine agent for local provider bootstrap

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years, 8 months ago by thumper
Modified:
10 years, 8 months ago
Reviewers:
gz, mp+174921, wallyworld
Visibility:
Public.

Description

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
Unified diffs Side-by-side diffs Delta from patch set Stats (+76 lines, -17 lines) Patch
A [revision details] View 1 chunk +2 lines, -0 lines 0 comments Download
M cmd/juju/bootstrap.go View 1 chunk +4 lines, -1 line 0 comments Download
M environs/local/config.go View 2 chunks +5 lines, -0 lines 0 comments Download
M environs/local/environ.go View 8 chunks +60 lines, -11 lines 6 comments Download
M environs/local/environprovider.go View 1 chunk +2 lines, -2 lines 0 comments Download
M environs/local/instance.go View 1 chunk +3 lines, -3 lines 0 comments Download

Messages

Total messages: 5
thumper
Please take a look.
10 years, 8 months ago (2013-07-16 05:09:57 UTC) #1
gz
LGTM. I wonder if uploading tools really is required for the local provider, seems synctools ...
10 years, 8 months ago (2013-07-16 10:06:57 UTC) #2
thumper
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#newcode370 environs/local/environ.go:370: tools.Binary.Series = version.CurrentSeries() On 2013/07/16 10:06:57, gz wrote: > ...
10 years, 8 months ago (2013-07-17 01:45:47 UTC) #3
wallyworld
The encapsulation breaks and tools version issues make me sad but I appreciate the need ...
10 years, 8 months ago (2013-07-17 02:33:41 UTC) #4
thumper
10 years, 8 months ago (2013-07-17 03:13:49 UTC) #5
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.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b