Add new option to remove-machine to tell the provisioner not to stop …

…the instance
modongsongrio · Aug 10, 2017 · dd58647 · dd58647
1 parent 7360104
commit dd58647
Show file tree

Hide file tree

Showing 26 changed files with 510 additions and 31 deletions.
diff --git a/api/client.go b/api/client.go
@@ -190,6 +190,21 @@ func (c *Client) ForceDestroyMachines(machines ...string) error {
 	return c.facade.FacadeCall("DestroyMachines", params, nil)
 }
 
+// DestroyMachinesWithParams removes a given set of machines and all associated units.
+//
+// NOTE(wallyworld) this exists only for backwards compatibility, when MachineManager
+// facade v4 is not available. The MachineManager.DestroyMachinesWithParams method
+// should be preferred.
+//
+// TODO(wallyworld) 2017-03-16 #1673323
+// Drop this in Juju 3.0.
+func (c *Client) DestroyMachinesWithParams(force, keep bool, machines ...string) error {
+	if keep {
+		return errors.NotSupportedf("destroy machine with keep-instance=true")
+	}
+	return c.DestroyMachines(machines...)
+}
+
 // GetModelConstraints returns the constraints for the model.
 func (c *Client) GetModelConstraints() (constraints.Value, error) {
 	results := new(params.GetConstraintsResults)

diff --git a/api/facadeversions.go b/api/facadeversions.go
@@ -47,7 +47,7 @@ var facadeVersions = map[string]int{
 	"LogForwarding":                1,
 	"Logger":                       1,
 	"MachineActions":               1,
-	"MachineManager":               3,
+	"MachineManager":               4,
 	"MachineUndertaker":            1,
 	"Machiner":                     1,
 	"MeterStatus":                  1,

diff --git a/api/machinemanager/machinemanager.go b/api/machinemanager/machinemanager.go
@@ -49,6 +49,48 @@ func (client *Client) ForceDestroyMachines(machines ...string) ([]params.Destroy
 	return client.destroyMachines("ForceDestroyMachine", machines)
 }
 
+// DestroyMachinesWithParams removes the given set of machines, the semantics of which
+// is determined by the force and keep parameters.
+// TODO(wallyworld) - for Juju 3.0, this should be the preferred api to use.
+func (client *Client) DestroyMachinesWithParams(force, keep bool, machines ...string) ([]params.DestroyMachineResult, error) {
+	if !keep {
+		if force {
+			return client.ForceDestroyMachines(machines...)
+		}
+		return client.DestroyMachines(machines...)
+	}
+	args := params.DestroyMachinesParams{
+		Force:       force,
+		Keep:        keep,
+		MachineTags: make([]string, 0, len(machines)),
+	}
+	allResults := make([]params.DestroyMachineResult, len(machines))
+	index := make([]int, 0, len(machines))
+	for i, machineId := range machines {
+		if !names.IsValidMachine(machineId) {
+			allResults[i].Error = &params.Error{
+				Message: errors.NotValidf("machine ID %q", machineId).Error(),
+			}
+			continue
+		}
+		index = append(index, i)
+		args.MachineTags = append(args.MachineTags, names.NewMachineTag(machineId).String())
+	}
+	if len(args.MachineTags) > 0 {
+		var result params.DestroyMachineResults
+		if err := client.facade.FacadeCall("DestroyMachineWithParams", args, &result); err != nil {
+			return nil, errors.Trace(err)
+		}
+		if n := len(result.Results); n != len(args.MachineTags) {
+			return nil, errors.Errorf("expected %d result(s), got %d", len(args.MachineTags), n)
+		}
+		for i, result := range result.Results {
+			allResults[index[i]] = result
+		}
+	}
+	return allResults, nil
+}
+
 func (client *Client) destroyMachines(method string, machines []string) ([]params.DestroyMachineResult, error) {
 	args := params.Entities{
 		Entities: make([]params.Entity, 0, len(machines)),

diff --git a/api/machinemanager/machinemanager_test.go b/api/machinemanager/machinemanager_test.go
@@ -180,3 +180,33 @@ func (s *MachinemanagerSuite) TestDestroyMachinesInvalidIds(c *gc.C) {
 	c.Assert(err, jc.ErrorIsNil)
 	c.Assert(results, jc.DeepEquals, expectedResults)
 }
+
+func (s *MachinemanagerSuite) TestDestroyMachinesWithParams(c *gc.C) {
+	expectedResults := []params.DestroyMachineResult{{
+		Error: &params.Error{Message: "boo"},
+	}, {
+		Info: &params.DestroyMachineInfo{
+			DestroyedUnits:   []params.Entity{{Tag: "unit-foo-0"}},
+			DestroyedStorage: []params.Entity{{Tag: "storage-pgdata-0"}},
+			DetachedStorage:  []params.Entity{{Tag: "storage-pgdata-1"}},
+		},
+	}}
+	client := newClient(func(objType string, version int, id, request string, a, response interface{}) error {
+		c.Assert(request, gc.Equals, "DestroyMachineWithParams")
+		c.Assert(a, jc.DeepEquals, params.DestroyMachinesParams{
+			Keep:  true,
+			Force: true,
+			MachineTags: []string{
+				"machine-0",
+				"machine-0-lxd-1",
+			},
+		})
+		c.Assert(response, gc.FitsTypeOf, &params.DestroyMachineResults{})
+		out := response.(*params.DestroyMachineResults)
+		*out = params.DestroyMachineResults{expectedResults}
+		return nil
+	})
+	results, err := client.DestroyMachinesWithParams(true, true, "0", "0/lxd/1")
+	c.Assert(err, jc.ErrorIsNil)
+	c.Assert(results, jc.DeepEquals, expectedResults)
+}
diff --git a/api/provisioner/machine.go b/api/provisioner/machine.go
@@ -6,6 +6,7 @@ package provisioner
 import (
 	"fmt"
 
+	"github.com/juju/errors"
 	"gopkg.in/juju/names.v2"
 
 	apiwatcher "github.com/juju/juju/api/watcher"
@@ -280,6 +281,30 @@ func (m *Machine) InstanceId() (instance.Id, error) {
 	return instance.Id(result.Result), nil
 }
 
+// KeepInstance returns the value of the keep-instance
+// for the machine.
+func (m *Machine) KeepInstance() (bool, error) {
+	var results params.BoolResults
+	args := params.Entities{
+		Entities: []params.Entity{{Tag: m.tag.String()}},
+	}
+	err := m.st.facade.FacadeCall("KeepInstance", args, &results)
+	if err != nil {
+		return false, err
+	}
+	if len(results.Results) != 1 {
+		return false, fmt.Errorf("expected 1 result, got %d", len(results.Results))
+	}
+	result := results.Results[0]
+	if result.Error != nil {
+		if params.IsCodeNotSupported(err) {
+			return false, errors.NewNotSupported(nil, "KeepInstance")
+		}
+		return false, result.Error
+	}
+	return result.Result, nil
+}
+
 // SetPassword sets the machine's password.
 func (m *Machine) SetPassword(password string) error {
 	var result params.ErrorResults

diff --git a/api/provisioner/provisioner_test.go b/api/provisioner/provisioner_test.go
@@ -357,6 +357,16 @@ func (s *provisionerSuite) TestSeries(c *gc.C) {
 	c.Assert(series, gc.Equals, "quantal")
 }
 
+func (s *provisionerSuite) TestKeepInstance(c *gc.C) {
+	err := s.machine.SetKeepInstance(true)
+	c.Assert(err, jc.ErrorIsNil)
+	apiMachine, err := s.provisioner.Machine(s.machine.Tag().(names.MachineTag))
+	c.Assert(err, jc.ErrorIsNil)
+	keep, err := apiMachine.KeepInstance()
+	c.Assert(err, jc.ErrorIsNil)
+	c.Assert(keep, jc.IsTrue)
+}
+
 func (s *provisionerSuite) TestDistributionGroup(c *gc.C) {
 	apiMachine, err := s.provisioner.Machine(s.machine.Tag().(names.MachineTag))
 	c.Assert(err, jc.ErrorIsNil)

diff --git a/apiserver/allfacades.go b/apiserver/allfacades.go
@@ -151,7 +151,8 @@ func AllFacades() *facade.Registry {
 	reg("MachineActions", 1, machineactions.NewExternalFacade)
 
 	reg("MachineManager", 2, machinemanager.NewMachineManagerAPI)
-	reg("MachineManager", 3, machinemanager.NewMachineManagerAPI) // Version 3 adds DestroyMachine and ForceDestroyMachine.
+	reg("MachineManager", 3, machinemanager.NewMachineManagerAPI)   // Version 3 adds DestroyMachine and ForceDestroyMachine.
+	reg("MachineManager", 4, machinemanager.NewMachineManagerAPIV4) // Version 4 adds DestroyMachineWithParams.
 
 	reg("MachineUndertaker", 1, machineundertaker.NewFacade)
 	reg("Machiner", 1, machine.NewMachinerAPI)

diff --git a/apiserver/machinemanager/machinemanager.go b/apiserver/machinemanager/machinemanager.go
@@ -7,6 +7,7 @@ import (
 	"fmt"
 
 	"github.com/juju/errors"
+	"github.com/juju/loggo"
 	"github.com/juju/utils/set"
 	"gopkg.in/juju/names.v2"
 
@@ -19,17 +20,37 @@ import (
 	"github.com/juju/juju/state"
 )
 
+var logger = loggo.GetLogger("juju.apiserver.machinemanager")
+
 // MachineManagerAPI provides access to the MachineManager API facade.
 type MachineManagerAPI struct {
 	st         stateInterface
 	authorizer facade.Authorizer
 	check      *common.BlockChecker
 }
 
+type MachineManagerAPIV4 struct {
+	*MachineManagerAPI
+}
+
 var getState = func(st *state.State) stateInterface {
 	return stateShim{st}
 }
 
+// NewMachineManagerAPIV4 creates a new server-side MachineManager API facade.
+func NewMachineManagerAPIV4(
+	st *state.State,
+	resources facade.Resources,
+	authorizer facade.Authorizer,
+) (*MachineManagerAPIV4, error) {
+
+	machineManagerAPI, err := NewMachineManagerAPI(st, resources, authorizer)
+	if err != nil {
+		return nil, errors.Trace(err)
+	}
+	return &MachineManagerAPIV4{machineManagerAPI}, nil
+}
+
 // NewMachineManagerAPI creates a new server-side MachineManager API facade.
 func NewMachineManagerAPI(
 	st *state.State,
@@ -174,15 +195,25 @@ func (mm *MachineManagerAPI) addOneMachine(p params.AddMachineParams) (*state.Ma
 
 // DestroyMachine removes a set of machines from the model.
 func (mm *MachineManagerAPI) DestroyMachine(args params.Entities) (params.DestroyMachineResults, error) {
-	return mm.destroyMachine(args, false)
+	return mm.destroyMachine(args, false, false)
 }
 
 // ForceDestroyMachine forcibly removes a set of machines from the model.
 func (mm *MachineManagerAPI) ForceDestroyMachine(args params.Entities) (params.DestroyMachineResults, error) {
-	return mm.destroyMachine(args, true)
+	return mm.destroyMachine(args, true, false)
 }
 
-func (mm *MachineManagerAPI) destroyMachine(args params.Entities, force bool) (params.DestroyMachineResults, error) {
+// DestroyMachineWithParams removes a set of machines from the model.
+func (mm *MachineManagerAPIV4) DestroyMachineWithParams(args params.DestroyMachinesParams) (params.DestroyMachineResults, error) {
+	logger.Criticalf("%+v", args)
+	entities := params.Entities{Entities: make([]params.Entity, len(args.MachineTags))}
+	for i, tag := range args.MachineTags {
+		entities.Entities[i].Tag = tag
+	}
+	return mm.destroyMachine(entities, args.Force, args.Keep)
+}
+
+func (mm *MachineManagerAPI) destroyMachine(args params.Entities, force, keep bool) (params.DestroyMachineResults, error) {
 	if err := mm.checkCanWrite(); err != nil {
 		return params.DestroyMachineResults{}, err
 	}
@@ -198,6 +229,12 @@ func (mm *MachineManagerAPI) destroyMachine(args params.Entities, force bool) (p
 		if err != nil {
 			return nil, err
 		}
+		if keep {
+			logger.Infof("destroy machine %v but keep instance", machineTag.Id())
+			if err := machine.SetKeepInstance(keep); err != nil {
+				return nil, err
+			}
+		}
 		var info params.DestroyMachineInfo
 		units, err := machine.Units()
 		if err != nil {

diff --git a/apiserver/machinemanager/machinemanager_test.go b/apiserver/machinemanager/machinemanager_test.go
@@ -141,6 +141,31 @@ func (s *MachineManagerSuite) TestDestroyMachine(c *gc.C) {
 	})
 }
 
+func (s *MachineManagerSuite) TestDestroyMachineWithParams(c *gc.C) {
+	apiV4 := machinemanager.MachineManagerAPIV4{s.api}
+	results, err := apiV4.DestroyMachineWithParams(params.DestroyMachinesParams{
+		Keep:        true,
+		Force:       true,
+		MachineTags: []string{"machine-0"},
+	})
+	c.Assert(err, jc.ErrorIsNil)
+	c.Assert(results, jc.DeepEquals, params.DestroyMachineResults{
+		Results: []params.DestroyMachineResult{{
+			Info: &params.DestroyMachineInfo{
+				DestroyedUnits: []params.Entity{
+					{"unit-foo-0"},
+					{"unit-foo-1"},
+					{"unit-foo-2"},
+				},
+				DestroyedStorage: []params.Entity{
+					{"storage-disks-0"},
+					{"storage-disks-1"},
+				},
+			},
+		}},
+	})
+}
+
 type mockState struct {
 	calls    int
 	machines []state.MachineTemplate
@@ -250,6 +275,10 @@ func (m *mockMachine) ForceDestroy() error {
 	return nil
 }
 
+func (m *mockMachine) SetKeepInstance(keep bool) error {
+	return nil
+}
+
 func (m *mockMachine) Units() ([]machinemanager.Unit, error) {
 	return []machinemanager.Unit{
 		&mockUnit{names.NewUnitTag("foo/0")},

diff --git a/apiserver/machinemanager/state.go b/apiserver/machinemanager/state.go
@@ -91,6 +91,7 @@ type Machine interface {
 	Destroy() error
 	ForceDestroy() error
 	Units() ([]Unit, error)
+	SetKeepInstance(bool) error
 }
 
 type machineShim struct {

diff --git a/apiserver/params/params.go b/apiserver/params/params.go
@@ -201,11 +201,20 @@ type AddMachinesResult struct {
 }
 
 // DestroyMachines holds parameters for the DestroyMachines call.
+// This is the legacy params struct used with the client facade.
+// TODO(wallyworld) - remove in Juju 3.0
 type DestroyMachines struct {
 	MachineNames []string `json:"machine-names"`
 	Force        bool     `json:"force"`
 }
 
+// DestroyMachinesParams holds parameters for the DestroyMachinesWithParams call.
+type DestroyMachinesParams struct {
+	MachineTags []string `json:"machine-tags"`
+	Force       bool     `json:"force,omitempty"`
+	Keep        bool     `json:"keep,omitempty"`
+}
+
 // ApplicationsDeploy holds the parameters for deploying one or more applications.
 type ApplicationsDeploy struct {
 	Applications []ApplicationDeploy `json:"applications"`

diff --git a/apiserver/provisioner/provisioner.go b/apiserver/provisioner/provisioner.go
@@ -348,6 +348,33 @@ func (p *ProvisionerAPI) Series(args params.Entities) (params.StringResults, err
 	return result, nil
 }
 
+// KeepInstance returns the keep-instance value for each given machine entity.
+func (p *ProvisionerAPI) KeepInstance(args params.Entities) (params.BoolResults, error) {
+	result := params.BoolResults{
+
+		Results: make([]params.BoolResult, len(args.Entities)),
+	}
+	canAccess, err := p.getAuthFunc()
+	if err != nil {
+		return result, err
+	}
+	for i, entity := range args.Entities {
+		tag, err := names.ParseMachineTag(entity.Tag)
+		if err != nil {
+			result.Results[i].Error = common.ServerError(common.ErrPerm)
+			continue
+		}
+		machine, err := p.getMachine(canAccess, tag)
+		if err == nil {
+			keep, err := machine.KeepInstance()
+			result.Results[i].Result = keep
+			result.Results[i].Error = common.ServerError(err)
+		}
+		result.Results[i].Error = common.ServerError(err)
+	}
+	return result, nil
+}
+
 // DistributionGroup returns, for each given machine entity,
 // a slice of instance.Ids that belong to the same distribution
 // group as that machine. This information may be used to