From d4bab4833151e432d8e60ca58ac69b28ddfc5935 Mon Sep 17 00:00:00 2001 From: Matt Schallert Date: Fri, 8 Mar 2019 23:37:54 -0500 Subject: [PATCH] [controller] fix incorrect label selectors We had been constructing the wrong selectors when querying children pods if the cluster had custom labels. --- pkg/controller/controller.go | 3 +- pkg/controller/controller_test.go | 17 ++++++++- pkg/controller/update_cluster.go | 4 +-- pkg/controller/update_cluster_test.go | 50 +++++++++++++++++++++++++-- pkg/k8sops/generators_test.go | 3 +- pkg/k8sops/statefulset.go | 3 +- 6 files changed, 69 insertions(+), 11 deletions(-) diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go index 3cce68a9..2aede64c 100644 --- 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 index bbe142df..d8c3aec9 100644 --- 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 index 6aaea5d4..02a8601f 100644 --- 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 index 745ed5fb..934c2847 100644 --- 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,10 +562,40 @@ 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 @@ -572,8 +603,21 @@ func TestValidatePlacementWithStatus_ErrNotFound(t *testing.T) { 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 index 9b53023c..20803be3 100644 --- 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 index 4ebcb38c..bfa87c7b 100644 --- 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, },