Merge lp:~thumper/golxc/nicer-destroy into lp:golxc

Proposed by Tim Penhey on 2013-09-30
Status: Merged
Merged at revision: 7
Proposed branch: lp:~thumper/golxc/nicer-destroy
Merge into: lp:golxc
Diff against target: 34 lines (+8/-1)
2 files modified
golxc.go (+5/-1)
golxc_test.go (+3/-0)
To merge this branch: bzr merge lp:~thumper/golxc/nicer-destroy
Reviewer Review Type Date Requested Status
John A Meinel 2013-09-30 Approve on 2013-09-30
Review via email: mp+188254@code.launchpad.net

Commit message

Don't stop a stopped container.

Newer versions of lxc > 0.9 error on lxc-stop if the
container is stopped.

The LXCSuite.TestStopNotRunning was failing (but it has
to be run as root, so awkward to test).

Description of the change

Don't stop a stopped container.

Newer versions of lxc > 0.9 error on lxc-stop if the
container is stopped.

The LXCSuite.TestStopNotRunning was failing (but it has
to be run as root, so awkward to test).

This change fixes it, with a small drive by. The start
command now waits for Started or Stopped. When the
lxc-start command is executed, the container goes into
the starting state, and from there it moves to running if
all is good, or stopped if there was an error starting it.
If we don't wait for stopped as well, golxc waits forever.

We do need a better solution here for tests. The output
and behaviour of lxc has changed over the versions, so we
should have a version check for the tests. I'll file a
tech-debt bug around this.

To post a comment you must log in.
Tim Penhey (thumper) wrote :

Please take a look.

John A Meinel (jameinel) wrote :

I'd like to see a test for the:
  return c.Wait(StateRunning, StateStopped)

So that we can make it clear why we need it (a machine that cannot start will transition from starting => stopped, and never show up as "started").

However, I can understand where the overhead of writing a test vs the actual patch doesn't balance well.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'golxc.go'
2--- golxc.go 2013-06-14 01:27:52 +0000
3+++ golxc.go 2013-09-30 02:41:04 +0000
4@@ -223,7 +223,7 @@
5 if err != nil {
6 return err
7 }
8- return c.Wait(StateRunning)
9+ return c.Wait(StateRunning, StateStopped)
10 }
11
12 // Stop terminates the running container.
13@@ -231,6 +231,10 @@
14 if !c.IsConstructed() {
15 return fmt.Errorf("container %q is not yet created", c.name)
16 }
17+ // If the container is not running, we are done.
18+ if !c.IsRunning() {
19+ return nil
20+ }
21 args := []string{
22 "-n", c.name,
23 }
24
25=== modified file 'golxc_test.go'
26--- golxc_test.go 2013-06-14 03:27:43 +0000
27+++ golxc_test.go 2013-09-30 02:41:04 +0000
28@@ -1,3 +1,6 @@
29+// Copyright 2013 Canonical Ltd.
30+// Licensed under the LGPLv3, see COPYING and COPYING.LESSER file for details.
31+
32 package golxc_test
33
34 import (

Subscribers

People subscribed via source and target branches

to all changes: