Merge lp:~thumper/juju-core/local-provider-machine-0 into lp:~go-bot/juju-core/trunk

[Tim Penhey]

Machine agent for local provider bootstrap

If you are using a local provider, always 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.

https://codereview.appspot.com/11327043/

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://codereview.appspot.com/11327043/

[Tim Penhey (thumper)]

Reviewers: mp+174921_code.launchpad.net,

Message:
Please take a look.

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)

Please review this at https://codereview.appspot.com/11327043/

Affected files:
   A [revision details]
   M cmd/juju/bootstrap.go
   M environs/local/config.go
   M environs/local/environ.go
   M environs/local/environprovider.go
   M environs/local/instance.go

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: <email address hidden>
+New revision: <email address hidden>

Index: cmd/juju/bootstrap.go
=== modified file 'cmd/juju/bootstrap.go'
--- cmd/juju/bootstrap.go 2013-05-02 15:55:42 +0000
+++ cmd/juju/bootstrap.go 2013-07-14 21:51:24 +0000
@@ -68,7 +68,10 @@
   if err != nil {
    return err
   }
-
+ // If we are using a local provider, always upload tools.
+ if environ.Config().Type() == "local" {
+ c.UploadTools = true
+ }
   if c.UploadTools {
    // Force version.Current, for consistency with subsequent upgrade-juju
    // (see UpgradeJujuCommand).

Index: environs/local/config.go
=== modified file 'environs/local/config.go'
--- environs/local/config.go 2013-07-09 02:28:22 +0000
+++ environs/local/config.go 2013-07-14 23:07:24 +0000
@@ -73,6 +73,10 @@
   return filepath.Join(c.rootDir(), "db")
  }

+func (c *environConfig) logDir() string {
+ return filepath.Join(c.rootDir(), "log")
+}
+
  func (c *environConfig) configFile(filename string) string {
   return filepath.Join(c.rootDir(), filename)
  }
@@ -107,6 +111,7 @@
    c.sharedStorageDir(),
    c.storageDir(),
    c.mongoDir(),
