New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add runtime representation of []v1.PreferredSchedulingTerm #96126
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -58,20 +58,10 @@ func NewLazyErrorNodeSelector(ns *v1.NodeSelector) *LazyErrorNodeSelector { | |
parsedTerms := make([]nodeSelectorTerm, 0, len(ns.NodeSelectorTerms)) | ||
for _, term := range ns.NodeSelectorTerms { | ||
// nil or empty term selects no objects | ||
if len(term.MatchExpressions) == 0 && len(term.MatchFields) == 0 { | ||
if isEmptyNodeSelectorTerm(&term) { | ||
continue | ||
} | ||
parsedTerms = append(parsedTerms, nodeSelectorTerm{}) | ||
parsedTerm := &parsedTerms[len(parsedTerms)-1] | ||
if len(term.MatchExpressions) != 0 { | ||
parsedTerm.matchLabels, parsedTerm.parseErr = nodeSelectorRequirementsAsSelector(term.MatchExpressions) | ||
if parsedTerm.parseErr != nil { | ||
continue | ||
} | ||
} | ||
if len(term.MatchFields) != 0 { | ||
parsedTerm.matchFields, parsedTerm.parseErr = nodeSelectorRequirementsAsFieldSelector(term.MatchFields) | ||
} | ||
parsedTerms = append(parsedTerms, newNodeSelectorTerm(&term)) | ||
} | ||
return &LazyErrorNodeSelector{ | ||
terms: parsedTerms, | ||
|
@@ -94,10 +84,7 @@ func (ns *LazyErrorNodeSelector) Match(node *v1.Node) (bool, error) { | |
return false, nil | ||
} | ||
nodeLabels := labels.Set(node.Labels) | ||
nodeFields := make(fields.Set) | ||
if len(node.Name) > 0 { | ||
nodeFields["metadata.name"] = node.Name | ||
} | ||
nodeFields := extractNodeFields(node) | ||
|
||
var errs []error | ||
for _, term := range ns.terms { | ||
|
@@ -113,12 +100,83 @@ func (ns *LazyErrorNodeSelector) Match(node *v1.Node) (bool, error) { | |
return false, errors.NewAggregate(errs) | ||
} | ||
|
||
// PreferredSchedulingTerms is a runtime representation of []v1.PreferredSchedulingTerms. | ||
type PreferredSchedulingTerms struct { | ||
terms []preferredSchedulingTerm | ||
} | ||
|
||
// NewPreferredSchedulingTerms returns a PreferredSchedulingTerms or all the parsing errors found. | ||
// If a v1.PreferredSchedulingTerm has a 0 weight, its parsing is skipped. | ||
func NewPreferredSchedulingTerms(terms []v1.PreferredSchedulingTerm) (*PreferredSchedulingTerms, error) { | ||
var errs []error | ||
parsedTerms := make([]preferredSchedulingTerm, 0, len(terms)) | ||
for _, term := range terms { | ||
if term.Weight == 0 || isEmptyNodeSelectorTerm(&term.Preference) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. could the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not really. Note that isEmptyNodeSelectorTerm receives v1.NodeSelectorTerm and is also used in the hard node affinity representation. |
||
continue | ||
} | ||
parsedTerm := preferredSchedulingTerm{ | ||
nodeSelectorTerm: newNodeSelectorTerm(&term.Preference), | ||
weight: int(term.Weight), | ||
} | ||
if parsedTerm.parseErr != nil { | ||
errs = append(errs, parsedTerm.parseErr) | ||
} else { | ||
parsedTerms = append(parsedTerms, parsedTerm) | ||
} | ||
} | ||
if len(errs) != 0 { | ||
return nil, errors.NewAggregate(errs) | ||
} | ||
return &PreferredSchedulingTerms{terms: parsedTerms}, nil | ||
} | ||
|
||
// Score returns a score for a Node: the sum of the weights of the terms that | ||
// match the Node. | ||
func (t *PreferredSchedulingTerms) Score(node *v1.Node) int64 { | ||
var score int64 | ||
nodeLabels := labels.Set(node.Labels) | ||
nodeFields := extractNodeFields(node) | ||
for _, term := range t.terms { | ||
// parse errors are reported in NewPreferredSchedulingTerms. | ||
if ok, _ := term.match(nodeLabels, nodeFields); ok { | ||
score += int64(term.weight) | ||
} | ||
} | ||
return score | ||
} | ||
|
||
func isEmptyNodeSelectorTerm(term *v1.NodeSelectorTerm) bool { | ||
return len(term.MatchExpressions) == 0 && len(term.MatchFields) == 0 | ||
} | ||
|
||
func extractNodeFields(n *v1.Node) fields.Set { | ||
f := make(fields.Set) | ||
if len(n.Name) > 0 { | ||
f["metadata.name"] = n.Name | ||
} | ||
return f | ||
} | ||
|
||
type nodeSelectorTerm struct { | ||
matchLabels labels.Selector | ||
matchFields fields.Selector | ||
parseErr error | ||
} | ||
|
||
func newNodeSelectorTerm(term *v1.NodeSelectorTerm) nodeSelectorTerm { | ||
var parsedTerm nodeSelectorTerm | ||
if len(term.MatchExpressions) != 0 { | ||
parsedTerm.matchLabels, parsedTerm.parseErr = nodeSelectorRequirementsAsSelector(term.MatchExpressions) | ||
if parsedTerm.parseErr != nil { | ||
return parsedTerm | ||
} | ||
} | ||
if len(term.MatchFields) != 0 { | ||
parsedTerm.matchFields, parsedTerm.parseErr = nodeSelectorRequirementsAsFieldSelector(term.MatchFields) | ||
} | ||
return parsedTerm | ||
} | ||
|
||
func (t *nodeSelectorTerm) match(nodeLabels labels.Set, nodeFields fields.Set) (bool, error) { | ||
if t.parseErr != nil { | ||
return false, t.parseErr | ||
|
@@ -197,3 +255,8 @@ func nodeSelectorRequirementsAsFieldSelector(nsr []v1.NodeSelectorRequirement) ( | |
|
||
return fields.AndSelectors(selectors...), nil | ||
} | ||
|
||
type preferredSchedulingTerm struct { | ||
nodeSelectorTerm | ||
weight int | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this still compute it for all nodes? Maybe I am missing where the computed affinity is stored, or that will be added in another PR, just curious
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct. For that, I would need to implement PreFilter, which is out of the scope for this PR. But I might leave that for 1.21. The priority is implementing #95738, for which parsing happens during plugin instantiation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good, just wanted to make sure I understood thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I created an issue and restored the TODO