juju-core

Merge lp:~thumper/juju-core/container-hardware-characteristics into lp:~go-bot/juju-core/trunk

  1. container-hardware-characteristics
  2. Merge into trunk
Proposed by Tim Penhey
Status: Merged
Approved by: Tim Penhey
Approved revision: no longer in the source branch.
Merged at revision: 2122
Proposed branch: lp:~thumper/juju-core/container-hardware-characteristics
Merge into: lp:~go-bot/juju-core/trunk
Prerequisite: lp:~thumper/juju-core/kvm-provisioner
Diff against target: 236 lines (+42/-28)
8 files modified
container/interface.go (+1/-1)
container/kvm/kvm.go (+10/-6)
container/kvm/live_test.go (+6/-2)
container/lxc/lxc.go (+14/-9)
container/testing/common.go (+3/-1)
provider/local/environ.go (+2/-3)
worker/provisioner/kvm-broker.go (+3/-3)
worker/provisioner/lxc-broker.go (+3/-3)
To merge this branch: bzr merge lp:~thumper/juju-core/container-hardware-characteristics
Reviewer Review Type Date Requested Status
Juju Engineering Pending
Review via email: mp+197644@code.launchpad.net

Commit message

StartContainer returns hardware characteristics.

Initial change to have the hardware characteristics
of containers returned. At this stage just the arch
is filled out.

https://codereview.appspot.com/36980043/

Description of the change

StartContainer returns hardware characteristics.

Initial change to have the hardware characteristics
of containers returned. At this stage just the arch
is filled out.

https://codereview.appspot.com/36980043/

To post a comment you must log in.
Revision history for this message
Tim Penhey (thumper) wrote :
Download full text (10.2 KiB)

Reviewers: mp+197644_code.launchpad.net,

Message:
Please take a look.

Description:
StartContainer returns hardware characteristics.

Initial change to have the hardware characteristics
of containers returned. At this stage just the arch
is filled out.

https://code.launchpad.net/~thumper/juju-core/container-hardware-characteristics/+merge/197644

Requires:
https://code.launchpad.net/~thumper/juju-core/kvm-provisioner/+merge/197629

(do not edit description out of merge proposal)

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

Affected files (+42, -28 lines):
   A [revision details]
   M container/interface.go
   M container/kvm/kvm.go
   M container/kvm/live_test.go
   M container/lxc/lxc.go
   M container/testing/common.go
   M provider/local/environ.go
   M worker/provisioner/kvm-broker.go
   M worker/provisioner/lxc-broker.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: container/interface.go
=== modified file 'container/interface.go'
--- container/interface.go 2013-11-20 01:07:26 +0000
+++ container/interface.go 2013-12-04 02:37:05 +0000
@@ -22,7 +22,7 @@
   StartContainer(
    machineConfig *cloudinit.MachineConfig,
    series string,
- network *NetworkConfig) (instance.Instance, error)
+ network *NetworkConfig) (instance.Instance,
*instance.HardwareCharacteristics, error)
   // StopContainer stops and destroyes the container identified by Instance.
   StopContainer(instance.Instance) error
   // ListContainers return a list of containers that have been started by

