Move service affinity predicate logic to its plugin.

Author: ahg-g

pkg/scheduler/algorithm/predicates/metadata.go

@@ -18,17 +18,15 @@ package predicates
 
 import (
 	"context"
-	"fmt"
 	"math"
 	"sync"
 
-	"k8s.io/klog"
-
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/client-go/util/workqueue"
+	"k8s.io/klog"
 	priorityutil "k8s.io/kubernetes/pkg/scheduler/algorithm/priorities/util"
 	schedulerlisters "k8s.io/kubernetes/pkg/scheduler/listers"
 	schedulernodeinfo "k8s.io/kubernetes/pkg/scheduler/nodeinfo"
@@ -129,56 +127,6 @@ type topologySpreadConstraint struct {
 	selector    labels.Selector
 }
 
-type serviceAffinityMetadata struct {
-	matchingPodList     []*v1.Pod
-	matchingPodServices []*v1.Service
-}
-
-func (m *serviceAffinityMetadata) addPod(addedPod *v1.Pod, pod *v1.Pod, node *v1.Node) {
-	// If addedPod is in the same namespace as the pod, update the list
-	// of matching pods if applicable.
-	if m == nil || addedPod.Namespace != pod.Namespace {
-		return
-	}
-
-	selector := CreateSelectorFromLabels(pod.Labels)
-	if selector.Matches(labels.Set(addedPod.Labels)) {
-		m.matchingPodList = append(m.matchingPodList, addedPod)
-	}
-}
-
-func (m *serviceAffinityMetadata) removePod(deletedPod *v1.Pod, node *v1.Node) {
-	deletedPodFullName := schedutil.GetPodFullName(deletedPod)
-
-	if m == nil ||
-		len(m.matchingPodList) == 0 ||
-		deletedPod.Namespace != m.matchingPodList[0].Namespace {
-		return
-	}
-
-	for i, pod := range m.matchingPodList {
-		if schedutil.GetPodFullName(pod) == deletedPodFullName {
-			m.matchingPodList = append(m.matchingPodList[:i], m.matchingPodList[i+1:]...)
-			break
-		}
-	}
-}
-
-func (m *serviceAffinityMetadata) clone() *serviceAffinityMetadata {
-	if m == nil {
-		return nil
-	}
-
-	copy := serviceAffinityMetadata{}
-
-	copy.matchingPodServices = append([]*v1.Service(nil),
-		m.matchingPodServices...)
-	copy.matchingPodList = append([]*v1.Pod(nil),
-		m.matchingPodList...)
-
-	return &copy
-}
-
 // PodAffinityMetadata pre-computed state for inter-pod affinity predicate.
 type PodAffinityMetadata struct {
 	// A map of topology pairs to the number of existing pods that has anti-affinity terms that match the "pod".
@@ -283,10 +231,8 @@ func (m *PodAffinityMetadata) Clone() *PodAffinityMetadata {
 
 // NOTE: When new fields are added/removed or logic is changed, please make sure that
 // RemovePod, AddPod, and ShallowCopy functions are updated to work with the new changes.
+// TODO(ahg-g): remove, not use anymore.
 type predicateMetadata struct {
-	pod *v1.Pod
-
-	serviceAffinityMetadata *serviceAffinityMetadata
 }
 
 // Ensure that predicateMetadata implements algorithm.Metadata.
@@ -318,9 +264,7 @@ func (f *MetadataProducerFactory) GetPredicateMetadata(pod *v1.Pod, sharedLister
 		return nil
 	}
 
-	predicateMetadata := &predicateMetadata{
-		pod: pod,
-	}
+	predicateMetadata := &predicateMetadata{}
 	for predicateName, precomputeFunc := range predicateMetadataProducers {
 		klog.V(10).Infof("Precompute: %v", predicateName)
 		precomputeFunc(predicateMetadata)
@@ -519,38 +463,19 @@ func (m *PodTopologySpreadMetadata) Clone() *PodTopologySpreadMetadata {
 // RemovePod changes predicateMetadata assuming that the given `deletedPod` is
 // deleted from the system.
 func (meta *predicateMetadata) RemovePod(deletedPod *v1.Pod, node *v1.Node) error {
-	deletedPodFullName := schedutil.GetPodFullName(deletedPod)
-	if deletedPodFullName == schedutil.GetPodFullName(meta.pod) {
-		return fmt.Errorf("deletedPod and meta.pod must not be the same")
-	}
-	meta.serviceAffinityMetadata.removePod(deletedPod, node)
-
 	return nil
 }
 
 // AddPod changes predicateMetadata assuming that the given `addedPod` is added to the
 // system.
 func (meta *predicateMetadata) AddPod(addedPod *v1.Pod, node *v1.Node) error {
-	addedPodFullName := schedutil.GetPodFullName(addedPod)
-	if addedPodFullName == schedutil.GetPodFullName(meta.pod) {
-		return fmt.Errorf("addedPod and meta.pod must not be the same")
-	}
-	if node == nil {
-		return fmt.Errorf("node not found")
-	}
-
-	meta.serviceAffinityMetadata.addPod(addedPod, meta.pod, node)
-
 	return nil
 }
 
 // ShallowCopy copies a metadata struct into a new struct and creates a copy of
 // its maps and slices, but it does not copy the contents of pointer values.
 func (meta *predicateMetadata) ShallowCopy() Metadata {
-	newPredMeta := &predicateMetadata{
-		pod: meta.pod,
-	}
-	newPredMeta.serviceAffinityMetadata = meta.serviceAffinityMetadata.clone()
+	newPredMeta := &predicateMetadata{}
 	return (Metadata)(newPredMeta)
 }
 

pkg/scheduler/algorithm/predicates/metadata_test.go

@@ -17,181 +17,16 @@ limitations under the License.
 package predicates
 
 import (
-	"fmt"
 	"reflect"
-	"sort"
 	"testing"
 
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
-	fakelisters "k8s.io/kubernetes/pkg/scheduler/listers/fake"
-	schedulernodeinfo "k8s.io/kubernetes/pkg/scheduler/nodeinfo"
 	nodeinfosnapshot "k8s.io/kubernetes/pkg/scheduler/nodeinfo/snapshot"
 	st "k8s.io/kubernetes/pkg/scheduler/testing"
 )
 
-// sortablePods lets us to sort pods.
-type sortablePods []*v1.Pod
-
-func (s sortablePods) Less(i, j int) bool {
-	return s[i].Namespace < s[j].Namespace ||
-		(s[i].Namespace == s[j].Namespace && s[i].Name < s[j].Name)
-}
-func (s sortablePods) Len() int      { return len(s) }
-func (s sortablePods) Swap(i, j int) { s[i], s[j] = s[j], s[i] }
-
-var _ sort.Interface = &sortablePods{}
-
-// sortableServices allows us to sort services.
-type sortableServices []*v1.Service
-
-func (s sortableServices) Less(i, j int) bool {
-	return s[i].Namespace < s[j].Namespace ||
-		(s[i].Namespace == s[j].Namespace && s[i].Name < s[j].Name)
-}
-func (s sortableServices) Len() int      { return len(s) }
-func (s sortableServices) Swap(i, j int) { s[i], s[j] = s[j], s[i] }
-
-var _ sort.Interface = &sortableServices{}
-
-// predicateMetadataEquivalent returns true if the two metadata are equivalent.
-// Note: this function does not compare podRequest.
-func predicateMetadataEquivalent(meta1, meta2 *predicateMetadata) error {
-	if !reflect.DeepEqual(meta1.pod, meta2.pod) {
-		return fmt.Errorf("pods are not the same")
-	}
-	if meta1.serviceAffinityMetadata != nil {
-		sortablePods1 := sortablePods(meta1.serviceAffinityMetadata.matchingPodList)
-		sort.Sort(sortablePods1)
-		sortablePods2 := sortablePods(meta2.serviceAffinityMetadata.matchingPodList)
-		sort.Sort(sortablePods2)
-		if !reflect.DeepEqual(sortablePods1, sortablePods2) {
-			return fmt.Errorf("serviceAffinityMatchingPodLists are not euqal")
-		}
-
-		sortableServices1 := sortableServices(meta1.serviceAffinityMetadata.matchingPodServices)
-		sort.Sort(sortableServices1)
-		sortableServices2 := sortableServices(meta2.serviceAffinityMetadata.matchingPodServices)
-		sort.Sort(sortableServices2)
-		if !reflect.DeepEqual(sortableServices1, sortableServices2) {
-			return fmt.Errorf("serviceAffinityMatchingPodServices are not euqal")
-		}
-	}
-	return nil
-}
-
-func TestPredicateMetadata_AddRemovePod(t *testing.T) {
-	var label1 = map[string]string{
-		"region": "r1",
-		"zone":   "z11",
-	}
-	var label2 = map[string]string{
-		"region": "r1",
-		"zone":   "z12",
-	}
-	var label3 = map[string]string{
-		"region": "r2",
-		"zone":   "z21",
-	}
-	selector1 := map[string]string{"foo": "bar"}
-
-	tests := []struct {
-		name         string
-		pendingPod   *v1.Pod
-		addedPod     *v1.Pod
-		existingPods []*v1.Pod
-		nodes        []*v1.Node
-		services     []*v1.Service
-	}{
-		{
-			name: "no anti-affinity or service affinity exist",
-			pendingPod: &v1.Pod{
-				ObjectMeta: metav1.ObjectMeta{Name: "pending", Labels: selector1},
-			},
-			existingPods: []*v1.Pod{
-				{ObjectMeta: metav1.ObjectMeta{Name: "p1", Labels: selector1},
-					Spec: v1.PodSpec{NodeName: "nodeA"},
-				},
-				{ObjectMeta: metav1.ObjectMeta{Name: "p2"},
-					Spec: v1.PodSpec{NodeName: "nodeC"},
-				},
-			},
-			addedPod: &v1.Pod{
-				ObjectMeta: metav1.ObjectMeta{Name: "addedPod", Labels: selector1},
-				Spec:       v1.PodSpec{NodeName: "nodeB"},
-			},
-			nodes: []*v1.Node{
-				{ObjectMeta: metav1.ObjectMeta{Name: "nodeA", Labels: label1}},
-				{ObjectMeta: metav1.ObjectMeta{Name: "nodeB", Labels: label2}},
-				{ObjectMeta: metav1.ObjectMeta{Name: "nodeC", Labels: label3}},
-			},
-		},
-		{
-			name: "metadata service-affinity data are updated correctly after adding and removing a pod",
-			pendingPod: &v1.Pod{
-				ObjectMeta: metav1.ObjectMeta{Name: "pending", Labels: selector1},
-			},
-			existingPods: []*v1.Pod{
-				{ObjectMeta: metav1.ObjectMeta{Name: "p1", Labels: selector1},
-					Spec: v1.PodSpec{NodeName: "nodeA"},
-				},
-				{ObjectMeta: metav1.ObjectMeta{Name: "p2"},
-					Spec: v1.PodSpec{NodeName: "nodeC"},
-				},
-			},
-			addedPod: &v1.Pod{
-				ObjectMeta: metav1.ObjectMeta{Name: "addedPod", Labels: selector1},
-				Spec:       v1.PodSpec{NodeName: "nodeB"},
-			},
-			services: []*v1.Service{{Spec: v1.ServiceSpec{Selector: selector1}}},
-			nodes: []*v1.Node{
-				{ObjectMeta: metav1.ObjectMeta{Name: "nodeA", Labels: label1}},
-				{ObjectMeta: metav1.ObjectMeta{Name: "nodeB", Labels: label2}},
-				{ObjectMeta: metav1.ObjectMeta{Name: "nodeC", Labels: label3}},
-			},
-		},
-	}
-
-	for _, test := range tests {
-		t.Run(test.name, func(t *testing.T) {
-			allPodLister := fakelisters.PodLister(append(test.existingPods, test.addedPod))
-			// getMeta creates predicate meta data given the list of pods.
-			getMeta := func(pods []*v1.Pod) (*predicateMetadata, map[string]*schedulernodeinfo.NodeInfo) {
-				s := nodeinfosnapshot.NewSnapshot(nodeinfosnapshot.CreateNodeInfoMap(pods, test.nodes))
-				_, precompute := NewServiceAffinityPredicate(s.NodeInfos(), s.Pods(), fakelisters.ServiceLister(test.services), nil)
-				RegisterPredicateMetadataProducer("ServiceAffinityMetaProducer", precompute)
-				factory := &MetadataProducerFactory{}
-				meta := factory.GetPredicateMetadata(test.pendingPod, s)
-				return meta.(*predicateMetadata), s.NodeInfoMap
-			}
-
-			// allPodsMeta is meta data produced when all pods, including test.addedPod
-			// are given to the metadata producer.
-			allPodsMeta, _ := getMeta(allPodLister)
-			// existingPodsMeta1 is meta data produced for test.existingPods (without test.addedPod).
-			existingPodsMeta1, nodeInfoMap := getMeta(test.existingPods)
-			// Add test.addedPod to existingPodsMeta1 and make sure meta is equal to allPodsMeta
-			nodeInfo := nodeInfoMap[test.addedPod.Spec.NodeName]
-			if err := existingPodsMeta1.AddPod(test.addedPod, nodeInfo.Node()); err != nil {
-				t.Errorf("error adding pod to meta: %v", err)
-			}
-			if err := predicateMetadataEquivalent(allPodsMeta, existingPodsMeta1); err != nil {
-				t.Errorf("meta data are not equivalent: %v", err)
-			}
-			// Remove the added pod and from existingPodsMeta1 an make sure it is equal
-			// to meta generated for existing pods.
-			existingPodsMeta2, _ := getMeta(fakelisters.PodLister(test.existingPods))
-			if err := existingPodsMeta1.RemovePod(test.addedPod, nodeInfo.Node()); err != nil {
-				t.Errorf("error removing pod from meta: %v", err)
-			}
-			if err := predicateMetadataEquivalent(existingPodsMeta1, existingPodsMeta2); err != nil {
-				t.Errorf("meta data are not equivalent: %v", err)
-			}
-		})
-	}
-}
-
 func TestPodAffinityMetadata_Clone(t *testing.T) {
 	source := &PodAffinityMetadata{
 		topologyToMatchedExistingAntiAffinityTerms: topologyToMatchedTermCount{
@@ -217,33 +52,6 @@ func TestPodAffinityMetadata_Clone(t *testing.T) {
 	}
 }
 
-// TestPredicateMetadata_ShallowCopy tests the ShallowCopy function. It is based
-// on the idea that shallow-copy should produce an object that is deep-equal to the original
-// object.
-func TestPredicateMetadata_ShallowCopy(t *testing.T) {
-	source := predicateMetadata{
-		pod: &v1.Pod{
-			ObjectMeta: metav1.ObjectMeta{
-				Name:      "test",
-				Namespace: "testns",
-			},
-		},
-		serviceAffinityMetadata: &serviceAffinityMetadata{
-			matchingPodList: []*v1.Pod{
-				{ObjectMeta: metav1.ObjectMeta{Name: "pod1"}},
-				{ObjectMeta: metav1.ObjectMeta{Name: "pod2"}},
-			},
-			matchingPodServices: []*v1.Service{
-				{ObjectMeta: metav1.ObjectMeta{Name: "service1"}},
-			},
-		},
-	}
-
-	if !reflect.DeepEqual(source.ShallowCopy().(*predicateMetadata), &source) {
-		t.Errorf("Copy is not equal to source!")
-	}
-}
-
 // TestGetTPMapMatchingIncomingAffinityAntiAffinity tests against method getTPMapMatchingIncomingAffinityAntiAffinity
 // on Anti Affinity cases
 func TestGetTPMapMatchingIncomingAffinityAntiAffinity(t *testing.T) {