From 3f2c0e17408d3e0cd9fe9102e7bd32061a8ce5a4 Mon Sep 17 00:00:00 2001 From: John Howard Date: Wed, 21 Sep 2022 11:51:27 -0700 Subject: [PATCH 1/4] Optimize turning a Set into a Selector MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes 112647 Benchmark: ``` $ benchstat /tmp/{old,new} name old time/op new time/op delta SelectorFromValidatedSet-48 397ns ± 1% 123ns ± 0% -68.87% (p=0.008 n=5+5) name old alloc/op new alloc/op delta SelectorFromValidatedSet-48 192B ± 0% 0B -100.00% (p=0.008 n=5+5) name old allocs/op new allocs/op delta SelectorFromValidatedSet-48 5.00 ± 0% 0.00 -100.00% (p=0.008 n=5+5) ``` --- .../podtopologyspread/filtering_test.go | 6 +- .../apimachinery/pkg/labels/selector.go | 108 +++++++++++++++--- .../apimachinery/pkg/labels/selector_test.go | 53 +++++++-- 3 files changed, 137 insertions(+), 30 deletions(-) diff --git a/pkg/scheduler/framework/plugins/podtopologyspread/filtering_test.go b/pkg/scheduler/framework/plugins/podtopologyspread/filtering_test.go index 30f50806a5a..b9abaaefc1d 100644 --- a/pkg/scheduler/framework/plugins/podtopologyspread/filtering_test.go +++ b/pkg/scheduler/framework/plugins/podtopologyspread/filtering_test.go @@ -505,7 +505,7 @@ func TestPreFilterState(t *testing.T) { { MaxSkew: 3, TopologyKey: "node", - Selector: mustConvertLabelSelectorAsSelector(t, st.MakeLabelSelector().Label("foo", "bar").Obj()), + Selector: labels.SelectorFromValidatedSet(labels.Set{"foo": "bar"}), MinDomains: 1, NodeAffinityPolicy: v1.NodeInclusionPolicyHonor, NodeTaintsPolicy: v1.NodeInclusionPolicyIgnore, @@ -513,7 +513,7 @@ func TestPreFilterState(t *testing.T) { { MaxSkew: 5, TopologyKey: "rack", - Selector: mustConvertLabelSelectorAsSelector(t, st.MakeLabelSelector().Label("foo", "bar").Obj()), + Selector: labels.SelectorFromValidatedSet(labels.Set{"foo": "bar"}), MinDomains: 1, NodeAffinityPolicy: v1.NodeInclusionPolicyHonor, NodeTaintsPolicy: v1.NodeInclusionPolicyIgnore, @@ -1283,7 +1283,7 @@ func TestPreFilterState(t *testing.T) { { MaxSkew: 1, TopologyKey: "zone", - Selector: mustConvertLabelSelectorAsSelector(t, st.MakeLabelSelector().Label("foo", "a").Obj()), + Selector: labels.SelectorFromValidatedSet(labels.Set{"foo": "a"}), MinDomains: 1, NodeAffinityPolicy: v1.NodeInclusionPolicyHonor, NodeTaintsPolicy: v1.NodeInclusionPolicyIgnore, diff --git a/staging/src/k8s.io/apimachinery/pkg/labels/selector.go b/staging/src/k8s.io/apimachinery/pkg/labels/selector.go index 6d6f562ad13..5155a3ca941 100644 --- a/staging/src/k8s.io/apimachinery/pkg/labels/selector.go +++ b/staging/src/k8s.io/apimachinery/pkg/labels/selector.go @@ -919,6 +919,14 @@ func SelectorFromSet(ls Set) Selector { 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 // nil and empty Sets are considered equivalent to Everything(). // The Set is validated client-side, which allows to catch errors early. @@ -926,17 +934,19 @@ func ValidatedSelectorFromSet(ls Set) (Selector, error) { if ls == nil || len(ls) == 0 { return internalSelector{}, nil } - requirements := make([]Requirement, 0, len(ls)) - for label, value := range ls { - r, err := NewRequirement(label, selection.Equals, []string{value}) - if err != nil { - return nil, err + + var allErrs field.ErrorList + for k, v := range ls { + if err := validateLabelKey(k, keyPath); err != nil { + allErrs = append(allErrs, err) + } + if err := validateLabelValue(k, v, value0Path); err != nil { + allErrs = append(allErrs, err) } - requirements = append(requirements, *r) } - // sort to have deterministic string representation - sort.Sort(ByKey(requirements)) - return internalSelector(requirements), nil + + // TODO: validate labels are valid label values + return setSelector(ls), allErrs.ToAggregate() } // SelectorFromValidatedSet returns a Selector which will match exactly the given Set. @@ -946,13 +956,7 @@ func SelectorFromValidatedSet(ls Set) Selector { if ls == nil || len(ls) == 0 { return internalSelector{} } - 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) + return setSelector(ls) } // ParseToRequirements takes a string representing a selector and returns a list of @@ -963,3 +967,75 @@ func SelectorFromValidatedSet(ls Set) Selector { func ParseToRequirements(selector string, opts ...field.PathOption) ([]Requirement, error) { return parse(selector, field.ToPath(opts...)) } + +type setSelector Set + +func (s setSelector) Matches(labels Labels) bool { + for k, v := range s { + if !labels.Has(k) || v != labels.Get(k) { + return false + } + } + return true +} + +func (s setSelector) Empty() bool { + return len(s) == 0 +} + +func (s setSelector) String() string { + keys := make([]string, 0, len(s)) + for k := range s { + keys = append(keys, k) + } + // Ensure deterministic output + sort.Strings(keys) + b := strings.Builder{} + for i, key := range keys { + last := i == len(keys)-1 + v := s[key] + b.WriteString(key) + b.WriteString("=") + b.WriteString(v) + if !last { + b.WriteString(",") + } + } + return b.String() +} + +func (s setSelector) Add(r ...Requirement) Selector { + return s.toFullSelector().Add(r...) +} + +func (s setSelector) Requirements() (requirements Requirements, selectable bool) { + return s.toFullSelector().Requirements() +} + +func (s setSelector) DeepCopySelector() Selector { + res := make(setSelector, len(s)) + for k, v := range s { + res[k] = v + } + return res +} + +func (s setSelector) RequiresExactMatch(label string) (value string, found bool) { + v, f := s[label] + return v, f +} + +func (s setSelector) toFullSelector() Selector { + if s == nil || len(s) == 0 { + return internalSelector{} + } + requirements := make([]Requirement, 0, len(s)) + for label, value := range s { + 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) +} + +var _ Selector = setSelector{} diff --git a/staging/src/k8s.io/apimachinery/pkg/labels/selector_test.go b/staging/src/k8s.io/apimachinery/pkg/labels/selector_test.go index 9d2730284c0..76fc82beca6 100644 --- a/staging/src/k8s.io/apimachinery/pkg/labels/selector_test.go +++ b/staging/src/k8s.io/apimachinery/pkg/labels/selector_test.go @@ -809,11 +809,48 @@ func BenchmarkSelectorFromValidatedSet(b *testing.B) { "foo": "foo", "bar": "bar", } + matchee := Set(map[string]string{ + "foo": "foo", + "bar": "bar", + "extra": "label", + }) for i := 0; i < b.N; i++ { - if SelectorFromValidatedSet(set).Empty() { + s := SelectorFromValidatedSet(set) + if s.Empty() { b.Errorf("Unexpected selector") } + if !s.Matches(matchee) { + b.Errorf("Unexpected match") + } + } +} + +func TestSetSelectorString(t *testing.T) { + cases := []struct { + set Set + out string + }{ + { + Set{}, + "", + }, + { + Set{"app": "foo"}, + "app=foo", + }, + { + Set{"app": "foo", "a": "b"}, + "a=b,app=foo", + }, + } + + for _, tt := range cases { + t.Run(tt.out, func(t *testing.T) { + if got := setSelector(tt.set).String(); tt.out != got { + t.Fatalf("expected %v, got %v", tt.out, got) + } + }) } } @@ -899,19 +936,13 @@ func TestValidatedSelectorFromSet(t *testing.T) { tests := []struct { name string input Set - expectedSelector internalSelector + expectedSelector Selector expectedError field.ErrorList }{ { - name: "Simple Set, no error", - input: Set{"key": "val"}, - expectedSelector: internalSelector{ - Requirement{ - key: "key", - operator: selection.Equals, - strValues: []string{"val"}, - }, - }, + name: "Simple Set, no error", + input: Set{"key": "val"}, + expectedSelector: setSelector{"key": "val"}, }, { name: "Invalid Set, value too long", From ce868cb7519fcf99c697e47a92ea8a31e665d9b0 Mon Sep 17 00:00:00 2001 From: John Howard Date: Wed, 12 Oct 2022 12:36:27 -0700 Subject: [PATCH 2/4] instead of changing existing method, add a new type --- .../podtopologyspread/filtering_test.go | 6 +- .../k8s.io/apimachinery/pkg/labels/labels.go | 2 + .../apimachinery/pkg/labels/selector.go | 64 ++++++++++--------- .../apimachinery/pkg/labels/selector_test.go | 38 +++++++++-- 4 files changed, 71 insertions(+), 39 deletions(-) diff --git a/pkg/scheduler/framework/plugins/podtopologyspread/filtering_test.go b/pkg/scheduler/framework/plugins/podtopologyspread/filtering_test.go index b9abaaefc1d..30f50806a5a 100644 --- a/pkg/scheduler/framework/plugins/podtopologyspread/filtering_test.go +++ b/pkg/scheduler/framework/plugins/podtopologyspread/filtering_test.go @@ -505,7 +505,7 @@ func TestPreFilterState(t *testing.T) { { MaxSkew: 3, TopologyKey: "node", - Selector: labels.SelectorFromValidatedSet(labels.Set{"foo": "bar"}), + Selector: mustConvertLabelSelectorAsSelector(t, st.MakeLabelSelector().Label("foo", "bar").Obj()), MinDomains: 1, NodeAffinityPolicy: v1.NodeInclusionPolicyHonor, NodeTaintsPolicy: v1.NodeInclusionPolicyIgnore, @@ -513,7 +513,7 @@ func TestPreFilterState(t *testing.T) { { MaxSkew: 5, TopologyKey: "rack", - Selector: labels.SelectorFromValidatedSet(labels.Set{"foo": "bar"}), + Selector: mustConvertLabelSelectorAsSelector(t, st.MakeLabelSelector().Label("foo", "bar").Obj()), MinDomains: 1, NodeAffinityPolicy: v1.NodeInclusionPolicyHonor, NodeTaintsPolicy: v1.NodeInclusionPolicyIgnore, @@ -1283,7 +1283,7 @@ func TestPreFilterState(t *testing.T) { { MaxSkew: 1, TopologyKey: "zone", - Selector: labels.SelectorFromValidatedSet(labels.Set{"foo": "a"}), + Selector: mustConvertLabelSelectorAsSelector(t, st.MakeLabelSelector().Label("foo", "a").Obj()), MinDomains: 1, NodeAffinityPolicy: v1.NodeInclusionPolicyHonor, NodeTaintsPolicy: v1.NodeInclusionPolicyIgnore, diff --git a/staging/src/k8s.io/apimachinery/pkg/labels/labels.go b/staging/src/k8s.io/apimachinery/pkg/labels/labels.go index 8360d842b6c..a7e275a449e 100644 --- a/staging/src/k8s.io/apimachinery/pkg/labels/labels.go +++ b/staging/src/k8s.io/apimachinery/pkg/labels/labels.go @@ -77,6 +77,8 @@ func (ls Set) AsValidatedSelector() (Selector, error) { // perform any validation. // According to our measurements this is significantly faster // 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 { return SelectorFromValidatedSet(ls) } diff --git a/staging/src/k8s.io/apimachinery/pkg/labels/selector.go b/staging/src/k8s.io/apimachinery/pkg/labels/selector.go index 5155a3ca941..5e1c980f41a 100644 --- a/staging/src/k8s.io/apimachinery/pkg/labels/selector.go +++ b/staging/src/k8s.io/apimachinery/pkg/labels/selector.go @@ -919,14 +919,6 @@ func SelectorFromSet(ls Set) Selector { 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 // nil and empty Sets are considered equivalent to Everything(). // 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 { return internalSelector{}, nil } - - var allErrs field.ErrorList - for k, v := range ls { - if err := validateLabelKey(k, keyPath); err != nil { - allErrs = append(allErrs, err) - } - if err := validateLabelValue(k, v, value0Path); err != nil { - allErrs = append(allErrs, err) + requirements := make([]Requirement, 0, len(ls)) + for label, value := range ls { + r, err := NewRequirement(label, selection.Equals, []string{value}) + if err != nil { + return nil, err } + requirements = append(requirements, *r) } - - // TODO: validate labels are valid label values - return setSelector(ls), allErrs.ToAggregate() + // sort to have deterministic string representation + sort.Sort(ByKey(requirements)) + return internalSelector(requirements), nil } // SelectorFromValidatedSet returns a Selector which will match exactly the given Set. // A nil and empty Sets are considered equivalent to Everything(). // 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 { if ls == nil || len(ls) == 0 { 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 @@ -968,9 +966,13 @@ func ParseToRequirements(selector string, opts ...field.PathOption) ([]Requireme 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 { if !labels.Has(k) || v != labels.Get(k) { return false @@ -979,11 +981,11 @@ func (s setSelector) Matches(labels Labels) bool { return true } -func (s setSelector) Empty() bool { +func (s SetSelector) Empty() bool { return len(s) == 0 } -func (s setSelector) String() string { +func (s SetSelector) String() string { keys := make([]string, 0, len(s)) for k := range s { keys = append(keys, k) @@ -1004,28 +1006,28 @@ func (s setSelector) String() string { return b.String() } -func (s setSelector) Add(r ...Requirement) Selector { +func (s SetSelector) Add(r ...Requirement) Selector { 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() } -func (s setSelector) DeepCopySelector() Selector { - res := make(setSelector, len(s)) +func (s SetSelector) DeepCopySelector() Selector { + res := make(SetSelector, len(s)) for k, v := range s { res[k] = v } 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] return v, f } -func (s setSelector) toFullSelector() Selector { +func (s SetSelector) toFullSelector() Selector { if s == nil || len(s) == 0 { return internalSelector{} } @@ -1038,4 +1040,4 @@ func (s setSelector) toFullSelector() Selector { return internalSelector(requirements) } -var _ Selector = setSelector{} +var _ Selector = SetSelector{} diff --git a/staging/src/k8s.io/apimachinery/pkg/labels/selector_test.go b/staging/src/k8s.io/apimachinery/pkg/labels/selector_test.go index 76fc82beca6..8557c12c772 100644 --- a/staging/src/k8s.io/apimachinery/pkg/labels/selector_test.go +++ b/staging/src/k8s.io/apimachinery/pkg/labels/selector_test.go @@ -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) { cases := []struct { set Set @@ -847,7 +869,7 @@ func TestSetSelectorString(t *testing.T) { for _, tt := range cases { 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) } }) @@ -936,13 +958,19 @@ func TestValidatedSelectorFromSet(t *testing.T) { tests := []struct { name string input Set - expectedSelector Selector + expectedSelector internalSelector expectedError field.ErrorList }{ { - name: "Simple Set, no error", - input: Set{"key": "val"}, - expectedSelector: setSelector{"key": "val"}, + name: "Simple Set, no error", + input: Set{"key": "val"}, + expectedSelector: internalSelector{ + Requirement{ + key: "key", + operator: selection.Equals, + strValues: []string{"val"}, + }, + }, }, { name: "Invalid Set, value too long", From 5b310565fa1fc1c9e781e5c5baf5371b3ce65758 Mon Sep 17 00:00:00 2001 From: John Howard Date: Wed, 2 Nov 2022 07:53:13 -0700 Subject: [PATCH 3/4] Address comments: re-use code, grow buffer, add comments --- .../apimachinery/pkg/labels/selector.go | 21 +++++++------------ 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/staging/src/k8s.io/apimachinery/pkg/labels/selector.go b/staging/src/k8s.io/apimachinery/pkg/labels/selector.go index 5e1c980f41a..7dc7d40fa98 100644 --- a/staging/src/k8s.io/apimachinery/pkg/labels/selector.go +++ b/staging/src/k8s.io/apimachinery/pkg/labels/selector.go @@ -970,6 +970,8 @@ func ParseToRequirements(selector string, opts ...field.PathOption) ([]Requireme // 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. +// None of the Selector methods mutate the underlying Set, but Add() and Requirements() convert to the +// less optimized version. type SetSelector Set func (s SetSelector) Matches(labels Labels) bool { @@ -994,14 +996,14 @@ func (s SetSelector) String() string { sort.Strings(keys) b := strings.Builder{} for i, key := range keys { - last := i == len(keys)-1 v := s[key] + b.Grow(len(key) + 2 + len(v)) + if i != 0 { + b.WriteString(",") + } b.WriteString(key) b.WriteString("=") b.WriteString(v) - if !last { - b.WriteString(",") - } } return b.String() } @@ -1028,16 +1030,7 @@ func (s SetSelector) RequiresExactMatch(label string) (value string, found bool) } func (s SetSelector) toFullSelector() Selector { - if s == nil || len(s) == 0 { - return internalSelector{} - } - requirements := make([]Requirement, 0, len(s)) - for label, value := range s { - 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) + return SelectorFromValidatedSet(Set(s)) } var _ Selector = SetSelector{} From 4e5487de13437f9e7f64e795d07e64db75ef8cb2 Mon Sep 17 00:00:00 2001 From: John Howard Date: Mon, 14 Nov 2022 08:39:04 -0800 Subject: [PATCH 4/4] Update name and docs --- .../k8s.io/apimachinery/pkg/labels/labels.go | 2 +- .../apimachinery/pkg/labels/selector.go | 38 ++++++++++--------- .../apimachinery/pkg/labels/selector_test.go | 4 +- 3 files changed, 24 insertions(+), 20 deletions(-) diff --git a/staging/src/k8s.io/apimachinery/pkg/labels/labels.go b/staging/src/k8s.io/apimachinery/pkg/labels/labels.go index a7e275a449e..19d823cef7b 100644 --- a/staging/src/k8s.io/apimachinery/pkg/labels/labels.go +++ b/staging/src/k8s.io/apimachinery/pkg/labels/labels.go @@ -77,7 +77,7 @@ func (ls Set) AsValidatedSelector() (Selector, error) { // perform any validation. // According to our measurements this is significantly faster // in codepaths that matter at high scale. -// Note: this method copies the Set; if the Set is immutable, consider wrapping it with SetSelector +// Note: this method copies the Set; if the Set is immutable, consider wrapping it with ValidatedSetSelector // instead, which does not copy. func (ls Set) AsSelectorPreValidated() Selector { return SelectorFromValidatedSet(ls) diff --git a/staging/src/k8s.io/apimachinery/pkg/labels/selector.go b/staging/src/k8s.io/apimachinery/pkg/labels/selector.go index 7dc7d40fa98..082e2bcf522 100644 --- a/staging/src/k8s.io/apimachinery/pkg/labels/selector.go +++ b/staging/src/k8s.io/apimachinery/pkg/labels/selector.go @@ -942,7 +942,7 @@ func ValidatedSelectorFromSet(ls Set) (Selector, error) { // SelectorFromValidatedSet returns a Selector which will match exactly the given Set. // A nil and empty Sets are considered equivalent to Everything(). // 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 +// Note: this method copies the Set; if the Set is immutable, consider wrapping it with ValidatedSetSelector // instead, which does not copy. func SelectorFromValidatedSet(ls Set) Selector { if ls == nil || len(ls) == 0 { @@ -966,15 +966,19 @@ func ParseToRequirements(selector string, opts ...field.PathOption) ([]Requireme return parse(selector, field.ToPath(opts...)) } -// SetSelector wraps a Set, allowing it to implement the Selector interface. Unlike +// ValidatedSetSelector 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. -// None of the Selector methods mutate the underlying Set, but Add() and Requirements() convert to the -// less optimized version. -type SetSelector Set +// Set. As a result, it is substantially more efficient. A nil and empty Sets are considered +// equivalent to Everything(). +// +// Callers MUST ensure the underlying Set is not mutated, and that it is already validated. If these +// constraints are not met, Set.AsValidatedSelector should be preferred +// +// None of the Selector methods mutate the underlying Set, but Add() and Requirements() convert to +// the less optimized version. +type ValidatedSetSelector Set -func (s SetSelector) Matches(labels Labels) bool { +func (s ValidatedSetSelector) Matches(labels Labels) bool { for k, v := range s { if !labels.Has(k) || v != labels.Get(k) { return false @@ -983,11 +987,11 @@ func (s SetSelector) Matches(labels Labels) bool { return true } -func (s SetSelector) Empty() bool { +func (s ValidatedSetSelector) Empty() bool { return len(s) == 0 } -func (s SetSelector) String() string { +func (s ValidatedSetSelector) String() string { keys := make([]string, 0, len(s)) for k := range s { keys = append(keys, k) @@ -1008,29 +1012,29 @@ func (s SetSelector) String() string { return b.String() } -func (s SetSelector) Add(r ...Requirement) Selector { +func (s ValidatedSetSelector) Add(r ...Requirement) Selector { return s.toFullSelector().Add(r...) } -func (s SetSelector) Requirements() (requirements Requirements, selectable bool) { +func (s ValidatedSetSelector) Requirements() (requirements Requirements, selectable bool) { return s.toFullSelector().Requirements() } -func (s SetSelector) DeepCopySelector() Selector { - res := make(SetSelector, len(s)) +func (s ValidatedSetSelector) DeepCopySelector() Selector { + res := make(ValidatedSetSelector, len(s)) for k, v := range s { res[k] = v } return res } -func (s SetSelector) RequiresExactMatch(label string) (value string, found bool) { +func (s ValidatedSetSelector) RequiresExactMatch(label string) (value string, found bool) { v, f := s[label] return v, f } -func (s SetSelector) toFullSelector() Selector { +func (s ValidatedSetSelector) toFullSelector() Selector { return SelectorFromValidatedSet(Set(s)) } -var _ Selector = SetSelector{} +var _ Selector = ValidatedSetSelector{} diff --git a/staging/src/k8s.io/apimachinery/pkg/labels/selector_test.go b/staging/src/k8s.io/apimachinery/pkg/labels/selector_test.go index 8557c12c772..4471969297e 100644 --- a/staging/src/k8s.io/apimachinery/pkg/labels/selector_test.go +++ b/staging/src/k8s.io/apimachinery/pkg/labels/selector_test.go @@ -838,7 +838,7 @@ func BenchmarkSetSelector(b *testing.B) { }) for i := 0; i < b.N; i++ { - s := SetSelector(set) + s := ValidatedSetSelector(set) if s.Empty() { b.Errorf("Unexpected selector") } @@ -869,7 +869,7 @@ func TestSetSelectorString(t *testing.T) { for _, tt := range cases { t.Run(tt.out, func(t *testing.T) { - if got := SetSelector(tt.set).String(); tt.out != got { + if got := ValidatedSetSelector(tt.set).String(); tt.out != got { t.Fatalf("expected %v, got %v", tt.out, got) } })