diff --git a/pkg/apis/extensions/helpers.go b/pkg/apis/extensions/helpers.go index f7be1a7dc4..df080da562 100644 --- a/pkg/apis/extensions/helpers.go +++ b/pkg/apis/extensions/helpers.go @@ -18,7 +18,6 @@ package extensions import ( "fmt" - "sort" "k8s.io/kubernetes/pkg/api" "k8s.io/kubernetes/pkg/labels" @@ -34,13 +33,13 @@ func PodSelectorAsSelector(ps *PodSelector) (labels.Selector, error) { if len(ps.MatchLabels)+len(ps.MatchExpressions) == 0 { return labels.Everything(), nil } - selector := labels.LabelSelector{} + selector := labels.NewSelector() for k, v := range ps.MatchLabels { - req, err := labels.NewRequirement(k, labels.InOperator, sets.NewString(v)) + r, err := labels.NewRequirement(k, labels.InOperator, sets.NewString(v)) if err != nil { return nil, err } - selector = append(selector, *req) + selector = selector.Add(*r) } for _, expr := range ps.MatchExpressions { var op labels.Operator @@ -56,13 +55,12 @@ func PodSelectorAsSelector(ps *PodSelector) (labels.Selector, error) { default: return nil, fmt.Errorf("%q is not a valid pod selector operator", expr.Operator) } - req, err := labels.NewRequirement(expr.Key, op, sets.NewString(expr.Values...)) + r, err := labels.NewRequirement(expr.Key, op, sets.NewString(expr.Values...)) if err != nil { return nil, err } - selector = append(selector, *req) + selector = selector.Add(*r) } - sort.Sort(labels.ByKey(selector)) return selector, nil } diff --git a/pkg/labels/selector.go b/pkg/labels/selector.go index bc80dcb9cb..1ded7e3754 100644 --- a/pkg/labels/selector.go +++ b/pkg/labels/selector.go @@ -37,21 +37,21 @@ type Selector interface { // String returns a human readable string that represents this selector. String() string - // Add add a specific requirement for the selector - Add(key string, operator Operator, values []string) Selector + // Add adds requirements to the Selector + Add(r ...Requirement) Selector } // Everything returns a selector that matches all labels. func Everything() Selector { - return LabelSelector{} + return internalSelector{} } type nothingSelector struct{} -func (n nothingSelector) Matches(_ Labels) bool { return false } -func (n nothingSelector) Empty() bool { return false } -func (n nothingSelector) String() string { return "" } -func (n nothingSelector) Add(_ string, _ Operator, _ []string) Selector { return n } +func (n nothingSelector) Matches(_ Labels) bool { return false } +func (n nothingSelector) Empty() bool { return false } +func (n nothingSelector) String() string { return "" } +func (n nothingSelector) Add(_ ...Requirement) Selector { return n } // Nothing returns a selector that matches no labels func Nothing() Selector { @@ -72,10 +72,13 @@ const ( ExistsOperator Operator = "exists" ) -//LabelSelector is a list of Requirements. -type LabelSelector []Requirement +func NewSelector() Selector { + return internalSelector(nil) +} -// Sort by obtain determisitic parser +type internalSelector []Requirement + +// Sort by key to obtain determisitic parser type ByKey []Requirement func (a ByKey) Len() int { return len(a) } @@ -178,8 +181,8 @@ func (r *Requirement) Values() sets.String { return ret } -// Return true if the LabelSelector doesn't restrict selection space -func (lsel LabelSelector) Empty() bool { +// Return true if the internalSelector doesn't restrict selection space +func (lsel internalSelector) Empty() bool { if lsel == nil { return true } @@ -228,22 +231,23 @@ func (r *Requirement) String() string { return buffer.String() } -// Add adds a requirement to the selector. It copies the current selector returning a new one -func (lsel LabelSelector) Add(key string, operator Operator, values []string) Selector { - var reqs []Requirement +// Add adds requirements to the selector. It copies the current selector returning a new one +func (lsel internalSelector) Add(reqs ...Requirement) Selector { + var sel internalSelector for _, item := range lsel { - reqs = append(reqs, item) + sel = append(sel, item) } - if r, err := NewRequirement(key, operator, sets.NewString(values...)); err == nil { - reqs = append(reqs, *r) + for _, r := range reqs { + sel = append(sel, r) } - return LabelSelector(reqs) + sort.Sort(ByKey(sel)) + return sel } -// Matches for a LabelSelector returns true if all +// Matches for a internalSelector returns true if all // its Requirements match the input Labels. If any // Requirement does not match, false is returned. -func (lsel LabelSelector) Matches(l Labels) bool { +func (lsel internalSelector) Matches(l Labels) bool { for _, req := range lsel { if matches := req.Matches(l); !matches { return false @@ -253,8 +257,8 @@ func (lsel LabelSelector) Matches(l Labels) bool { } // String returns a comma-separated string of all -// the LabelSelector Requirements' human-readable strings. -func (lsel LabelSelector) String() string { +// the internalSelector Requirements' human-readable strings. +func (lsel internalSelector) String() string { var reqs []string for _, req := range lsel { reqs = append(reqs, req.String()) @@ -473,10 +477,10 @@ func (p *Parser) scan() { // parse runs the left recursive descending algorithm // on input string. It returns a list of Requirement objects. -func (p *Parser) parse() ([]Requirement, error) { +func (p *Parser) parse() (internalSelector, error) { p.scan() // init scannedItems - var requirements []Requirement + var requirements internalSelector for { tok, lit := p.lookahead(Values) switch tok { @@ -692,7 +696,7 @@ func Parse(selector string) (Selector, error) { items, error := p.parse() if error == nil { sort.Sort(ByKey(items)) // sort to grant determistic parsing - return LabelSelector(items), error + return internalSelector(items), error } return nil, error } @@ -717,18 +721,18 @@ func validateLabelValue(v string) error { // nil and empty Sets are considered equivalent to Everything(). func SelectorFromSet(ls Set) Selector { if ls == nil { - return LabelSelector{} + return internalSelector{} } - var requirements []Requirement + var requirements internalSelector for label, value := range ls { if r, err := NewRequirement(label, EqualsOperator, sets.NewString(value)); err != nil { //TODO: double check errors when input comes from serialization? - return LabelSelector{} + return internalSelector{} } else { requirements = append(requirements, *r) } } // sort to have deterministic string representation sort.Sort(ByKey(requirements)) - return LabelSelector(requirements) + return internalSelector(requirements) } diff --git a/pkg/labels/selector_test.go b/pkg/labels/selector_test.go index 32269c8f2c..efe97f7d8b 100644 --- a/pkg/labels/selector_test.go +++ b/pkg/labels/selector_test.go @@ -168,8 +168,8 @@ func TestSetIsEmpty(t *testing.T) { if !(Set{}).AsSelector().Empty() { t.Errorf("Empty set should be empty") } - if !(LabelSelector(nil)).Empty() { - t.Errorf("Nil LabelSelector should be empty") + if !(NewSelector()).Empty() { + t.Errorf("Nil Selector should be empty") } } @@ -313,30 +313,31 @@ func TestRequirementConstructor(t *testing.T) { func TestToString(t *testing.T) { var req Requirement toStringTests := []struct { - In *LabelSelector + In *internalSelector Out string Valid bool }{ - {&LabelSelector{ + + {&internalSelector{ getRequirement("x", InOperator, sets.NewString("abc", "def"), t), getRequirement("y", NotInOperator, sets.NewString("jkl"), t), getRequirement("z", ExistsOperator, nil, t)}, "x in (abc,def),y notin (jkl),z", true}, - {&LabelSelector{ + {&internalSelector{ getRequirement("x", NotInOperator, sets.NewString("abc", "def"), t), getRequirement("y", NotEqualsOperator, sets.NewString("jkl"), t), getRequirement("z", DoesNotExistOperator, nil, t)}, "x notin (abc,def),y!=jkl,!z", true}, - {&LabelSelector{ + {&internalSelector{ getRequirement("x", InOperator, sets.NewString("abc", "def"), t), req}, // adding empty req for the trailing ',' "x in (abc,def),", false}, - {&LabelSelector{ + {&internalSelector{ getRequirement("x", NotInOperator, sets.NewString("abc"), t), getRequirement("y", InOperator, sets.NewString("jkl", "mno"), t), getRequirement("z", NotInOperator, sets.NewString(""), t)}, "x notin (abc),y in (jkl,mno),z notin ()", true}, - {&LabelSelector{ + {&internalSelector{ getRequirement("x", EqualsOperator, sets.NewString("abc"), t), getRequirement("y", DoubleEqualsOperator, sets.NewString("jkl"), t), getRequirement("z", NotEqualsOperator, sets.NewString("a"), t), @@ -352,37 +353,37 @@ func TestToString(t *testing.T) { } } -func TestRequirementLabelSelectorMatching(t *testing.T) { +func TestRequirementSelectorMatching(t *testing.T) { var req Requirement labelSelectorMatchingTests := []struct { Set Set - Sel *LabelSelector + Sel Selector Match bool }{ - {Set{"x": "foo", "y": "baz"}, &LabelSelector{ + {Set{"x": "foo", "y": "baz"}, &internalSelector{ req, }, false}, - {Set{"x": "foo", "y": "baz"}, &LabelSelector{ + {Set{"x": "foo", "y": "baz"}, &internalSelector{ getRequirement("x", InOperator, sets.NewString("foo"), t), getRequirement("y", NotInOperator, sets.NewString("alpha"), t), }, true}, - {Set{"x": "foo", "y": "baz"}, &LabelSelector{ + {Set{"x": "foo", "y": "baz"}, &internalSelector{ getRequirement("x", InOperator, sets.NewString("foo"), t), getRequirement("y", InOperator, sets.NewString("alpha"), t), }, false}, - {Set{"y": ""}, &LabelSelector{ + {Set{"y": ""}, &internalSelector{ getRequirement("x", NotInOperator, sets.NewString(""), t), getRequirement("y", ExistsOperator, nil, t), }, true}, - {Set{"y": ""}, &LabelSelector{ + {Set{"y": ""}, &internalSelector{ getRequirement("x", DoesNotExistOperator, nil, t), getRequirement("y", ExistsOperator, nil, t), }, true}, - {Set{"y": ""}, &LabelSelector{ + {Set{"y": ""}, &internalSelector{ getRequirement("x", NotInOperator, sets.NewString(""), t), getRequirement("y", DoesNotExistOperator, nil, t), }, false}, - {Set{"y": "baz"}, &LabelSelector{ + {Set{"y": "baz"}, &internalSelector{ getRequirement("x", InOperator, sets.NewString(""), t), }, false}, } @@ -400,75 +401,75 @@ func TestSetSelectorParser(t *testing.T) { Match bool Valid bool }{ - {"", LabelSelector(nil), true, true}, - {"\rx", LabelSelector{ + {"", NewSelector(), true, true}, + {"\rx", internalSelector{ getRequirement("x", ExistsOperator, nil, t), }, true, true}, - {"this-is-a-dns.domain.com/key-with-dash", LabelSelector{ + {"this-is-a-dns.domain.com/key-with-dash", internalSelector{ getRequirement("this-is-a-dns.domain.com/key-with-dash", ExistsOperator, nil, t), }, true, true}, - {"this-is-another-dns.domain.com/key-with-dash in (so,what)", LabelSelector{ + {"this-is-another-dns.domain.com/key-with-dash in (so,what)", internalSelector{ getRequirement("this-is-another-dns.domain.com/key-with-dash", InOperator, sets.NewString("so", "what"), t), }, true, true}, - {"0.1.2.domain/99 notin (10.10.100.1, tick.tack.clock)", LabelSelector{ + {"0.1.2.domain/99 notin (10.10.100.1, tick.tack.clock)", internalSelector{ getRequirement("0.1.2.domain/99", NotInOperator, sets.NewString("10.10.100.1", "tick.tack.clock"), t), }, true, true}, - {"foo in (abc)", LabelSelector{ + {"foo in (abc)", internalSelector{ getRequirement("foo", InOperator, sets.NewString("abc"), t), }, true, true}, - {"x notin\n (abc)", LabelSelector{ + {"x notin\n (abc)", internalSelector{ getRequirement("x", NotInOperator, sets.NewString("abc"), t), }, true, true}, - {"x notin \t (abc,def)", LabelSelector{ + {"x notin \t (abc,def)", internalSelector{ getRequirement("x", NotInOperator, sets.NewString("abc", "def"), t), }, true, true}, - {"x in (abc,def)", LabelSelector{ + {"x in (abc,def)", internalSelector{ getRequirement("x", InOperator, sets.NewString("abc", "def"), t), }, true, true}, - {"x in (abc,)", LabelSelector{ + {"x in (abc,)", internalSelector{ getRequirement("x", InOperator, sets.NewString("abc", ""), t), }, true, true}, - {"x in ()", LabelSelector{ + {"x in ()", internalSelector{ getRequirement("x", InOperator, sets.NewString(""), t), }, true, true}, - {"x notin (abc,,def),bar,z in (),w", LabelSelector{ + {"x notin (abc,,def),bar,z in (),w", internalSelector{ getRequirement("bar", ExistsOperator, nil, t), getRequirement("w", ExistsOperator, nil, t), getRequirement("x", NotInOperator, sets.NewString("abc", "", "def"), t), getRequirement("z", InOperator, sets.NewString(""), t), }, true, true}, - {"x,y in (a)", LabelSelector{ + {"x,y in (a)", internalSelector{ getRequirement("y", InOperator, sets.NewString("a"), t), getRequirement("x", ExistsOperator, nil, t), }, false, true}, - {"x=a", LabelSelector{ + {"x=a", internalSelector{ getRequirement("x", EqualsOperator, sets.NewString("a"), t), }, true, true}, - {"x=a,y!=b", LabelSelector{ + {"x=a,y!=b", internalSelector{ getRequirement("x", EqualsOperator, sets.NewString("a"), t), getRequirement("y", NotEqualsOperator, sets.NewString("b"), t), }, true, true}, - {"x=a,y!=b,z in (h,i,j)", LabelSelector{ + {"x=a,y!=b,z in (h,i,j)", internalSelector{ getRequirement("x", EqualsOperator, sets.NewString("a"), t), getRequirement("y", NotEqualsOperator, sets.NewString("b"), t), getRequirement("z", InOperator, sets.NewString("h", "i", "j"), t), }, true, true}, - {"x=a||y=b", LabelSelector{}, false, false}, + {"x=a||y=b", internalSelector{}, false, false}, {"x,,y", nil, true, false}, {",x,y", nil, true, false}, {"x nott in (y)", nil, true, false}, - {"x notin ( )", LabelSelector{ + {"x notin ( )", internalSelector{ getRequirement("x", NotInOperator, sets.NewString(""), t), }, true, true}, - {"x notin (, a)", LabelSelector{ + {"x notin (, a)", internalSelector{ getRequirement("x", NotInOperator, sets.NewString("", "a"), t), }, true, true}, {"a in (xyz),", nil, true, false}, {"a in (xyz)b notin ()", nil, true, false}, - {"a ", LabelSelector{ + {"a ", internalSelector{ getRequirement("a", ExistsOperator, nil, t), }, true, true}, - {"a in (x,y,notin, z,in)", LabelSelector{ + {"a in (x,y,notin, z,in)", internalSelector{ getRequirement("a", InOperator, sets.NewString("in", "notin", "x", "y", "z"), t), }, true, true}, // operator 'in' inside list of identifiers {"a in (xyz abc)", nil, false, false}, // no comma @@ -483,7 +484,7 @@ func TestSetSelectorParser(t *testing.T) { } else if err == nil && !ssp.Valid { t.Errorf("Parse(%s) => %+v expected error", ssp.In, sel) } else if ssp.Match && !reflect.DeepEqual(sel, ssp.Out) { - t.Errorf("Parse(%s) => parse output '%v' doesn't match '%v' expected match", ssp.In, sel, ssp.Out) + t.Errorf("Parse(%s) => parse output '%#v' doesn't match '%#v' expected match", ssp.In, sel, ssp.Out) } } } @@ -499,6 +500,7 @@ func getRequirement(key string, op Operator, vals sets.String, t *testing.T) Req func TestAdd(t *testing.T) { testCases := []struct { + name string sel Selector key string operator Operator @@ -506,27 +508,33 @@ func TestAdd(t *testing.T) { refSelector Selector }{ { - LabelSelector{}, + "keyInOperator", + internalSelector{}, "key", InOperator, []string{"value"}, - LabelSelector{Requirement{"key", InOperator, sets.NewString("value")}}, + internalSelector{Requirement{"key", InOperator, sets.NewString("value")}}, }, { - LabelSelector{Requirement{"key", InOperator, sets.NewString("value")}}, + "keyEqualsOperator", + internalSelector{Requirement{"key", InOperator, sets.NewString("value")}}, "key2", EqualsOperator, []string{"value2"}, - LabelSelector{ + internalSelector{ Requirement{"key", InOperator, sets.NewString("value")}, Requirement{"key2", EqualsOperator, sets.NewString("value2")}, }, }, } for _, ts := range testCases { - ts.sel = ts.sel.Add(ts.key, ts.operator, ts.values) + req, err := NewRequirement(ts.key, ts.operator, sets.NewString(ts.values...)) + if err != nil { + t.Errorf("%s - Unable to create labels.Requirement", ts.name) + } + ts.sel = ts.sel.Add(*req) if !reflect.DeepEqual(ts.sel, ts.refSelector) { - t.Errorf("Expected %v found %v", ts.refSelector, ts.sel) + t.Errorf("%s - Expected %v found %v", ts.name, ts.refSelector, ts.sel) } } }