Reviewers: mp+218773_code.launchpad.net,

Message:
Please take a look.

Description:
Introduce lxc-use-clone, ContainerManagerConfig

A new lxc-use-clone config attribute and a new
ContainerManagerConfig provisioner API are introduced.
If lxc-use-clone is set to true, then the LXC
provisioner will be configured to use cloning
regardless of provider type. This setting is currently
immutable; additional work will be required to support
reacting to changes and reconfiguring the provisioner.

Fixes lp:1315216

https://code.launchpad.net/~axwalk/juju-core/lp1315216-lxc-use-clone/+merge/218773

(do not edit description out of merge proposal)


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

Affected files (+85, -13 lines):
   A [revision details]
   M environs/config/config.go
   M environs/config/config_test.go
   M state/api/params/params.go
   M state/api/provisioner/provisioner.go
   M state/apiserver/provisioner/provisioner.go
   M worker/provisioner/container_initialisation.go
   M worker/provisioner/kvm-broker.go
   M worker/provisioner/kvm-broker_test.go
   M worker/provisioner/lxc-broker.go
   M worker/provisioner/lxc-broker_test.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: tarmac-20140508054219-e6ylw5luw58d6zhx
+New revision: <email address hidden>

Index: environs/config/config.go
=== modified file 'environs/config/config.go'
--- environs/config/config.go	2014-04-11 17:51:58 +0000
+++ environs/config/config.go	2014-05-08 09:56:27 +0000
@@ -698,6 +698,13 @@
  	return c.defined["test-mode"].(bool)
  }

+// LXCUseClone reports whether the LXC provisioner should create a
+// template and use cloning to speed up container provisioning.
+func (c *Config) LXCUseClone() bool {
+	v, _ := c.defined["lxc-use-clone"].(bool)
+	return v
+}
+
  // UnknownAttrs returns a copy of the raw configuration attributes
  // that are supposedly specific to the environment type. They could
  // also be wrong attributes, though. Only the specific environment
@@ -774,6 +781,7 @@
  	"bootstrap-addresses-delay": schema.ForceInt(),
  	"test-mode":                 schema.Bool(),
  	"proxy-ssh":                 schema.Bool(),
+	"lxc-use-clone":             schema.Bool(),

  	// Deprecated fields, retain for backwards compatibility.
  	"tools-url": schema.String(),
@@ -831,9 +839,10 @@
  	// Authentication string sent with requests to the charm store
  	"charm-store-auth": "",
  	// Previously image-stream could be set to an empty value
-	"image-stream": "",
-	"test-mode":    false,
-	"proxy-ssh":    false,
+	"image-stream":  "",
+	"test-mode":     false,
+	"proxy-ssh":     false,
+	"lxc-use-clone": false,
  }

  func allowEmpty(attr string) bool {
@@ -894,6 +903,7 @@
  	"bootstrap-timeout",
  	"bootstrap-retry-delay",
  	"bootstrap-addresses-delay",
+	"lxc-use-clone",
  }

  var (


Index: environs/config/config_test.go
=== modified file 'environs/config/config_test.go'
--- environs/config/config_test.go	2014-04-11 18:13:56 +0000
+++ environs/config/config_test.go	2014-05-08 09:56:27 +0000
@@ -1028,6 +1028,7 @@
  	attrs["tools-url"] = ""
  	attrs["image-stream"] = ""
  	attrs["proxy-ssh"] = false
+	attrs["lxc-use-clone"] = false

  	// Default firewall mode is instance
  	attrs["firewall-mode"] = string(config.FwInstance)


Index: worker/provisioner/container_initialisation.go
=== modified file 'worker/provisioner/container_initialisation.go'
--- worker/provisioner/container_initialisation.go	2014-03-13 17:21:05 +0000
+++ worker/provisioner/container_initialisation.go	2014-05-08 09:56:27 +0000
@@ -14,6 +14,7 @@
  	"launchpad.net/juju-core/environs"
  	"launchpad.net/juju-core/instance"
  	"launchpad.net/juju-core/state"
+	"launchpad.net/juju-core/state/api/params"
  	apiprovisioner "launchpad.net/juju-core/state/api/provisioner"
  	"launchpad.net/juju-core/state/api/watcher"
  	"launchpad.net/juju-core/worker"
@@ -71,7 +72,7 @@
  }

  // Handle is called whenever containers change on the machine being  
watched.
-// All machines start out with so containers so the first time Handle is  
called,
+// Machines start out with no containers so the first time Handle is  
called,
  // it will be because a container has been added.
  func (cs *ContainerSetup) Handle(containerIds []string) (resultError  
error) {
  	// Consume the initial watcher event.
@@ -142,6 +143,21 @@
  	}
  	var initialiser container.Initialiser
  	var broker environs.InstanceBroker
+
+	// Ask the provisioner for the container manager configuration.
+	managerConfigResult, err := cs.provisioner.ContainerManagerConfig(
+		params.ContainerManagerConfigParams{Type: containerType},
+	)
+	if params.IsCodeNotImplemented(err) {
+		// We currently don't support upgrading;
+		// revert to the old configuration.
+		managerConfigResult.ManagerConfig =  
container.ManagerConfig{container.ConfigName: "juju"}
+	}
+	if err != nil {
+		return nil, nil, err
+	}
+	managerConfig :=  
container.ManagerConfig(managerConfigResult.ManagerConfig)
+
  	switch containerType {
  	case instance.LXC:
  		series, err := cs.machine.Series()
@@ -150,13 +166,13 @@
  		}

  		initialiser = lxc.NewContainerInitialiser(series)
-		broker, err = NewLxcBroker(cs.provisioner, tools, cs.config)
+		broker, err = NewLxcBroker(cs.provisioner, tools, cs.config,  
managerConfig)
  		if err != nil {
  			return nil, nil, err
  		}
  	case instance.KVM:
  		initialiser = kvm.NewContainerInitialiser()
-		broker, err = NewKvmBroker(cs.provisioner, tools, cs.config)
+		broker, err = NewKvmBroker(cs.provisioner, tools, cs.config,  
managerConfig)
  		if err != nil {
  			logger.Errorf("failed to create new kvm broker")
  			return nil, nil, err


Index: state/api/params/params.go
=== modified file 'state/api/params/params.go'
--- state/api/params/params.go	2014-04-23 16:28:17 +0000
+++ state/api/params/params.go	2014-05-08 09:56:27 +0000
@@ -597,6 +597,18 @@
  	}
  }

+// ContainerManagerConfigParams contains the parameters for the
+// ContainerManagerConfig provisioner API call.
+type ContainerManagerConfigParams struct {
+	Type instance.ContainerType
+}
+
+// ContainerManagerConfig contains information from the environment config
+// that is needed for configuring the container manager.
+type ContainerManagerConfig struct {
+	ManagerConfig map[string]string
+}
+
  // ContainerConfig contains information from the environment config that is
  // needed for container cloud-init.
  type ContainerConfig struct {


Index: worker/provisioner/kvm-broker.go
=== modified file 'worker/provisioner/kvm-broker.go'
--- worker/provisioner/kvm-broker.go	2014-04-18 16:37:28 +0000
+++ worker/provisioner/kvm-broker.go	2014-05-08 09:56:27 +0000
@@ -26,8 +26,9 @@
  	api APICalls,
  	tools *tools.Tools,
  	agentConfig agent.Config,
+	managerConfig container.ManagerConfig,
  ) (environs.InstanceBroker, error) {
-	manager, err :=  
kvm.NewContainerManager(container.ManagerConfig{container.ConfigName: "juju"})
+	manager, err := kvm.NewContainerManager(managerConfig)
  	if err != nil {
  		return nil, err
  	}


Index: worker/provisioner/kvm-broker_test.go
=== modified file 'worker/provisioner/kvm-broker_test.go'
--- worker/provisioner/kvm-broker_test.go	2014-04-17 12:53:23 +0000
+++ worker/provisioner/kvm-broker_test.go	2014-05-08 09:56:27 +0000
@@ -13,6 +13,7 @@

  	"launchpad.net/juju-core/agent"
  	"launchpad.net/juju-core/constraints"
+	"launchpad.net/juju-core/container"
  	"launchpad.net/juju-core/container/kvm/mock"
  	kvmtesting "launchpad.net/juju-core/container/kvm/testing"
  	"launchpad.net/juju-core/environs"
@@ -75,7 +76,8 @@
  			CACert:            coretesting.CACert,
  		})
  	c.Assert(err, gc.IsNil)
-	s.broker, err = provisioner.NewKvmBroker(&fakeAPI{}, tools, s.agentConfig)
+	managerConfig := container.ManagerConfig{container.ConfigName: "juju"}
+	s.broker, err = provisioner.NewKvmBroker(&fakeAPI{}, tools,  
s.agentConfig, managerConfig)
  	c.Assert(err, gc.IsNil)
  }

@@ -219,7 +221,8 @@
  	agentConfig := s.AgentConfigForTag(c, machineTag)
  	tools, err := s.provisioner.Tools(agentConfig.Tag())
  	c.Assert(err, gc.IsNil)
-	broker, err := provisioner.NewKvmBroker(s.provisioner, tools, agentConfig)
+	managerConfig := container.ManagerConfig{container.ConfigName: "juju"}
+	broker, err := provisioner.NewKvmBroker(s.provisioner, tools,  
agentConfig, managerConfig)
  	c.Assert(err, gc.IsNil)
  	return provisioner.NewContainerProvisioner(instance.KVM, s.provisioner,  
agentConfig, broker)
  }


Index: state/api/provisioner/provisioner.go
=== modified file 'state/api/provisioner/provisioner.go'
--- state/api/provisioner/provisioner.go	2014-04-12 05:53:58 +0000
+++ state/api/provisioner/provisioner.go	2014-05-08 09:56:27 +0000
@@ -115,6 +115,11 @@
  	return result.Tools, nil
  }

+func (st *State) ContainerManagerConfig(args  
params.ContainerManagerConfigParams) (result params.ContainerManagerConfig,  
err error) {
+	err = st.call("ContainerManagerConfig", args, &result)
+	return result, err
+}
+
  // ContainerConfig returns information from the environment config that are
  // needed for container cloud-init.
  func (st *State) ContainerConfig() (result params.ContainerConfig, err  
error) {


Index: state/apiserver/provisioner/provisioner.go
=== modified file 'state/apiserver/provisioner/provisioner.go'
--- state/apiserver/provisioner/provisioner.go	2014-04-22 09:23:39 +0000
+++ state/apiserver/provisioner/provisioner.go	2014-05-08 09:56:27 +0000
@@ -7,6 +7,7 @@
  	"fmt"

  	"launchpad.net/juju-core/constraints"
+	"launchpad.net/juju-core/container"
  	"launchpad.net/juju-core/instance"
  	"launchpad.net/juju-core/names"
  	"launchpad.net/juju-core/state"
@@ -190,6 +191,24 @@
  	return result, nil
  }

+// ContainerManagerConfig returns information from the environment config  
that are
+// needed for configuring the container manager.
+func (p *ProvisionerAPI) ContainerManagerConfig(args  
params.ContainerManagerConfigParams) (params.ContainerManagerConfig, error)  
{
+	var result params.ContainerManagerConfig
+	config, err := p.st.EnvironConfig()
+	if err != nil {
+		return result, err
+	}
+	cfg := make(map[string]string)
+	cfg[container.ConfigName] = "juju"
+	switch args.Type {
+	case instance.LXC:
+		cfg["use-clone"] = fmt.Sprint(config.LXCUseClone())
+	}
+	result.ManagerConfig = cfg
+	return result, nil
+}
+
  // ContainerConfig returns information from the environment config that are
  // needed for container cloud-init.
  func (p *ProvisionerAPI) ContainerConfig() (params.ContainerConfig, error)  
{


Index: worker/provisioner/lxc-broker.go
=== modified file 'worker/provisioner/lxc-broker.go'
--- worker/provisioner/lxc-broker.go	2014-04-18 16:37:28 +0000
+++ worker/provisioner/lxc-broker.go	2014-05-08 09:56:27 +0000
@@ -27,8 +27,8 @@
  	ContainerConfig() (params.ContainerConfig, error)
  }

-func NewLxcBroker(api APICalls, tools *tools.Tools, agentConfig  
agent.Config) (environs.InstanceBroker, error) {
-	manager, err :=  
lxc.NewContainerManager(container.ManagerConfig{container.ConfigName: "juju"})
+func NewLxcBroker(api APICalls, tools *tools.Tools, agentConfig  
agent.Config, managerConfig container.ManagerConfig)  
(environs.InstanceBroker, error) {
+	manager, err := lxc.NewContainerManager(managerConfig)
  	if err != nil {
  		return nil, err
  	}


Index: worker/provisioner/lxc-broker_test.go
=== modified file 'worker/provisioner/lxc-broker_test.go'
--- worker/provisioner/lxc-broker_test.go	2014-04-17 12:53:23 +0000
+++ worker/provisioner/lxc-broker_test.go	2014-05-08 09:56:27 +0000
@@ -14,6 +14,7 @@

  	"launchpad.net/juju-core/agent"
  	"launchpad.net/juju-core/constraints"
+	"launchpad.net/juju-core/container"
  	"launchpad.net/juju-core/container/lxc/mock"
  	lxctesting "launchpad.net/juju-core/container/lxc/testing"
  	"launchpad.net/juju-core/environs"
@@ -77,7 +78,8 @@
  			CACert:            coretesting.CACert,
  		})
  	c.Assert(err, gc.IsNil)
-	s.broker, err = provisioner.NewLxcBroker(&fakeAPI{}, tools, s.agentConfig)
+	managerConfig := container.ManagerConfig{container.ConfigName: "juju"}
+	s.broker, err = provisioner.NewLxcBroker(&fakeAPI{}, tools,  
s.agentConfig, managerConfig)
  	c.Assert(err, gc.IsNil)
  }

@@ -253,7 +255,8 @@
  	agentConfig := s.AgentConfigForTag(c, parentMachineTag)
  	tools, err := s.provisioner.Tools(agentConfig.Tag())
  	c.Assert(err, gc.IsNil)
-	broker, err := provisioner.NewLxcBroker(s.provisioner, tools, agentConfig)
+	managerConfig := container.ManagerConfig{container.ConfigName: "juju"}
+	broker, err := provisioner.NewLxcBroker(s.provisioner, tools,  
agentConfig, managerConfig)
  	c.Assert(err, gc.IsNil)
  	return provisioner.NewContainerProvisioner(instance.LXC, s.provisioner,  
agentConfig, broker)
  }