diff --git a/pkg/api/helpers.go b/pkg/api/helpers.go index c1da66284f4..3de57d4b9d2 100644 --- a/pkg/api/helpers.go +++ b/pkg/api/helpers.go @@ -417,7 +417,7 @@ func NodeSelectorRequirementsAsSelector(nsm []NodeSelectorRequirement) (labels.S default: return nil, fmt.Errorf("%q is not a valid node selector operator", expr.Operator) } - r, err := labels.NewRequirement(expr.Key, op, sets.NewString(expr.Values...)) + r, err := labels.NewRequirement(expr.Key, op, expr.Values) if err != nil { return nil, err } diff --git a/pkg/api/unversioned/helpers.go b/pkg/api/unversioned/helpers.go index 5bd9051cbca..aed3f0edac7 100644 --- a/pkg/api/unversioned/helpers.go +++ b/pkg/api/unversioned/helpers.go @@ -21,7 +21,6 @@ import ( "k8s.io/kubernetes/pkg/labels" "k8s.io/kubernetes/pkg/selection" - "k8s.io/kubernetes/pkg/util/sets" ) // LabelSelectorAsSelector converts the LabelSelector api type into a struct that implements @@ -36,7 +35,7 @@ func LabelSelectorAsSelector(ps *LabelSelector) (labels.Selector, error) { } selector := labels.NewSelector() for k, v := range ps.MatchLabels { - r, err := labels.NewRequirement(k, selection.Equals, sets.NewString(v)) + r, err := labels.NewRequirement(k, selection.Equals, []string{v}) if err != nil { return nil, err } @@ -56,7 +55,7 @@ func LabelSelectorAsSelector(ps *LabelSelector) (labels.Selector, error) { default: return nil, fmt.Errorf("%q is not a valid pod selector operator", expr.Operator) } - r, err := labels.NewRequirement(expr.Key, op, sets.NewString(expr.Values...)) + r, err := labels.NewRequirement(expr.Key, op, append([]string(nil), expr.Values...)) if err != nil { return nil, err } diff --git a/pkg/labels/selector.go b/pkg/labels/selector.go index 703cf7aa1bf..705f42477ef 100644 --- a/pkg/labels/selector.go +++ b/pkg/labels/selector.go @@ -91,9 +91,12 @@ func (a ByKey) Less(i, j int) bool { return a[i].key < a[j].key } // Requirement implements both set based match and exact match // Requirement should be initialized via NewRequirement constructor for creating a valid Requirement. type Requirement struct { - key string - operator selection.Operator - strValues sets.String + key string + operator selection.Operator + // In huge majority of cases we have at most one value here. + // It is generally faster to operate on a single-element slice + // than on a single-element map, so we have a slice here. + strValues []string } // NewRequirement is the constructor for a Requirement. @@ -107,7 +110,7 @@ type Requirement struct { // of characters. See validateLabelKey for more details. // // The empty string is a valid value in the input values set. -func NewRequirement(key string, op selection.Operator, vals sets.String) (*Requirement, error) { +func NewRequirement(key string, op selection.Operator, vals []string) (*Requirement, error) { if err := validateLabelKey(key); err != nil { return nil, err } @@ -128,8 +131,8 @@ func NewRequirement(key string, op selection.Operator, vals sets.String) (*Requi if len(vals) != 1 { return nil, fmt.Errorf("for 'Gt', 'Lt' operators, exactly one value is required") } - for val := range vals { - if _, err := strconv.ParseInt(val, 10, 64); err != nil { + for i := range vals { + if _, err := strconv.ParseInt(vals[i], 10, 64); err != nil { return nil, fmt.Errorf("for 'Gt', 'Lt' operators, the value must be an integer") } } @@ -137,14 +140,24 @@ func NewRequirement(key string, op selection.Operator, vals sets.String) (*Requi return nil, fmt.Errorf("operator '%v' is not recognized", op) } - for v := range vals { - if err := validateLabelValue(v); err != nil { + for i := range vals { + if err := validateLabelValue(vals[i]); err != nil { return nil, err } } + sort.Strings(vals) return &Requirement{key: key, operator: op, strValues: vals}, nil } +func (r *Requirement) hasValue(value string) bool { + for i := range r.strValues { + if r.strValues[i] == value { + return true + } + } + return false +} + // Matches returns true if the Requirement matches the input Labels. // There is a match in the following cases: // (1) The operator is Exists and Labels has the Requirement's key. @@ -162,12 +175,12 @@ func (r *Requirement) Matches(ls Labels) bool { if !ls.Has(r.key) { return false } - return r.strValues.Has(ls.Get(r.key)) + return r.hasValue(ls.Get(r.key)) case selection.NotIn, selection.NotEquals: if !ls.Has(r.key) { return true } - return !r.strValues.Has(ls.Get(r.key)) + return !r.hasValue(ls.Get(r.key)) case selection.Exists: return ls.Has(r.key) case selection.DoesNotExist: @@ -189,10 +202,10 @@ func (r *Requirement) Matches(ls Labels) bool { } var rValue int64 - for strValue := range r.strValues { - rValue, err = strconv.ParseInt(strValue, 10, 64) + for i := range r.strValues { + rValue, err = strconv.ParseInt(r.strValues[i], 10, 64) if err != nil { - glog.V(10).Infof("ParseInt failed for value %+v in requirement %#v, for 'Gt', 'Lt' operators, the value must be an integer", strValue, r) + glog.V(10).Infof("ParseInt failed for value %+v in requirement %#v, for 'Gt', 'Lt' operators, the value must be an integer", r.strValues[i], r) return false } } @@ -210,8 +223,8 @@ func (r *Requirement) Operator() selection.Operator { } func (r *Requirement) Values() sets.String { ret := sets.String{} - for k := range r.strValues { - ret.Insert(k) + for i := range r.strValues { + ret.Insert(r.strValues[i]) } return ret } @@ -258,9 +271,9 @@ func (r *Requirement) String() string { buffer.WriteString("(") } if len(r.strValues) == 1 { - buffer.WriteString(r.strValues.List()[0]) + buffer.WriteString(r.strValues[0]) } else { // only > 1 since == 0 prohibited by NewRequirement - buffer.WriteString(strings.Join(r.strValues.List(), ",")) + buffer.WriteString(strings.Join(r.strValues, ",")) } switch r.operator { @@ -561,7 +574,7 @@ func (p *Parser) parseRequirement() (*Requirement, error) { return nil, err } if operator == selection.Exists || operator == selection.DoesNotExist { // operator found lookahead set checked - return NewRequirement(key, operator, nil) + return NewRequirement(key, operator, []string{}) } operator, err = p.parseOperator() if err != nil { @@ -577,7 +590,7 @@ func (p *Parser) parseRequirement() (*Requirement, error) { if err != nil { return nil, err } - return NewRequirement(key, operator, values) + return NewRequirement(key, operator, values.List()) } @@ -784,7 +797,7 @@ func SelectorFromSet(ls Set) Selector { } var requirements internalSelector for label, value := range ls { - if r, err := NewRequirement(label, selection.Equals, sets.NewString(value)); err != nil { + if r, err := NewRequirement(label, selection.Equals, []string{value}); err != nil { //TODO: double check errors when input comes from serialization? return internalSelector{} } else { @@ -805,7 +818,7 @@ func SelectorFromValidatedSet(ls Set) Selector { } var requirements internalSelector for label, value := range ls { - requirements = append(requirements, Requirement{key: label, operator: selection.Equals, strValues: sets.NewString(value)}) + requirements = append(requirements, Requirement{key: label, operator: selection.Equals, strValues: []string{value}}) } // sort to have deterministic string representation sort.Sort(ByKey(requirements)) diff --git a/pkg/labels/selector_test.go b/pkg/labels/selector_test.go index 76987d7e138..2d0147b3d99 100644 --- a/pkg/labels/selector_test.go +++ b/pkg/labels/selector_test.go @@ -320,7 +320,7 @@ func TestRequirementConstructor(t *testing.T) { {strings.Repeat("a", 254), selection.Exists, nil, false}, //breaks DNS rule that len(key) <= 253 } for _, rc := range requirementConstructorTests { - if _, err := NewRequirement(rc.Key, rc.Op, rc.Vals); err == nil && !rc.Success { + if _, err := NewRequirement(rc.Key, rc.Op, rc.Vals.List()); err == nil && !rc.Success { t.Errorf("expected error with key:%#v op:%v vals:%v, got no error", rc.Key, rc.Op, rc.Vals) } else if err != nil && rc.Success { t.Errorf("expected no error with key:%#v op:%v vals:%v, got:%v", rc.Key, rc.Op, rc.Vals, err) @@ -525,7 +525,7 @@ func TestSetSelectorParser(t *testing.T) { } func getRequirement(key string, op selection.Operator, vals sets.String, t *testing.T) Requirement { - req, err := NewRequirement(key, op, vals) + req, err := NewRequirement(key, op, vals.List()) if err != nil { t.Errorf("NewRequirement(%v, %v, %v) resulted in error:%v", key, op, vals, err) return Requirement{} @@ -548,22 +548,22 @@ func TestAdd(t *testing.T) { "key", selection.In, []string{"value"}, - internalSelector{Requirement{"key", selection.In, sets.NewString("value")}}, + internalSelector{Requirement{"key", selection.In, []string{"value"}}}, }, { "keyEqualsOperator", - internalSelector{Requirement{"key", selection.In, sets.NewString("value")}}, + internalSelector{Requirement{"key", selection.In, []string{"value"}}}, "key2", selection.Equals, []string{"value2"}, internalSelector{ - Requirement{"key", selection.In, sets.NewString("value")}, - Requirement{"key2", selection.Equals, sets.NewString("value2")}, + Requirement{"key", selection.In, []string{"value"}}, + Requirement{"key2", selection.Equals, []string{"value2"}}, }, }, } for _, ts := range testCases { - req, err := NewRequirement(ts.key, ts.operator, sets.NewString(ts.values...)) + req, err := NewRequirement(ts.key, ts.operator, ts.values) if err != nil { t.Errorf("%s - Unable to create labels.Requirement", ts.name) } diff --git a/pkg/registry/generic/registry/store_test.go b/pkg/registry/generic/registry/store_test.go index 5e3a3d5ef7d..0fd1e77d7a0 100644 --- a/pkg/registry/generic/registry/store_test.go +++ b/pkg/registry/generic/registry/store_test.go @@ -43,7 +43,6 @@ import ( etcdtesting "k8s.io/kubernetes/pkg/storage/etcd/testing" "k8s.io/kubernetes/pkg/storage/storagebackend/factory" storagetesting "k8s.io/kubernetes/pkg/storage/testing" - "k8s.io/kubernetes/pkg/util/sets" "k8s.io/kubernetes/pkg/util/validation/field" "k8s.io/kubernetes/pkg/util/wait" ) @@ -109,7 +108,7 @@ func NewTestGenericStoreRegistry(t *testing.T) (factory.DestroyFunc, *Store) { func matchPodName(names ...string) storage.SelectionPredicate { // Note: even if pod name is a field, we have to use labels, // because field selector doesn't support "IN" operator. - l, err := labels.NewRequirement("name", selection.In, sets.NewString(names...)) + l, err := labels.NewRequirement("name", selection.In, names) if err != nil { panic("Labels requirement must validate successfully") }