instead of changing existing method, add a new type

This commit is contained in:
John Howard 2022-10-12 12:36:27 -07:00
parent 3f2c0e1740
commit ce868cb751
4 changed files with 71 additions and 39 deletions

View File

@ -505,7 +505,7 @@ func TestPreFilterState(t *testing.T) {
{ {
MaxSkew: 3, MaxSkew: 3,
TopologyKey: "node", TopologyKey: "node",
Selector: labels.SelectorFromValidatedSet(labels.Set{"foo": "bar"}), Selector: mustConvertLabelSelectorAsSelector(t, st.MakeLabelSelector().Label("foo", "bar").Obj()),
MinDomains: 1, MinDomains: 1,
NodeAffinityPolicy: v1.NodeInclusionPolicyHonor, NodeAffinityPolicy: v1.NodeInclusionPolicyHonor,
NodeTaintsPolicy: v1.NodeInclusionPolicyIgnore, NodeTaintsPolicy: v1.NodeInclusionPolicyIgnore,
@ -513,7 +513,7 @@ func TestPreFilterState(t *testing.T) {
{ {
MaxSkew: 5, MaxSkew: 5,
TopologyKey: "rack", TopologyKey: "rack",
Selector: labels.SelectorFromValidatedSet(labels.Set{"foo": "bar"}), Selector: mustConvertLabelSelectorAsSelector(t, st.MakeLabelSelector().Label("foo", "bar").Obj()),
MinDomains: 1, MinDomains: 1,
NodeAffinityPolicy: v1.NodeInclusionPolicyHonor, NodeAffinityPolicy: v1.NodeInclusionPolicyHonor,
NodeTaintsPolicy: v1.NodeInclusionPolicyIgnore, NodeTaintsPolicy: v1.NodeInclusionPolicyIgnore,
@ -1283,7 +1283,7 @@ func TestPreFilterState(t *testing.T) {
{ {
MaxSkew: 1, MaxSkew: 1,
TopologyKey: "zone", TopologyKey: "zone",
Selector: labels.SelectorFromValidatedSet(labels.Set{"foo": "a"}), Selector: mustConvertLabelSelectorAsSelector(t, st.MakeLabelSelector().Label("foo", "a").Obj()),
MinDomains: 1, MinDomains: 1,
NodeAffinityPolicy: v1.NodeInclusionPolicyHonor, NodeAffinityPolicy: v1.NodeInclusionPolicyHonor,
NodeTaintsPolicy: v1.NodeInclusionPolicyIgnore, NodeTaintsPolicy: v1.NodeInclusionPolicyIgnore,

View File

@ -77,6 +77,8 @@ func (ls Set) AsValidatedSelector() (Selector, error) {
// perform any validation. // perform any validation.
// According to our measurements this is significantly faster // According to our measurements this is significantly faster
// in codepaths that matter at high scale. // in codepaths that matter at high scale.
// Note: this method copies the Set; if the Set is immutable, consider wrapping it with SetSelector
// instead, which does not copy.
func (ls Set) AsSelectorPreValidated() Selector { func (ls Set) AsSelectorPreValidated() Selector {
return SelectorFromValidatedSet(ls) return SelectorFromValidatedSet(ls)
} }

View File

@ -919,14 +919,6 @@ func SelectorFromSet(ls Set) Selector {
return SelectorFromValidatedSet(ls) return SelectorFromValidatedSet(ls)
} }
var (
// Create constants for hot-path selectors
value0Path, keyPath = func() (*field.Path, *field.Path) {
path := field.ToPath()
return path.Child("values").Index(0), path.Child("key")
}()
)
// ValidatedSelectorFromSet returns a Selector which will match exactly the given Set. A // ValidatedSelectorFromSet returns a Selector which will match exactly the given Set. A
// nil and empty Sets are considered equivalent to Everything(). // nil and empty Sets are considered equivalent to Everything().
// The Set is validated client-side, which allows to catch errors early. // The Set is validated client-side, which allows to catch errors early.
@ -934,29 +926,35 @@ func ValidatedSelectorFromSet(ls Set) (Selector, error) {
if ls == nil || len(ls) == 0 { if ls == nil || len(ls) == 0 {
return internalSelector{}, nil return internalSelector{}, nil
} }
requirements := make([]Requirement, 0, len(ls))
var allErrs field.ErrorList for label, value := range ls {
for k, v := range ls { r, err := NewRequirement(label, selection.Equals, []string{value})
if err := validateLabelKey(k, keyPath); err != nil { if err != nil {
allErrs = append(allErrs, err) return nil, err
}
if err := validateLabelValue(k, v, value0Path); err != nil {
allErrs = append(allErrs, err)
} }
requirements = append(requirements, *r)
} }
// sort to have deterministic string representation
// TODO: validate labels are valid label values sort.Sort(ByKey(requirements))
return setSelector(ls), allErrs.ToAggregate() return internalSelector(requirements), nil
} }
// SelectorFromValidatedSet returns a Selector which will match exactly the given Set. // SelectorFromValidatedSet returns a Selector which will match exactly the given Set.
// A nil and empty Sets are considered equivalent to Everything(). // A nil and empty Sets are considered equivalent to Everything().
// It assumes that Set is already validated and doesn't do any validation. // It assumes that Set is already validated and doesn't do any validation.
// Note: this method copies the Set; if the Set is immutable, consider wrapping it with SetSelector
// instead, which does not copy.
func SelectorFromValidatedSet(ls Set) Selector { func SelectorFromValidatedSet(ls Set) Selector {
if ls == nil || len(ls) == 0 { if ls == nil || len(ls) == 0 {
return internalSelector{} return internalSelector{}
} }
return setSelector(ls) requirements := make([]Requirement, 0, len(ls))
for label, value := range ls {
requirements = append(requirements, Requirement{key: label, operator: selection.Equals, strValues: []string{value}})
}
// sort to have deterministic string representation
sort.Sort(ByKey(requirements))
return internalSelector(requirements)
} }
// ParseToRequirements takes a string representing a selector and returns a list of // ParseToRequirements takes a string representing a selector and returns a list of
@ -968,9 +966,13 @@ func ParseToRequirements(selector string, opts ...field.PathOption) ([]Requireme
return parse(selector, field.ToPath(opts...)) return parse(selector, field.ToPath(opts...))
} }
type setSelector Set // SetSelector wraps a Set, allowing it to implement the Selector interface. Unlike
// Set.AsSelectorPreValidated (which copies the input Set), this type simply wraps the underlying
// Set. As a result, it is substantially more efficient, but requires the caller to not mutate the
// Set.
type SetSelector Set
func (s setSelector) Matches(labels Labels) bool { func (s SetSelector) Matches(labels Labels) bool {
for k, v := range s { for k, v := range s {
if !labels.Has(k) || v != labels.Get(k) { if !labels.Has(k) || v != labels.Get(k) {
return false return false
@ -979,11 +981,11 @@ func (s setSelector) Matches(labels Labels) bool {
return true return true
} }
func (s setSelector) Empty() bool { func (s SetSelector) Empty() bool {
return len(s) == 0 return len(s) == 0
} }
func (s setSelector) String() string { func (s SetSelector) String() string {
keys := make([]string, 0, len(s)) keys := make([]string, 0, len(s))
for k := range s { for k := range s {
keys = append(keys, k) keys = append(keys, k)
@ -1004,28 +1006,28 @@ func (s setSelector) String() string {
return b.String() return b.String()
} }
func (s setSelector) Add(r ...Requirement) Selector { func (s SetSelector) Add(r ...Requirement) Selector {
return s.toFullSelector().Add(r...) return s.toFullSelector().Add(r...)
} }
func (s setSelector) Requirements() (requirements Requirements, selectable bool) { func (s SetSelector) Requirements() (requirements Requirements, selectable bool) {
return s.toFullSelector().Requirements() return s.toFullSelector().Requirements()
} }
func (s setSelector) DeepCopySelector() Selector { func (s SetSelector) DeepCopySelector() Selector {
res := make(setSelector, len(s)) res := make(SetSelector, len(s))
for k, v := range s { for k, v := range s {
res[k] = v res[k] = v
} }
return res return res
} }
func (s setSelector) RequiresExactMatch(label string) (value string, found bool) { func (s SetSelector) RequiresExactMatch(label string) (value string, found bool) {
v, f := s[label] v, f := s[label]
return v, f return v, f
} }
func (s setSelector) toFullSelector() Selector { func (s SetSelector) toFullSelector() Selector {
if s == nil || len(s) == 0 { if s == nil || len(s) == 0 {
return internalSelector{} return internalSelector{}
} }
@ -1038,4 +1040,4 @@ func (s setSelector) toFullSelector() Selector {
return internalSelector(requirements) return internalSelector(requirements)
} }
var _ Selector = setSelector{} var _ Selector = SetSelector{}

View File

@ -826,6 +826,28 @@ func BenchmarkSelectorFromValidatedSet(b *testing.B) {
} }
} }
func BenchmarkSetSelector(b *testing.B) {
set := map[string]string{
"foo": "foo",
"bar": "bar",
}
matchee := Set(map[string]string{
"foo": "foo",
"bar": "bar",
"extra": "label",
})
for i := 0; i < b.N; i++ {
s := SetSelector(set)
if s.Empty() {
b.Errorf("Unexpected selector")
}
if !s.Matches(matchee) {
b.Errorf("Unexpected match")
}
}
}
func TestSetSelectorString(t *testing.T) { func TestSetSelectorString(t *testing.T) {
cases := []struct { cases := []struct {
set Set set Set
@ -847,7 +869,7 @@ func TestSetSelectorString(t *testing.T) {
for _, tt := range cases { for _, tt := range cases {
t.Run(tt.out, func(t *testing.T) { t.Run(tt.out, func(t *testing.T) {
if got := setSelector(tt.set).String(); tt.out != got { if got := SetSelector(tt.set).String(); tt.out != got {
t.Fatalf("expected %v, got %v", tt.out, got) t.Fatalf("expected %v, got %v", tt.out, got)
} }
}) })
@ -936,13 +958,19 @@ func TestValidatedSelectorFromSet(t *testing.T) {
tests := []struct { tests := []struct {
name string name string
input Set input Set
expectedSelector Selector expectedSelector internalSelector
expectedError field.ErrorList expectedError field.ErrorList
}{ }{
{ {
name: "Simple Set, no error", name: "Simple Set, no error",
input: Set{"key": "val"}, input: Set{"key": "val"},
expectedSelector: setSelector{"key": "val"}, expectedSelector: internalSelector{
Requirement{
key: "key",
operator: selection.Equals,
strValues: []string{"val"},
},
},
}, },
{ {
name: "Invalid Set, value too long", name: "Invalid Set, value too long",