From 7127915a66d3bf949d55bb3f4ded5456089dd0ee Mon Sep 17 00:00:00 2001 From: Hongchao Deng Date: Mon, 4 Jul 2016 16:02:42 -0700 Subject: [PATCH] selector: make sure value of GT and LT is integer --- pkg/api/helpers_test.go | 8 ++-- pkg/labels/selector.go | 20 ++++---- pkg/labels/selector_test.go | 46 +++++++++---------- .../algorithm/predicates/predicates_test.go | 5 +- 4 files changed, 41 insertions(+), 38 deletions(-) diff --git a/pkg/api/helpers_test.go b/pkg/api/helpers_test.go index 8732f3b8ea9..70f9bf609b6 100644 --- a/pkg/api/helpers_test.go +++ b/pkg/api/helpers_test.go @@ -212,17 +212,17 @@ func TestNodeSelectorRequirementsAsSelector(t *testing.T) { in: []NodeSelectorRequirement{{ Key: "foo", Operator: NodeSelectorOpGt, - Values: []string{"1.1"}, + Values: []string{"1"}, }}, - out: mustParse("foo>1.1"), + out: mustParse("foo>1"), }, { in: []NodeSelectorRequirement{{ Key: "bar", Operator: NodeSelectorOpLt, - Values: []string{"7.1"}, + Values: []string{"7"}, }}, - out: mustParse("bar<7.1"), + out: mustParse("bar<7"), }, } diff --git a/pkg/labels/selector.go b/pkg/labels/selector.go index 4a0254f259e..641fa76ea39 100644 --- a/pkg/labels/selector.go +++ b/pkg/labels/selector.go @@ -108,7 +108,7 @@ type Requirement struct { // (2) If the operator is In or NotIn, the values set must be non-empty. // (3) If the operator is Equals, DoubleEquals, or NotEquals, the values set must contain one value. // (4) If the operator is Exists or DoesNotExist, the value set must be empty. -// (5) If the operator is Gt or Lt, the values set must contain only one value. +// (5) If the operator is Gt or Lt, the values set must contain only one value, which will be interpreted as an integer. // (6) The key is invalid due to its length, or sequence // of characters. See validateLabelKey for more details. // @@ -135,8 +135,8 @@ func NewRequirement(key string, op Operator, vals sets.String) (*Requirement, er return nil, fmt.Errorf("for 'Gt', 'Lt' operators, exactly one value is required") } for val := range vals { - if _, err := strconv.ParseFloat(val, 64); err != nil { - return nil, fmt.Errorf("for 'Gt', 'Lt' operators, the value must be a number") + if _, err := strconv.ParseInt(val, 10, 64); err != nil { + return nil, fmt.Errorf("for 'Gt', 'Lt' operators, the value must be an integer") } } default: @@ -160,6 +160,8 @@ func NewRequirement(key string, op Operator, vals sets.String) (*Requirement, er // Labels' value for that key is not in Requirement's value set. // (4) The operator is DoesNotExist or NotIn and Labels does not have the // Requirement's key. +// (5) The operator is GreaterThanOperator or LessThanOperator, and Labels has +// the Requirement's key and the corresponding value satisfies mathematical inequality. func (r *Requirement) Matches(ls Labels) bool { switch r.operator { case InOperator, EqualsOperator, DoubleEqualsOperator: @@ -180,23 +182,23 @@ func (r *Requirement) Matches(ls Labels) bool { if !ls.Has(r.key) { return false } - lsValue, err := strconv.ParseFloat(ls.Get(r.key), 64) + lsValue, err := strconv.ParseInt(ls.Get(r.key), 10, 64) if err != nil { - glog.V(10).Infof("Parse float failed for value %+v in label %+v, %+v", ls.Get(r.key), ls, err) + glog.V(10).Infof("ParseInt failed for value %+v in label %+v, %+v", ls.Get(r.key), ls, err) return false } - // There should be only one strValue in r.strValues, and can be converted to a float number. + // There should be only one strValue in r.strValues, and can be converted to a integer. if len(r.strValues) != 1 { glog.V(10).Infof("Invalid values count %+v of requirement %+v, for 'Gt', 'Lt' operators, exactly one value is required", len(r.strValues), r) return false } - var rValue float64 + var rValue int64 for strValue := range r.strValues { - rValue, err = strconv.ParseFloat(strValue, 64) + rValue, err = strconv.ParseInt(strValue, 10, 64) if err != nil { - glog.V(10).Infof("Parse float failed for value %+v in requirement %+v, for 'Gt', 'Lt' operators, the value must be a number", strValue, r) + glog.V(10).Infof("ParseInt failed for value %+v in requirement %+v, for 'Gt', 'Lt' operators, the value must be an integer", strValue, r) return false } } diff --git a/pkg/labels/selector_test.go b/pkg/labels/selector_test.go index 79d4c026b30..a659a339fd8 100644 --- a/pkg/labels/selector_test.go +++ b/pkg/labels/selector_test.go @@ -34,8 +34,8 @@ func TestSelectorParse(t *testing.T) { "x=,z= ", "x= ,z= ", "!x", - "x>1.1", - "x>1.1,z<5.3", + "x>1", + "x>1,z<5", } testBadStrings := []string{ "x=a||y=b", @@ -110,16 +110,16 @@ func TestSelectorMatches(t *testing.T) { expectMatch(t, "notin=in", Set{"notin": "in"}) // in and notin in exactMatch expectMatch(t, "x", Set{"x": "z"}) expectMatch(t, "!x", Set{"y": "z"}) - expectMatch(t, "x>1.1", Set{"x": "1.2"}) - expectMatch(t, "x<1.1", Set{"x": "0.8"}) + expectMatch(t, "x>1", Set{"x": "2"}) + expectMatch(t, "x<1", Set{"x": "0"}) expectNoMatch(t, "x=z", Set{}) expectNoMatch(t, "x=y", Set{"x": "z"}) expectNoMatch(t, "x=y,z=w", Set{"x": "w", "z": "w"}) expectNoMatch(t, "x!=y,z!=w", Set{"x": "z", "z": "w"}) expectNoMatch(t, "x", Set{"y": "z"}) expectNoMatch(t, "!x", Set{"x": "z"}) - expectNoMatch(t, "x>1.1", Set{"x": "0.8"}) - expectNoMatch(t, "x<1.1", Set{"x": "1.1"}) + expectNoMatch(t, "x>1", Set{"x": "0"}) + expectNoMatch(t, "x<1", Set{"x": "2"}) labelset := Set{ "foo": "bar", @@ -235,8 +235,8 @@ func TestLexerSequence(t *testing.T) { {"()", []Token{OpenParToken, ClosedParToken}}, {"x in (),y", []Token{IdentifierToken, InToken, OpenParToken, ClosedParToken, CommaToken, IdentifierToken}}, {"== != (), = notin", []Token{DoubleEqualsToken, NotEqualsToken, OpenParToken, ClosedParToken, CommaToken, EqualsToken, NotInToken}}, - {"key>1.1", []Token{IdentifierToken, GreaterThanToken, IdentifierToken}}, - {"key<0.8", []Token{IdentifierToken, LessThanToken, IdentifierToken}}, + {"key>2", []Token{IdentifierToken, GreaterThanToken, IdentifierToken}}, + {"key<1", []Token{IdentifierToken, LessThanToken, IdentifierToken}}, } for _, v := range testcases { var literals []string @@ -274,8 +274,8 @@ func TestParserLookahead(t *testing.T) { {"", []Token{EndOfStringToken}}, {"x in (),y", []Token{IdentifierToken, InToken, OpenParToken, ClosedParToken, CommaToken, IdentifierToken, EndOfStringToken}}, {"== != (), = notin", []Token{DoubleEqualsToken, NotEqualsToken, OpenParToken, ClosedParToken, CommaToken, EqualsToken, NotInToken, EndOfStringToken}}, - {"key>1.1", []Token{IdentifierToken, GreaterThanToken, IdentifierToken, EndOfStringToken}}, - {"key<0.8", []Token{IdentifierToken, LessThanToken, IdentifierToken, EndOfStringToken}}, + {"key>2", []Token{IdentifierToken, GreaterThanToken, IdentifierToken, EndOfStringToken}}, + {"key<1", []Token{IdentifierToken, LessThanToken, IdentifierToken, EndOfStringToken}}, } for _, v := range testcases { p := &Parser{l: &Lexer{s: v.s, pos: 0}, position: 0} @@ -312,8 +312,8 @@ func TestRequirementConstructor(t *testing.T) { {"x", DoesNotExistOperator, nil, true}, {"1foo", InOperator, sets.NewString("bar"), true}, {"1234", InOperator, sets.NewString("bar"), true}, - {"y", GreaterThanOperator, sets.NewString("1.1"), true}, - {"z", LessThanOperator, sets.NewString("5.3"), true}, + {"y", GreaterThanOperator, sets.NewString("1"), true}, + {"z", LessThanOperator, sets.NewString("6"), true}, {"foo", GreaterThanOperator, sets.NewString("bar"), false}, {"barz", LessThanOperator, sets.NewString("blah"), false}, {strings.Repeat("a", 254), ExistsOperator, nil, false}, //breaks DNS rule that len(key) <= 253 @@ -361,10 +361,10 @@ func TestToString(t *testing.T) { getRequirement("z", ExistsOperator, nil, t)}, "x=abc,y==jkl,z!=a,z", true}, {&internalSelector{ - getRequirement("x", GreaterThanOperator, sets.NewString("2.4"), t), - getRequirement("y", LessThanOperator, sets.NewString("7.1"), t), + getRequirement("x", GreaterThanOperator, sets.NewString("2"), t), + getRequirement("y", LessThanOperator, sets.NewString("8"), t), getRequirement("z", ExistsOperator, nil, t)}, - "x>2.4,y<7.1,z", true}, + "x>2,y<8,z", true}, } for _, ts := range toStringTests { if out := ts.In.String(); out == "" && ts.Valid { @@ -408,11 +408,11 @@ func TestRequirementSelectorMatching(t *testing.T) { {Set{"y": "baz"}, &internalSelector{ getRequirement("x", InOperator, sets.NewString(""), t), }, false}, - {Set{"z": "1.2"}, &internalSelector{ - getRequirement("z", GreaterThanOperator, sets.NewString("1.0"), t), + {Set{"z": "2"}, &internalSelector{ + getRequirement("z", GreaterThanOperator, sets.NewString("1"), t), }, true}, - {Set{"z": "v1.2"}, &internalSelector{ - getRequirement("z", GreaterThanOperator, sets.NewString("1.0"), t), + {Set{"z": "v2"}, &internalSelector{ + getRequirement("z", GreaterThanOperator, sets.NewString("1"), t), }, false}, } for _, lsm := range labelSelectorMatchingTests { @@ -473,11 +473,11 @@ func TestSetSelectorParser(t *testing.T) { {"x=a", internalSelector{ getRequirement("x", EqualsOperator, sets.NewString("a"), t), }, true, true}, - {"x>1.1", internalSelector{ - getRequirement("x", GreaterThanOperator, sets.NewString("1.1"), t), + {"x>1", internalSelector{ + getRequirement("x", GreaterThanOperator, sets.NewString("1"), t), }, true, true}, - {"x<7.1", internalSelector{ - getRequirement("x", LessThanOperator, sets.NewString("7.1"), t), + {"x<7", internalSelector{ + getRequirement("x", LessThanOperator, sets.NewString("7"), t), }, true, true}, {"x=a,y!=b", internalSelector{ getRequirement("x", EqualsOperator, sets.NewString("a"), t), diff --git a/plugin/pkg/scheduler/algorithm/predicates/predicates_test.go b/plugin/pkg/scheduler/algorithm/predicates/predicates_test.go index 80e234932c5..1a94b1767e7 100644 --- a/plugin/pkg/scheduler/algorithm/predicates/predicates_test.go +++ b/plugin/pkg/scheduler/algorithm/predicates/predicates_test.go @@ -719,7 +719,7 @@ func TestPodFitsSelector(t *testing.T) { "matchExpressions": [{ "key": "kernel-version", "operator": "Gt", - "values": ["2.4"] + "values": ["0204"] }] }] }}}`, @@ -727,7 +727,8 @@ func TestPodFitsSelector(t *testing.T) { }, }, labels: map[string]string{ - "kernel-version": "2.6", + // We use two digit to denote major version and two digit for minior version. + "kernel-version": "0206", }, fits: true, test: "Pod with matchExpressions using Gt operator that matches the existing node",