launchpad-reviewers team mailing list archive

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

Raphaël Badin has proposed merging lp:~rvb/juju-core/destroy-env2 into lp:~maas-maintainers/juju-core/maas-provider-skeleton.

Commit message:
Implement environ.Destroy().

Requested reviews:
  MAAS Maintainers (maas-maintainers)

For more details, see:
https://code.launchpad.net/~rvb/juju-core/destroy-env2/+merge/153176

environ.Destroy() and storage.deleteAll() are mostly cargo-culted from the openstack provider.
-- 
https://code.launchpad.net/~rvb/juju-core/destroy-env2/+merge/153176
Your team MAAS Maintainers is requested to review the proposed merge of lp:~rvb/juju-core/destroy-env2 into lp:~maas-maintainers/juju-core/maas-provider-skeleton.

=== modified file 'environs/maas/environ.go'
--- environs/maas/environ.go	2013-03-13 11:06:51 +0000
+++ environs/maas/environ.go	2013-03-13 15:51:43 +0000
@@ -258,7 +258,7 @@
 	}
 	params := url.Values{
 		"distro_series": {tools.Series},
-// TODO: How do we pass user_data!?
+		// TODO: How do we pass user_data!?
 		//"user_data": {userdata},
 	}
 	// Initialize err to a non-nil value as a sentinel for the following
@@ -410,9 +410,36 @@
 	return environs.EmptyStorage
 }
 
-func (environ *maasEnviron) Destroy([]environs.Instance) error {
+func (environ *maasEnviron) Destroy(ensureInsts []environs.Instance) error {
 	log.Printf("environs/maas: destroying environment %q", environ.name)
-	panic("Not implemented.")
+	insts, err := environ.AllInstances()
+	if err != nil {
+		return fmt.Errorf("cannot get instances: %v", err)
+	}
+	found := make(map[state.InstanceId]bool)
+	for _, inst := range insts {
+		found[inst.Id()] = true
+	}
+
+	// Add any instances we've been told about but haven't yet shown
+	// up in the instance list.
+	for _, inst := range ensureInsts {
+		id := state.InstanceId(inst.(*maasInstance).Id())
+		if !found[id] {
+			insts = append(insts, inst)
+			found[id] = true
+		}
+	}
+	err = environ.StopInstances(insts)
+	if err != nil {
+		return err
+	}
+
+	// To properly observe e.storageUnlocked we need to get its value while
+	// holding e.ecfgMutex. e.Storage() does this for us, then we convert
+	// back to the (*storage) to access the private deleteAll() method.
+	st := environ.Storage().(*maasStorage)
+	return st.deleteAll()
 }
 
 func (*maasEnviron) AssignmentPolicy() state.AssignmentPolicy {

=== modified file 'environs/maas/environ_test.go'
--- environs/maas/environ_test.go	2013-03-13 11:06:51 +0000
+++ environs/maas/environ_test.go	2013-03-13 15:51:43 +0000
@@ -233,6 +233,27 @@
 	c.Check(err, Not(IsNil))
 }
 
+func (suite *EnvironSuite) TestDestroy(c *C) {
+	env := suite.makeEnviron()
+	suite.getInstance("test1")
+	instance := suite.getInstance("test2")
+	data := makeRandomBytes(10)
+	suite.testMAASObject.TestServer.NewFile("filename", data)
+
+	err := env.Destroy([]environs.Instance{instance})
+
+	c.Check(err, IsNil)
+	// Instances have been stopped.
+	operations := suite.testMAASObject.TestServer.NodeOperations()
+	expectedOperations := map[string][]string{"test1": {"release"}, "test2": {"release"}}
+	c.Check(operations, DeepEquals, expectedOperations)
+	storage := NewStorage(env)
+	// Files have been cleaned up.
+	listing, err := storage.List("")
+	c.Assert(err, IsNil)
+	c.Assert(listing, DeepEquals, []string{})
+}
+
 func (suite *EnvironSuite) TestQuiesceStateFileFailsOnBrokenStateFile(c *C) {
 	const content = "@#$(*&Y%!"
 	reader := bytes.NewReader([]byte(content))

=== modified file 'environs/maas/storage.go'
--- environs/maas/storage.go	2013-03-08 07:19:11 +0000
+++ environs/maas/storage.go	2013-03-13 15:51:43 +0000
@@ -179,3 +179,33 @@
 	stor.getSnapshot().maasClientUnlocked.GetSubObject(name).Delete()
 	return nil
 }
+
+func (stor *maasStorage) deleteAll() error {
+	names, err := stor.List("")
+	if err != nil {
+		return err
+	}
+	// Remove all the objects in parallel so that we incur less round-trips.
+	// If we're in danger of having hundreds of objects,
+	// we'll want to change this to limit the number
+	// of concurrent operations.
+	var wg sync.WaitGroup
+	wg.Add(len(names))
+	errc := make(chan error, len(names))
+	for _, name := range names {
+		name := name
+		go func() {
+			if err := stor.Remove(name); err != nil {
+				errc <- err
+			}
+			wg.Done()
+		}()
+	}
+	wg.Wait()
+	select {
+	case err := <-errc:
+		return fmt.Errorf("cannot delete all provider state: %v", err)
+	default:
+	}
+	return nil
+}

=== modified file 'environs/maas/storage_test.go'
--- environs/maas/storage_test.go	2013-03-11 13:14:49 +0000
+++ environs/maas/storage_test.go	2013-03-13 15:51:43 +0000
@@ -369,3 +369,17 @@
 	c.Assert(err, IsNil)
 	c.Check(data, DeepEquals, content)
 }
+
+func (s *StorageSuite) TestDeleteAllDeletesAllFiles(c *C) {
+	storage := s.makeStorage("get-retrieves-file")
+	const filename1 = "stored-data1"
+	s.fakeStoredFile(storage, filename1)
+	const filename2 = "stored-data2"
+	s.fakeStoredFile(storage, filename2)
+
+	err := storage.deleteAll()
+	c.Assert(err, IsNil)
+	listing, err := storage.List("")
+	c.Assert(err, IsNil)
+	c.Assert(listing, DeepEquals, []string{})
+}