[controller] fix incorrect label selectors

We had been constructing the wrong selectors when querying children pods if the cluster had custom labels.
m3db · Mar 10, 2019 · d4bab48 · d4bab48
1 parent 5eb9f37
commit d4bab48
Show file tree

Hide file tree

Showing 6 changed files with 69 additions and 11 deletions.
diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go
@@ -564,7 +564,8 @@ func instancesInIsoGroup(pl m3placement.Placement, isoGroup string) []m3placemen
 // This func is currently read-only, but if we end up modifying statefulsets
 // we'll have to deepcopy.
 func (c *Controller) getChildStatefulSets(cluster *myspec.M3DBCluster) ([]*appsv1.StatefulSet, error) {
-	statefulSets, err := c.statefulSetLister.StatefulSets(cluster.Namespace).List(klabels.Set(cluster.Labels).AsSelector())
+	labels := labels.BaseLabels(cluster)
+	statefulSets, err := c.statefulSetLister.StatefulSets(cluster.Namespace).List(klabels.Set(labels).AsSelector())
 	if err != nil {
 		runtime.HandleError(fmt.Errorf("error listing statefulsets: %v", err))
 		return nil, err

diff --git a/pkg/controller/controller_test.go b/pkg/controller/controller_test.go
@@ -97,10 +97,25 @@ func TestGetChildStatefulSets(t *testing.T) {
 		{
 			cluster: newMeta("cluster1", map[string]string{"foo": "bar"}),
 			sets: []*metav1.ObjectMeta{
-				newMeta("set1", map[string]string{"foo": "bar"}),
+				newMeta("set1", map[string]string{
+					"foo": "bar",
+					"operator.m3db.io/app":     "m3db",
+					"operator.m3db.io/cluster": "cluster1",
+				}),
 			},
 			expChildren: []string{"set1"},
 		},
+		{
+			cluster: newMeta("cluster1", map[string]string{"foo": "bar"}),
+			sets: []*metav1.ObjectMeta{
+				newMeta("set1", map[string]string{
+					"foo": "bar",
+					"operator.m3db.io/app":     "m3db",
+					"operator.m3db.io/cluster": "cluster2",
+				}),
+			},
+			expChildren: []string{},
+		},
 	}
 
 	for _, test := range tests {

diff --git a/pkg/controller/update_cluster.go b/pkg/controller/update_cluster.go
@@ -180,8 +180,8 @@ func (c *Controller) validatePlacementWithStatus(cluster *myspec.M3DBCluster) (*
 		ReplicationFactor: cluster.Spec.ReplicationFactor,
 	}
 
-	targetLabels := map[string]string{}
-	for k, v := range cluster.Labels {
+	targetLabels := labels.BaseLabels(cluster)
+	for k, v := range cluster.Spec.Labels {
 		targetLabels[k] = v
 	}
 	targetLabels[labels.Component] = labels.ComponentM3DBNode

diff --git a/pkg/controller/update_cluster_test.go b/pkg/controller/update_cluster_test.go
@@ -23,6 +23,7 @@ package controller
 import (
 	"errors"
 	"fmt"
+	"reflect"
 	"sort"
 	"strconv"
 	"testing"
@@ -438,7 +439,7 @@ func TestExpandPlacementForSet(t *testing.T) {
 
 	pods := podsForClusterSet(cluster, set, 3)
 	deps := newTestDeps(t, &testOpts{
-		kubeObjects: append(objectsFromPods(pods...)),
+		kubeObjects: objectsFromPods(pods...),
 		crdObjects:  []runtime.Object{cluster},
 	})
 	placementMock := deps.placementClient
@@ -561,19 +562,62 @@ func TestValidatePlacementWithStatus(t *testing.T) {
 	require.NotNil(t, clusterReturn)
 }
 
+type placementInstancesMatcher struct {
+	instanceNames []string
+}
+
+func (p placementInstancesMatcher) Matches(x interface{}) bool {
+	pl := x.(*admin.PlacementInitRequest)
+	plInsts := []string{}
+	for _, inst := range pl.Instances {
+		plInsts = append(plInsts, inst.Id)
+	}
+
+	sort.Strings(p.instanceNames)
+	sort.Strings(plInsts)
+
+	fmt.Println(plInsts)
+	fmt.Println(p.instanceNames)
+
+	return reflect.DeepEqual(p.instanceNames, plInsts)
+}
+
+func (p placementInstancesMatcher) String() string {
+	return fmt.Sprintf("matches whether instance names are equal to %v", p.instanceNames)
+}
+
 func TestValidatePlacementWithStatus_ErrNotFound(t *testing.T) {
 	cluster := getFixture("cluster-3-zones.yaml", t)
+	set, err := k8sops.GenerateStatefulSet(cluster, "us-fake1-a", 3)
+	require.NoError(t, err)
+	set.Status.ReadyReplicas = 3
+	pods := podsForClusterSet(cluster, set, 3)
+
 	deps := newTestDeps(t, &testOpts{
-		crdObjects: []runtime.Object{cluster},
+		kubeObjects: objectsFromPods(pods...),
+		crdObjects:  []runtime.Object{cluster},
 	})
 
 	placementMock := deps.placementClient
 	defer deps.cleanup()
 
 	controller := deps.newController()
 
+	idProvider := deps.idProvider
+	expInsts := []string{
+		`{"name":"cluster-zones-rep0-0","uid":"0"}`,
+		`{"name":"cluster-zones-rep0-1","uid":"1"}`,
+		`{"name":"cluster-zones-rep0-2","uid":"2"}`,
+	}
+	for _, pod := range pods {
+		pod := pod
+		idProvider.EXPECT().Identity(newPodNameMatcher(pod.Name), gomock.Any()).Return(identityForPod(pod), nil).AnyTimes()
+	}
+	matcher := placementInstancesMatcher{
+		instanceNames: expInsts,
+	}
 	placementMock.EXPECT().Get().Return(nil, pkgerrors.Wrap(m3admin.ErrNotFound, "foo"))
-	placementMock.EXPECT().Init(gomock.Any())
+	placementMock.EXPECT().Init(matcher)
 
 	clusterReturn, err := controller.validatePlacementWithStatus(cluster)
 

diff --git a/pkg/k8sops/generators_test.go b/pkg/k8sops/generators_test.go
@@ -122,8 +122,7 @@ func TestGenerateStatefulSet(t *testing.T) {
 			},
 		},
 		Spec: appsv1.StatefulSetSpec{
-			ServiceName:         "m3dbnode-m3db-cluster",
-			PodManagementPolicy: "Parallel",
+			ServiceName: "m3dbnode-m3db-cluster",
 			Selector: &metav1.LabelSelector{
 				MatchLabels: labels,
 			},

diff --git a/pkg/k8sops/statefulset.go b/pkg/k8sops/statefulset.go
@@ -229,8 +229,7 @@ func NewBaseStatefulSet(ssName, isolationGroup string, cluster *myspec.M3DBClust
 			Labels: objLabels,
 		},
 		Spec: appsv1.StatefulSetSpec{
-			ServiceName:         HeadlessServiceName(clusterName),
-			PodManagementPolicy: "Parallel",
+			ServiceName: HeadlessServiceName(clusterName),
 			Selector: &metav1.LabelSelector{
 				MatchLabels: objLabels,
 			},