Merge lp:~thumper/juju-core/kvm-manager-start-stop into lp:~go-bot/juju-core/trunk

Proposed by Tim Penhey
Status: Merged
Approved by: Tim Penhey
Approved revision: no longer in the source branch.
Merged at revision: 2097
Proposed branch: lp:~thumper/juju-core/kvm-manager-start-stop
Merge into: lp:~go-bot/juju-core/trunk
Prerequisite: lp:~thumper/juju-core/local-provider-syslog
Diff against target: 681 lines (+434/-30)
11 files modified
container/kvm/container.go (+56/-5)
container/kvm/containerfactory.go (+22/-6)
container/kvm/interface.go (+11/-2)
container/kvm/kvm.go (+34/-3)
container/kvm/kvm_test.go (+10/-4)
container/kvm/libvirt.go (+128/-0)
container/kvm/live_test.go (+139/-0)
container/kvm/mock/mock-kvm.go (+7/-1)
container/kvm/mock/mock-kvm_test.go (+18/-8)
container/userdata.go (+1/-1)
log/log.go (+8/-0)
To merge this branch: bzr merge lp:~thumper/juju-core/kvm-manager-start-stop
Reviewer Review Type Date Requested Status
Juju Engineering Pending
Review via email: mp+196072@code.launchpad.net

Commit message

Implement kvm start, stop and list.

The implementation here uses a mix of:
 * uvt-simplestream-libvirt
 * uvt-kvm
 * virsh

These are provided by the uvtool package and the
dependencies. The implementation of the low level
calls out to the shell are in container/kvm/libvirt.go

These low level calls are then used by the container
factory and container instances in container/kvm.

These are used by the container manager instance.

There are some live tests for the kvm calls.
  cd container/kvm
  sudo -E go test
This will take quite some time the first time it is
run on any particular machine as it is syncing the
kvm machine instances. But it does work \o/
The machines won't actually boot functional juju
machines in the test because the tools location is
fake.

https://codereview.appspot.com/30020044/

Description of the change

Implement kvm start, stop and list.

The implementation here uses a mix of:
 * uvt-simplestream-libvirt
 * uvt-kvm
 * virsh

These are provided by the uvtool package and the
dependencies. The implementation of the low level
calls out to the shell are in container/kvm/libvirt.go

These low level calls are then used by the container
factory and container instances in container/kvm.

These are used by the container manager instance.

There are some live tests for the kvm calls.
  cd container/kvm
  sudo -E go test
This will take quite some time the first time it is
run on any particular machine as it is syncing the
kvm machine instances. But it does work \o/
The machines won't actually boot functional juju
machines in the test because the tools location is
fake.

https://codereview.appspot.com/30020044/

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

Reviewers: mp+196072_code.launchpad.net,

Message:
Please take a look.

Description:
Implement kvm start, stop and list.

The implementation here uses a mix of:
  * uvt-simplestream-libvirt
  * uvt-kvm
  * virsh

These are provided by the uvtool package and the
dependencies. The implementation of the low level
calls out to the shell are in container/kvm/libvirt.go

These low level calls are then used by the container
factory and container instances in container/kvm.

These are used by the container manager instance.

There are some live tests for the kvm calls.
   cd container/kvm
   sudo -E go test
This will take quite some time the first time it is
run on any particular machine as it is syncing the
kvm machine instances. But it does work \o/
The machines won't actually boot functional juju
machines in the test because the tools location is
fake.

https://code.launchpad.net/~thumper/juju-core/kvm-manager-start-stop/+merge/196072

Requires:
https://code.launchpad.net/~thumper/juju-core/local-provider-syslog/+merge/196048

(do not edit description out of merge proposal)

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

Affected files (+424, -29 lines):
   A [revision details]
   M container/kvm/container.go
   M container/kvm/containerfactory.go
   M container/kvm/interface.go
   M container/kvm/kvm.go
   M container/kvm/kvm_test.go
   A container/kvm/libvirt.go
   A container/kvm/live_test.go
   M container/kvm/mock/mock-kvm.go
   M container/kvm/mock/mock-kvm_test.go
   M container/userdata.go
   M log/log.go

Revision history for this message
Ian Booth (wallyworld) wrote :
Download full text (3.3 KiB)

LTGM with some suggestions.

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

https://codereview.appspot.com/30020044/diff/1/container/kvm/container.go#newcode16
container/kvm/container.go:16: started *bool
Perhaps a comment explaining why a pointer? ie inband signalling of
unknown state

https://codereview.appspot.com/30020044/diff/1/container/kvm/container.go#newcode36
container/kvm/container.go:36: bridge := ""
I'd prefer var bridge string here because "" is not a valid value

