launchpad-reviewers team mailing list archive

[Raphaël Badin <raphael.badin@xxxxxxxxxxxxx>]

Raphaël Badin has proposed merging lp:~rvb/juju-core/mpv-start-stop into lp:~maas-maintainers/juju-core/maas-provider-skeleton with lp:~rvb/juju-core/mpv-instance as a prerequisite.

Commit message:
Add support to start/stop instances.

Requested reviews:
  MAAS Maintainers (maas-maintainers)

For more details, see:
https://code.launchpad.net/~rvb/juju-core/mpv-start-stop/+merge/147167

Without the ability to monkey patch stuff, properly testing the behavior of StopInstances in the face of error is tricky.
-- 
https://code.launchpad.net/~rvb/juju-core/mpv-start-stop/+merge/147167
Your team MAAS Maintainers is requested to review the proposed merge of lp:~rvb/juju-core/mpv-start-stop into lp:~maas-maintainers/juju-core/maas-provider-skeleton.

=== modified file 'environs/maas/environ.go'
--- environs/maas/environ.go	2013-02-08 11:36:22 +0000
+++ environs/maas/environ.go	2013-02-08 11:36:22 +0000
@@ -8,6 +8,7 @@
 	"launchpad.net/juju-core/log"
 	"launchpad.net/juju-core/state"
 	"launchpad.net/juju-core/state/api"
+	"net/url"
 	"sync"
 )
 
@@ -83,12 +84,40 @@
 	return nil
 }
 
-func (*maasEnviron) StartInstance(machineId string, info *state.Info, apiInfo *api.Info, tools *state.Tools) (environs.Instance, error) {
-	panic("Not implemented.")
+func (environ *maasEnviron) StartInstance(machineId string, info *state.Info, apiInfo *api.Info, tools *state.Tools) (environs.Instance, error) {
+	node := environ.maasClientUnlocked.GetSubObject(machineId)
+	refreshedNode, err := node.Get()
+	if err != nil {
+		return nil, err
+	}
+	_, refreshErr := refreshedNode.CallPost("start", url.Values{})
+	if refreshErr != nil {
+		return nil, refreshErr
+	}
+	instance := &maasInstance{maasObject: &refreshedNode, environ: environ}
+	return instance, nil
 }
 
-func (*maasEnviron) StopInstances([]environs.Instance) error {
-	panic("Not implemented.")
+func (environ *maasEnviron) StopInstances(instances []environs.Instance) error {
+	// Shortcut to exit quickly if 'instances' is an empty slice or nil.
+	if len(instances) == 0 {
+		return nil
+	}
+	// Iterate over all the instances and send the "stop" signal,
+	// collecting the errors returned.
+	var errors []error
+	for _, instance := range instances {
+		maasInstance := instance.(*maasInstance)
+		_, errPost := (*maasInstance.maasObject).CallPost("stop", url.Values{})
+		errors = append(errors, errPost)
+	}
+	// Return the first error encountered, if any.
+	for _, err := range errors {
+		if err != nil {
+			return err
+		}
+	}
+	return nil
 }
 
 // Instances returns the environs.Instance objects corresponding to the given
@@ -147,8 +176,8 @@
 	panic("Not implemented.")
 }
 
-func (env *maasEnviron) Destroy([]environs.Instance) error {
-	log.Printf("environs/maas: destroying environment %q", env.name)
+func (environ *maasEnviron) Destroy([]environs.Instance) error {
+	log.Printf("environs/maas: destroying environment %q", environ.name)
 	panic("Not implemented.")
 }
 

=== modified file 'environs/maas/environ_test.go'
--- environs/maas/environ_test.go	2013-02-08 11:36:22 +0000
+++ environs/maas/environ_test.go	2013-02-08 11:36:22 +0000
@@ -125,3 +125,47 @@
 	c.Check(len(instances), Equals, 1)
 	c.Check(string(instances[0].Id()), Equals, resourceURI1)
 }
+
+func (suite *EnvironSuite) TestStartInstanceStartsInstance(c *C) {
+	input := `{"system_id": "test"}`
+	node := suite.testMAASObject.TestServer.NewNode(input)
+	resourceURI, _ := node.GetField("resource_uri")
+
+	instance, err := suite.environ.StartInstance(resourceURI, nil, nil, nil)
+
+	c.Check(err, IsNil)
+	c.Check(string(instance.Id()), Equals, resourceURI)
+	operations := suite.testMAASObject.TestServer.NodeOperations()
+	actions, found := operations["test"]
+	c.Check(found, Equals, true)
+	c.Check(actions, DeepEquals, []string{"start"})
+}
+
+func (suite *EnvironSuite) getInstance(systemId string) *maasInstance {
+	input := `{"system_id": "` + systemId + `"}`
+	node := suite.testMAASObject.TestServer.NewNode(input)
+	return &maasInstance{&node, suite.environ}
+}
+
+func (suite *EnvironSuite) TestStopInstancesReturnsIfParameterEmpty(c *C) {
+	suite.getInstance("test1")
+
+	err := suite.environ.StopInstances([]environs.Instance{})
+	c.Check(err, IsNil)
+	operations := suite.testMAASObject.TestServer.NodeOperations()
+	c.Check(operations, DeepEquals, map[string][]string{})
+}
+
+func (suite *EnvironSuite) TestStopInstancesStopsInstances(c *C) {
+	instance1 := suite.getInstance("test1")
+	instance2 := suite.getInstance("test2")
+	suite.getInstance("test3")
+	instances := []environs.Instance{instance1, instance2}
+
+	err := suite.environ.StopInstances(instances)
+
+	c.Check(err, IsNil)
+	operations := suite.testMAASObject.TestServer.NodeOperations()
+	expectedOperations := map[string][]string{"test1": {"stop"}, "test2": {"stop"}}
+	c.Check(operations, DeepEquals, expectedOperations)
+}