From 898a6444e3a0325b38dba8d77b2bc37d758c289c Mon Sep 17 00:00:00 2001 From: Wojciech Tyczynski Date: Fri, 22 Jul 2016 12:48:35 +0200 Subject: [PATCH] Return pointer for Affinity in api helper --- pkg/api/helpers.go | 9 +++++---- pkg/api/validation/validation.go | 3 +++ plugin/pkg/admission/antiaffinity/admission.go | 2 +- .../scheduler/algorithm/predicates/predicates.go | 16 ++++++++++++---- .../algorithm/predicates/predicates_test.go | 2 +- .../algorithm/priorities/interpod_affinity.go | 8 ++++---- .../algorithm/priorities/node_affinity.go | 2 +- plugin/pkg/scheduler/schedulercache/node_info.go | 11 +++++++++-- 8 files changed, 36 insertions(+), 17 deletions(-) diff --git a/pkg/api/helpers.go b/pkg/api/helpers.go index b1881914231..08245e50069 100644 --- a/pkg/api/helpers.go +++ b/pkg/api/helpers.go @@ -436,15 +436,16 @@ const ( // GetAffinityFromPod gets the json serialized affinity data from Pod.Annotations // and converts it to the Affinity type in api. -func GetAffinityFromPodAnnotations(annotations map[string]string) (Affinity, error) { - var affinity Affinity +func GetAffinityFromPodAnnotations(annotations map[string]string) (*Affinity, error) { if len(annotations) > 0 && annotations[AffinityAnnotationKey] != "" { + var affinity Affinity err := json.Unmarshal([]byte(annotations[AffinityAnnotationKey]), &affinity) if err != nil { - return affinity, err + return nil, err } + return &affinity, nil } - return affinity, nil + return nil, nil } // GetTolerationsFromPodAnnotations gets the json serialized tolerations data from Pod.Annotations diff --git a/pkg/api/validation/validation.go b/pkg/api/validation/validation.go index 0fe02c5dc8d..e08bb1e5d60 100644 --- a/pkg/api/validation/validation.go +++ b/pkg/api/validation/validation.go @@ -1863,6 +1863,9 @@ func ValidateAffinityInPodAnnotations(annotations map[string]string, fldPath *fi allErrs = append(allErrs, field.Invalid(fldPath, api.AffinityAnnotationKey, err.Error())) return allErrs } + if affinity == nil { + return allErrs + } affinityFldPath := fldPath.Child(api.AffinityAnnotationKey) if affinity.NodeAffinity != nil { diff --git a/plugin/pkg/admission/antiaffinity/admission.go b/plugin/pkg/admission/antiaffinity/admission.go index f211022b3a3..50cb5a5f8cb 100644 --- a/plugin/pkg/admission/antiaffinity/admission.go +++ b/plugin/pkg/admission/antiaffinity/admission.go @@ -63,7 +63,7 @@ func (p *plugin) Admit(attributes admission.Attributes) (err error) { glog.V(5).Infof("Invalid Affinity detected, but we will leave handling of this to validation phase") return nil } - if affinity.PodAntiAffinity != nil { + if affinity != nil && affinity.PodAntiAffinity != nil { var podAntiAffinityTerms []api.PodAffinityTerm if len(affinity.PodAntiAffinity.RequiredDuringSchedulingIgnoredDuringExecution) != 0 { podAntiAffinityTerms = affinity.PodAntiAffinity.RequiredDuringSchedulingIgnoredDuringExecution diff --git a/plugin/pkg/scheduler/algorithm/predicates/predicates.go b/plugin/pkg/scheduler/algorithm/predicates/predicates.go index 71b96c228f3..161e668f19e 100644 --- a/plugin/pkg/scheduler/algorithm/predicates/predicates.go +++ b/plugin/pkg/scheduler/algorithm/predicates/predicates.go @@ -512,7 +512,7 @@ func podMatchesNodeLabels(pod *api.Pod, node *api.Node) bool { // 5. zero-length non-nil []NodeSelectorRequirement matches no nodes also, just for simplicity // 6. non-nil empty NodeSelectorRequirement is not allowed nodeAffinityMatches := true - if affinity.NodeAffinity != nil { + if affinity != nil && affinity.NodeAffinity != nil { nodeAffinity := affinity.NodeAffinity // if no required NodeAffinity requirements, will do no-op, means select all nodes. // TODO: Replace next line with subsequent commented-out line when implement RequiredDuringSchedulingRequiredDuringExecution. @@ -809,14 +809,19 @@ func (checker *PodAffinityChecker) InterPodAffinityMatches(pod *api.Pod, meta in // Check if the current node match the inter-pod affinity scheduling constraints. // Hard inter-pod affinity is not symmetric, check only when affinity.PodAffinity exists. - if affinity.PodAffinity != nil { + if affinity != nil && affinity.PodAffinity != nil { if !checker.NodeMatchesHardPodAffinity(pod, allPods, node, affinity.PodAffinity) { return false, ErrPodAffinityNotMatch } } - // Hard inter-pod anti-affinity is symmetric, we should always check it. - if !checker.NodeMatchesHardPodAntiAffinity(pod, allPods, node, affinity.PodAntiAffinity) { + // Hard inter-pod anti-affinity is symmetric, we should always check it + // (also when affinity or affinity.PodAntiAffinity is nil). + var antiAffinity *api.PodAntiAffinity + if affinity != nil { + antiAffinity = affinity.PodAntiAffinity + } + if !checker.NodeMatchesHardPodAntiAffinity(pod, allPods, node, antiAffinity) { return false, ErrPodAffinityNotMatch } @@ -933,6 +938,9 @@ func (checker *PodAffinityChecker) NodeMatchesHardPodAntiAffinity(pod *api.Pod, glog.V(10).Infof("Failed to get Affinity from Pod %+v, err: %+v", podName(pod), err) return false } + if epAffinity == nil { + continue + } epNode, err := checker.info.GetNodeInfo(ep.Spec.NodeName) if err != nil { glog.V(10).Infof("Failed to get node from Pod %+v, err: %+v", podName(ep), err) diff --git a/plugin/pkg/scheduler/algorithm/predicates/predicates_test.go b/plugin/pkg/scheduler/algorithm/predicates/predicates_test.go index 48295b01da1..caaef840b2e 100755 --- a/plugin/pkg/scheduler/algorithm/predicates/predicates_test.go +++ b/plugin/pkg/scheduler/algorithm/predicates/predicates_test.go @@ -2500,7 +2500,7 @@ func TestInterPodAffinityWithMultipleNodes(t *testing.T) { if err != nil { t.Errorf("unexpected error: %v", err) } - if affinity.NodeAffinity != nil { + if affinity != nil && affinity.NodeAffinity != nil { nodeInfo := schedulercache.NewNodeInfo() nodeInfo.SetNode(&node) fits2, err := PodSelectorMatches(test.pod, PredicateMetadata(test.pod), nodeInfo) diff --git a/plugin/pkg/scheduler/algorithm/priorities/interpod_affinity.go b/plugin/pkg/scheduler/algorithm/priorities/interpod_affinity.go index 269762a59d8..dd4d2ea0811 100644 --- a/plugin/pkg/scheduler/algorithm/priorities/interpod_affinity.go +++ b/plugin/pkg/scheduler/algorithm/priorities/interpod_affinity.go @@ -127,14 +127,14 @@ func (ipa *InterPodAffinity) CalculateInterPodAffinityPriority(pod *api.Pod, nod return nil, err } - if affinity.PodAffinity != nil { + if affinity != nil && affinity.PodAffinity != nil { // For every soft pod affinity term of , if matches the term, // increment for every node in the cluster with the same // value as that of `s node by the term`s weight. terms := affinity.PodAffinity.PreferredDuringSchedulingIgnoredDuringExecution pm.processTerms(terms, pod, existingPod, existingPodNode, 1) } - if affinity.PodAntiAffinity != nil { + if affinity != nil && affinity.PodAntiAffinity != nil { // For every soft pod anti-affinity term of , if matches the term, // decrement for every node in the cluster with the same // value as that of `s node by the term`s weight. @@ -142,7 +142,7 @@ func (ipa *InterPodAffinity) CalculateInterPodAffinityPriority(pod *api.Pod, nod pm.processTerms(terms, pod, existingPod, existingPodNode, -1) } - if existingPodAffinity.PodAffinity != nil { + if existingPodAffinity != nil && existingPodAffinity.PodAffinity != nil { // For every hard pod affinity term of , if matches the term, // increment for every node in the cluster with the same // value as that of 's node by the constant @@ -162,7 +162,7 @@ func (ipa *InterPodAffinity) CalculateInterPodAffinityPriority(pod *api.Pod, nod terms := existingPodAffinity.PodAffinity.PreferredDuringSchedulingIgnoredDuringExecution pm.processTerms(terms, existingPod, pod, existingPodNode, 1) } - if existingPodAffinity.PodAntiAffinity != nil { + if existingPodAffinity != nil && existingPodAffinity.PodAntiAffinity != nil { // For every soft pod anti-affinity term of , if matches the term, // decrement for every node in the cluster with the same // value as that of 's node by the term's weight. diff --git a/plugin/pkg/scheduler/algorithm/priorities/node_affinity.go b/plugin/pkg/scheduler/algorithm/priorities/node_affinity.go index 53b8b87ffc1..97a2c9b6544 100644 --- a/plugin/pkg/scheduler/algorithm/priorities/node_affinity.go +++ b/plugin/pkg/scheduler/algorithm/priorities/node_affinity.go @@ -41,7 +41,7 @@ func CalculateNodeAffinityPriority(pod *api.Pod, nodeNameToInfo map[string]*sche // A nil element of PreferredDuringSchedulingIgnoredDuringExecution matches no objects. // An element of PreferredDuringSchedulingIgnoredDuringExecution that refers to an // empty PreferredSchedulingTerm matches all objects. - if affinity.NodeAffinity != nil && affinity.NodeAffinity.PreferredDuringSchedulingIgnoredDuringExecution != nil { + if affinity != nil && affinity.NodeAffinity != nil && affinity.NodeAffinity.PreferredDuringSchedulingIgnoredDuringExecution != nil { // Match PreferredDuringSchedulingIgnoredDuringExecution term by term. for _, preferredSchedulingTerm := range affinity.NodeAffinity.PreferredDuringSchedulingIgnoredDuringExecution { if preferredSchedulingTerm.Weight == 0 { diff --git a/plugin/pkg/scheduler/schedulercache/node_info.go b/plugin/pkg/scheduler/schedulercache/node_info.go index 4db5f3cb16f..5d041b0cd0d 100644 --- a/plugin/pkg/scheduler/schedulercache/node_info.go +++ b/plugin/pkg/scheduler/schedulercache/node_info.go @@ -153,6 +153,14 @@ func (n *NodeInfo) String() string { return fmt.Sprintf("&NodeInfo{Pods:%v, RequestedResource:%#v, NonZeroRequest: %#v}", podKeys, n.requestedResource, n.nonzeroRequest) } +func hasPodAffinityConstraints(pod *api.Pod) bool { + affinity, err := api.GetAffinityFromPodAnnotations(pod.Annotations) + if err != nil || affinity == nil { + return false + } + return affinity.PodAffinity != nil || affinity.PodAntiAffinity != nil +} + // addPod adds pod information to this NodeInfo. func (n *NodeInfo) addPod(pod *api.Pod) { cpu, mem, nvidia_gpu, non0_cpu, non0_mem := calculateResource(pod) @@ -162,8 +170,7 @@ func (n *NodeInfo) addPod(pod *api.Pod) { n.nonzeroRequest.MilliCPU += non0_cpu n.nonzeroRequest.Memory += non0_mem n.pods = append(n.pods, pod) - // TODO: This should return pointer to avoid allocations. - if affinity, err := api.GetAffinityFromPodAnnotations(pod.Annotations); err == nil && (affinity.PodAffinity != nil || affinity.PodAntiAffinity != nil) { + if hasPodAffinityConstraints(pod) { n.podsWithAffinity = append(n.podsWithAffinity, pod) } n.generation++