https://codereview.appspot.com/30020044/diff/1/container/kvm/container.go#newcode60
container/kvm/container.go:60: if c.IsRunning() {
Perhaps
id !c.isRunning() {
   logger.Debugf(...)
   return nil
}
other stuff....

because the other stuff is more lines of code and I think it reads
better that way

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

https://codereview.appspot.com/30020044/diff/1/container/kvm/kvm.go#newcode78
container/kvm/kvm.go:78: return nil, err
Generally, inside these sorts of methods implementing an interface, I
think we have been doing stuff like
return nil, fmt.Errorf("doing blah blah: %v", err)

to give some content to the final error message, rather that just return
nil, err

https://codereview.appspot.com/30020044/diff/1/container/kvm/kvm.go#newcode84
container/kvm/kvm.go:84: return nil, err
ditto

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

https://codereview.appspot.com/30020044/diff/1/container/kvm/kvm_test.go#newcode24
container/kvm/kvm_test.go:24:
Aren't there tests we can do for start, stop etc when using mocks?

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

https://codereview.appspot.com/30020044/diff/1/container/kvm/libvirt.go#newcode5
container/kvm/libvirt.go:5:
Could we add some docstring saying what this file does and also what
packages it depends on being installed?

https://codereview.appspot.com/30020044/diff/1/container/kvm/libvirt.go#newcode22
container/kvm/libvirt.go:22: func run(command string, args ...string)
(output string, err error) {
This looks like pretty generic boilerplate. Shouldn't we put this in a
utils package somewhere?

https://codereview.appspot.com/30020044/diff/1/container/kvm/libvirt.go#newcode53
container/kvm/libvirt.go:53: UserData string
Could we rename this to UserDataFile cause that's what it is and it
could be confused for the actual user data content otherwise. That's
what I thought till I read the rest of the code.

https://codereview.appspot.com/30020044/diff/1/container/kvm/libvirt.go#newcode107
container/kvm/libvirt.go:107: // Perhaps regex matching is the easiest
way to match the lines.
s/Perhaps//

https://codereview.appspot.com/30020044/diff/1/container/kvm/libvirt.go#newcode112
container/kvm/libvirt.go:112: machineStatus :=
machineListPattern.ExpandString(nil, "$hostname $status", output, s)
machineStatus is confusing as a variable name, since it could look like
is is being used to store the value of "status" from "id hostname
stat...

Read more...

Revision history for this message
Tim Penhey (thumper) wrote :
Download full text (4.3 KiB)

I'll push the diff once I have fixed the first one. It seems a bit
screwed up right now.

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

https://codereview.appspot.com/30020044/diff/1/container/kvm/container.go#newcode16
container/kvm/container.go:16: started *bool
On 2013/11/21 23:57:26, wallyworld wrote:
> Perhaps a comment explaining why a pointer? ie inband signalling of
unknown
> state

Done.

https://codereview.appspot.com/30020044/diff/1/container/kvm/container.go#newcode36
container/kvm/container.go:36: bridge := ""
On 2013/11/21 23:57:26, wallyworld wrote:
> I'd prefer var bridge string here because "" is not a valid value

Changed it, but "" is a technically valid value. If it isn't set, we
don't pass the --bridge value down to the underlying uvt-kvm call.

https://codereview.appspot.com/30020044/diff/1/container/kvm/container.go#newcode60
container/kvm/container.go:60: if c.IsRunning() {
On 2013/11/21 23:57:26, wallyworld wrote:
> Perhaps
> id !c.isRunning() {
> logger.Debugf(...)
> return nil
> }
> other stuff....

> because the other stuff is more lines of code and I think it reads
better that
> way

Done.

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

https://codereview.appspot.com/30020044/diff/1/container/kvm/kvm.go#newcode78
container/kvm/kvm.go:78: return nil, err
On 2013/11/21 23:57:26, wallyworld wrote:
> Generally, inside these sorts of methods implementing an interface, I
think we
> have been doing stuff like
> return nil, fmt.Errorf("doing blah blah: %v", err)

> to give some content to the final error message, rather that just
return nil,
> err

Done.

https://codereview.appspot.com/30020044/diff/1/container/kvm/kvm.go#newcode84
container/kvm/kvm.go:84: return nil, err
On 2013/11/21 23:57:26, wallyworld wrote:
> ditto

Done.

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

https://codereview.appspot.com/30020044/diff/1/container/kvm/kvm_test.go#newcode24
container/kvm/kvm_test.go:24:
On 2013/11/21 23:57:26, wallyworld wrote:
> Aren't there tests we can do for start, stop etc when using mocks?

Yes, and we actually do already. This comment was to remind myself to
implement the live tests, which I did.

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

https://codereview.appspot.com/30020044/diff/1/container/kvm/libvirt.go#newcode5
container/kvm/libvirt.go:5:
On 2013/11/21 23:57:26, wallyworld wrote:
> Could we add some docstring saying what this file does and also what
packages it
> depends on being installed?

Done.

https://codereview.appspot.com/30020044/diff/1/container/kvm/libvirt.go#newcode22
container/kvm/libvirt.go:22: func run(command string, args ...string)
(output string, err error) {
On 2013/11/21 23:57:26, wallyworld wrote:
> This looks like pretty generic boilerplate. Shouldn't we put this in a
utils
> package somewhere?

Possibly, but it did provide me a very place to ramp up the logging to
test what was going on. I'd be a little loathed to rem...

Read more...

Revision history for this message
Tim Penhey (thumper) wrote :

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'container/kvm/container.go'
--- container/kvm/container.go 2013-11-12 21:00:12 +0000
+++ container/kvm/container.go 2013-11-25 04:11:41 +0000
@@ -5,12 +5,18 @@
55
6import (6import (
7 "fmt"7 "fmt"
8
9 "launchpad.net/juju-core/container"
10 "launchpad.net/juju-core/log"
8)11)
912
10type kvmContainer struct {13type kvmContainer struct {
11 factory *containerFactory14 factory *containerFactory
12 name string15 name string
13 started bool16 // started is a three state boolean, true, false, or unknown
17 // this allows for checking when we don't know, but using a
18 // value if we already know it (like in the list situation).
19 started *bool
14}20}
1521
16var _ Container = (*kvmContainer)(nil)22var _ Container = (*kvmContainer)(nil)
@@ -19,16 +25,61 @@
19 return c.name25 return c.name
20}26}
2127
22func (c *kvmContainer) Start() error {28func (c *kvmContainer) Start(
23 return fmt.Errorf("not implemented")29 series string,
30 arch string,
31 userDataFile string,
32 network *container.NetworkConfig,
33) error {
34 // TODO: handle memory, cpu, disk etc.
35 logger.Debugf("Synchronise images for %s %s", series, arch)
36 if err := SyncImages(series, arch); err != nil {
37 return err
38 }
39 var bridge string
40 if network != nil {
41 if network.NetworkType == container.BridgeNetwork {
42 bridge = network.Device
43 } else {
44 return log.LoggedErrorf(logger, "Non-bridge network devices not yet supported")
45 }
46 }
47 logger.Debugf("Create the machine %s", c.name)
48 if err := CreateMachine(CreateMachineParams{
49 Hostname: c.name,
50 Series: series,
51 Arch: arch,
52 UserDataFile: userDataFile,
53 NetworkBridge: bridge,
54 }); err != nil {
55 return err
56 }
57
58 logger.Debugf("Set machine %s to autostart", c.name)
59 return AutostartMachine(c.name)
24}60}
2561
26func (c *kvmContainer) Stop() error {62func (c *kvmContainer) Stop() error {
27 return fmt.Errorf("not implemented")63 if !c.IsRunning() {
64 logger.Debugf("%s is already stopped", c.name)
65 return nil
66 }
67 // Make started state unknown again.
68 c.started = nil
69 logger.Debugf("Stop %s", c.name)
70 return DestroyMachine(c.name)
28}71}
2972
30func (c *kvmContainer) IsRunning() bool {73func (c *kvmContainer) IsRunning() bool {
31 return c.started74 if c.started != nil {
75 return *c.started
76 }
77 machines, err := ListMachines()
78 if err != nil {
79 return false
80 }
81 c.started = isRunning(machines[c.name])
82 return *c.started
32}83}
3384
34func (c *kvmContainer) String() string {85func (c *kvmContainer) String() string {
3586
=== modified file 'container/kvm/containerfactory.go'
--- container/kvm/containerfactory.go 2013-11-12 21:00:12 +0000
+++ container/kvm/containerfactory.go 2013-11-25 04:11:41 +0000
@@ -3,10 +3,6 @@
33
4package kvm4package kvm
55
6import (
7 "fmt"
8)
9
10type containerFactory struct {6type containerFactory struct {
11}7}
128
@@ -19,6 +15,26 @@
19 }15 }
20}16}
2117
22func (factory *containerFactory) List() ([]Container, error) {18func isRunning(value string) *bool {
23 return nil, fmt.Errorf("Not yet implemented")19 var result *bool = new(bool)
20 if value == "running" {
21 *result = true
22 }
23 return result
24}
25
26func (factory *containerFactory) List() (result []Container, err error) {
27 machines, err := ListMachines()
28 if err != nil {
29 return nil, err
30 }
31 for hostname, status := range machines {
32 result = append(result, &kvmContainer{
33 factory: factory,
34 name: hostname,
35 started: isRunning(status),
36 })
37
38 }
39 return result, nil
24}40}
2541
=== modified file 'container/kvm/interface.go'
--- container/kvm/interface.go 2013-11-12 21:00:12 +0000
+++ container/kvm/interface.go 2013-11-25 04:11:41 +0000
@@ -3,6 +3,10 @@
33
4package kvm4package kvm
55
6import (
7 "launchpad.net/juju-core/container"
8)
9
6// Container represents a virtualized container instance and provides10// Container represents a virtualized container instance and provides
7// operations to create, maintain and destroy the container.11// operations to create, maintain and destroy the container.
8type Container interface {12type Container interface {
@@ -11,8 +15,13 @@
11 Name() string15 Name() string
1216
13 // Start runs the container as a daemon.17 // Start runs the container as a daemon.
14 // TODO: determine parameters18 // TODO: add constraints.
15 Start() error19 Start(
20 series string,
21 arch string,
22 userDataFile string,
23 network *container.NetworkConfig,
24 ) error
1625
17 // Stop terminates the running container.26 // Stop terminates the running container.
18 Stop() error27 Stop() error
1928
=== modified file 'container/kvm/kvm.go'
--- container/kvm/kvm.go 2013-11-13 01:35:46 +0000
+++ container/kvm/kvm.go 2013-11-25 04:11:41 +0000
@@ -13,6 +13,9 @@
13 "launchpad.net/juju-core/container"13 "launchpad.net/juju-core/container"
14 "launchpad.net/juju-core/environs/cloudinit"14 "launchpad.net/juju-core/environs/cloudinit"
15 "launchpad.net/juju-core/instance"15 "launchpad.net/juju-core/instance"
16 "launchpad.net/juju-core/log"
17 "launchpad.net/juju-core/names"
18 "launchpad.net/juju-core/version"
16)19)
1720
18var (21var (
@@ -60,11 +63,39 @@
60 machineConfig *cloudinit.MachineConfig,63 machineConfig *cloudinit.MachineConfig,
61 series string,64 series string,
62 network *container.NetworkConfig) (instance.Instance, error) {65 network *container.NetworkConfig) (instance.Instance, error) {
63 return nil, fmt.Errorf("not yet implemented")66
67 name := names.MachineTag(machineConfig.MachineId)
68 if manager.name != "" {
69 name = fmt.Sprintf("%s-%s", manager.name, name)
70 }
71 // Note here that the kvmObjectFacotry only returns a valid container
72 // object, and doesn't actually construct the underlying kvm container on
73 // disk.
74 kvmContainer := KvmObjectFactory.New(name)
75
76 // Create the cloud-init.
77 directory, err := container.NewDirectory(name)
78 if err != nil {
79 return nil, fmt.Errorf("failed to create container directory: %v", err)
80 }
81 logger.Tracef("write cloud-init")
82 userDataFilename, err := container.WriteUserData(machineConfig, directory)
83 if err != nil {
84 return nil, log.LoggedErrorf(logger, "failed to write user data: %v", err)
85 }
86 // Create the container.
87 logger.Tracef("create the container")
88 if err := kvmContainer.Start(series, version.Current.Arch, userDataFilename, network); err != nil {
89 return nil, log.LoggedErrorf(logger, "kvm container creation failed: %v", err)
90 }
91 logger.Tracef("kvm container created")
92 return &kvmInstance{kvmContainer, name}, nil
64}93}
6594
66func (manager *containerManager) StopContainer(instance.Instance) error {95func (manager *containerManager) StopContainer(instance instance.Instance) error {
67 return fmt.Errorf("not yet implemented")96 name := string(instance.Id())
97 kvmContainer := KvmObjectFactory.New(name)
98 return kvmContainer.Stop()
68}99}
69100
70func (manager *containerManager) ListContainers() (result []instance.Instance, err error) {101func (manager *containerManager) ListContainers() (result []instance.Instance, err error) {
71102
=== modified file 'container/kvm/kvm_test.go'
--- container/kvm/kvm_test.go 2013-11-12 21:00:12 +0000
+++ container/kvm/kvm_test.go 2013-11-25 04:11:41 +0000
@@ -11,6 +11,7 @@
11 kvmtesting "launchpad.net/juju-core/container/kvm/testing"11 kvmtesting "launchpad.net/juju-core/container/kvm/testing"
12 "launchpad.net/juju-core/instance"12 "launchpad.net/juju-core/instance"
13 jc "launchpad.net/juju-core/testing/checkers"13 jc "launchpad.net/juju-core/testing/checkers"
14 "launchpad.net/juju-core/version"
14)15)
1516
16type KVMSuite struct {17type KVMSuite struct {
@@ -19,7 +20,11 @@
1920
20var _ = gc.Suite(&KVMSuite{})21var _ = gc.Suite(&KVMSuite{})
2122
22// TODO: work out how to test the actual kvm implementations.23func (*KVMSuite) TestManagerNameNeeded(c *gc.C) {
24 manager, err := kvm.NewContainerManager(container.ManagerConfig{})
25 c.Assert(err, gc.ErrorMatches, "name is required")
26 c.Assert(manager, gc.IsNil)
27}
2328
24func (*KVMSuite) TestListInitiallyEmpty(c *gc.C) {29func (*KVMSuite) TestListInitiallyEmpty(c *gc.C) {
25 manager, err := kvm.NewContainerManager(container.ManagerConfig{Name: "test"})30 manager, err := kvm.NewContainerManager(container.ManagerConfig{Name: "test"})
@@ -30,9 +35,10 @@
30}35}
3136
32func (s *KVMSuite) createRunningContainer(c *gc.C, name string) kvm.Container {37func (s *KVMSuite) createRunningContainer(c *gc.C, name string) kvm.Container {
33 container := s.Factory.New(name)38 kvmContainer := s.Factory.New(name)
34 c.Assert(container.Start(), gc.IsNil)39 network := container.BridgeNetworkConfig("testbr0")
35 return container40 c.Assert(kvmContainer.Start("quantal", version.Current.Arch, "userdata.txt", network), gc.IsNil)
41 return kvmContainer
36}42}
3743
38func (s *KVMSuite) TestListMatchesManagerName(c *gc.C) {44func (s *KVMSuite) TestListMatchesManagerName(c *gc.C) {
3945
=== added file 'container/kvm/libvirt.go'
--- container/kvm/libvirt.go 1970-01-01 00:00:00 +0000
+++ container/kvm/libvirt.go 2013-11-25 04:11:41 +0000
@@ -0,0 +1,128 @@
1// Copyright 2013 Canonical Ltd.
2// Licensed under the AGPLv3, see LICENCE file for details.
3
4package kvm
5
6// This file contains wrappers around the following executables:
7// uvt-simplestreams-libvirt
8// uvt-kvm
9// virsh
10// Those executables are found in the following packages:
11// uvtool-libvirt
12// libvirt-bin
13//
14// These executables provide Juju's interface to dealing with kvm containers.
15// The define how we start, stop and list running containers on the host
16
17import (
18 "fmt"
19 "os/exec"
20 "regexp"
21 "strings"
22)
23
24var (
25 // The regular expression for breaking up the results of 'virsh list'
26 // (?m) - specify that this is a multiline regex
27 // first part is the opaque identifier we don't care about
28 // then the hostname, and lastly the status.
29 machineListPattern = regexp.MustCompile(`(?m)^\s+\d+\s+(?P<hostname>[-\w]+)\s+(?P<status>.+)\s*$`)
30)
31
32// run the command and return the combined output.
33func run(command string, args ...string) (output string, err error) {
34 logger.Tracef("%s %v", command, args)
35 cmd := exec.Command(command, args...)
36 out, err := cmd.CombinedOutput()
37 output = string(out)
38 logger.Tracef("output: %v", output)
39 if err != nil {
40 return output, err
41 }
42 if !cmd.ProcessState.Success() {
43 return output, fmt.Errorf("%s returned non-zero exit", command)
44 }
45 return output, nil
46}
47
48// SyncImages updates the local cached images by reading the simplestreams
49// data and downloading the cloud images to the uvtool pool (used by libvirt).
50func SyncImages(series string, arch string) error {
51 args := []string{
52 "sync",
53 fmt.Sprintf("arch=%s", arch),
54 fmt.Sprintf("release=%s", series),
55 }
56 _, err := run("uvt-simplestreams-libvirt", args...)
57 return err
58}
59
60type CreateMachineParams struct {
61 Hostname string
62 Series string
63 Arch string
64 UserDataFile string
65 NetworkBridge string
66 // TODO memory, cpu and disk
67}
68
69// CreateMachine creates a virtual machine and starts it.
70func CreateMachine(params CreateMachineParams) error {
71 if params.Hostname == "" {
72 return fmt.Errorf("Hostname is required")
73 }
74 args := []string{
75 "create",
76 "--log-console-output", // do wonder where this goes...
77 }
78 if params.UserDataFile != "" {
79 args = append(args, "--user-data", params.UserDataFile)
80 }
81 if params.NetworkBridge != "" {
82 args = append(args, "--bridge", params.NetworkBridge)
83 }
84 // TODO add memory, cpu and disk prior to hostname
85 args = append(args, params.Hostname)
86 if params.Series != "" {
87 args = append(args, fmt.Sprintf("release=%s", params.Series))
88 }
89 if params.Arch != "" {
90 args = append(args, fmt.Sprintf("arch=%s", params.Arch))
91 }
92 output, err := run("uvt-kvm", args...)
93 logger.Debugf("is this the logged output?:\n%s", output)
94 return err
95}
96
97// DestroyMachine destroys the virtual machine identified by hostname.
98func DestroyMachine(hostname string) error {
99 _, err := run("uvt-kvm", "destroy", hostname)
100 return err
101}
102
103// AutostartMachine indicates that the virtual machines should automatically
104// restart when the host restarts.
105func AutostartMachine(hostname string) error {
106 _, err := run("virsh", "autostart", hostname)
107 return err
108}
109
110// ListMachines returns a map of machine name to state, where state is one of:
111// running, idle, paused, shutdown, shut off, crashed, dying, pmsuspended.
112func ListMachines() (map[string]string, error) {
113 output, err := run("virsh", "-q", "list", "--all")
114 if err != nil {
115 return nil, err
116 }
117 // Split the output into lines.
118 // Regex matching is the easiest way to match the lines.
119 // id hostname status
120 // separated by whitespace, with whitespace at the start too.
121 result := make(map[string]string)
122 for _, s := range machineListPattern.FindAllStringSubmatchIndex(output, -1) {
123 hostnameAndStatus := machineListPattern.ExpandString(nil, "$hostname $status", output, s)
124 parts := strings.SplitN(string(hostnameAndStatus), " ", 2)
125 result[parts[0]] = parts[1]
126 }
127 return result, nil
128}
0129
=== added file 'container/kvm/live_test.go'
--- container/kvm/live_test.go 1970-01-01 00:00:00 +0000
+++ container/kvm/live_test.go 2013-11-25 04:11:41 +0000
@@ -0,0 +1,139 @@
1// Copyright 2013 Canonical Ltd.
2// Licensed under the AGPLv3, see LICENCE file for details.
3
4package kvm_test
5
6import (
7 "os"
8 "runtime"
9
10 gc "launchpad.net/gocheck"
11 "launchpad.net/loggo"
12
13 "launchpad.net/juju-core/constraints"
14 "launchpad.net/juju-core/container"
15 "launchpad.net/juju-core/container/kvm"
16 "launchpad.net/juju-core/environs"
17 "launchpad.net/juju-core/environs/config"
18 "launchpad.net/juju-core/instance"
19 jujutesting "launchpad.net/juju-core/juju/testing"
20 coretesting "launchpad.net/juju-core/testing"
21 "launchpad.net/juju-core/testing/testbase"
22 "launchpad.net/juju-core/tools"
23 "launchpad.net/juju-core/version"
24)
25
26type LiveSuite struct {
27 testbase.LoggingSuite
28 ContainerDir string
29 RemovedDir string
30}
31
32var _ = gc.Suite(&LiveSuite{})
33
34func (s *LiveSuite) SetUpTest(c *gc.C) {
35 s.LoggingSuite.SetUpTest(c)
36 // Skip if not linux
37 if runtime.GOOS != "linux" {
38 c.Skip("not running linux")
39 }
40 // Skip if not running as root.
41 if os.Getuid() != 0 {
42 c.Skip("not running as root")
43 }
44 s.ContainerDir = c.MkDir()
45 s.PatchValue(&container.ContainerDir, s.ContainerDir)
46 s.RemovedDir = c.MkDir()
47 s.PatchValue(&container.RemovedContainerDir, s.RemovedDir)
48 loggo.GetLogger("juju.container").SetLogLevel(loggo.TRACE)
49}
50
51func (s *LiveSuite) newManager(c *gc.C, name string) container.Manager {
52 manager, err := kvm.NewContainerManager(
53 container.ManagerConfig{
54 Name: name,
55 LogDir: c.MkDir(),
56 })
57 c.Assert(err, gc.IsNil)
58 return manager
59}
60
61func assertNumberOfContainers(c *gc.C, manager container.Manager, count int) {
62 containers, err := manager.ListContainers()
63 c.Assert(err, gc.IsNil)
64 c.Assert(containers, gc.HasLen, count)
65}
66
67func (s *LiveSuite) TestNoInitialContainers(c *gc.C) {
68 manager := s.newManager(c, "test")
69 assertNumberOfContainers(c, manager, 0)
70}
71
72func shutdownMachines(manager container.Manager) func(*gc.C) {
73 return func(c *gc.C) {
74 instances, err := manager.ListContainers()
75 c.Assert(err, gc.IsNil)
76 for _, instance := range instances {
77 err := manager.StopContainer(instance)
78 c.Check(err, gc.IsNil)
79 }
80 }
81}
82
83func startContainer(c *gc.C, manager container.Manager, machineId string) instance.Instance {
84 machineNonce := "fake-nonce"
85 stateInfo := jujutesting.FakeStateInfo(machineId)
86 apiInfo := jujutesting.FakeAPIInfo(machineId)
87 machineConfig := environs.NewMachineConfig(machineId, machineNonce, stateInfo, apiInfo)
88 network := container.BridgeNetworkConfig("virbr0")
89
90 machineConfig.Tools = &tools.Tools{
91 Version: version.MustParseBinary("2.3.4-foo-bar"),
92 URL: "http://tools.testing.invalid/2.3.4-foo-bar.tgz",
93 }
94 environConfig := dummyConfig(c)
95 err := environs.FinishMachineConfig(machineConfig, environConfig, constraints.Value{})
96 c.Assert(err, gc.IsNil)
97
98 instance, err := manager.StartContainer(machineConfig, "precise", network)
99 c.Assert(err, gc.IsNil)
100 return instance
101}
102
103func (s *LiveSuite) TestShutdownMachines(c *gc.C) {
104 manager := s.newManager(c, "test")
105 startContainer(c, manager, "1/kvm/0")
106 startContainer(c, manager, "1/kvm/1")
107 assertNumberOfContainers(c, manager, 2)
108
109 shutdownMachines(manager)(c)
110 assertNumberOfContainers(c, manager, 0)
111}
112
113func (s *LiveSuite) TestManagerIsolation(c *gc.C) {
114 firstManager := s.newManager(c, "first")
115 s.AddCleanup(shutdownMachines(firstManager))
116
117 startContainer(c, firstManager, "1/kvm/0")
118 startContainer(c, firstManager, "1/kvm/1")
119
120 secondManager := s.newManager(c, "second")
121 s.AddCleanup(shutdownMachines(secondManager))
122
123 startContainer(c, secondManager, "1/kvm/0")
124
125 assertNumberOfContainers(c, firstManager, 2)
126 assertNumberOfContainers(c, secondManager, 1)
127}
128
129func dummyConfig(c *gc.C) *config.Config {
130 testConfig, err := config.New(config.UseDefaults, coretesting.FakeConfig())
131 c.Assert(err, gc.IsNil)
132 testConfig, err = testConfig.Apply(map[string]interface{}{
133 "type": "dummy",
134 "state-server": false,
135 "agent-version": version.Current.Number.String(),
136 })
137 c.Assert(err, gc.IsNil)
138 return testConfig
139}
0140
=== modified file 'container/kvm/mock/mock-kvm.go'
--- container/kvm/mock/mock-kvm.go 2013-11-12 21:00:12 +0000
+++ container/kvm/mock/mock-kvm.go 2013-11-25 04:11:41 +0000
@@ -6,6 +6,7 @@
6import (6import (
7 "fmt"7 "fmt"
88
9 "launchpad.net/juju-core/container"
9 "launchpad.net/juju-core/container/kvm"10 "launchpad.net/juju-core/container/kvm"
10)11)
1112
@@ -66,7 +67,12 @@
66 return mock.name67 return mock.name
67}68}
6869
69func (mock *mockContainer) Start() error {70func (mock *mockContainer) Start(
71 series string,
72 arch string,
73 userDataFile string,
74 network *container.NetworkConfig,
75) error {
70 if mock.started {76 if mock.started {
71 return fmt.Errorf("container is already running")77 return fmt.Errorf("container is already running")
72 }78 }
7379
=== modified file 'container/kvm/mock/mock-kvm_test.go'
--- container/kvm/mock/mock-kvm_test.go 2013-11-12 21:13:21 +0000
+++ container/kvm/mock/mock-kvm_test.go 2013-11-25 04:11:41 +0000
@@ -6,17 +6,27 @@
6import (6import (
7 gc "launchpad.net/gocheck"7 gc "launchpad.net/gocheck"
88
9 "launchpad.net/juju-core/container"
9 "launchpad.net/juju-core/container/kvm"10 "launchpad.net/juju-core/container/kvm"
10 "launchpad.net/juju-core/container/kvm/mock"11 "launchpad.net/juju-core/container/kvm/mock"
11 jc "launchpad.net/juju-core/testing/checkers"12 jc "launchpad.net/juju-core/testing/checkers"
12 "launchpad.net/juju-core/testing/testbase"13 "launchpad.net/juju-core/testing/testbase"
14 "launchpad.net/juju-core/version"
13)15)
1416
15type MockSuite struct {17type MockSuite struct {
16 testbase.LoggingSuite18 testbase.LoggingSuite
17}19}
1820
19var _ = gc.Suite(&MockSuite{})21var (
22 _ = gc.Suite(&MockSuite{})
23
24 // Start args
25 series = "quantal"
26 arch = version.Current.Arch
27 userData = "/some/path/to/userdata.txt"
28 network = container.BridgeNetworkConfig("testbr0")
29)
2030
21func (*MockSuite) TestListInitiallyEmpty(c *gc.C) {31func (*MockSuite) TestListInitiallyEmpty(c *gc.C) {
22 factory := mock.MockFactory()32 factory := mock.MockFactory()
@@ -52,7 +62,7 @@
52func (*MockSuite) TestContainerStartStarts(c *gc.C) {62func (*MockSuite) TestContainerStartStarts(c *gc.C) {
53 factory := mock.MockFactory()63 factory := mock.MockFactory()
54 container := factory.New("first")64 container := factory.New("first")
55 err := container.Start()65 err := container.Start(series, arch, userData, network)
56 c.Assert(err, gc.IsNil)66 c.Assert(err, gc.IsNil)
57 c.Assert(container.IsRunning(), jc.IsTrue)67 c.Assert(container.IsRunning(), jc.IsTrue)
58}68}
@@ -60,16 +70,16 @@
60func (*MockSuite) TestContainerStartingRunningErrors(c *gc.C) {70func (*MockSuite) TestContainerStartingRunningErrors(c *gc.C) {
61 factory := mock.MockFactory()71 factory := mock.MockFactory()
62 container := factory.New("first")72 container := factory.New("first")
63 err := container.Start()73 err := container.Start(series, arch, userData, network)
64 c.Assert(err, gc.IsNil)74 c.Assert(err, gc.IsNil)
65 err = container.Start()75 err = container.Start(series, arch, userData, network)
66 c.Assert(err, gc.ErrorMatches, "container is already running")76 c.Assert(err, gc.ErrorMatches, "container is already running")
67}77}
6878
69func (*MockSuite) TestContainerStoppingRunningStops(c *gc.C) {79func (*MockSuite) TestContainerStoppingRunningStops(c *gc.C) {
70 factory := mock.MockFactory()80 factory := mock.MockFactory()
71 container := factory.New("first")81 container := factory.New("first")
72 err := container.Start()82 err := container.Start(series, arch, userData, network)
73 c.Assert(err, gc.IsNil)83 c.Assert(err, gc.IsNil)
74 err = container.Stop()84 err = container.Stop()
75 c.Assert(err, gc.IsNil)85 c.Assert(err, gc.IsNil)
@@ -132,8 +142,8 @@
132142
133 first := factory.New("first")143 first := factory.New("first")
134 second := factory.New("second")144 second := factory.New("second")
135 first.Start()145 first.Start(series, arch, userData, network)
136 second.Start()146 second.Start(series, arch, userData, network)
137 second.Stop()147 second.Stop()
138 first.Stop()148 first.Stop()
139149
@@ -151,7 +161,7 @@
151 factory.AddListener(second)161 factory.AddListener(second)
152162
153 container := factory.New("container")163 container := factory.New("container")
154 container.Start()164 container.Start(series, arch, userData, network)
155 container.Stop()165 container.Stop()
156166
157 c.Assert(<-first, gc.Equals, mock.Event{mock.Started, "container"})167 c.Assert(<-first, gc.Equals, mock.Event{mock.Started, "container"})
158168
=== modified file 'container/userdata.go'
--- container/userdata.go 2013-11-14 08:27:15 +0000
+++ container/userdata.go 2013-11-25 04:11:41 +0000
@@ -17,7 +17,7 @@
17)17)
1818
19var (19var (
20 logger = loggo.GetLogger("juju.container.lxc")20 logger = loggo.GetLogger("juju.container")
21 aptHTTPProxyRE = regexp.MustCompile(`(?i)^Acquire::HTTP::Proxy\s+"([^"]+)";$`)21 aptHTTPProxyRE = regexp.MustCompile(`(?i)^Acquire::HTTP::Proxy\s+"([^"]+)";$`)
22)22)
2323
2424
=== modified file 'log/log.go'
--- log/log.go 2013-06-10 04:25:51 +0000
+++ log/log.go 2013-11-25 04:11:41 +0000
@@ -4,6 +4,8 @@
4package log4package log
55
6import (6import (
7 "fmt"
8
7 "launchpad.net/loggo"9 "launchpad.net/loggo"
8)10)
911
@@ -41,3 +43,9 @@
41 logger.Logf(loggo.DEBUG, format, a...)43 logger.Logf(loggo.DEBUG, format, a...)
42 return nil44 return nil
43}45}
46
47// Log the error and return an error with the same text.
48func LoggedErrorf(logger loggo.Logger, format string, a ...interface{}) error {
49 logger.Logf(loggo.ERROR, format, a...)
50 return fmt.Errorf(format, a...)
51}

Subscribers

People subscribed via source and target branches

to status/vote changes: