Addressed reviewer comments

This commit is contained in:
Bobby (Babak) Salamat 2018-04-12 12:02:14 -07:00
parent bdb22ebbe1
commit 3041698e52
3 changed files with 42 additions and 36 deletions

View File

@ -56,10 +56,10 @@ type predicateMetadata struct {
matchingAntiAffinityTerms map[string][]matchingPodAntiAffinityTerm matchingAntiAffinityTerms map[string][]matchingPodAntiAffinityTerm
// A map of node name to a list of Pods on the node that can potentially match // A map of node name to a list of Pods on the node that can potentially match
// the affinity rules of the "pod". // the affinity rules of the "pod".
matchingAffinityPods map[string][]*v1.Pod nodeNameToMatchingAffinityPods map[string][]*v1.Pod
// A map of node name to a list of Pods on the node that can potentially match // A map of node name to a list of Pods on the node that can potentially match
// the anti-affinity rules of the "pod". // the anti-affinity rules of the "pod".
matchingAntiAffinityPods map[string][]*v1.Pod nodeNameToMatchingAntiAffinityPods map[string][]*v1.Pod
serviceAffinityInUse bool serviceAffinityInUse bool
serviceAffinityMatchingPodList []*v1.Pod serviceAffinityMatchingPodList []*v1.Pod
serviceAffinityMatchingPodServices []*v1.Service serviceAffinityMatchingPodServices []*v1.Service
@ -128,8 +128,8 @@ func (pfactory *PredicateMetadataFactory) GetMetadata(pod *v1.Pod, nodeNameToInf
podRequest: GetResourceRequest(pod), podRequest: GetResourceRequest(pod),
podPorts: schedutil.GetContainerPorts(pod), podPorts: schedutil.GetContainerPorts(pod),
matchingAntiAffinityTerms: matchingTerms, matchingAntiAffinityTerms: matchingTerms,
matchingAffinityPods: affinityPods, nodeNameToMatchingAffinityPods: affinityPods,
matchingAntiAffinityPods: antiAffinityPods, nodeNameToMatchingAntiAffinityPods: antiAffinityPods,
} }
for predicateName, precomputeFunc := range predicateMetadataProducers { for predicateName, precomputeFunc := range predicateMetadataProducers {
glog.V(10).Infof("Precompute: %v", predicateName) glog.V(10).Infof("Precompute: %v", predicateName)
@ -152,17 +152,23 @@ func (meta *predicateMetadata) RemovePod(deletedPod *v1.Pod) error {
podNodeName := deletedPod.Spec.NodeName podNodeName := deletedPod.Spec.NodeName
if affinity != nil && len(podNodeName) > 0 { if affinity != nil && len(podNodeName) > 0 {
if affinity.PodAffinity != nil { if affinity.PodAffinity != nil {
for i, p := range meta.matchingAffinityPods[podNodeName] { for i, p := range meta.nodeNameToMatchingAffinityPods[podNodeName] {
if p == deletedPod { if p == deletedPod {
meta.matchingAffinityPods[podNodeName] = append(meta.matchingAffinityPods[podNodeName][:i], meta.matchingAffinityPods[podNodeName][i+1:]...) s := meta.nodeNameToMatchingAffinityPods[podNodeName]
s[i] = s[len(s)-1]
s = s[:len(s)-1]
meta.nodeNameToMatchingAffinityPods[podNodeName] = s
break break
} }
} }
} }
if affinity.PodAntiAffinity != nil { if affinity.PodAntiAffinity != nil {
for i, p := range meta.matchingAntiAffinityPods[podNodeName] { for i, p := range meta.nodeNameToMatchingAntiAffinityPods[podNodeName] {
if p == deletedPod { if p == deletedPod {
meta.matchingAntiAffinityPods[podNodeName] = append(meta.matchingAntiAffinityPods[podNodeName][:i], meta.matchingAntiAffinityPods[podNodeName][i+1:]...) s := meta.nodeNameToMatchingAntiAffinityPods[podNodeName]
s[i] = s[len(s)-1]
s = s[:len(s)-1]
meta.nodeNameToMatchingAntiAffinityPods[podNodeName] = s
break break
} }
} }
@ -210,32 +216,32 @@ func (meta *predicateMetadata) AddPod(addedPod *v1.Pod, nodeInfo *schedulercache
meta.matchingAntiAffinityTerms[addedPodFullName] = podMatchingTerms meta.matchingAntiAffinityTerms[addedPodFullName] = podMatchingTerms
} }
} }
// Add the pod to matchingAffinityPods and matchingAntiAffinityPods if needed. // Add the pod to nodeNameToMatchingAffinityPods and nodeNameToMatchingAntiAffinityPods if needed.
affinity := meta.pod.Spec.Affinity affinity := meta.pod.Spec.Affinity
podNodeName := addedPod.Spec.NodeName podNodeName := addedPod.Spec.NodeName
if affinity != nil && len(podNodeName) > 0 { if affinity != nil && len(podNodeName) > 0 {
if targetPodMatchesAffinityOfPod(meta.pod, addedPod) { if targetPodMatchesAffinityOfPod(meta.pod, addedPod) {
found := false found := false
for _, p := range meta.matchingAffinityPods[podNodeName] { for _, p := range meta.nodeNameToMatchingAffinityPods[podNodeName] {
if p == addedPod { if p == addedPod {
found = true found = true
break break
} }
} }
if !found { if !found {
meta.matchingAffinityPods[podNodeName] = append(meta.matchingAffinityPods[podNodeName], addedPod) meta.nodeNameToMatchingAffinityPods[podNodeName] = append(meta.nodeNameToMatchingAffinityPods[podNodeName], addedPod)
} }
} }
if targetPodMatchesAntiAffinityOfPod(meta.pod, addedPod) { if targetPodMatchesAntiAffinityOfPod(meta.pod, addedPod) {
found := false found := false
for _, p := range meta.matchingAntiAffinityPods[podNodeName] { for _, p := range meta.nodeNameToMatchingAntiAffinityPods[podNodeName] {
if p == addedPod { if p == addedPod {
found = true found = true
break break
} }
} }
if !found { if !found {
meta.matchingAntiAffinityPods[podNodeName] = append(meta.matchingAntiAffinityPods[podNodeName], addedPod) meta.nodeNameToMatchingAntiAffinityPods[podNodeName] = append(meta.nodeNameToMatchingAntiAffinityPods[podNodeName], addedPod)
} }
} }
} }
@ -266,13 +272,13 @@ func (meta *predicateMetadata) ShallowCopy() algorithm.PredicateMetadata {
for k, v := range meta.matchingAntiAffinityTerms { for k, v := range meta.matchingAntiAffinityTerms {
newPredMeta.matchingAntiAffinityTerms[k] = append([]matchingPodAntiAffinityTerm(nil), v...) newPredMeta.matchingAntiAffinityTerms[k] = append([]matchingPodAntiAffinityTerm(nil), v...)
} }
newPredMeta.matchingAffinityPods = make(map[string][]*v1.Pod) newPredMeta.nodeNameToMatchingAffinityPods = make(map[string][]*v1.Pod)
for k, v := range meta.matchingAffinityPods { for k, v := range meta.nodeNameToMatchingAffinityPods {
newPredMeta.matchingAffinityPods[k] = append([]*v1.Pod(nil), v...) newPredMeta.nodeNameToMatchingAffinityPods[k] = append([]*v1.Pod(nil), v...)
} }
newPredMeta.matchingAntiAffinityPods = make(map[string][]*v1.Pod) newPredMeta.nodeNameToMatchingAntiAffinityPods = make(map[string][]*v1.Pod)
for k, v := range meta.matchingAntiAffinityPods { for k, v := range meta.nodeNameToMatchingAntiAffinityPods {
newPredMeta.matchingAntiAffinityPods[k] = append([]*v1.Pod(nil), v...) newPredMeta.nodeNameToMatchingAntiAffinityPods[k] = append([]*v1.Pod(nil), v...)
} }
newPredMeta.serviceAffinityMatchingPodServices = append([]*v1.Service(nil), newPredMeta.serviceAffinityMatchingPodServices = append([]*v1.Service(nil),
meta.serviceAffinityMatchingPodServices...) meta.serviceAffinityMatchingPodServices...)

View File

@ -118,15 +118,15 @@ func predicateMetadataEquivalent(meta1, meta2 *predicateMetadata) error {
if !reflect.DeepEqual(meta1.matchingAntiAffinityTerms, meta2.matchingAntiAffinityTerms) { if !reflect.DeepEqual(meta1.matchingAntiAffinityTerms, meta2.matchingAntiAffinityTerms) {
return fmt.Errorf("matchingAntiAffinityTerms are not euqal") return fmt.Errorf("matchingAntiAffinityTerms are not euqal")
} }
sortNodePodMap(meta1.matchingAffinityPods) sortNodePodMap(meta1.nodeNameToMatchingAffinityPods)
sortNodePodMap(meta2.matchingAffinityPods) sortNodePodMap(meta2.nodeNameToMatchingAffinityPods)
if !reflect.DeepEqual(meta1.matchingAffinityPods, meta2.matchingAffinityPods) { if !reflect.DeepEqual(meta1.nodeNameToMatchingAffinityPods, meta2.nodeNameToMatchingAffinityPods) {
return fmt.Errorf("matchingAffinityPods are not euqal") return fmt.Errorf("nodeNameToMatchingAffinityPods are not euqal")
} }
sortNodePodMap(meta1.matchingAntiAffinityPods) sortNodePodMap(meta1.nodeNameToMatchingAntiAffinityPods)
sortNodePodMap(meta2.matchingAntiAffinityPods) sortNodePodMap(meta2.nodeNameToMatchingAntiAffinityPods)
if !reflect.DeepEqual(meta1.matchingAntiAffinityPods, meta2.matchingAntiAffinityPods) { if !reflect.DeepEqual(meta1.nodeNameToMatchingAntiAffinityPods, meta2.nodeNameToMatchingAntiAffinityPods) {
return fmt.Errorf("matchingAntiAffinityPods are not euqal") return fmt.Errorf("nodeNameToMatchingAntiAffinityPods are not euqal")
} }
if meta1.serviceAffinityInUse { if meta1.serviceAffinityInUse {
sortablePods1 := sortablePods(meta1.serviceAffinityMatchingPodList) sortablePods1 := sortablePods(meta1.serviceAffinityMatchingPodList)
@ -473,7 +473,7 @@ func TestPredicateMetadata_ShallowCopy(t *testing.T) {
}, },
}, },
}, },
matchingAffinityPods: map[string][]*v1.Pod{ nodeNameToMatchingAffinityPods: map[string][]*v1.Pod{
"nodeA": { "nodeA": {
&v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "p1", Labels: selector1}, &v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "p1", Labels: selector1},
Spec: v1.PodSpec{NodeName: "nodeA"}, Spec: v1.PodSpec{NodeName: "nodeA"},
@ -490,7 +490,7 @@ func TestPredicateMetadata_ShallowCopy(t *testing.T) {
}, },
}, },
}, },
matchingAntiAffinityPods: map[string][]*v1.Pod{ nodeNameToMatchingAntiAffinityPods: map[string][]*v1.Pod{
"nodeN": { "nodeN": {
&v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "p1", Labels: selector1}, &v1.Pod{ObjectMeta: metav1.ObjectMeta{Name: "p1", Labels: selector1},
Spec: v1.PodSpec{NodeName: "nodeN"}, Spec: v1.PodSpec{NodeName: "nodeN"},

View File

@ -1423,7 +1423,7 @@ func (c *PodAffinityChecker) satisfiesPodsAffinityAntiAffinity(pod *v1.Pod,
} }
if predicateMeta, ok := meta.(*predicateMetadata); ok { if predicateMeta, ok := meta.(*predicateMetadata); ok {
// Check all affinity terms. // Check all affinity terms.
matchingPods := predicateMeta.matchingAffinityPods matchingPods := predicateMeta.nodeNameToMatchingAffinityPods
for _, term := range GetPodAffinityTerms(affinity.PodAffinity) { for _, term := range GetPodAffinityTerms(affinity.PodAffinity) {
termMatches, err := c.anyMatchingPodInTopology(pod, matchingPods, nodeInfo, &term) termMatches, err := c.anyMatchingPodInTopology(pod, matchingPods, nodeInfo, &term)
if err != nil { if err != nil {
@ -1445,7 +1445,7 @@ func (c *PodAffinityChecker) satisfiesPodsAffinityAntiAffinity(pod *v1.Pod,
} }
// Check all anti-affinity terms. // Check all anti-affinity terms.
matchingPods = predicateMeta.matchingAntiAffinityPods matchingPods = predicateMeta.nodeNameToMatchingAntiAffinityPods
for _, term := range GetPodAntiAffinityTerms(affinity.PodAntiAffinity) { for _, term := range GetPodAntiAffinityTerms(affinity.PodAntiAffinity) {
termMatches, err := c.anyMatchingPodInTopology(pod, matchingPods, nodeInfo, &term) termMatches, err := c.anyMatchingPodInTopology(pod, matchingPods, nodeInfo, &term)
if err != nil || termMatches { if err != nil || termMatches {