[controller] Support multi instance placement add

[notbdu]

Modifies the placement client Add() API to accept multiple instances.

[codecov]

Codecov Report

Merging #275 (9447ac0) into master (7a6466e) will increase coverage by 0.00%.
The diff coverage is 63.82%.

@@           Coverage Diff           @@
##           master     #275   +/-   ##
=======================================
  Coverage   76.01%   76.02%           
=======================================
  Files          32       32           
  Lines        2381     2394   +13     
=======================================
+ Hits         1810     1820   +10     
- Misses        427      429    +2     
- Partials      144      145    +1     

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7a6466e...9447ac0. Read the comment docs.

[schallert]

LGTM. A few minor nits.

One general question: do our current "ready replica" checks guarantee that we'll add all pods at once, rather than accidentally adding one at a time?

For example, say we scale a set from 2 to 4 pods. At time T1, 1/2 new pods is up. Are our readiness checks structured in such a way that we won't immediate add the 3rd pod, and will wait until we can evaluate the two new ones at once?

[schallert]

Nit: take advantage of structured logging here (i.e. logger.Error("error creating instance for pod", zap.String("pod", ...

[schallert]

This is a pretty different convention than how build up logs, and this isn't very perf-sensitive code.

I would vote for just adding to an array of strings, which are the names of the instances / pods, and then calling zap.Strings.

[schallert]

Same thing here re: taking the change to structure the below .Error log.

[schallert]

Nit: this ls already imported as corev1 on the line above.

[notbdu]

In practice, I observed the operator bringing up a pod at a time (waiting for each to be ready before proceeding) and then expanding the placement when all requested pods are up/ready/bootstrapped (w/ no shards). I observed this pattern testing in rc scaling from 2 -> 4 and 2 -> 6 nodes.

In the code it looks like we add a pod at a time here:

m3db-operator/pkg/controller/controller.go

Lines 710 to 716 in 7a6466e

	var newCount int32
	if current < desired {
	newCount = current + 1
	} else {
	newCount = current - 1
	}
	setLogger.Info("resizing set, desired != current", zap.Int32("newSize", newCount))

And then when the number of desired == current pods we expand the placement here (placement shouldn't be expanded earlier than this point):

m3db-operator/pkg/controller/controller.go

Lines 688 to 701 in 7a6466e

	if desired == current {
	// If the set is at its desired size, and all pods in the set are in the
	// placement, there's nothing we need to do for this set.
	if current == inPlacement {
	continue
	}
	// If the set is at its desired size but there's pods in the set that are
	// absent from the placement, add pods to placement.
	if inPlacement < current {
	setLogger.Info("expanding placement for set")
	return c.expandPlacementForSet(cluster, set, group, placement)
	}
	}

Can observe this in the bootstrap metrics as well (2 -> 4 and then 2 -> 6):