Repository: m3db/m3db-operator
Pull request #312 (2022-03-04 10:34:39)

move to batch placement remove API
Moves to a newer batch remove API. Previously delete would call same API under the hood [here](https://github.com/m3db/m3/blob/master/src/cluster/placementhandler/delete.go#L129) and [here](https://github.com/m3db/m3/blob/master/src/cluster/placementhandler/remove.go#L145).


Patch:
diff --git a/pkg/controller/m3admin_client.go b/pkg/controller/m3admin_client.go
index e896efa..801ce87 100644
--- a/pkg/controller/m3admin_client.go
+++ b/pkg/controller/m3admin_client.go
@@ -273,7 +273,7 @@ func (c errorPlacementClient) Add([]*placementpb.Instance) error {
 	return c.err
 }
 
-func (c errorPlacementClient) Remove(string) error {
+func (c errorPlacementClient) Remove([]string) error {
 	return c.err
 }
 
diff --git a/pkg/controller/update_cluster.go b/pkg/controller/update_cluster.go
index 2c5facf..6ed2589 100644
--- a/pkg/controller/update_cluster.go
+++ b/pkg/controller/update_cluster.go
@@ -563,7 +563,7 @@ func (c *M3DBController) shrinkPlacementForSet(
 	}
 
 	c.logger.Info("removing pod from placement", zap.String("instance", removeInst.ID()))
-	return c.adminClient.placementClientForCluster(cluster).Remove(removeInst.ID())
+	return c.adminClient.placementClientForCluster(cluster).Remove([]string{removeInst.ID()})
 }
 
 // findPodInstanceToRemove returns the pod (and associated placement instace)
diff --git a/pkg/m3admin/placement/client.go b/pkg/m3admin/placement/client.go
index ea70bff..b16f4a4 100644
--- a/pkg/m3admin/placement/client.go
+++ b/pkg/m3admin/placement/client.go
@@ -22,7 +22,6 @@ package placement
 
 import (
 	"errors"
-	"fmt"
 	"net/http"
 
 	"github.com/m3db/m3db-operator/pkg/m3admin"
@@ -38,7 +37,7 @@ const (
 	placementBaseURL    = "/api/v1/services/m3db/placement"
 	placementInitURL    = placementBaseURL + "/init"
 	placementReplaceURL = placementBaseURL + "/replace"
-	placementRemoveFmt  = placementBaseURL + "/%s"
+	placementRemoveURL  = placementBaseURL + "/remove"
 	placementSetURL     = placementBaseURL + "/set"
 )
 
@@ -129,9 +128,12 @@ func (p *placementClient) Add(instances []*placementpb.Instance) error {
 	return nil
 }
 
-func (p *placementClient) Remove(id string) error {
-	url := fmt.Sprintf(p.url+placementRemoveFmt, id)
-	return p.client.DoHTTPJSONPBRequest(http.MethodDelete, url, nil, nil)
+func (p *placementClient) Remove(instanceIds []string) error {
+	url := p.url + placementRemoveURL
+	req := &admin.PlacementRemoveRequest{
+		InstanceIds: instanceIds,
+	}
+	return p.client.DoHTTPJSONPBRequest(http.MethodPost, url, req, nil)
 }
 
 func (p *placementClient) Replace(leavingInstanceID string, newInst placementpb.Instance) error {
diff --git a/pkg/m3admin/placement/client_mock.go b/pkg/m3admin/placement/client_mock.go
index 236a47c..be57d8d 100644
--- a/pkg/m3admin/placement/client_mock.go
+++ b/pkg/m3admin/placement/client_mock.go
@@ -115,17 +115,17 @@ func (mr *MockClientMockRecorder) Init(request interface{}) *gomock.Call {
 }
 
 // Remove mocks base method.
-func (m *MockClient) Remove(id string) error {
+func (m *MockClient) Remove(instanceIds []string) error {
 	m.ctrl.T.Helper()
-	ret := m.ctrl.Call(m, "Remove", id)
+	ret := m.ctrl.Call(m, "Remove", instanceIds)
 	ret0, _ := ret[0].(error)
 	return ret0
 }
 
 // Remove indicates an expected call of Remove.
-func (mr *MockClientMockRecorder) Remove(id interface{}) *gomock.Call {
+func (mr *MockClientMockRecorder) Remove(instanceIds interface{}) *gomock.Call {
 	mr.mock.ctrl.T.Helper()
-	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Remove", reflect.TypeOf((*MockClient)(nil).Remove), id)
+	return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Remove", reflect.TypeOf((*MockClient)(nil).Remove), instanceIds)
 }
 
 // Replace mocks base method.
diff --git a/pkg/m3admin/placement/types.go b/pkg/m3admin/placement/types.go
index 113168b..36a4f30 100644
--- a/pkg/m3admin/placement/types.go
+++ b/pkg/m3admin/placement/types.go
@@ -34,12 +34,12 @@ type Client interface {
 	Set(request *admin.PlacementSetRequest) error
 	// Get will provide the current placement
 	Get() (placement m3placement.Placement, err error)
-	// Delete will delete the current placment
+	// Delete will delete the current placement
 	Delete() error
 	// Add will add an instance to the placement
 	Add(instances []*placementpb.Instance) error
-	// Remove removes a given instance with the given ID from the placement.
-	Remove(id string) error
+	// Remove removes instances with the given IDs from the placement.
+	Remove(instanceIds []string) error
 	// Replace replaces one instance with another.
 	Replace(leavingInstanceID string, newInstance placementpb.Instance) error
 }


Test patch:
diff --git a/pkg/controller/m3admin_client_test.go b/pkg/controller/m3admin_client_test.go
index ef71602..7ee4373 100644
--- a/pkg/controller/m3admin_client_test.go
+++ b/pkg/controller/m3admin_client_test.go
@@ -242,7 +242,7 @@ func TestErrorPlacementClient(t *testing.T) {
 	err = cl.Add([]*placementpb.Instance{{}})
 	assert.Equal(t, clErr, err)
 
-	err = cl.Remove("foo")
+	err = cl.Remove([]string{"foo"})
 	assert.Equal(t, clErr, err)
 
 	err = cl.Replace("foo", placementpb.Instance{})
diff --git a/pkg/controller/update_cluster_test.go b/pkg/controller/update_cluster_test.go
index e17ecff..7806be7 100644
--- a/pkg/controller/update_cluster_test.go
+++ b/pkg/controller/update_cluster_test.go
@@ -695,14 +695,14 @@ func TestShrinkPlacementForSet(t *testing.T) {
 	pl := placementFromPods(t, cluster, pods, deps.idProvider)
 
 	// Expect the last pod to be removed.
-	placementMock.EXPECT().Remove(`{"name":"cluster-zones-rep0-2","uid":"2"}`)
+	placementMock.EXPECT().Remove([]string{`{"name":"cluster-zones-rep0-2","uid":"2"}`})
 	err = controller.shrinkPlacementForSet(cluster, set, pl)
 	assert.NoError(t, err)
 
 	// If there are more pods in the set then in the placement, we expect the last
 	// in the set to be removed.
 	pl = placementFromPods(t, cluster, pods[:2], deps.idProvider)
-	placementMock.EXPECT().Remove(`{"name":"cluster-zones-rep0-1","uid":"1"}`)
+	placementMock.EXPECT().Remove([]string{`{"name":"cluster-zones-rep0-1","uid":"1"}`})
 	err = controller.shrinkPlacementForSet(cluster, set, pl)
 	assert.NoError(t, err)
 }
diff --git a/pkg/m3admin/placement/client_test.go b/pkg/m3admin/placement/client_test.go
index 016eac0..ce0c029 100644
--- a/pkg/m3admin/placement/client_test.go
+++ b/pkg/m3admin/placement/client_test.go
@@ -201,7 +201,7 @@ func TestGetErr(t *testing.T) {
 
 func TestRemove(t *testing.T) {
 	s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		if r.URL.String() != "/api/v1/services/m3db/placement/instFoo" || r.Method != http.MethodDelete {
+		if r.URL.String() != "/api/v1/services/m3db/placement/remove" || r.Method != http.MethodPost {
 			w.WriteHeader(404)
 			return
 		}
@@ -211,7 +211,7 @@ func TestRemove(t *testing.T) {
 	defer s.Close()
 
 	client := newPlacementClient(t, s.URL)
-	err := client.Remove("instFoo")
+	err := client.Remove([]string{"instFoo"})
 	assert.NoError(t, err)
 }