Merge pull request juju#10325 from manadart/2.6-ha-partial-placement

jujubot · web-flow · commit 470bfa59b3c0 · 2019-06-13T12:30:58.000+01:00
juju#10325 ## Description of change Backport of juju#10185 from the develop branch.
diff --git a/apiserver/facades/client/highavailability/highavailability.go b/apiserver/facades/client/highavailability/highavailability.go
@@ -170,11 +170,11 @@ func getReferenceController(st *state.State, machineIds []string) (*state.Machin
 	controllerId := controllerIds[0]
 
 	// Load the controller machine and get its constraints.
-	controller, err := st.Machine(strconv.Itoa(controllerId))
+	cm, err := st.Machine(strconv.Itoa(controllerId))
 	if err != nil {
 		return nil, errors.Annotatef(err, "reading controller id %v", controllerId)
 	}
-	return controller, nil
+	return cm, nil
 }
 
 // validateCurrentControllers checks for a scenario where there is no HA space
@@ -189,11 +189,11 @@ func validateCurrentControllers(st *state.State, cfg controller.Config, machineI
 
 	var badIds []string
 	for _, id := range machineIds {
-		controller, err := st.Machine(id)
+		cm, err := st.Machine(id)
 		if err != nil {
 			return errors.Annotatef(err, "reading controller id %v", id)
 		}
-		addresses := controller.Addresses()
+		addresses := cm.Addresses()
 		if len(addresses) == 0 {
 			// machines without any address are essentially not started yet
 			continue
diff --git a/cmd/juju/commands/enableha.go b/cmd/juju/commands/enableha.go
@@ -227,7 +227,7 @@ func (c *enableHACommand) Run(ctx *cmd.Context) error {
 		return err
 	}
 
-	defer haClient.Close()
+	defer func() { _ = haClient.Close() }()
 	enableHAResult, err := haClient.EnableHA(
 		c.NumControllers,
 		c.Constraints,
diff --git a/state/enableha.go b/state/enableha.go
@@ -184,18 +184,20 @@ func (st *State) enableHAIntentionOps(
 ) ([]txn.Op, ControllersChanges, error) {
 	var ops []txn.Op
 	var change ControllersChanges
+
 	for _, m := range intent.promote {
 		ops = append(ops, promoteControllerOps(m)...)
 		change.Promoted = append(change.Promoted, m.doc.Id)
 	}
+
 	for _, m := range intent.convert {
 		ops = append(ops, convertControllerOps(m)...)
 		change.Converted = append(change.Converted, m.doc.Id)
 	}
-	// Use any placement directives that have been provided
-	// when adding new machines, until the directives have
-	// been all used up. Ignore constraints for provided machines.
-	// Set up a helper function to do the work required.
+
+	// Use any placement directives that have been provided when adding new
+	// machines, until the directives have been all used up.
+	// Ignore constraints for provided machines.
 	placementCount := 0
 	getPlacementConstraints := func() (string, constraints.Value) {
 		if placementCount >= len(intent.placement) {
@@ -205,6 +207,7 @@ func (st *State) enableHAIntentionOps(
 		placementCount++
 		return result, constraints.Value{}
 	}
+
 	mdocs := make([]*machineDoc, intent.newCount)
 	for i := range mdocs {
 		placement, cons := getPlacementConstraints()
@@ -224,8 +227,8 @@ func (st *State) enableHAIntentionOps(
 		mdocs[i] = mdoc
 		ops = append(ops, addOps...)
 		change.Added = append(change.Added, mdoc.Id)
-
 	}
+
 	for _, m := range intent.maintain {
 		tag, err := names.ParseTag(m.Tag().String())
 		if err != nil {
@@ -258,15 +261,21 @@ type enableHAIntent struct {
 func (st *State) enableHAIntentions(info *ControllerInfo, placement []string) (*enableHAIntent, error) {
 	var intent enableHAIntent
 	for _, s := range placement {
-		// TODO(natefinch): unscoped placements shouldn't ever get here (though
-		// they do currently).  We should fix up the CLI to always add a scope
-		// to placements and then we can remove the need to deal with unscoped
-		// placements.
+		// TODO(natefinch): Unscoped placements can end up here, though they
+		// should not. We should fix up the CLI to always add a scope,
+		// then we can remove the need to deal with unscoped placements.
+
+		// Append unscoped placements to the intentions.
+		// These will be used if/when adding new controllers is required.
+		// These placements will be interpreted as availability zones.
 		p, err := instance.ParsePlacement(s)
 		if err == instance.ErrPlacementScopeMissing {
 			intent.placement = append(intent.placement, s)
 			continue
 		}
+
+		// Placements for machines are "consumed" by appending such machines as
+		// candidates for promotion to controllers.
 		if err == nil && p.Scope == instance.MachineScope {
 			if names.IsContainerMachine(p.Directive) {
 				return nil, errors.New("container placement directives not supported")
@@ -280,7 +289,6 @@ func (st *State) enableHAIntentions(info *ControllerInfo, placement []string) (*
 				return nil, errors.Errorf("machine for placement directive %q is already a controller", s)
 			}
 			intent.convert = append(intent.convert, m)
-			intent.placement = append(intent.placement, s)
 			continue
 		}
 		return nil, errors.Errorf("unsupported placement directive %q", s)
diff --git a/state/enableha_test.go b/state/enableha_test.go

Original file line number	Diff line number	Diff line change
`@@ -227,7 +227,7 @@ func (c enableHACommand) Run(ctx cmd.Context) error {`
`227`	`227`	`return err`
`228`	`228`	`}`
`229`	`229`
`230`		`- defer haClient.Close()`
	`230`	`+ defer func() { _ = haClient.Close() }()`
`231`	`231`	`enableHAResult, err := haClient.EnableHA(`
`232`	`232`	`c.NumControllers,`
`233`	`233`	`c.Constraints,`