+ c.logDir(),
   } {
    logger.Tracef("creating directory %s", dirname)
    if err := os.MkdirAll(dirname, 0755); err != nil {

Index: environs/local/environ.go
=== modified file 'environs/local/environ.go'
--- environs/local/environ.go 2013-07-12 02:08:36 +0000
+++ environs/local/environ.go 2013-07-14 23:07:24 +0000
@@ -7,6 +7,7 @@
   "fmt"
   "io/ioutil"
   "net"
+ "net/url"
   "os"
   "path/filepath"
   "sync"
@@ -22,6 +23,7 @@
   "launchpad.net/juju-core/state/api"
   "launchpad.net/juju-core/upstart"
   "launchpad.net/juju-core/utils"
+ "launchpad.net/juju-core/version"
  )

  // lxcBridgeName is the name of the network interface that the local
provider
@@ -64,6 +66,10 @@
   return "juju-db-" + env.config.names...

[Martin Packman (gz)]

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#newcode370
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/

[Tim Penhey (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:
> 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.

https://codereview.appspot.com/11327043/

[Ian Booth (wallyworld)]

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#newcode363
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#newcode370
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/

[Tim Penhey (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#newcode363
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#newcode370
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.

https://codereview.appspot.com/11327043/

=== modified file 'cmd/juju/bootstrap.go'
--- cmd/juju/bootstrap.go	2013-05-02 15:55:42 +0000
+++ cmd/juju/bootstrap.go	2013-07-17 03:17:27 +0000
@@ -68,7 +68,10 @@
 	if err != nil {
 		return err
 	}
-
+	// If we are using a local provider, always upload tools.
+	if environ.Config().Type() == "local" {
+		c.UploadTools = true
+	}
 	if c.UploadTools {
 		// Force version.Current, for consistency with subsequent upgrade-juju
 		// (see UpgradeJujuCommand).

=== modified file 'environs/local/config.go'
--- environs/local/config.go	2013-07-16 21:56:22 +0000
+++ environs/local/config.go	2013-07-17 03:17:27 +0000
@@ -73,6 +73,10 @@
 	return filepath.Join(c.rootDir(), "db")
 }
 
+func (c *environConfig) logDir() string {
+	return filepath.Join(c.rootDir(), "log")
+}
+
 func (c *environConfig) configFile(filename string) string {
 	return filepath.Join(c.rootDir(), filename)
 }
@@ -104,6 +108,7 @@
 		c.sharedStorageDir(),
 		c.storageDir(),
 		c.mongoDir(),
+		c.logDir(),
 	} {
 		logger.Tracef("creating directory %s", dirname)
 		if err := os.MkdirAll(dirname, 0755); err != nil {

=== modified file 'environs/local/environ.go'
--- environs/local/environ.go	2013-07-17 00:26:31 +0000
+++ environs/local/environ.go	2013-07-17 03:17:27 +0000
@@ -7,6 +7,7 @@
 	"fmt"
 	"io/ioutil"
 	"net"
+	"net/url"
 	"os"
 	"path/filepath"
 	"sync"
@@ -22,6 +23,7 @@
 	"launchpad.net/juju-core/state/api"
 	"launchpad.net/juju-core/upstart"
 	"launchpad.net/juju-core/utils"
+	"launchpad.net/juju-core/version"
 )
 
 // lxcBridgeName is the name of the network interface that the local provider
@@ -64,6 +66,10 @@
 	return "juju-db-" + env.config.namespace()
 }
 
+func (env *localEnviron) machineAgentServiceName() string {
+	return "juju-agent-" + env.config.namespace()
+}
+
 // ensureCertOwner checks to make sure that the cert files created
 // by the bootstrap command are owned by the user and not root.
 func (env *localEnviron) ensureCertOwner() error {
@@ -143,11 +149,7 @@
 	}
 	defer stateConnection.Close()
 
-	// TODO(thumper): upload tools into the storage
-
-	// TODO(thumper): start the machine agent for machine-0
-
-	return nil
+	return env.setupLocalMachineAgent(cons)
 }
 
 // StateInfo is specified in the Environ interface.
@@ -219,12 +221,12 @@
 	info *state.Info,
 	apiInfo *api.Info,
 ) (instance.Instance, *instance.HardwareCharacteristics, error) {
-	return nil, nil, fmt.Errorf("not implemented")
+	return nil, nil, fmt.Errorf("start instance not implemented")
 }
 
 // StopInstances is specified in the Environ interface.
 func (env *localEnviron) StopInstances([]instance.Instance) error {
-	return fmt.Errorf("not implemented")
+	return fmt.Errorf("stop instance not implemented")
 }
 
 // Instances is specified in the Environ interface.
@@ -285,17 +287,17 @@
 
 // OpenPorts is specified in the Environ interface.
 func (env *localEnviron) OpenPorts(ports []instance.Port) error {
-	return fmt.Errorf("not implemented")
+	return fmt.Errorf("open ports not implemented")
 }
 
 // ClosePorts is specified in the Environ interface.
 func (env *localEnviron) ClosePorts(ports []instance.Port) error {
-	return fmt.Errorf("not implemented")
+	return fmt.Errorf("close ports not implemented")
 }
 
 // Ports is specified in the Environ interface.
 func (env *localEnviron) Ports() ([]instance.Port, error) {
-	return nil, fmt.Errorf("not implemented")
+	return nil, nil
 }
 
 // Provider is specified in the Environ interface.
@@ -340,6 +342,53 @@
 	return cert, key, nil
 }
 
+func (env *localEnviron) setupLocalMachineAgent(cons constraints.Value) error {
+	dataDir := env.config.rootDir()
+	toolList, err := environs.FindBootstrapTools(env, cons)
+	if err != nil {
+		return err
+	}
+	// ensure we have at least one valid tools
+	if len(toolList) == 0 {
+		return fmt.Errorf("No bootstrap tools found")
+	}
+	// unpack the first tools into the agent dir.
+	tools := toolList[0]
+	logger.Infof("tools: %#v", tools)
+	// brutally abuse our knowledge of storage to directly open the file
+	toolsUrl, err := url.Parse(tools.URL)
+	toolsLocation := filepath.Join(env.config.storageDir(), toolsUrl.Path)
+	logger.Infof("tools location: %v", toolsLocation)
+	toolsFile, err := os.Open(toolsLocation)
+	defer toolsFile.Close()
+	// Again, brutally abuse our knowledge here.
+
+	// The tools that FindBootstrapTools has returned us are based on the
+	// default series in the config.  However we are running potentially on a
+	// different series.  When the machine agent is started, it will be
+	// looking based on the current series, so we need to override the series
+	// returned in the tools to be the current series.
+	tools.Binary.Series = version.CurrentSeries()
+	err = agent.UnpackTools(dataDir, tools, toolsFile)
+
+	machineId := "0" // Always machine 0
+	tag := state.MachineTag(machineId)
+	toolsDir := agent.SharedToolsDir(dataDir, tools.Binary)
+	logDir := env.config.logDir()
+	logConfig := "--debug" // TODO(thumper): specify loggo config
+	agent := upstart.MachineAgentUpstartService(
+		env.machineAgentServiceName(),
+		toolsDir, dataDir, logDir, tag, machineId, logConfig)
+
+	agent.InitDir = upstartScriptLocation
+	logger.Infof("installing service %s to %s", env.machineAgentServiceName(), agent.InitDir)
+	if err := agent.Install(); err != nil {
+		logger.Errorf("could not install machine agent service: %v", err)
+		return err
+	}
+	return nil
+}
+
 func (env *localEnviron) findBridgeAddress() (string, error) {
 	bridge, err := net.InterfaceByName(lxcBridgeName)
 	if err != nil {
@@ -370,7 +419,7 @@
 		StateServerCert: cert,
 		StateServerKey:  key,
 		StatePort:       env.config.StatePort(),
-		APIPort:         env.config.StatePort(),
+		APIPort:         env.config.APIPort(),
 		MachineNonce:    state.BootstrapNonce,
 	}
 	if err := conf.Write(); err != nil {

=== modified file 'environs/local/environprovider.go'
--- environs/local/environprovider.go	2013-07-16 22:32:42 +0000
+++ environs/local/environprovider.go	2013-07-17 03:17:27 +0000
@@ -107,12 +107,12 @@
 
 // PublicAddress implements environs.EnvironProvider.PublicAddress.
 func (environProvider) PublicAddress() (string, error) {
-	return "", fmt.Errorf("not implemented")
+	return "", fmt.Errorf("public address not implemented")
 }
 
 // PrivateAddress implements environs.EnvironProvider.PrivateAddress.
 func (environProvider) PrivateAddress() (string, error) {
-	return "", fmt.Errorf("not implemented")
+	return "", fmt.Errorf("private address not implemented")
 }
 
 // InstanceId implements environs.EnvironProvider.InstanceId.

=== modified file 'environs/local/instance.go'
--- environs/local/instance.go	2013-07-17 00:26:31 +0000
+++ environs/local/instance.go	2013-07-17 03:17:27 +0000
@@ -55,7 +55,7 @@
 
 // Ports implements instance.Instance.Ports.
 func (inst *localInstance) Ports(machineId string) ([]instance.Port, error) {
-	return nil, fmt.Errorf("not implemented")
+	return nil, nil
 }
 
 // Add a string representation of the id.