From 6d3af1f8ff76f5a26646c8583234943aea29c905 Mon Sep 17 00:00:00 2001 From: Daniel Smith Date: Mon, 16 Jun 2014 13:21:53 -0700 Subject: [PATCH 01/12] Make success status public so it will actually get sent to clients --- pkg/apiserver/apiserver.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/apiserver/apiserver.go b/pkg/apiserver/apiserver.go index 8da4c6e7be9..96373711b5d 100644 --- a/pkg/apiserver/apiserver.go +++ b/pkg/apiserver/apiserver.go @@ -38,7 +38,7 @@ type RESTStorage interface { // Status is a return value for calls that don't return other objects type Status struct { - success bool + Success bool } // ApiServer is an HTTPHandler that delegates to RESTStorage objects. @@ -195,7 +195,7 @@ func (server *ApiServer) handleREST(parts []string, url *url.URL, req *http.Requ server.error(err, w) return } - server.write(200, Status{success: true}, w) + server.write(200, Status{Success: true}, w) return case "PUT": if len(parts) != 2 { From 3ab2f8a3a21e62638ff07a564164d2e60ae1a101 Mon Sep 17 00:00:00 2001 From: Daniel Smith Date: Mon, 16 Jun 2014 17:49:50 -0700 Subject: [PATCH 02/12] First piece of improving labels --- pkg/apiserver/apiserver.go | 17 ++- pkg/apiserver/apiserver_test.go | 3 +- pkg/apiserver/labelquery.go | 221 +++++++++++++++++++++++++++++++ pkg/apiserver/labelquery_test.go | 80 +++++++++++ pkg/client/client_test.go | 24 ---- 5 files changed, 313 insertions(+), 32 deletions(-) create mode 100644 pkg/apiserver/labelquery.go create mode 100644 pkg/apiserver/labelquery_test.go diff --git a/pkg/apiserver/apiserver.go b/pkg/apiserver/apiserver.go index 96373711b5d..8d589f1e6f4 100644 --- a/pkg/apiserver/apiserver.go +++ b/pkg/apiserver/apiserver.go @@ -28,7 +28,7 @@ import ( // RESTStorage is a generic interface for RESTful storage services type RESTStorage interface { - List(*url.URL) (interface{}, error) + List(LabelQuery) (interface{}, error) Get(id string) (interface{}, error) Delete(id string) error Extract(body string) (interface{}, error) @@ -141,28 +141,33 @@ func (server *ApiServer) readBody(req *http.Request) (string, error) { // PUT /foo/bar update 'bar' // DELETE /foo/bar delete 'bar' // Returns 404 if the method/pattern doesn't match one of these entries -func (server *ApiServer) handleREST(parts []string, url *url.URL, req *http.Request, w http.ResponseWriter, storage RESTStorage) { +func (server *ApiServer) handleREST(parts []string, requestUrl *url.URL, req *http.Request, w http.ResponseWriter, storage RESTStorage) { switch req.Method { case "GET": switch len(parts) { case 1: - controllers, err := storage.List(url) + query, err := ParseLabelQuery(requestUrl.Query().Get("labels")) + if err != nil { + server.error(err, w) + return + } + controllers, err := storage.List(query) if err != nil { server.error(err, w) return } server.write(200, controllers, w) case 2: - pod, err := storage.Get(parts[1]) + item, err := storage.Get(parts[1]) if err != nil { server.error(err, w) return } - if pod == nil { + if item == nil { server.notFound(req, w) return } - server.write(200, pod, w) + server.write(200, item, w) default: server.notFound(req, w) } diff --git a/pkg/apiserver/apiserver_test.go b/pkg/apiserver/apiserver_test.go index 1b80d297f10..cc53ec101ad 100644 --- a/pkg/apiserver/apiserver_test.go +++ b/pkg/apiserver/apiserver_test.go @@ -22,7 +22,6 @@ import ( "io/ioutil" "net/http" "net/http/httptest" - "net/url" "reflect" "testing" ) @@ -50,7 +49,7 @@ type SimpleRESTStorage struct { updated Simple } -func (storage *SimpleRESTStorage) List(*url.URL) (interface{}, error) { +func (storage *SimpleRESTStorage) List(LabelQuery) (interface{}, error) { result := SimpleList{ Items: storage.list, } diff --git a/pkg/apiserver/labelquery.go b/pkg/apiserver/labelquery.go new file mode 100644 index 00000000000..1c7d174bf24 --- /dev/null +++ b/pkg/apiserver/labelquery.go @@ -0,0 +1,221 @@ +/* +Copyright 2014 Google Inc. All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package apiserver + +import ( + "fmt" + "strings" +) + +// Labels allows you to present labels. +type Labels interface { + Get(label string) (value string) +} + +// A map of label:value. +type LabelSet map[string]string + +func (ls LabelSet) String() string { + query := make([]string, 0, len(ls)) + for key, value := range ls { + query = append(query, key+"="+value) + } + return strings.Join(query, ",") +} + +func (ls LabelSet) Get(label string) string { + return ls[label] +} + +// Represents a query. +type LabelQuery interface { + // Returns true if this query matches the given set of labels. + Matches(Labels) bool + + // Prints a human readable version of this label query. + String() string +} + +// A single term of a label query. +type labelQueryTerm struct { + // Not inverts the meaning of the items in this term. + not bool + + // Exactly one of the below three items should be used. + + // If non-nil, we match LabelSet l iff l[*label] == *value. + label, value *string + + // A list of terms which must all match for this query term to return true. + and []labelQueryTerm + + // A list of terms, any one of which will cause this query term to return true. + // Parsing/printing not implemented. + or []labelQueryTerm +} + +func (l *labelQueryTerm) Matches(ls Labels) bool { + matches := !l.not + switch { + case l.label != nil && l.value != nil: + if ls.Get(*l.label) == *l.value { + return matches + } + return !matches + case len(l.and) > 0: + for i := range l.and { + if !l.and[i].Matches(ls) { + return !matches + } + } + return matches + case len(l.or) > 0: + for i := range l.or { + if l.or[i].Matches(ls) { + return matches + } + } + return !matches + } + + // Empty queries match everything + return matches +} + +func try(queryPiece, op string) (lhs, rhs string, ok bool) { + pieces := strings.Split(queryPiece, op) + if len(pieces) == 2 { + return pieces[0], pieces[1], true + } + return "", "", false +} + +// Takes a string repsenting a label query and returns an object suitable for matching, or an error. +func ParseLabelQuery(query string) (LabelQuery, error) { + parts := strings.Split(query, ",") + var items []labelQueryTerm + for _, part := range parts { + if part == "" { + continue + } + if lhs, rhs, ok := try(part, "!="); ok { + items = append(items, labelQueryTerm{not: true, label: &lhs, value: &rhs}) + } else if lhs, rhs, ok := try(part, "=="); ok { + items = append(items, labelQueryTerm{label: &lhs, value: &rhs}) + } else if lhs, rhs, ok := try(part, "="); ok { + items = append(items, labelQueryTerm{label: &lhs, value: &rhs}) + } else { + return nil, fmt.Errorf("invalid label query: '%s'; can't understand '%s'", query, part) + } + } + if len(items) == 1 { + return &items[0], nil + } + return &labelQueryTerm{and: items}, nil +} + +// Returns this query as a string in a form that ParseLabelQuery can parse. +func (l *labelQueryTerm) String() (out string) { + if len(l.and) > 0 { + for _, part := range l.and { + if out != "" { + out += "," + } + out += part.String() + } + return + } else if l.label != nil && l.value != nil { + op := "=" + if l.not { + op = "!=" + } + return fmt.Sprintf("%v%v%v", *l.label, op, *l.value) + } + return "" +} + +/* +type parseErr struct { + Reason string + Pos token.Pos +} + +func (p parseErr) Error() string { + return fmt.Sprintf("%v: %v", p.Reason, p.Pos) +} + +func fromLiteral(expr *ast.BinaryExpr) (*labelQueryTerm, error) { + lhs, ok := expr.X.(*ast.Ident) + if !ok { + return nil, parseErr{"expected literal", expr.X.Pos()} + } + +} + +func fromBinaryExpr(expr *ast.BinaryExpr) (*labelQueryTerm, error) { + switch expr.Op { + case token.EQL, token.NEQ: + return fromLiteral(expr) + } + lhs, err := fromExpr(expr.X) + if err != nil { + return nil, err + } + rhs, err := fromExpr(expr.Y) + if err != nil { + return nil, err + } + switch expr.Op { + case token.AND, token.LAND: + return &labelQueryTerm{And: []LabelQuery{lhs, rhs}} + case token.OR, token.LOR: + return &labelQueryTerm{Or: []LabelQuery{lhs, rhs}} + } +} + +func fromUnaryExpr(expr *ast.UnaryExpr) (*labelQueryTerm, error) { + if expr.Op == token.NOT { + lqt, err := fromExpr(expr.X) + if err != nil { + return nil, err + } + lqt.not = !lqt.not + return lqt, nil + } + return nil, parseErr{"unrecognized unary expression", expr.OpPos} +} + +func fromExpr(expr ast.Expr) (*labelQueryTerm, error) { + switch v := expr.(type) { + case *ast.UnaryExpr: + return fromUnaryExpr(v) + case *ast.BinaryExpr: + return fromBinaryExpr(v) + } + return nil, parseErr{"unrecognized expression type", expr.Pos()} +} + +// Takes a string repsenting a label query and returns an object suitable for matching, or an error. +func ParseLabelQuery(query string) (LabelQuery, error) { + expr, err := parser.ParseExpr(query) + if err != nil { + return nil, err + } + log.Printf("%v: %v (%#v)", query, expr, expr) + return fromExpr(expr) +} +*/ diff --git a/pkg/apiserver/labelquery_test.go b/pkg/apiserver/labelquery_test.go new file mode 100644 index 00000000000..acc3964ee78 --- /dev/null +++ b/pkg/apiserver/labelquery_test.go @@ -0,0 +1,80 @@ +/* +Copyright 2014 Google Inc. All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package apiserver + +import ( + "testing" +) + +func TestLabelQueryParse(t *testing.T) { + testGoodStrings := []string{ + "x=a,y=b,z=c", + "", + "x!=a,y=b", + } + testBadStrings := []string{ + "x=a||y=b", + "x==a==b", + } + for _, test := range testGoodStrings { + lq, err := ParseLabelQuery(test) + if err != nil { + t.Errorf("%v: error %v (%#v)\n", test, err, err) + } + if test != lq.String() { + t.Errorf("%v restring gave: %v\n", test, lq.String()) + } + } + for _, test := range testBadStrings { + _, err := ParseLabelQuery(test) + if err == nil { + t.Errorf("%v: did not get expected error\n", test) + } + } +} + +func shouldMatch(t *testing.T, query string, ls LabelSet) { + lq, err := ParseLabelQuery(query) + if err != nil { + t.Errorf("Unable to parse %v as a query\n", query) + return + } + if !lq.Matches(ls) { + t.Errorf("Wanted %s to match %s, but it did not.\n", query, ls) + } +} + +func shouldNotMatch(t *testing.T, query string, ls LabelSet) { + lq, err := ParseLabelQuery(query) + if err != nil { + t.Errorf("Unable to parse %v as a query\n", query) + return + } + if lq.Matches(ls) { + t.Errorf("Wanted '%s' to not match %s, but it did.", query, ls) + } +} + +func TestSimpleLabel(t *testing.T) { + shouldMatch(t, "", LabelSet{"x": "y"}) + shouldMatch(t, "x=y", LabelSet{"x": "y"}) + shouldMatch(t, "x=y,z=w", LabelSet{"x": "y", "z": "w"}) + shouldMatch(t, "x!=y,z!=w", LabelSet{"x": "z", "z": "a"}) + shouldNotMatch(t, "x=y", LabelSet{"x": "z"}) + shouldNotMatch(t, "x=y,z=w", LabelSet{"x": "w", "z": "w"}) + shouldNotMatch(t, "x!=y,z!=w", LabelSet{"x": "z", "z": "w"}) +} diff --git a/pkg/client/client_test.go b/pkg/client/client_test.go index eb0f619c999..9cd7cf404f6 100644 --- a/pkg/client/client_test.go +++ b/pkg/client/client_test.go @@ -245,30 +245,6 @@ func expectEqual(t *testing.T, expected, observed interface{}) { } } -func TestEncodeDecodeLabelQuery(t *testing.T) { - queryIn := map[string]string{ - "foo": "bar", - "baz": "blah", - } - queryString, _ := url.QueryUnescape(EncodeLabelQuery(queryIn)) - queryOut := DecodeLabelQuery(queryString) - expectEqual(t, queryIn, queryOut) -} - -func TestDecodeEmpty(t *testing.T) { - query := DecodeLabelQuery("") - if len(query) != 0 { - t.Errorf("Unexpected query: %#v", query) - } -} - -func TestDecodeBad(t *testing.T) { - query := DecodeLabelQuery("foo") - if len(query) != 0 { - t.Errorf("Unexpected query: %#v", query) - } -} - func TestGetController(t *testing.T) { expectedController := api.ReplicationController{ JSONBase: api.JSONBase{ From 1c6342a79439c6fbd6e8de464d45b3e5c3fdc2cc Mon Sep 17 00:00:00 2001 From: Daniel Smith Date: Mon, 16 Jun 2014 17:57:57 -0700 Subject: [PATCH 03/12] Move labels to own package --- pkg/labels/doc.go | 19 ++++++++ pkg/{apiserver => labels}/labelquery.go | 47 +++++++++++--------- pkg/{apiserver => labels}/labelquery_test.go | 12 ++--- 3 files changed, 50 insertions(+), 28 deletions(-) create mode 100644 pkg/labels/doc.go rename pkg/{apiserver => labels}/labelquery.go (79%) rename pkg/{apiserver => labels}/labelquery_test.go (90%) diff --git a/pkg/labels/doc.go b/pkg/labels/doc.go new file mode 100644 index 00000000000..54318e41bb8 --- /dev/null +++ b/pkg/labels/doc.go @@ -0,0 +1,19 @@ +/* +Copyright 2014 Google Inc. All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +// Package labels implements a simple label system, parsing and matching queries +// with sets of labels. +package labels diff --git a/pkg/apiserver/labelquery.go b/pkg/labels/labelquery.go similarity index 79% rename from pkg/apiserver/labelquery.go rename to pkg/labels/labelquery.go index 1c7d174bf24..1cf1c95f5b2 100644 --- a/pkg/apiserver/labelquery.go +++ b/pkg/labels/labelquery.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package apiserver +package labels import ( "fmt" @@ -26,9 +26,11 @@ type Labels interface { Get(label string) (value string) } -// A map of label:value. +// A map of label:value. Implements Labels. type LabelSet map[string]string +// All labels listed as a human readable string. Conveiently, exactly the format +// that ParseQuery takes. func (ls LabelSet) String() string { query := make([]string, 0, len(ls)) for key, value := range ls { @@ -37,12 +39,13 @@ func (ls LabelSet) String() string { return strings.Join(query, ",") } +// Implement Labels interface. func (ls LabelSet) Get(label string) string { return ls[label] } -// Represents a query. -type LabelQuery interface { +// Represents a label query. +type Query interface { // Returns true if this query matches the given set of labels. Matches(Labels) bool @@ -51,7 +54,7 @@ type LabelQuery interface { } // A single term of a label query. -type labelQueryTerm struct { +type queryTerm struct { // Not inverts the meaning of the items in this term. not bool @@ -61,14 +64,14 @@ type labelQueryTerm struct { label, value *string // A list of terms which must all match for this query term to return true. - and []labelQueryTerm + and []queryTerm // A list of terms, any one of which will cause this query term to return true. // Parsing/printing not implemented. - or []labelQueryTerm + or []queryTerm } -func (l *labelQueryTerm) Matches(ls Labels) bool { +func (l *queryTerm) Matches(ls Labels) bool { matches := !l.not switch { case l.label != nil && l.value != nil: @@ -105,19 +108,19 @@ func try(queryPiece, op string) (lhs, rhs string, ok bool) { } // Takes a string repsenting a label query and returns an object suitable for matching, or an error. -func ParseLabelQuery(query string) (LabelQuery, error) { +func ParseQuery(query string) (Query, error) { parts := strings.Split(query, ",") - var items []labelQueryTerm + var items []queryTerm for _, part := range parts { if part == "" { continue } if lhs, rhs, ok := try(part, "!="); ok { - items = append(items, labelQueryTerm{not: true, label: &lhs, value: &rhs}) + items = append(items, queryTerm{not: true, label: &lhs, value: &rhs}) } else if lhs, rhs, ok := try(part, "=="); ok { - items = append(items, labelQueryTerm{label: &lhs, value: &rhs}) + items = append(items, queryTerm{label: &lhs, value: &rhs}) } else if lhs, rhs, ok := try(part, "="); ok { - items = append(items, labelQueryTerm{label: &lhs, value: &rhs}) + items = append(items, queryTerm{label: &lhs, value: &rhs}) } else { return nil, fmt.Errorf("invalid label query: '%s'; can't understand '%s'", query, part) } @@ -125,11 +128,11 @@ func ParseLabelQuery(query string) (LabelQuery, error) { if len(items) == 1 { return &items[0], nil } - return &labelQueryTerm{and: items}, nil + return &queryTerm{and: items}, nil } -// Returns this query as a string in a form that ParseLabelQuery can parse. -func (l *labelQueryTerm) String() (out string) { +// Returns this query as a string in a form that ParseQuery can parse. +func (l *queryTerm) String() (out string) { if len(l.and) > 0 { for _, part := range l.and { if out != "" { @@ -158,7 +161,7 @@ func (p parseErr) Error() string { return fmt.Sprintf("%v: %v", p.Reason, p.Pos) } -func fromLiteral(expr *ast.BinaryExpr) (*labelQueryTerm, error) { +func fromLiteral(expr *ast.BinaryExpr) (*queryTerm, error) { lhs, ok := expr.X.(*ast.Ident) if !ok { return nil, parseErr{"expected literal", expr.X.Pos()} @@ -166,7 +169,7 @@ func fromLiteral(expr *ast.BinaryExpr) (*labelQueryTerm, error) { } -func fromBinaryExpr(expr *ast.BinaryExpr) (*labelQueryTerm, error) { +func fromBinaryExpr(expr *ast.BinaryExpr) (*queryTerm, error) { switch expr.Op { case token.EQL, token.NEQ: return fromLiteral(expr) @@ -181,13 +184,13 @@ func fromBinaryExpr(expr *ast.BinaryExpr) (*labelQueryTerm, error) { } switch expr.Op { case token.AND, token.LAND: - return &labelQueryTerm{And: []LabelQuery{lhs, rhs}} + return &queryTerm{And: []LabelQuery{lhs, rhs}} case token.OR, token.LOR: - return &labelQueryTerm{Or: []LabelQuery{lhs, rhs}} + return &queryTerm{Or: []LabelQuery{lhs, rhs}} } } -func fromUnaryExpr(expr *ast.UnaryExpr) (*labelQueryTerm, error) { +func fromUnaryExpr(expr *ast.UnaryExpr) (*queryTerm, error) { if expr.Op == token.NOT { lqt, err := fromExpr(expr.X) if err != nil { @@ -199,7 +202,7 @@ func fromUnaryExpr(expr *ast.UnaryExpr) (*labelQueryTerm, error) { return nil, parseErr{"unrecognized unary expression", expr.OpPos} } -func fromExpr(expr ast.Expr) (*labelQueryTerm, error) { +func fromExpr(expr ast.Expr) (*queryTerm, error) { switch v := expr.(type) { case *ast.UnaryExpr: return fromUnaryExpr(v) diff --git a/pkg/apiserver/labelquery_test.go b/pkg/labels/labelquery_test.go similarity index 90% rename from pkg/apiserver/labelquery_test.go rename to pkg/labels/labelquery_test.go index acc3964ee78..f0f3f83fd4f 100644 --- a/pkg/apiserver/labelquery_test.go +++ b/pkg/labels/labelquery_test.go @@ -14,13 +14,13 @@ See the License for the specific language governing permissions and limitations under the License. */ -package apiserver +package labels import ( "testing" ) -func TestLabelQueryParse(t *testing.T) { +func TestQueryParse(t *testing.T) { testGoodStrings := []string{ "x=a,y=b,z=c", "", @@ -31,7 +31,7 @@ func TestLabelQueryParse(t *testing.T) { "x==a==b", } for _, test := range testGoodStrings { - lq, err := ParseLabelQuery(test) + lq, err := ParseQuery(test) if err != nil { t.Errorf("%v: error %v (%#v)\n", test, err, err) } @@ -40,7 +40,7 @@ func TestLabelQueryParse(t *testing.T) { } } for _, test := range testBadStrings { - _, err := ParseLabelQuery(test) + _, err := ParseQuery(test) if err == nil { t.Errorf("%v: did not get expected error\n", test) } @@ -48,7 +48,7 @@ func TestLabelQueryParse(t *testing.T) { } func shouldMatch(t *testing.T, query string, ls LabelSet) { - lq, err := ParseLabelQuery(query) + lq, err := ParseQuery(query) if err != nil { t.Errorf("Unable to parse %v as a query\n", query) return @@ -59,7 +59,7 @@ func shouldMatch(t *testing.T, query string, ls LabelSet) { } func shouldNotMatch(t *testing.T, query string, ls LabelSet) { - lq, err := ParseLabelQuery(query) + lq, err := ParseQuery(query) if err != nil { t.Errorf("Unable to parse %v as a query\n", query) return From 154ec0db1e242dae73ab48c191a39d17420dd67a Mon Sep 17 00:00:00 2001 From: Daniel Smith Date: Mon, 16 Jun 2014 18:03:44 -0700 Subject: [PATCH 04/12] apiserver builds again --- pkg/apiserver/apiserver.go | 6 +- pkg/apiserver/apiserver_test.go | 4 +- pkg/labels/labels.go | 44 ++++++++++++ pkg/labels/{labelquery.go => query.go} | 95 -------------------------- 4 files changed, 51 insertions(+), 98 deletions(-) create mode 100644 pkg/labels/labels.go rename pkg/labels/{labelquery.go => query.go} (59%) diff --git a/pkg/apiserver/apiserver.go b/pkg/apiserver/apiserver.go index 8d589f1e6f4..353a1844973 100644 --- a/pkg/apiserver/apiserver.go +++ b/pkg/apiserver/apiserver.go @@ -24,11 +24,13 @@ import ( "net/http" "net/url" "strings" + + "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" ) // RESTStorage is a generic interface for RESTful storage services type RESTStorage interface { - List(LabelQuery) (interface{}, error) + List(labels.Query) (interface{}, error) Get(id string) (interface{}, error) Delete(id string) error Extract(body string) (interface{}, error) @@ -146,7 +148,7 @@ func (server *ApiServer) handleREST(parts []string, requestUrl *url.URL, req *ht case "GET": switch len(parts) { case 1: - query, err := ParseLabelQuery(requestUrl.Query().Get("labels")) + query, err := labels.ParseQuery(requestUrl.Query().Get("labels")) if err != nil { server.error(err, w) return diff --git a/pkg/apiserver/apiserver_test.go b/pkg/apiserver/apiserver_test.go index cc53ec101ad..41197d5b7e3 100644 --- a/pkg/apiserver/apiserver_test.go +++ b/pkg/apiserver/apiserver_test.go @@ -24,6 +24,8 @@ import ( "net/http/httptest" "reflect" "testing" + + "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" ) // TODO: This doesn't reduce typing enough to make it worth the less readable errors. Remove. @@ -49,7 +51,7 @@ type SimpleRESTStorage struct { updated Simple } -func (storage *SimpleRESTStorage) List(LabelQuery) (interface{}, error) { +func (storage *SimpleRESTStorage) List(labels.Query) (interface{}, error) { result := SimpleList{ Items: storage.list, } diff --git a/pkg/labels/labels.go b/pkg/labels/labels.go new file mode 100644 index 00000000000..96382c49cf4 --- /dev/null +++ b/pkg/labels/labels.go @@ -0,0 +1,44 @@ +/* +Copyright 2014 Google Inc. All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package labels + +import ( + "strings" +) + +// Labels allows you to present labels independently from their storage. +type Labels interface { + Get(label string) (value string) +} + +// A map of label:value. Implements Labels. +type LabelSet map[string]string + +// All labels listed as a human readable string. Conveiently, exactly the format +// that ParseQuery takes. +func (ls LabelSet) String() string { + query := make([]string, 0, len(ls)) + for key, value := range ls { + query = append(query, key+"="+value) + } + return strings.Join(query, ",") +} + +// Implement Labels interface. +func (ls LabelSet) Get(label string) string { + return ls[label] +} diff --git a/pkg/labels/labelquery.go b/pkg/labels/query.go similarity index 59% rename from pkg/labels/labelquery.go rename to pkg/labels/query.go index 1cf1c95f5b2..7755361bc12 100644 --- a/pkg/labels/labelquery.go +++ b/pkg/labels/query.go @@ -21,29 +21,6 @@ import ( "strings" ) -// Labels allows you to present labels. -type Labels interface { - Get(label string) (value string) -} - -// A map of label:value. Implements Labels. -type LabelSet map[string]string - -// All labels listed as a human readable string. Conveiently, exactly the format -// that ParseQuery takes. -func (ls LabelSet) String() string { - query := make([]string, 0, len(ls)) - for key, value := range ls { - query = append(query, key+"="+value) - } - return strings.Join(query, ",") -} - -// Implement Labels interface. -func (ls LabelSet) Get(label string) string { - return ls[label] -} - // Represents a label query. type Query interface { // Returns true if this query matches the given set of labels. @@ -150,75 +127,3 @@ func (l *queryTerm) String() (out string) { } return "" } - -/* -type parseErr struct { - Reason string - Pos token.Pos -} - -func (p parseErr) Error() string { - return fmt.Sprintf("%v: %v", p.Reason, p.Pos) -} - -func fromLiteral(expr *ast.BinaryExpr) (*queryTerm, error) { - lhs, ok := expr.X.(*ast.Ident) - if !ok { - return nil, parseErr{"expected literal", expr.X.Pos()} - } - -} - -func fromBinaryExpr(expr *ast.BinaryExpr) (*queryTerm, error) { - switch expr.Op { - case token.EQL, token.NEQ: - return fromLiteral(expr) - } - lhs, err := fromExpr(expr.X) - if err != nil { - return nil, err - } - rhs, err := fromExpr(expr.Y) - if err != nil { - return nil, err - } - switch expr.Op { - case token.AND, token.LAND: - return &queryTerm{And: []LabelQuery{lhs, rhs}} - case token.OR, token.LOR: - return &queryTerm{Or: []LabelQuery{lhs, rhs}} - } -} - -func fromUnaryExpr(expr *ast.UnaryExpr) (*queryTerm, error) { - if expr.Op == token.NOT { - lqt, err := fromExpr(expr.X) - if err != nil { - return nil, err - } - lqt.not = !lqt.not - return lqt, nil - } - return nil, parseErr{"unrecognized unary expression", expr.OpPos} -} - -func fromExpr(expr ast.Expr) (*queryTerm, error) { - switch v := expr.(type) { - case *ast.UnaryExpr: - return fromUnaryExpr(v) - case *ast.BinaryExpr: - return fromBinaryExpr(v) - } - return nil, parseErr{"unrecognized expression type", expr.Pos()} -} - -// Takes a string repsenting a label query and returns an object suitable for matching, or an error. -func ParseLabelQuery(query string) (LabelQuery, error) { - expr, err := parser.ParseExpr(query) - if err != nil { - return nil, err - } - log.Printf("%v: %v (%#v)", query, expr, expr) - return fromExpr(expr) -} -*/ From ad2ec27e9183dfdf70674cd749a2a0dbe8ca14d6 Mon Sep 17 00:00:00 2001 From: Daniel Smith Date: Mon, 16 Jun 2014 18:17:08 -0700 Subject: [PATCH 05/12] Implement label queries for controller registry --- pkg/registry/controller_registry.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/pkg/registry/controller_registry.go b/pkg/registry/controller_registry.go index 13f22a44857..b739082dcc3 100644 --- a/pkg/registry/controller_registry.go +++ b/pkg/registry/controller_registry.go @@ -18,10 +18,10 @@ package registry import ( "encoding/json" - "net/url" "github.com/GoogleCloudPlatform/kubernetes/pkg/api" "github.com/GoogleCloudPlatform/kubernetes/pkg/apiserver" + "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" ) // Implementation of RESTStorage for the api server. @@ -35,13 +35,14 @@ func MakeControllerRegistryStorage(registry ControllerRegistry) apiserver.RESTSt } } -func (storage *ControllerRegistryStorage) List(*url.URL) (interface{}, error) { +func (storage *ControllerRegistryStorage) List(query labels.Query) (interface{}, error) { result := api.ReplicationControllerList{JSONBase: api.JSONBase{Kind: "cluster#replicationControllerList"}} controllers, err := storage.registry.ListControllers() if err == nil { - result = api.ReplicationControllerList{ - JSONBase: api.JSONBase{Kind: "cluster#replicationControllerList"}, - Items: controllers, + for _, controller := range controllers { + if query.Matches(labels.LabelSet(controller.Labels)) { + result.Items = append(result.Items, controller) + } } } return result, err From 7d05ba4dc4304457cbc081caee44122a83c52835 Mon Sep 17 00:00:00 2001 From: Daniel Smith Date: Mon, 16 Jun 2014 19:10:43 -0700 Subject: [PATCH 06/12] Implement new label system --- pkg/labels/labelquery_test.go | 80 --------------- pkg/labels/query_test.go | 123 +++++++++++++++++++++++ pkg/registry/controller_registry_test.go | 5 +- pkg/registry/endpoints.go | 3 +- pkg/registry/etcd_registry.go | 5 +- pkg/registry/etcd_registry_test.go | 7 +- pkg/registry/interfaces.go | 18 +++- pkg/registry/memory_registry.go | 5 +- pkg/registry/memory_registry_test.go | 5 +- pkg/registry/pod_registry.go | 36 +------ pkg/registry/pod_registry_test.go | 71 +------------ pkg/registry/service_registry.go | 20 ++-- 12 files changed, 169 insertions(+), 209 deletions(-) delete mode 100644 pkg/labels/labelquery_test.go create mode 100644 pkg/labels/query_test.go diff --git a/pkg/labels/labelquery_test.go b/pkg/labels/labelquery_test.go deleted file mode 100644 index f0f3f83fd4f..00000000000 --- a/pkg/labels/labelquery_test.go +++ /dev/null @@ -1,80 +0,0 @@ -/* -Copyright 2014 Google Inc. All rights reserved. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package labels - -import ( - "testing" -) - -func TestQueryParse(t *testing.T) { - testGoodStrings := []string{ - "x=a,y=b,z=c", - "", - "x!=a,y=b", - } - testBadStrings := []string{ - "x=a||y=b", - "x==a==b", - } - for _, test := range testGoodStrings { - lq, err := ParseQuery(test) - if err != nil { - t.Errorf("%v: error %v (%#v)\n", test, err, err) - } - if test != lq.String() { - t.Errorf("%v restring gave: %v\n", test, lq.String()) - } - } - for _, test := range testBadStrings { - _, err := ParseQuery(test) - if err == nil { - t.Errorf("%v: did not get expected error\n", test) - } - } -} - -func shouldMatch(t *testing.T, query string, ls LabelSet) { - lq, err := ParseQuery(query) - if err != nil { - t.Errorf("Unable to parse %v as a query\n", query) - return - } - if !lq.Matches(ls) { - t.Errorf("Wanted %s to match %s, but it did not.\n", query, ls) - } -} - -func shouldNotMatch(t *testing.T, query string, ls LabelSet) { - lq, err := ParseQuery(query) - if err != nil { - t.Errorf("Unable to parse %v as a query\n", query) - return - } - if lq.Matches(ls) { - t.Errorf("Wanted '%s' to not match %s, but it did.", query, ls) - } -} - -func TestSimpleLabel(t *testing.T) { - shouldMatch(t, "", LabelSet{"x": "y"}) - shouldMatch(t, "x=y", LabelSet{"x": "y"}) - shouldMatch(t, "x=y,z=w", LabelSet{"x": "y", "z": "w"}) - shouldMatch(t, "x!=y,z!=w", LabelSet{"x": "z", "z": "a"}) - shouldNotMatch(t, "x=y", LabelSet{"x": "z"}) - shouldNotMatch(t, "x=y,z=w", LabelSet{"x": "w", "z": "w"}) - shouldNotMatch(t, "x!=y,z!=w", LabelSet{"x": "z", "z": "w"}) -} diff --git a/pkg/labels/query_test.go b/pkg/labels/query_test.go new file mode 100644 index 00000000000..d42a4ede620 --- /dev/null +++ b/pkg/labels/query_test.go @@ -0,0 +1,123 @@ +/* +Copyright 2014 Google Inc. All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package labels + +import ( + "testing" +) + +func TestQueryParse(t *testing.T) { + testGoodStrings := []string{ + "x=a,y=b,z=c", + "", + "x!=a,y=b", + } + testBadStrings := []string{ + "x=a||y=b", + "x==a==b", + } + for _, test := range testGoodStrings { + lq, err := ParseQuery(test) + if err != nil { + t.Errorf("%v: error %v (%#v)\n", test, err, err) + } + if test != lq.String() { + t.Errorf("%v restring gave: %v\n", test, lq.String()) + } + } + for _, test := range testBadStrings { + _, err := ParseQuery(test) + if err == nil { + t.Errorf("%v: did not get expected error\n", test) + } + } +} + +func expectMatch(t *testing.T, query string, ls LabelSet) { + lq, err := ParseQuery(query) + if err != nil { + t.Errorf("Unable to parse %v as a query\n", query) + return + } + if !lq.Matches(ls) { + t.Errorf("Wanted %s to match '%s', but it did not.\n", query, ls) + } +} + +func expectNoMatch(t *testing.T, query string, ls LabelSet) { + lq, err := ParseQuery(query) + if err != nil { + t.Errorf("Unable to parse %v as a query\n", query) + return + } + if lq.Matches(ls) { + t.Errorf("Wanted '%s' to not match '%s', but it did.", query, ls) + } +} + +func TestEverything(t *testing.T) { + if !Everything().Matches(LabelSet{"x": "y"}) { + t.Errorf("Nil query didn't match") + } +} + +func TestLabelQueryMatches(t *testing.T) { + expectMatch(t, "", LabelSet{"x": "y"}) + expectMatch(t, "x=y", LabelSet{"x": "y"}) + expectMatch(t, "x=y,z=w", LabelSet{"x": "y", "z": "w"}) + expectMatch(t, "x!=y,z!=w", LabelSet{"x": "z", "z": "a"}) + expectNoMatch(t, "x=y", LabelSet{"x": "z"}) + expectNoMatch(t, "x=y,z=w", LabelSet{"x": "w", "z": "w"}) + expectNoMatch(t, "x!=y,z!=w", LabelSet{"x": "z", "z": "w"}) + + labelset := LabelSet{ + "foo": "bar", + "baz": "blah", + } + expectMatch(t, "foo=bar", labelset) + expectMatch(t, "baz=blah", labelset) + expectMatch(t, "foo=bar,baz=blah", labelset) + expectNoMatch(t, "foo=blah", labelset) + expectNoMatch(t, "baz=bar", labelset) + expectNoMatch(t, "foo=bar,foobar=bar,baz=blah", labelset) +} + +func expectMatchDirect(t *testing.T, query, ls LabelSet) { + if !QueryFromSet(query).Matches(ls) { + t.Errorf("Wanted %s to match '%s', but it did not.\n", query, ls) + } +} + +func expectNoMatchDirect(t *testing.T, query, ls LabelSet) { + if QueryFromSet(query).Matches(ls) { + t.Errorf("Wanted '%s' to not match '%s', but it did.", query, ls) + } +} + +func TestLabelSetMatches(t *testing.T) { + labelset := LabelSet{ + "foo": "bar", + "baz": "blah", + } + expectMatchDirect(t, LabelSet{}, labelset) + expectMatchDirect(t, LabelSet{"foo": "bar"}, labelset) + expectMatchDirect(t, LabelSet{"baz": "blah"}, labelset) + expectMatchDirect(t, LabelSet{"foo": "bar", "baz": "blah"}, labelset) + expectNoMatchDirect(t, LabelSet{"foo": "=blah"}, labelset) + expectNoMatchDirect(t, LabelSet{"baz": "=bar"}, labelset) + expectNoMatchDirect(t, LabelSet{"foo": "=bar", "foobar": "bar", "baz": "blah"}, labelset) +} diff --git a/pkg/registry/controller_registry_test.go b/pkg/registry/controller_registry_test.go index b3d9d7ce5ec..14eac7e4986 100644 --- a/pkg/registry/controller_registry_test.go +++ b/pkg/registry/controller_registry_test.go @@ -23,6 +23,7 @@ import ( "testing" "github.com/GoogleCloudPlatform/kubernetes/pkg/api" + "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" ) type MockControllerRegistry struct { @@ -71,7 +72,7 @@ func TestListEmptyControllerList(t *testing.T) { storage := ControllerRegistryStorage{ registry: &mockRegistry, } - controllers, err := storage.List(nil) + controllers, err := storage.List(labels.Everything()) expectNoError(t, err) if len(controllers.(api.ReplicationControllerList).Items) != 0 { t.Errorf("Unexpected non-zero ctrl list: %#v", controllers) @@ -96,7 +97,7 @@ func TestListControllerList(t *testing.T) { storage := ControllerRegistryStorage{ registry: &mockRegistry, } - controllersObj, err := storage.List(nil) + controllersObj, err := storage.List(labels.Everything()) controllers := controllersObj.(api.ReplicationControllerList) expectNoError(t, err) if len(controllers.Items) != 2 { diff --git a/pkg/registry/endpoints.go b/pkg/registry/endpoints.go index 46ce7f304be..cee7b424607 100644 --- a/pkg/registry/endpoints.go +++ b/pkg/registry/endpoints.go @@ -20,6 +20,7 @@ import ( "log" "github.com/GoogleCloudPlatform/kubernetes/pkg/api" + "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" ) func MakeEndpointController(serviceRegistry ServiceRegistry, podRegistry PodRegistry) *EndpointController { @@ -41,7 +42,7 @@ func (e *EndpointController) SyncServiceEndpoints() error { } var resultErr error for _, service := range services.Items { - pods, err := e.podRegistry.ListPods(&service.Labels) + pods, err := e.podRegistry.ListPods(labels.QueryFromSet(labels.LabelSet(service.Labels))) if err != nil { log.Printf("Error syncing service: %#v, skipping.", service) resultErr = err diff --git a/pkg/registry/etcd_registry.go b/pkg/registry/etcd_registry.go index b1c212c55d9..8bd882ced29 100644 --- a/pkg/registry/etcd_registry.go +++ b/pkg/registry/etcd_registry.go @@ -23,6 +23,7 @@ import ( "github.com/coreos/go-etcd/etcd" "github.com/GoogleCloudPlatform/kubernetes/pkg/api" + "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" ) // TODO: Need to add a reconciler loop that makes sure that things in pods are reflected into @@ -66,7 +67,7 @@ func makePodKey(machine, podID string) string { return "/registry/hosts/" + machine + "/pods/" + podID } -func (registry *EtcdRegistry) ListPods(query *map[string]string) ([]api.Pod, error) { +func (registry *EtcdRegistry) ListPods(query labels.Query) ([]api.Pod, error) { pods := []api.Pod{} for _, machine := range registry.machines { machinePods, err := registry.listPodsForMachine(machine) @@ -74,7 +75,7 @@ func (registry *EtcdRegistry) ListPods(query *map[string]string) ([]api.Pod, err return pods, err } for _, pod := range machinePods { - if LabelsMatch(pod, query) { + if query.Matches(labels.LabelSet(pod.Labels)) { pods = append(pods, pod) } } diff --git a/pkg/registry/etcd_registry_test.go b/pkg/registry/etcd_registry_test.go index d117ea018ef..a9872b888fd 100644 --- a/pkg/registry/etcd_registry_test.go +++ b/pkg/registry/etcd_registry_test.go @@ -21,6 +21,7 @@ import ( "testing" "github.com/GoogleCloudPlatform/kubernetes/pkg/api" + "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" "github.com/GoogleCloudPlatform/kubernetes/pkg/util" "github.com/coreos/go-etcd/etcd" ) @@ -307,7 +308,7 @@ func TestEtcdEmptyListPods(t *testing.T) { E: nil, } registry := MakeTestEtcdRegistry(fakeClient, []string{"machine"}) - pods, err := registry.ListPods(nil) + pods, err := registry.ListPods(labels.Everything()) expectNoError(t, err) if len(pods) != 0 { t.Errorf("Unexpected pod list: %#v", pods) @@ -322,7 +323,7 @@ func TestEtcdListPodsNotFound(t *testing.T) { E: &etcd.EtcdError{ErrorCode: 100}, } registry := MakeTestEtcdRegistry(fakeClient, []string{"machine"}) - pods, err := registry.ListPods(nil) + pods, err := registry.ListPods(labels.Everything()) expectNoError(t, err) if len(pods) != 0 { t.Errorf("Unexpected pod list: %#v", pods) @@ -348,7 +349,7 @@ func TestEtcdListPods(t *testing.T) { E: nil, } registry := MakeTestEtcdRegistry(fakeClient, []string{"machine"}) - pods, err := registry.ListPods(nil) + pods, err := registry.ListPods(labels.Everything()) expectNoError(t, err) if len(pods) != 2 || pods[0].ID != "foo" || pods[1].ID != "bar" { t.Errorf("Unexpected pod list: %#v", pods) diff --git a/pkg/registry/interfaces.go b/pkg/registry/interfaces.go index db27abde8da..bf0e2728e97 100644 --- a/pkg/registry/interfaces.go +++ b/pkg/registry/interfaces.go @@ -17,13 +17,13 @@ package registry import ( "github.com/GoogleCloudPlatform/kubernetes/pkg/api" + "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" ) -// PodRegistry is an interface implemented by things that know how to store Pod objects +// PodRegistry is an interface implemented by things that know how to store Pod objects. type PodRegistry interface { // ListPods obtains a list of pods that match query. - // Query may be nil in which case all pods are returned. - ListPods(query *map[string]string) ([]api.Pod, error) + ListPods(query labels.Query) ([]api.Pod, error) // Get a specific pod GetPod(podID string) (*api.Pod, error) // Create a pod based on a specification, schedule it onto a specific machine. @@ -34,7 +34,7 @@ type PodRegistry interface { DeletePod(podID string) error } -// ControllerRegistry is an interface for things that know how to store Controllers +// ControllerRegistry is an interface for things that know how to store Controllers. type ControllerRegistry interface { ListControllers() ([]api.ReplicationController, error) GetController(controllerId string) (*api.ReplicationController, error) @@ -42,3 +42,13 @@ type ControllerRegistry interface { UpdateController(controller api.ReplicationController) error DeleteController(controllerId string) error } + +// ServiceRegistry is an interface for things that know how to store services. +type ServiceRegistry interface { + ListServices() (api.ServiceList, error) + CreateService(svc api.Service) error + GetService(name string) (*api.Service, error) + DeleteService(name string) error + UpdateService(svc api.Service) error + UpdateEndpoints(e api.Endpoints) error +} diff --git a/pkg/registry/memory_registry.go b/pkg/registry/memory_registry.go index 894a3854251..0ff4263344e 100644 --- a/pkg/registry/memory_registry.go +++ b/pkg/registry/memory_registry.go @@ -17,6 +17,7 @@ package registry import ( "github.com/GoogleCloudPlatform/kubernetes/pkg/api" + "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" ) // An implementation of PodRegistry and ControllerRegistry that is backed by memory @@ -35,10 +36,10 @@ func MakeMemoryRegistry() *MemoryRegistry { } } -func (registry *MemoryRegistry) ListPods(labelQuery *map[string]string) ([]api.Pod, error) { +func (registry *MemoryRegistry) ListPods(query labels.Query) ([]api.Pod, error) { result := []api.Pod{} for _, value := range registry.podData { - if LabelsMatch(value, labelQuery) { + if query.Matches(labels.LabelSet(value.Labels)) { result = append(result, value) } } diff --git a/pkg/registry/memory_registry_test.go b/pkg/registry/memory_registry_test.go index 3de7902c60f..b9adb26d9c8 100644 --- a/pkg/registry/memory_registry_test.go +++ b/pkg/registry/memory_registry_test.go @@ -19,11 +19,12 @@ import ( "testing" "github.com/GoogleCloudPlatform/kubernetes/pkg/api" + "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" ) func TestListPodsEmpty(t *testing.T) { registry := MakeMemoryRegistry() - pods, err := registry.ListPods(nil) + pods, err := registry.ListPods(labels.Everything()) expectNoError(t, err) if len(pods) != 0 { t.Errorf("Unexpected pod list: %#v", pods) @@ -33,7 +34,7 @@ func TestListPodsEmpty(t *testing.T) { func TestMemoryListPods(t *testing.T) { registry := MakeMemoryRegistry() registry.CreatePod("machine", api.Pod{JSONBase: api.JSONBase{ID: "foo"}}) - pods, err := registry.ListPods(nil) + pods, err := registry.ListPods(labels.Everything()) expectNoError(t, err) if len(pods) != 1 || pods[0].ID != "foo" { t.Errorf("Unexpected pod list: %#v", pods) diff --git a/pkg/registry/pod_registry.go b/pkg/registry/pod_registry.go index 95b150f99aa..c57a7c7f725 100644 --- a/pkg/registry/pod_registry.go +++ b/pkg/registry/pod_registry.go @@ -18,11 +18,11 @@ package registry import ( "encoding/json" "fmt" - "net/url" "github.com/GoogleCloudPlatform/kubernetes/pkg/api" "github.com/GoogleCloudPlatform/kubernetes/pkg/apiserver" "github.com/GoogleCloudPlatform/kubernetes/pkg/client" + "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" ) // PodRegistryStorage implements the RESTStorage interface in terms of a PodRegistry @@ -40,41 +40,11 @@ func MakePodRegistryStorage(registry PodRegistry, containerInfo client.Container } } -// LabelMatch tests to see if a Pod's labels map contains 'key' mapping to 'value' -func LabelMatch(pod api.Pod, queryKey, queryValue string) bool { - for key, value := range pod.Labels { - if queryKey == key && queryValue == value { - return true - } - } - return false -} - -// LabelMatch tests to see if a Pod's labels map contains all key/value pairs in 'labelQuery' -func LabelsMatch(pod api.Pod, labelQuery *map[string]string) bool { - if labelQuery == nil { - return true - } - for key, value := range *labelQuery { - if !LabelMatch(pod, key, value) { - return false - } - } - return true -} - -func (storage *PodRegistryStorage) List(url *url.URL) (interface{}, error) { +func (storage *PodRegistryStorage) List(query labels.Query) (interface{}, error) { var result api.PodList - var query *map[string]string - if url != nil { - queryMap := client.DecodeLabelQuery(url.Query().Get("labels")) - query = &queryMap - } pods, err := storage.registry.ListPods(query) if err == nil { - result = api.PodList{ - Items: pods, - } + result.Items = pods } result.Kind = "cluster#podList" return result, err diff --git a/pkg/registry/pod_registry_test.go b/pkg/registry/pod_registry_test.go index 217192e4fc9..3e6bda6208f 100644 --- a/pkg/registry/pod_registry_test.go +++ b/pkg/registry/pod_registry_test.go @@ -22,6 +22,7 @@ import ( "testing" "github.com/GoogleCloudPlatform/kubernetes/pkg/api" + "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" ) type MockPodRegistry struct { @@ -35,7 +36,7 @@ func expectNoError(t *testing.T, err error) { } } -func (registry *MockPodRegistry) ListPods(*map[string]string) ([]api.Pod, error) { +func (registry *MockPodRegistry) ListPods(labels.Query) ([]api.Pod, error) { return registry.pods, registry.err } @@ -135,74 +136,6 @@ func TestExtractJson(t *testing.T) { } } -func expectLabelMatch(t *testing.T, pod api.Pod, key, value string) { - if !LabelMatch(pod, key, value) { - t.Errorf("Unexpected match failure: %#v %s %s", pod, key, value) - } -} - -func expectNoLabelMatch(t *testing.T, pod api.Pod, key, value string) { - if LabelMatch(pod, key, value) { - t.Errorf("Unexpected match success: %#v %s %s", pod, key, value) - } -} - -func expectLabelsMatch(t *testing.T, pod api.Pod, query *map[string]string) { - if !LabelsMatch(pod, query) { - t.Errorf("Unexpected match failure: %#v %#v", pod, *query) - } -} - -func expectNoLabelsMatch(t *testing.T, pod api.Pod, query *map[string]string) { - if LabelsMatch(pod, query) { - t.Errorf("Unexpected match success: %#v %#v", pod, *query) - } -} - -func TestLabelMatch(t *testing.T) { - pod := api.Pod{ - Labels: map[string]string{ - "foo": "bar", - "baz": "blah", - }, - } - expectLabelMatch(t, pod, "foo", "bar") - expectLabelMatch(t, pod, "baz", "blah") - expectNoLabelMatch(t, pod, "foo", "blah") - expectNoLabelMatch(t, pod, "baz", "bar") -} - -func TestLabelsMatch(t *testing.T) { - pod := api.Pod{ - Labels: map[string]string{ - "foo": "bar", - "baz": "blah", - }, - } - expectLabelsMatch(t, pod, &map[string]string{}) - expectLabelsMatch(t, pod, &map[string]string{ - "foo": "bar", - }) - expectLabelsMatch(t, pod, &map[string]string{ - "baz": "blah", - }) - expectLabelsMatch(t, pod, &map[string]string{ - "foo": "bar", - "baz": "blah", - }) - expectNoLabelsMatch(t, pod, &map[string]string{ - "foo": "blah", - }) - expectNoLabelsMatch(t, pod, &map[string]string{ - "baz": "bar", - }) - expectNoLabelsMatch(t, pod, &map[string]string{ - "foo": "bar", - "foobar": "bar", - "baz": "blah", - }) -} - func TestMakePodStatus(t *testing.T) { status := makePodStatus(map[string]interface{}{}) if status != "Pending" { diff --git a/pkg/registry/service_registry.go b/pkg/registry/service_registry.go index 218cc0eb4cc..102ffe174cc 100644 --- a/pkg/registry/service_registry.go +++ b/pkg/registry/service_registry.go @@ -17,23 +17,14 @@ package registry import ( "encoding/json" - "net/url" "strconv" "strings" "github.com/GoogleCloudPlatform/kubernetes/pkg/api" "github.com/GoogleCloudPlatform/kubernetes/pkg/apiserver" + "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" ) -type ServiceRegistry interface { - ListServices() (api.ServiceList, error) - CreateService(svc api.Service) error - GetService(name string) (*api.Service, error) - DeleteService(name string) error - UpdateService(svc api.Service) error - UpdateEndpoints(e api.Endpoints) error -} - type ServiceRegistryStorage struct { registry ServiceRegistry } @@ -59,12 +50,19 @@ func GetServiceEnvironmentVariables(registry ServiceRegistry, machine string) ([ return result, nil } -func (sr *ServiceRegistryStorage) List(*url.URL) (interface{}, error) { +func (sr *ServiceRegistryStorage) List(query labels.Query) (interface{}, error) { list, err := sr.registry.ListServices() if err != nil { return nil, err } list.Kind = "cluster#serviceList" + var filtered []api.Service + for _, service := range list.Items { + if query.Matches(labels.LabelSet(service.Labels)) { + filtered = append(filtered, service) + } + } + list.Items = filtered return list, err } From 5c3e4fab58e982025761c56341ced936fa2a726a Mon Sep 17 00:00:00 2001 From: Daniel Smith Date: Mon, 16 Jun 2014 19:17:23 -0700 Subject: [PATCH 07/12] add another test --- pkg/labels/labels_test.go | 43 +++++++++++++++++++++++++++++++++++++++ pkg/labels/query.go | 16 +++++++++++++++ 2 files changed, 59 insertions(+) create mode 100644 pkg/labels/labels_test.go diff --git a/pkg/labels/labels_test.go b/pkg/labels/labels_test.go new file mode 100644 index 00000000000..a843074a408 --- /dev/null +++ b/pkg/labels/labels_test.go @@ -0,0 +1,43 @@ +/* +Copyright 2014 Google Inc. All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package labels + +import ( + "testing" +) + +func matches(t *testing.T, ls LabelSet, want string) { + if ls.String() != want { + t.Errorf("Expected '%s', but got '%s'", want, ls.String()) + } +} + +func TestLabelSetString(t *testing.T) { + matches(t, LabelSet{"x": "y"}, "x=y") + matches(t, LabelSet{"foo": "bar"}, "foo=bar") + matches(t, LabelSet{"foo": "bar", "baz": "qup"}, "foo=bar,baz=qup") + + // TODO: Make our label representation robust enough to handel labels + // with ",=!" characters in their names. +} + +func TestLabelGet(t *testing.T) { + ls := LabelSet{"x": "y"} + if ls.Get("x") != "y" { + t.Errorf("LabelSet.Get is broken") + } +} diff --git a/pkg/labels/query.go b/pkg/labels/query.go index 7755361bc12..0ad1c5d7a41 100644 --- a/pkg/labels/query.go +++ b/pkg/labels/query.go @@ -30,6 +30,11 @@ type Query interface { String() string } +// Everything returns a query that matches all labels. +func Everything() Query { + return &queryTerm{} +} + // A single term of a label query. type queryTerm struct { // Not inverts the meaning of the items in this term. @@ -84,6 +89,17 @@ func try(queryPiece, op string) (lhs, rhs string, ok bool) { return "", "", false } +// Given a LabelSet, return a Query which will match exactly that LabelSet. +func QueryFromSet(ls LabelSet) Query { + var query queryTerm + for l, v := range ls { + // Make a copy, because we're taking the address below + label, value := l, v + query.and = append(query.and, queryTerm{label: &label, value: &value}) + } + return &query +} + // Takes a string repsenting a label query and returns an object suitable for matching, or an error. func ParseQuery(query string) (Query, error) { parts := strings.Split(query, ",") From c534d070e5a5cd3fadad68e912b5d78d8edb34ad Mon Sep 17 00:00:00 2001 From: Daniel Smith Date: Mon, 16 Jun 2014 19:22:46 -0700 Subject: [PATCH 08/12] Rename LabelSet labels.Set --- pkg/labels/labels.go | 6 ++-- pkg/labels/labels_test.go | 14 ++++----- pkg/labels/query.go | 6 ++-- pkg/labels/query_test.go | 44 ++++++++++++++--------------- pkg/registry/controller_registry.go | 2 +- pkg/registry/endpoints.go | 2 +- pkg/registry/etcd_registry.go | 2 +- pkg/registry/memory_registry.go | 2 +- pkg/registry/service_registry.go | 2 +- 9 files changed, 40 insertions(+), 40 deletions(-) diff --git a/pkg/labels/labels.go b/pkg/labels/labels.go index 96382c49cf4..38ec2ac0e94 100644 --- a/pkg/labels/labels.go +++ b/pkg/labels/labels.go @@ -26,11 +26,11 @@ type Labels interface { } // A map of label:value. Implements Labels. -type LabelSet map[string]string +type Set map[string]string // All labels listed as a human readable string. Conveiently, exactly the format // that ParseQuery takes. -func (ls LabelSet) String() string { +func (ls Set) String() string { query := make([]string, 0, len(ls)) for key, value := range ls { query = append(query, key+"="+value) @@ -39,6 +39,6 @@ func (ls LabelSet) String() string { } // Implement Labels interface. -func (ls LabelSet) Get(label string) string { +func (ls Set) Get(label string) string { return ls[label] } diff --git a/pkg/labels/labels_test.go b/pkg/labels/labels_test.go index a843074a408..76db00b951a 100644 --- a/pkg/labels/labels_test.go +++ b/pkg/labels/labels_test.go @@ -20,24 +20,24 @@ import ( "testing" ) -func matches(t *testing.T, ls LabelSet, want string) { +func matches(t *testing.T, ls Set, want string) { if ls.String() != want { t.Errorf("Expected '%s', but got '%s'", want, ls.String()) } } -func TestLabelSetString(t *testing.T) { - matches(t, LabelSet{"x": "y"}, "x=y") - matches(t, LabelSet{"foo": "bar"}, "foo=bar") - matches(t, LabelSet{"foo": "bar", "baz": "qup"}, "foo=bar,baz=qup") +func TestSetString(t *testing.T) { + matches(t, Set{"x": "y"}, "x=y") + matches(t, Set{"foo": "bar"}, "foo=bar") + matches(t, Set{"foo": "bar", "baz": "qup"}, "foo=bar,baz=qup") // TODO: Make our label representation robust enough to handel labels // with ",=!" characters in their names. } func TestLabelGet(t *testing.T) { - ls := LabelSet{"x": "y"} + ls := Set{"x": "y"} if ls.Get("x") != "y" { - t.Errorf("LabelSet.Get is broken") + t.Errorf("Set.Get is broken") } } diff --git a/pkg/labels/query.go b/pkg/labels/query.go index 0ad1c5d7a41..00f01bcb8b7 100644 --- a/pkg/labels/query.go +++ b/pkg/labels/query.go @@ -42,7 +42,7 @@ type queryTerm struct { // Exactly one of the below three items should be used. - // If non-nil, we match LabelSet l iff l[*label] == *value. + // If non-nil, we match Set l iff l[*label] == *value. label, value *string // A list of terms which must all match for this query term to return true. @@ -89,8 +89,8 @@ func try(queryPiece, op string) (lhs, rhs string, ok bool) { return "", "", false } -// Given a LabelSet, return a Query which will match exactly that LabelSet. -func QueryFromSet(ls LabelSet) Query { +// Given a Set, return a Query which will match exactly that Set. +func QueryFromSet(ls Set) Query { var query queryTerm for l, v := range ls { // Make a copy, because we're taking the address below diff --git a/pkg/labels/query_test.go b/pkg/labels/query_test.go index d42a4ede620..38dae8b4896 100644 --- a/pkg/labels/query_test.go +++ b/pkg/labels/query_test.go @@ -47,7 +47,7 @@ func TestQueryParse(t *testing.T) { } } -func expectMatch(t *testing.T, query string, ls LabelSet) { +func expectMatch(t *testing.T, query string, ls Set) { lq, err := ParseQuery(query) if err != nil { t.Errorf("Unable to parse %v as a query\n", query) @@ -58,7 +58,7 @@ func expectMatch(t *testing.T, query string, ls LabelSet) { } } -func expectNoMatch(t *testing.T, query string, ls LabelSet) { +func expectNoMatch(t *testing.T, query string, ls Set) { lq, err := ParseQuery(query) if err != nil { t.Errorf("Unable to parse %v as a query\n", query) @@ -70,21 +70,21 @@ func expectNoMatch(t *testing.T, query string, ls LabelSet) { } func TestEverything(t *testing.T) { - if !Everything().Matches(LabelSet{"x": "y"}) { + if !Everything().Matches(Set{"x": "y"}) { t.Errorf("Nil query didn't match") } } func TestLabelQueryMatches(t *testing.T) { - expectMatch(t, "", LabelSet{"x": "y"}) - expectMatch(t, "x=y", LabelSet{"x": "y"}) - expectMatch(t, "x=y,z=w", LabelSet{"x": "y", "z": "w"}) - expectMatch(t, "x!=y,z!=w", LabelSet{"x": "z", "z": "a"}) - expectNoMatch(t, "x=y", LabelSet{"x": "z"}) - expectNoMatch(t, "x=y,z=w", LabelSet{"x": "w", "z": "w"}) - expectNoMatch(t, "x!=y,z!=w", LabelSet{"x": "z", "z": "w"}) + expectMatch(t, "", Set{"x": "y"}) + expectMatch(t, "x=y", Set{"x": "y"}) + expectMatch(t, "x=y,z=w", Set{"x": "y", "z": "w"}) + expectMatch(t, "x!=y,z!=w", Set{"x": "z", "z": "a"}) + 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"}) - labelset := LabelSet{ + labelset := Set{ "foo": "bar", "baz": "blah", } @@ -96,28 +96,28 @@ func TestLabelQueryMatches(t *testing.T) { expectNoMatch(t, "foo=bar,foobar=bar,baz=blah", labelset) } -func expectMatchDirect(t *testing.T, query, ls LabelSet) { +func expectMatchDirect(t *testing.T, query, ls Set) { if !QueryFromSet(query).Matches(ls) { t.Errorf("Wanted %s to match '%s', but it did not.\n", query, ls) } } -func expectNoMatchDirect(t *testing.T, query, ls LabelSet) { +func expectNoMatchDirect(t *testing.T, query, ls Set) { if QueryFromSet(query).Matches(ls) { t.Errorf("Wanted '%s' to not match '%s', but it did.", query, ls) } } -func TestLabelSetMatches(t *testing.T) { - labelset := LabelSet{ +func TestSetMatches(t *testing.T) { + labelset := Set{ "foo": "bar", "baz": "blah", } - expectMatchDirect(t, LabelSet{}, labelset) - expectMatchDirect(t, LabelSet{"foo": "bar"}, labelset) - expectMatchDirect(t, LabelSet{"baz": "blah"}, labelset) - expectMatchDirect(t, LabelSet{"foo": "bar", "baz": "blah"}, labelset) - expectNoMatchDirect(t, LabelSet{"foo": "=blah"}, labelset) - expectNoMatchDirect(t, LabelSet{"baz": "=bar"}, labelset) - expectNoMatchDirect(t, LabelSet{"foo": "=bar", "foobar": "bar", "baz": "blah"}, labelset) + expectMatchDirect(t, Set{}, labelset) + expectMatchDirect(t, Set{"foo": "bar"}, labelset) + expectMatchDirect(t, Set{"baz": "blah"}, labelset) + expectMatchDirect(t, Set{"foo": "bar", "baz": "blah"}, labelset) + expectNoMatchDirect(t, Set{"foo": "=blah"}, labelset) + expectNoMatchDirect(t, Set{"baz": "=bar"}, labelset) + expectNoMatchDirect(t, Set{"foo": "=bar", "foobar": "bar", "baz": "blah"}, labelset) } diff --git a/pkg/registry/controller_registry.go b/pkg/registry/controller_registry.go index b739082dcc3..1824ba965a2 100644 --- a/pkg/registry/controller_registry.go +++ b/pkg/registry/controller_registry.go @@ -40,7 +40,7 @@ func (storage *ControllerRegistryStorage) List(query labels.Query) (interface{}, controllers, err := storage.registry.ListControllers() if err == nil { for _, controller := range controllers { - if query.Matches(labels.LabelSet(controller.Labels)) { + if query.Matches(labels.Set(controller.Labels)) { result.Items = append(result.Items, controller) } } diff --git a/pkg/registry/endpoints.go b/pkg/registry/endpoints.go index cee7b424607..5601db2f136 100644 --- a/pkg/registry/endpoints.go +++ b/pkg/registry/endpoints.go @@ -42,7 +42,7 @@ func (e *EndpointController) SyncServiceEndpoints() error { } var resultErr error for _, service := range services.Items { - pods, err := e.podRegistry.ListPods(labels.QueryFromSet(labels.LabelSet(service.Labels))) + pods, err := e.podRegistry.ListPods(labels.QueryFromSet(labels.Set(service.Labels))) if err != nil { log.Printf("Error syncing service: %#v, skipping.", service) resultErr = err diff --git a/pkg/registry/etcd_registry.go b/pkg/registry/etcd_registry.go index 8bd882ced29..0f8062c96b5 100644 --- a/pkg/registry/etcd_registry.go +++ b/pkg/registry/etcd_registry.go @@ -75,7 +75,7 @@ func (registry *EtcdRegistry) ListPods(query labels.Query) ([]api.Pod, error) { return pods, err } for _, pod := range machinePods { - if query.Matches(labels.LabelSet(pod.Labels)) { + if query.Matches(labels.Set(pod.Labels)) { pods = append(pods, pod) } } diff --git a/pkg/registry/memory_registry.go b/pkg/registry/memory_registry.go index 0ff4263344e..a2175525914 100644 --- a/pkg/registry/memory_registry.go +++ b/pkg/registry/memory_registry.go @@ -39,7 +39,7 @@ func MakeMemoryRegistry() *MemoryRegistry { func (registry *MemoryRegistry) ListPods(query labels.Query) ([]api.Pod, error) { result := []api.Pod{} for _, value := range registry.podData { - if query.Matches(labels.LabelSet(value.Labels)) { + if query.Matches(labels.Set(value.Labels)) { result = append(result, value) } } diff --git a/pkg/registry/service_registry.go b/pkg/registry/service_registry.go index 102ffe174cc..aaf5c4f68a5 100644 --- a/pkg/registry/service_registry.go +++ b/pkg/registry/service_registry.go @@ -58,7 +58,7 @@ func (sr *ServiceRegistryStorage) List(query labels.Query) (interface{}, error) list.Kind = "cluster#serviceList" var filtered []api.Service for _, service := range list.Items { - if query.Matches(labels.LabelSet(service.Labels)) { + if query.Matches(labels.Set(service.Labels)) { filtered = append(filtered, service) } } From c27ad1390d9371a948f096dbb9dfbc19e1042d82 Mon Sep 17 00:00:00 2001 From: Daniel Smith Date: Mon, 16 Jun 2014 19:27:59 -0700 Subject: [PATCH 09/12] etcd does some trickery that was avoiding the pid capturing in our tests. Run in subshell. --- hack/integration-test.sh | 2 +- hack/local-up-cluster.sh | 2 +- hack/local-up.sh | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/hack/integration-test.sh b/hack/integration-test.sh index cc0b79f8485..482b15cc5fc 100755 --- a/hack/integration-test.sh +++ b/hack/integration-test.sh @@ -27,7 +27,7 @@ $(dirname $0)/build-go.sh ETCD_DIR=$(mktemp -d -t kube-integration.XXXXXX) trap "rm -rf ${ETCD_DIR}" EXIT -etcd -name test -data-dir ${ETCD_DIR} > /tmp/etcd.log & +(etcd -name test -data-dir ${ETCD_DIR} > /tmp/etcd.log) & ETCD_PID=$! sleep 5 diff --git a/hack/local-up-cluster.sh b/hack/local-up-cluster.sh index 36b1c44e6ec..95f9406aa2b 100755 --- a/hack/local-up-cluster.sh +++ b/hack/local-up-cluster.sh @@ -33,7 +33,7 @@ echo "Starting etcd" ETCD_DIR=$(mktemp -d -t kube-integration.XXXXXX) trap "rm -rf ${ETCD_DIR}" EXIT -etcd -name test -data-dir ${ETCD_DIR} &> /tmp/etcd.log & +(etcd -name test -data-dir ${ETCD_DIR} &> /tmp/etcd.log) & ETCD_PID=$! sleep 5 diff --git a/hack/local-up.sh b/hack/local-up.sh index 1308420ee7e..ff1ee378772 100755 --- a/hack/local-up.sh +++ b/hack/local-up.sh @@ -40,7 +40,7 @@ echo "Starting etcd" ETCD_DIR=$(mktemp -d -t kube-integration.XXXXXX) trap "rm -rf ${ETCD_DIR}" EXIT -etcd -name test -data-dir ${ETCD_DIR} > /tmp/etcd.log & +(etcd -name test -data-dir ${ETCD_DIR} > /tmp/etcd.log) & ETCD_PID=$! sleep 5 From 3b980bd9dcfe4551ae417f99dc3015eadb193347 Mon Sep 17 00:00:00 2001 From: Daniel Smith Date: Mon, 16 Jun 2014 20:05:22 -0700 Subject: [PATCH 10/12] Make deterministic --- pkg/labels/labels.go | 3 +++ pkg/labels/labels_test.go | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/pkg/labels/labels.go b/pkg/labels/labels.go index 38ec2ac0e94..86971d3f841 100644 --- a/pkg/labels/labels.go +++ b/pkg/labels/labels.go @@ -17,6 +17,7 @@ limitations under the License. package labels import ( + "sort" "strings" ) @@ -35,6 +36,8 @@ func (ls Set) String() string { for key, value := range ls { query = append(query, key+"="+value) } + // Sort for determinism. + sort.StringSlice(query).Sort() return strings.Join(query, ",") } diff --git a/pkg/labels/labels_test.go b/pkg/labels/labels_test.go index 76db00b951a..74db659acaa 100644 --- a/pkg/labels/labels_test.go +++ b/pkg/labels/labels_test.go @@ -29,7 +29,7 @@ func matches(t *testing.T, ls Set, want string) { func TestSetString(t *testing.T) { matches(t, Set{"x": "y"}, "x=y") matches(t, Set{"foo": "bar"}, "foo=bar") - matches(t, Set{"foo": "bar", "baz": "qup"}, "foo=bar,baz=qup") + matches(t, Set{"foo": "bar", "baz": "qup"}, "baz=qup,foo=bar") // TODO: Make our label representation robust enough to handel labels // with ",=!" characters in their names. From c4e575d4ac2931a183471dc9d402e33d856f443e Mon Sep 17 00:00:00 2001 From: Daniel Smith Date: Mon, 16 Jun 2014 22:04:28 -0700 Subject: [PATCH 11/12] switch to different types for different parts of the label query --- pkg/labels/labels.go | 2 +- pkg/labels/query.go | 122 ++++++++++++++++++------------------------- 2 files changed, 51 insertions(+), 73 deletions(-) diff --git a/pkg/labels/labels.go b/pkg/labels/labels.go index 86971d3f841..156e18561c3 100644 --- a/pkg/labels/labels.go +++ b/pkg/labels/labels.go @@ -29,7 +29,7 @@ type Labels interface { // A map of label:value. Implements Labels. type Set map[string]string -// All labels listed as a human readable string. Conveiently, exactly the format +// All labels listed as a human readable string. Conveniently, exactly the format // that ParseQuery takes. func (ls Set) String() string { query := make([]string, 0, len(ls)) diff --git a/pkg/labels/query.go b/pkg/labels/query.go index 00f01bcb8b7..5e563924a3c 100644 --- a/pkg/labels/query.go +++ b/pkg/labels/query.go @@ -32,53 +32,50 @@ type Query interface { // Everything returns a query that matches all labels. func Everything() Query { - return &queryTerm{} + return andTerm{} } -// A single term of a label query. -type queryTerm struct { - // Not inverts the meaning of the items in this term. - not bool - - // Exactly one of the below three items should be used. - - // If non-nil, we match Set l iff l[*label] == *value. - label, value *string - - // A list of terms which must all match for this query term to return true. - and []queryTerm - - // A list of terms, any one of which will cause this query term to return true. - // Parsing/printing not implemented. - or []queryTerm +type hasTerm struct { + label, value string } -func (l *queryTerm) Matches(ls Labels) bool { - matches := !l.not - switch { - case l.label != nil && l.value != nil: - if ls.Get(*l.label) == *l.value { - return matches +func (t *hasTerm) Matches(ls Labels) bool { + return ls.Get(t.label) == t.value +} + +func (t *hasTerm) String() string { + return fmt.Sprintf("%v=%v", t.label, t.value) +} + +type notHasTerm struct { + label, value string +} + +func (t *notHasTerm) Matches(ls Labels) bool { + return ls.Get(t.label) != t.value +} + +func (t *notHasTerm) String() string { + return fmt.Sprintf("%v!=%v", t.label, t.value) +} + +type andTerm []Query + +func (t andTerm) Matches(ls Labels) bool { + for _, q := range t { + if !q.Matches(ls) { + return false } - return !matches - case len(l.and) > 0: - for i := range l.and { - if !l.and[i].Matches(ls) { - return !matches - } - } - return matches - case len(l.or) > 0: - for i := range l.or { - if l.or[i].Matches(ls) { - return matches - } - } - return !matches } + return true +} - // Empty queries match everything - return matches +func (t andTerm) String() string { + var terms []string + for _, q := range t { + terms = append(terms, q.String()) + } + return strings.Join(terms, ",") } func try(queryPiece, op string) (lhs, rhs string, ok bool) { @@ -91,55 +88,36 @@ func try(queryPiece, op string) (lhs, rhs string, ok bool) { // Given a Set, return a Query which will match exactly that Set. func QueryFromSet(ls Set) Query { - var query queryTerm - for l, v := range ls { - // Make a copy, because we're taking the address below - label, value := l, v - query.and = append(query.and, queryTerm{label: &label, value: &value}) + var items []Query + for label, value := range ls { + items = append(items, &hasTerm{label: label, value: value}) } - return &query + if len(items) == 1 { + return items[0] + } + return andTerm(items) } // Takes a string repsenting a label query and returns an object suitable for matching, or an error. func ParseQuery(query string) (Query, error) { parts := strings.Split(query, ",") - var items []queryTerm + var items []Query for _, part := range parts { if part == "" { continue } if lhs, rhs, ok := try(part, "!="); ok { - items = append(items, queryTerm{not: true, label: &lhs, value: &rhs}) + items = append(items, ¬HasTerm{label: lhs, value: rhs}) } else if lhs, rhs, ok := try(part, "=="); ok { - items = append(items, queryTerm{label: &lhs, value: &rhs}) + items = append(items, &hasTerm{label: lhs, value: rhs}) } else if lhs, rhs, ok := try(part, "="); ok { - items = append(items, queryTerm{label: &lhs, value: &rhs}) + items = append(items, &hasTerm{label: lhs, value: rhs}) } else { return nil, fmt.Errorf("invalid label query: '%s'; can't understand '%s'", query, part) } } if len(items) == 1 { - return &items[0], nil + return items[0], nil } - return &queryTerm{and: items}, nil -} - -// Returns this query as a string in a form that ParseQuery can parse. -func (l *queryTerm) String() (out string) { - if len(l.and) > 0 { - for _, part := range l.and { - if out != "" { - out += "," - } - out += part.String() - } - return - } else if l.label != nil && l.value != nil { - op := "=" - if l.not { - op = "!=" - } - return fmt.Sprintf("%v%v%v", *l.label, op, *l.value) - } - return "" + return andTerm(items), nil } From e10e5b99d5b2f3d14b4e2587f691fc65622944a8 Mon Sep 17 00:00:00 2001 From: Daniel Smith Date: Mon, 16 Jun 2014 22:21:43 -0700 Subject: [PATCH 12/12] Fix typo --- pkg/labels/labels_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/labels/labels_test.go b/pkg/labels/labels_test.go index 74db659acaa..c2df41703c3 100644 --- a/pkg/labels/labels_test.go +++ b/pkg/labels/labels_test.go @@ -31,7 +31,7 @@ func TestSetString(t *testing.T) { matches(t, Set{"foo": "bar"}, "foo=bar") matches(t, Set{"foo": "bar", "baz": "qup"}, "baz=qup,foo=bar") - // TODO: Make our label representation robust enough to handel labels + // TODO: Make our label representation robust enough to handle labels // with ",=!" characters in their names. }