Code review comment for lp:~waigani/juju-core/lxc-autorestart-setting

Reviewers: mp+210117_code.launchpad.net,

Message:
Please take a look.

Description:
LXC AutoRestart Setting

lxc.containerManager gains "autorestart bool",
and sets that from container.ManagerConfig, if
not in container.ManagerConfig, then manager
default is true.

https://code.launchpad.net/~waigani/juju-core/lxc-autorestart-setting/+merge/210117

Requires:
https://code.launchpad.net/~waigani/juju-core/managers-should-warn-for-any-unknown-options/+merge/210114

(do not edit description out of merge proposal)

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

Affected files (+26, -3 lines):
   A [revision details]
   M container/lxc/lxc.go
   M container/lxc/lxc_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: <email address hidden>
+New revision: <email address hidden>

Index: container/lxc/lxc.go
=== modified file 'container/lxc/lxc.go'
--- container/lxc/lxc.go 2014-03-10 02:51:31 +0000
+++ container/lxc/lxc.go 2014-03-10 03:33:38 +0000
@@ -42,8 +42,9 @@
  }

  type containerManager struct {
- name string
- logdir string
+ name string
+ logdir string
+ autorestart bool
  }

  // containerManager implements container.Manager.
@@ -63,11 +64,21 @@
   if logDir == "" {
    logDir = agent.DefaultLogDir
   }
+ autoRestart := false
+ if conf["autoRestart"] == "" || conf["autoRestart"] == "true" {
+ autoRestart = true
+ }
+ delete(conf, "autoRestart")
+
   for k, v := range conf {
    logger.Warningf(`Found unused config option with key: "%v" and
value: "%v"`, k, v)
   }

- return &containerManager{name: name, logdir: logDir}, nil
+ return &containerManager{name: name, logdir: logDir, autorestart:
autoRestart}, nil
+}
+
+func (manager *containerManager) AutoRestart() bool {
+ return manager.autorestart
  }

  func (manager *containerManager) StartContainer(

Index: container/lxc/lxc_test.go
=== modified file 'container/lxc/lxc_test.go'
--- container/lxc/lxc_test.go 2014-03-10 02:51:31 +0000
+++ container/lxc/lxc_test.go 2014-03-10 03:33:38 +0000
@@ -58,6 +58,16 @@
   c.Assert(c.GetTestLog(), gc.Matches, `^.*WARNING juju.container.lxc Found
unused config option with key: "shazam" and value: "Captain Marvel"\n*`)
  }

+func (*LxcSuite) TestAutoRestartDefaultTrue(c *gc.C) {
+ _, err := lxc.NewContainerManager(container.ManagerConfig{
+ container.ConfigName: "startMeUp",
+ })
+ c.Assert(err, gc.IsNil)
+
+ // How do we test autorestart ?
+ // c.Assert(manager.somehowGetAutoRestart(?), gc.Equals, true)
+}
+
  func (s *LxcSuite) TestStartContainer(c *gc.C) {
   manager := s.makeManager(c, "test")
   instance := containertesting.StartContainer(c, manager, "1/lxc/0")