Index: container/kvm/kvm.go
=== modified file 'container/kvm/kvm.go'
--- container/kvm/kvm.go 2013-12-04 00:52:35 +0000
+++ container/kvm/kvm.go 2013-12-04 02:37:05 +0000
@@ -63,7 +63,7 @@
  func (manager *containerManager) StartContainer(
   machineConfig *cloudinit.MachineConfig,
   series string,
- network *container.NetworkConfig) (instance.Instance, error) {
+ network *container.NetworkConfig) (instance.Instance,
*instance.HardwareCharacteristics, error) {

   name := names.MachineTag(machineConfig.MachineId)
   if manager.name != "" {
@@ -77,20 +77,24 @@
   // Create the cloud-init.
   directory, err := container.NewDirectory(name)
   if err != nil {
- return nil, fmt.Errorf("failed to create container directory: %v", err)
+ return nil, nil, fmt.Errorf("failed to create container directory: %v",
err)
   }
   logger.Tracef("write cloud-init")
   userDataFilename, err := container.WriteUserData(machineConfig, directory)
   if err != nil {
- return nil, log.LoggedErrorf(logger, "failed to write user data: %v",
err)
+ return nil, nil, log.LoggedErrorf(logger, "failed to write user
data: %v", err)
   }
   // Create the container.
+ arch := version.Current.Arch
+ hardware := &instance.HardwareCharacteristics{
+ Arch: &arch,
+ }
   logger.Tracef("create the container")
- if err := kvmContainer.Start(series, version.Current.Arch,
userDataFilename, network); er...

Revision history for this message
Ian Booth (wallyworld) wrote :

LGTM but I'd like to see the tests more proscriptive. eg we know what
arch has to be so test that rather than no nil.

https://codereview.appspot.com/36980043/diff/1/container/kvm/live_test.go
File container/kvm/live_test.go (right):

https://codereview.appspot.com/36980043/diff/1/container/kvm/live_test.go#newcode101
container/kvm/live_test.go:101: c.Assert(hardware, gc.Not(gc.Equals),
&instance.HardwareCharacteristics{})
I do think we should be asserting an actual expected value, not just
"not nil"

https://codereview.appspot.com/36980043/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'container/interface.go'
--- container/interface.go 2013-11-20 01:07:26 +0000
+++ container/interface.go 2013-12-04 22:36:30 +0000
@@ -22,7 +22,7 @@
22 StartContainer(22 StartContainer(
23 machineConfig *cloudinit.MachineConfig,23 machineConfig *cloudinit.MachineConfig,
24 series string,24 series string,
25 network *NetworkConfig) (instance.Instance, error)25 network *NetworkConfig) (instance.Instance, *instance.HardwareCharacteristics, error)
26 // StopContainer stops and destroyes the container identified by Instance.26 // StopContainer stops and destroyes the container identified by Instance.
27 StopContainer(instance.Instance) error27 StopContainer(instance.Instance) error
28 // ListContainers return a list of containers that have been started by28 // ListContainers return a list of containers that have been started by
2929
=== modified file 'container/kvm/kvm.go'
--- container/kvm/kvm.go 2013-12-04 00:52:35 +0000
+++ container/kvm/kvm.go 2013-12-04 22:36:30 +0000
@@ -63,7 +63,7 @@
63func (manager *containerManager) StartContainer(63func (manager *containerManager) StartContainer(
64 machineConfig *cloudinit.MachineConfig,64 machineConfig *cloudinit.MachineConfig,
65 series string,65 series string,
66 network *container.NetworkConfig) (instance.Instance, error) {66 network *container.NetworkConfig) (instance.Instance, *instance.HardwareCharacteristics, error) {
6767
68 name := names.MachineTag(machineConfig.MachineId)68 name := names.MachineTag(machineConfig.MachineId)
69 if manager.name != "" {69 if manager.name != "" {
@@ -77,20 +77,24 @@
77 // Create the cloud-init.77 // Create the cloud-init.
78 directory, err := container.NewDirectory(name)78 directory, err := container.NewDirectory(name)
79 if err != nil {79 if err != nil {
80 return nil, fmt.Errorf("failed to create container directory: %v", err)80 return nil, nil, fmt.Errorf("failed to create container directory: %v", err)
81 }81 }
82 logger.Tracef("write cloud-init")82 logger.Tracef("write cloud-init")
83 userDataFilename, err := container.WriteUserData(machineConfig, directory)83 userDataFilename, err := container.WriteUserData(machineConfig, directory)
84 if err != nil {84 if err != nil {
85 return nil, log.LoggedErrorf(logger, "failed to write user data: %v", err)85 return nil, nil, log.LoggedErrorf(logger, "failed to write user data: %v", err)
86 }86 }
87 // Create the container.87 // Create the container.
88 arch := version.Current.Arch
89 hardware := &instance.HardwareCharacteristics{
90 Arch: &arch,
91 }
88 logger.Tracef("create the container")92 logger.Tracef("create the container")
89 if err := kvmContainer.Start(series, version.Current.Arch, userDataFilename, network); err != nil {93 if err := kvmContainer.Start(series, arch, userDataFilename, network); err != nil {
90 return nil, log.LoggedErrorf(logger, "kvm container creation failed: %v", err)94 return nil, nil, log.LoggedErrorf(logger, "kvm container creation failed: %v", err)
91 }95 }
92 logger.Tracef("kvm container created")96 logger.Tracef("kvm container created")
93 return &kvmInstance{kvmContainer, name}, nil97 return &kvmInstance{kvmContainer, name}, hardware, nil
94}98}
9599
96func (manager *containerManager) StopContainer(instance instance.Instance) error {100func (manager *containerManager) StopContainer(instance instance.Instance) error {
97101
=== modified file 'container/kvm/live_test.go'
--- container/kvm/live_test.go 2013-11-21 09:47:12 +0000
+++ container/kvm/live_test.go 2013-12-04 22:36:30 +0000
@@ -95,9 +95,13 @@
95 err := environs.FinishMachineConfig(machineConfig, environConfig, constraints.Value{})95 err := environs.FinishMachineConfig(machineConfig, environConfig, constraints.Value{})
96 c.Assert(err, gc.IsNil)96 c.Assert(err, gc.IsNil)
9797
98 instance, err := manager.StartContainer(machineConfig, "precise", network)98 inst, hardware, err := manager.StartContainer(machineConfig, "precise", network)
99 c.Assert(err, gc.IsNil)99 c.Assert(err, gc.IsNil)
100 return instance100 c.Assert(hardware, gc.NotNil)
101 arch := version.Current.Arch
102 expected := instance.HardwareCharacteristics{Arch: &arch}.String()
103 c.Assert(hardware.String(), gc.Equals, expected)
104 return inst
101}105}
102106
103func (s *LiveSuite) TestShutdownMachines(c *gc.C) {107func (s *LiveSuite) TestShutdownMachines(c *gc.C) {
104108
=== modified file 'container/lxc/lxc.go'
--- container/lxc/lxc.go 2013-12-01 23:54:35 +0000
+++ container/lxc/lxc.go 2013-12-04 22:36:30 +0000
@@ -17,6 +17,7 @@
17 "launchpad.net/juju-core/environs/cloudinit"17 "launchpad.net/juju-core/environs/cloudinit"
18 "launchpad.net/juju-core/instance"18 "launchpad.net/juju-core/instance"
19 "launchpad.net/juju-core/names"19 "launchpad.net/juju-core/names"
20 "launchpad.net/juju-core/version"
20)21)
2122
22var logger = loggo.GetLogger("juju.container.lxc")23var logger = loggo.GetLogger("juju.container.lxc")
@@ -61,7 +62,7 @@
61func (manager *containerManager) StartContainer(62func (manager *containerManager) StartContainer(
62 machineConfig *cloudinit.MachineConfig,63 machineConfig *cloudinit.MachineConfig,
63 series string,64 series string,
64 network *container.NetworkConfig) (instance.Instance, error) {65 network *container.NetworkConfig) (instance.Instance, *instance.HardwareCharacteristics, error) {
6566
66 name := names.MachineTag(machineConfig.MachineId)67 name := names.MachineTag(machineConfig.MachineId)
67 if manager.name != "" {68 if manager.name != "" {
@@ -75,19 +76,19 @@
75 // Create the cloud-init.76 // Create the cloud-init.
76 directory, err := container.NewDirectory(name)77 directory, err := container.NewDirectory(name)
77 if err != nil {78 if err != nil {
78 return nil, err79 return nil, nil, err
79 }80 }
80 logger.Tracef("write cloud-init")81 logger.Tracef("write cloud-init")
81 userDataFilename, err := container.WriteUserData(machineConfig, directory)82 userDataFilename, err := container.WriteUserData(machineConfig, directory)
82 if err != nil {83 if err != nil {
83 logger.Errorf("failed to write user data: %v", err)84 logger.Errorf("failed to write user data: %v", err)
84 return nil, err85 return nil, nil, err
85 }86 }
86 logger.Tracef("write the lxc.conf file")87 logger.Tracef("write the lxc.conf file")
87 configFile, err := writeLxcConfig(network, directory, manager.logdir)88 configFile, err := writeLxcConfig(network, directory, manager.logdir)
88 if err != nil {89 if err != nil {
89 logger.Errorf("failed to write config file: %v", err)90 logger.Errorf("failed to write config file: %v", err)
90 return nil, err91 return nil, nil, err
91 }92 }
92 templateParams := []string{93 templateParams := []string{
93 "--debug", // Debug errors in the cloud image94 "--debug", // Debug errors in the cloud image
@@ -99,19 +100,19 @@
99 logger.Tracef("create the container")100 logger.Tracef("create the container")
100 if err := lxcContainer.Create(configFile, defaultTemplate, templateParams...); err != nil {101 if err := lxcContainer.Create(configFile, defaultTemplate, templateParams...); err != nil {
101 logger.Errorf("lxc container creation failed: %v", err)102 logger.Errorf("lxc container creation failed: %v", err)
102 return nil, err103 return nil, nil, err
103 }104 }
104 // Make sure that the mount dir has been created.105 // Make sure that the mount dir has been created.
105 logger.Tracef("make the mount dir for the shard logs")106 logger.Tracef("make the mount dir for the shard logs")
106 if err := os.MkdirAll(internalLogDir(name), 0755); err != nil {107 if err := os.MkdirAll(internalLogDir(name), 0755); err != nil {
107 logger.Errorf("failed to create internal /var/log/juju mount dir: %v", err)108 logger.Errorf("failed to create internal /var/log/juju mount dir: %v", err)
108 return nil, err109 return nil, nil, err
109 }110 }
110 logger.Tracef("lxc container created")111 logger.Tracef("lxc container created")
111 // Now symlink the config file into the restart directory.112 // Now symlink the config file into the restart directory.
112 containerConfigFile := filepath.Join(LxcContainerDir, name, "config")113 containerConfigFile := filepath.Join(LxcContainerDir, name, "config")
113 if err := os.Symlink(containerConfigFile, restartSymlink(name)); err != nil {114 if err := os.Symlink(containerConfigFile, restartSymlink(name)); err != nil {
114 return nil, err115 return nil, nil, err
115 }116 }
116 logger.Tracef("auto-restart link created")117 logger.Tracef("auto-restart link created")
117118
@@ -126,10 +127,14 @@
126 // setting it ourselves.127 // setting it ourselves.
127 if err = lxcContainer.Start("", consoleFile); err != nil {128 if err = lxcContainer.Start("", consoleFile); err != nil {
128 logger.Errorf("container failed to start: %v", err)129 logger.Errorf("container failed to start: %v", err)
129 return nil, err130 return nil, nil, err
131 }
132 arch := version.Current.Arch
133 hardware := &instance.HardwareCharacteristics{
134 Arch: &arch,
130 }135 }
131 logger.Tracef("container started")136 logger.Tracef("container started")
132 return &lxcInstance{lxcContainer, name}, nil137 return &lxcInstance{lxcContainer, name}, hardware, nil
133}138}
134139
135func (manager *containerManager) StopContainer(instance instance.Instance) error {140func (manager *containerManager) StopContainer(instance instance.Instance) error {
136141
=== modified file 'container/testing/common.go'
--- container/testing/common.go 2013-12-04 04:14:44 +0000
+++ container/testing/common.go 2013-12-04 22:36:30 +0000
@@ -29,8 +29,10 @@
2929
30 series := "series"30 series := "series"
31 network := container.BridgeNetworkConfig("nic42")31 network := container.BridgeNetworkConfig("nic42")
32 inst, err := manager.StartContainer(machineConfig, series, network)32 inst, hardware, err := manager.StartContainer(machineConfig, series, network)
33 c.Assert(err, gc.IsNil)33 c.Assert(err, gc.IsNil)
34 c.Assert(hardware, gc.NotNil)
35 c.Assert(hardware.String(), gc.Not(gc.Equals), "")
34 return inst36 return inst
35}37}
3638
3739
=== modified file 'provider/local/environ.go'
--- provider/local/environ.go 2013-12-04 00:41:41 +0000
+++ provider/local/environ.go 2013-12-04 22:36:30 +0000
@@ -295,12 +295,11 @@
295 return nil, nil, err295 return nil, nil, err
296 }296 }
297 machineConfig.AgentEnvironment[agent.Namespace] = env.config.namespace()297 machineConfig.AgentEnvironment[agent.Namespace] = env.config.namespace()
298 inst, err := env.containerManager.StartContainer(machineConfig, series, network)298 inst, hardware, err := env.containerManager.StartContainer(machineConfig, series, network)
299 if err != nil {299 if err != nil {
300 return nil, nil, err300 return nil, nil, err
301 }301 }
302 // TODO(thumper): return some hardware characteristics.302 return inst, hardware, nil
303 return inst, nil, nil
304}303}
305304
306// StartInstance is specified in the InstanceBroker interface.305// StartInstance is specified in the InstanceBroker interface.
307306
=== modified file 'worker/provisioner/kvm-broker.go'
--- worker/provisioner/kvm-broker.go 2013-12-04 04:01:16 +0000
+++ worker/provisioner/kvm-broker.go 2013-12-04 22:36:30 +0000
@@ -90,13 +90,13 @@
90 return nil, nil, err90 return nil, nil, err
91 }91 }
9292
93 inst, err := broker.manager.StartContainer(machineConfig, series, network)93 inst, hardware, err := broker.manager.StartContainer(machineConfig, series, network)
94 if err != nil {94 if err != nil {
95 kvmLogger.Errorf("failed to start container: %v", err)95 kvmLogger.Errorf("failed to start container: %v", err)
96 return nil, nil, err96 return nil, nil, err
97 }97 }
98 kvmLogger.Infof("started kvm container for machineId: %s, %s", machineId, inst.Id())98 kvmLogger.Infof("started kvm container for machineId: %s, %s, %s", machineId, inst.Id(), hardware.String())
99 return inst, nil, nil99 return inst, hardware, nil
100}100}
101101
102// StopInstances shuts down the given instances.102// StopInstances shuts down the given instances.
103103
=== modified file 'worker/provisioner/lxc-broker.go'
--- worker/provisioner/lxc-broker.go 2013-12-04 04:01:16 +0000
+++ worker/provisioner/lxc-broker.go 2013-12-04 22:36:30 +0000
@@ -80,13 +80,13 @@
80 return nil, nil, err80 return nil, nil, err
81 }81 }
8282
83 inst, err := broker.manager.StartContainer(machineConfig, series, network)83 inst, hardware, err := broker.manager.StartContainer(machineConfig, series, network)
84 if err != nil {84 if err != nil {
85 lxcLogger.Errorf("failed to start container: %v", err)85 lxcLogger.Errorf("failed to start container: %v", err)
86 return nil, nil, err86 return nil, nil, err
87 }87 }
88 lxcLogger.Infof("started lxc container for machineId: %s, %s", machineId, inst.Id())88 lxcLogger.Infof("started lxc container for machineId: %s, %s, %s", machineId, inst.Id(), hardware.String())
89 return inst, nil, nil89 return inst, hardware, nil
90}90}
9191
92// StopInstances shuts down the given instances.92// StopInstances shuts down the given instances.

Subscribers

People subscribed via source and target branches

to status/vote changes: