diff --git a/pkg/api/v1beta1/conversion.go b/pkg/api/v1beta1/conversion.go index ad816ecc578..b1496605ba4 100644 --- a/pkg/api/v1beta1/conversion.go +++ b/pkg/api/v1beta1/conversion.go @@ -1291,26 +1291,4 @@ func init() { // If one of the conversion functions is malformed, detect it immediately. panic(err) } - - // Add field conversion funcs. - err = newer.Scheme.AddFieldLabelConversionFunc("v1beta1", "pods", - func(label, value string) (string, string, error) { - switch label { - case "name": - return "name", value, nil - case "DesiredState.Host": - return "Status.Host", value, nil - case "DesiredState.Status": - podStatus := PodStatus(value) - var internalValue newer.PodPhase - newer.Scheme.Convert(&podStatus, &internalValue) - return "Status.Phase", string(internalValue), nil - default: - return "", "", fmt.Errorf("field label not supported: %s", label) - } - }) - if err != nil { - // If one of the conversion functions is malformed, detect it immediately. - panic(err) - } } diff --git a/pkg/api/v1beta2/conversion.go b/pkg/api/v1beta2/conversion.go index 2ce1a1c85e5..a0ac51de0d0 100644 --- a/pkg/api/v1beta2/conversion.go +++ b/pkg/api/v1beta2/conversion.go @@ -1206,26 +1206,4 @@ func init() { // If one of the conversion functions is malformed, detect it immediately. panic(err) } - - // Add field conversion funcs. - err = newer.Scheme.AddFieldLabelConversionFunc("v1beta2", "pods", - func(label, value string) (string, string, error) { - switch label { - case "name": - return "name", value, nil - case "DesiredState.Host": - return "Status.Host", value, nil - case "DesiredState.Status": - podStatus := PodStatus(value) - var internalValue newer.PodPhase - newer.Scheme.Convert(&podStatus, &internalValue) - return "Status.Phase", string(internalValue), nil - default: - return "", "", fmt.Errorf("field label not supported: %s", label) - } - }) - if err != nil { - // If one of the conversion functions is malformed, detect it immediately. - panic(err) - } } diff --git a/pkg/api/v1beta3/conversion.go b/pkg/api/v1beta3/conversion.go deleted file mode 100644 index c84b301d419..00000000000 --- a/pkg/api/v1beta3/conversion.go +++ /dev/null @@ -1,44 +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 v1beta3 - -import ( - "fmt" - - newer "github.com/GoogleCloudPlatform/kubernetes/pkg/api" -) - -func init() { - // Add field conversion funcs. - err := newer.Scheme.AddFieldLabelConversionFunc("v1beta3", "pods", - func(label, value string) (string, string, error) { - switch label { - case "name": - fallthrough - case "Status.Phase": - fallthrough - case "Status.Host": - return label, value, nil - default: - return "", "", fmt.Errorf("field label not supported: %s", label) - } - }) - if err != nil { - // If one of the conversion functions is malformed, detect it immediately. - panic(err) - } -} diff --git a/pkg/apiserver/api_installer.go b/pkg/apiserver/api_installer.go index e1854ae012f..48c6066838e 100644 --- a/pkg/apiserver/api_installer.go +++ b/pkg/apiserver/api_installer.go @@ -242,7 +242,7 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage RESTStorage addParams(route, action.Params) ws.Route(route) case "LIST": // List all resources of a kind. - route := ws.GET(action.Path).To(ListResource(lister, ctxFn, action.Namer, codec, a.group.info)). + route := ws.GET(action.Path).To(ListResource(lister, ctxFn, action.Namer, codec)). Filter(m). Doc("list objects of kind " + kind). Operation("list" + kind). diff --git a/pkg/apiserver/resthandler.go b/pkg/apiserver/resthandler.go index a3fc2e3a0d1..496e6a253ee 100644 --- a/pkg/apiserver/resthandler.go +++ b/pkg/apiserver/resthandler.go @@ -18,7 +18,6 @@ package apiserver import ( "net/http" - "net/url" gpath "path" "time" @@ -80,24 +79,8 @@ func GetResource(r RESTGetter, ctxFn ContextFunc, namer ScopeNamer, codec runtim } } -func parseSelectorQueryParams(query url.Values, version, apiResource string) (label, field labels.Selector, err error) { - label, err = labels.ParseSelector(query.Get("labels")) - if err != nil { - return nil, nil, err - } - - convertToInternalVersionFunc := func(label, value string) (newLabel, newValue string, err error) { - return api.Scheme.ConvertFieldLabel(version, apiResource, label, value) - } - field, err = labels.ParseAndTransformSelector(query.Get("fields"), convertToInternalVersionFunc) - if err != nil { - return nil, nil, err - } - return label, field, nil -} - // ListResource returns a function that handles retrieving a list of resources from a RESTStorage object. -func ListResource(r RESTLister, ctxFn ContextFunc, namer ScopeNamer, codec runtime.Codec, requestInfoResolver *APIRequestInfoResolver) restful.RouteFunction { +func ListResource(r RESTLister, ctxFn ContextFunc, namer ScopeNamer, codec runtime.Codec) restful.RouteFunction { return func(req *restful.Request, res *restful.Response) { w := res.ResponseWriter @@ -109,13 +92,12 @@ func ListResource(r RESTLister, ctxFn ContextFunc, namer ScopeNamer, codec runti ctx := ctxFn(req) ctx = api.WithNamespace(ctx, namespace) - requestInfo, err := requestInfoResolver.GetAPIRequestInfo(req.Request) + label, err := labels.ParseSelector(req.Request.URL.Query().Get("labels")) if err != nil { errorJSON(err, codec, w) return } - - label, field, err := parseSelectorQueryParams(req.Request.URL.Query(), requestInfo.APIVersion, requestInfo.Resource) + field, err := labels.ParseSelector(req.Request.URL.Query().Get("fields")) if err != nil { errorJSON(err, codec, w) return diff --git a/pkg/apiserver/watch.go b/pkg/apiserver/watch.go index c9e501b16bd..e7698182c97 100644 --- a/pkg/apiserver/watch.go +++ b/pkg/apiserver/watch.go @@ -18,6 +18,7 @@ package apiserver import ( "net/http" + "net/url" "path" "regexp" "strings" @@ -26,6 +27,7 @@ import ( "github.com/GoogleCloudPlatform/kubernetes/pkg/api" "github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors" "github.com/GoogleCloudPlatform/kubernetes/pkg/httplog" + "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" "github.com/GoogleCloudPlatform/kubernetes/pkg/runtime" "github.com/GoogleCloudPlatform/kubernetes/pkg/watch" watchjson "github.com/GoogleCloudPlatform/kubernetes/pkg/watch/json" @@ -54,6 +56,25 @@ func (h *WatchHandler) setSelfLinkAddName(obj runtime.Object, req *http.Request) return h.linker.SetSelfLink(obj, newURL.String()) } +func getWatchParams(query url.Values) (label, field labels.Selector, resourceVersion string, err error) { + s, perr := labels.ParseSelector(query.Get("labels")) + if perr != nil { + err = perr + return + } + label = s + + s, perr = labels.ParseSelector(query.Get("fields")) + if perr != nil { + err = perr + return + } + field = s + + resourceVersion = query.Get("resourceVersion") + return +} + var connectionUpgradeRegex = regexp.MustCompile("(^|.*,\\s*)upgrade($|\\s*,)") func isWebsocketRequest(req *http.Request) bool { @@ -96,13 +117,11 @@ func (h *WatchHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) { return } - label, field, err := parseSelectorQueryParams(req.URL.Query(), requestInfo.APIVersion, apiResource) + label, field, resourceVersion, err := getWatchParams(req.URL.Query()) if err != nil { httpCode = errorJSON(err, h.codec, w) return } - - resourceVersion := req.URL.Query().Get("resourceVersion") watching, err := watcher.Watch(ctx, label, field, resourceVersion) if err != nil { httpCode = errorJSON(err, h.codec, w) diff --git a/pkg/apiserver/watch_test.go b/pkg/apiserver/watch_test.go index d7b44be6307..d4a702addc8 100644 --- a/pkg/apiserver/watch_test.go +++ b/pkg/apiserver/watch_test.go @@ -25,7 +25,6 @@ import ( "testing" "github.com/GoogleCloudPlatform/kubernetes/pkg/api" - "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" "github.com/GoogleCloudPlatform/kubernetes/pkg/runtime" "github.com/GoogleCloudPlatform/kubernetes/pkg/watch" "golang.org/x/net/websocket" @@ -165,19 +164,15 @@ func TestWatchHTTP(t *testing.T) { } func TestWatchParamParsing(t *testing.T) { - api.Scheme.AddFieldLabelConversionFunc(testVersion, "foo", - func(label, value string) (string, string, error) { - return label, value, nil - }) simpleStorage := &SimpleRESTStorage{} handler := Handle(map[string]RESTStorage{ "foo": simpleStorage, - }, codec, "/api", testVersion, selfLinker, admissionControl, requestContextMapper, mapper) + }, codec, "/api", "version", selfLinker, admissionControl, requestContextMapper, mapper) server := httptest.NewServer(handler) defer server.Close() dest, _ := url.Parse(server.URL) - dest.Path = "/api/" + testVersion + "/watch/foo" + dest.Path = "/api/version/watch/foo" table := []struct { rawQuery string @@ -199,10 +194,10 @@ func TestWatchParamParsing(t *testing.T) { fieldSelector: "Host=", namespace: api.NamespaceDefault, }, { - rawQuery: "namespace=watchother&fields=id%3dfoo&resourceVersion=1492", + rawQuery: "namespace=watchother&fields=ID%3dfoo&resourceVersion=1492", resourceVersion: "1492", labelSelector: "", - fieldSelector: "id=foo", + fieldSelector: "ID=foo", namespace: "watchother", }, { rawQuery: "", @@ -214,8 +209,8 @@ func TestWatchParamParsing(t *testing.T) { } for _, item := range table { - simpleStorage.requestedLabelSelector = labels.Everything() - simpleStorage.requestedFieldSelector = labels.Everything() + simpleStorage.requestedLabelSelector = nil + simpleStorage.requestedFieldSelector = nil simpleStorage.requestedResourceVersion = "5" // Prove this is set in all cases simpleStorage.requestedResourceNamespace = "" dest.RawQuery = item.rawQuery diff --git a/pkg/client/cache/listwatch_test.go b/pkg/client/cache/listwatch_test.go index 3f3d60f1149..43529761d9d 100644 --- a/pkg/client/cache/listwatch_test.go +++ b/pkg/client/cache/listwatch_test.go @@ -48,15 +48,6 @@ func buildResourcePath(prefix, namespace, resource string) string { return path.Join(base, resource) } -func getHostFieldLabel() string { - switch testapi.Version() { - case "v1beta1", "v1beta2": - return "DesiredState.Host" - default: - return "Status.Host" - } -} - // buildQueryValues is a convenience function for knowing if a namespace should be in a query param or not func buildQueryValues(namespace string, query url.Values) url.Values { v := url.Values{} @@ -93,17 +84,17 @@ func TestListWatchesCanList(t *testing.T) { }, // pod with "assigned" field selector. { - location: buildLocation(buildResourcePath("", api.NamespaceAll, "pods"), buildQueryValues(api.NamespaceAll, url.Values{"fields": []string{getHostFieldLabel() + "="}})), + location: buildLocation(buildResourcePath("", api.NamespaceAll, "pods"), buildQueryValues(api.NamespaceAll, url.Values{"fields": []string{"DesiredState.Host="}})), resource: "pods", namespace: api.NamespaceAll, - fieldSelector: labels.Set{getHostFieldLabel(): ""}.AsSelector(), + fieldSelector: labels.Set{"DesiredState.Host": ""}.AsSelector(), }, // pod in namespace "foo" { - location: buildLocation(buildResourcePath("", "foo", "pods"), buildQueryValues("foo", url.Values{"fields": []string{getHostFieldLabel() + "="}})), + location: buildLocation(buildResourcePath("", "foo", "pods"), buildQueryValues("foo", url.Values{"fields": []string{"DesiredState.Host="}})), resource: "pods", namespace: "foo", - fieldSelector: labels.Set{getHostFieldLabel(): ""}.AsSelector(), + fieldSelector: labels.Set{"DesiredState.Host": ""}.AsSelector(), }, } for _, item := range table { @@ -147,19 +138,19 @@ func TestListWatchesCanWatch(t *testing.T) { }, // pod with "assigned" field selector. { - location: buildLocation(buildResourcePath("watch", api.NamespaceAll, "pods"), buildQueryValues(api.NamespaceAll, url.Values{"fields": []string{getHostFieldLabel() + "="}, "resourceVersion": []string{"0"}})), + location: buildLocation(buildResourcePath("watch", api.NamespaceAll, "pods"), buildQueryValues(api.NamespaceAll, url.Values{"fields": []string{"DesiredState.Host="}, "resourceVersion": []string{"0"}})), rv: "0", resource: "pods", namespace: api.NamespaceAll, - fieldSelector: labels.Set{getHostFieldLabel(): ""}.AsSelector(), + fieldSelector: labels.Set{"DesiredState.Host": ""}.AsSelector(), }, // pod with namespace foo and assigned field selector { - location: buildLocation(buildResourcePath("watch", "foo", "pods"), buildQueryValues("foo", url.Values{"fields": []string{getHostFieldLabel() + "="}, "resourceVersion": []string{"0"}})), + location: buildLocation(buildResourcePath("watch", "foo", "pods"), buildQueryValues("foo", url.Values{"fields": []string{"DesiredState.Host="}, "resourceVersion": []string{"0"}})), rv: "0", resource: "pods", namespace: "foo", - fieldSelector: labels.Set{getHostFieldLabel(): ""}.AsSelector(), + fieldSelector: labels.Set{"DesiredState.Host": ""}.AsSelector(), }, } diff --git a/pkg/kubelet/config/apiserver.go b/pkg/kubelet/config/apiserver.go index b8062504f86..cfcac66f008 100644 --- a/pkg/kubelet/config/apiserver.go +++ b/pkg/kubelet/config/apiserver.go @@ -28,7 +28,7 @@ import ( // NewSourceApiserver creates a config source that watches and pulls from the apiserver. func NewSourceApiserver(client *client.Client, hostname string, updates chan<- interface{}) { - lw := cache.NewListWatchFromClient(client, "pods", api.NamespaceAll, labels.OneTermEqualSelector(getHostFieldLabel(client.APIVersion()), hostname)) + lw := cache.NewListWatchFromClient(client, "pods", api.NamespaceAll, labels.OneTermEqualSelector("Status.Host", hostname)) newSourceApiserverFromLW(lw, updates) } @@ -51,12 +51,3 @@ func newSourceApiserverFromLW(lw cache.ListerWatcher, updates chan<- interface{} } cache.NewReflector(lw, &api.Pod{}, cache.NewUndeltaStore(send, cache.MetaNamespaceKeyFunc)).Run() } - -func getHostFieldLabel(apiVersion string) string { - switch apiVersion { - case "v1beta1", "v1beta2": - return "DesiredState.Host" - default: - return "Status.Host" - } -} diff --git a/pkg/labels/selector.go b/pkg/labels/selector.go index cbf94413619..eba8b708d29 100644 --- a/pkg/labels/selector.go +++ b/pkg/labels/selector.go @@ -800,21 +800,6 @@ func SelectorFromSetParse(ls Set) (Selector, error) { // ParseSelector takes a string representing a selector and returns an // object suitable for matching, or an error. func ParseSelector(selector string) (Selector, error) { - return parseSelector(selector, - func(lhs, rhs string) (newLhs, newRhs string, err error) { - return lhs, rhs, nil - }) -} - -// Parses the selector and runs them through the given TransformFunc. -func ParseAndTransformSelector(selector string, fn TransformFunc) (Selector, error) { - return parseSelector(selector, fn) -} - -// Function to transform selectors. -type TransformFunc func(label, value string) (newLabel, newValue string, err error) - -func parseSelector(selector string, fn TransformFunc) (Selector, error) { parts := strings.Split(selector, ",") sort.StringSlice(parts).Sort() var items []Selector @@ -823,22 +808,10 @@ func parseSelector(selector string, fn TransformFunc) (Selector, error) { continue } if lhs, rhs, ok := try(part, "!="); ok { - lhs, rhs, err := fn(lhs, rhs) - if err != nil { - return nil, err - } items = append(items, ¬HasTerm{label: lhs, value: rhs}) } else if lhs, rhs, ok := try(part, "=="); ok { - lhs, rhs, err := fn(lhs, rhs) - if err != nil { - return nil, err - } items = append(items, &hasTerm{label: lhs, value: rhs}) } else if lhs, rhs, ok := try(part, "="); ok { - lhs, rhs, err := fn(lhs, rhs) - if err != nil { - return nil, err - } items = append(items, &hasTerm{label: lhs, value: rhs}) } else { return nil, fmt.Errorf("invalid selector: '%s'; can't understand '%s'", selector, part) diff --git a/pkg/registry/pod/rest.go b/pkg/registry/pod/rest.go index 1172041cfe7..53c1df1a403 100644 --- a/pkg/registry/pod/rest.go +++ b/pkg/registry/pod/rest.go @@ -23,6 +23,7 @@ import ( "github.com/GoogleCloudPlatform/kubernetes/pkg/api" "github.com/GoogleCloudPlatform/kubernetes/pkg/api/errors" "github.com/GoogleCloudPlatform/kubernetes/pkg/api/rest" + "github.com/GoogleCloudPlatform/kubernetes/pkg/api/v1beta1" "github.com/GoogleCloudPlatform/kubernetes/pkg/api/validation" "github.com/GoogleCloudPlatform/kubernetes/pkg/labels" "github.com/GoogleCloudPlatform/kubernetes/pkg/registry/generic" @@ -119,10 +120,18 @@ func MatchPod(label, field labels.Selector) generic.Matcher { // PodToSelectableFields returns a label set that represents the object // TODO: fields are not labels, and the validation rules for them do not apply. func PodToSelectableFields(pod *api.Pod) labels.Set { + // TODO we are populating both Status and DesiredState because selectors are not aware of API versions + // see https://github.com/GoogleCloudPlatform/kubernetes/pull/2503 + + var olderPodStatus v1beta1.PodStatus + api.Scheme.Convert(pod.Status.Phase, &olderPodStatus) + return labels.Set{ - "name": pod.Name, - "Status.Phase": string(pod.Status.Phase), - "Status.Host": pod.Status.Host, + "name": pod.Name, + "Status.Phase": string(pod.Status.Phase), + "Status.Host": pod.Status.Host, + "DesiredState.Status": string(olderPodStatus), + "DesiredState.Host": pod.Status.Host, } } diff --git a/pkg/runtime/scheme.go b/pkg/runtime/scheme.go index 253c4957f88..dc713d80e84 100644 --- a/pkg/runtime/scheme.go +++ b/pkg/runtime/scheme.go @@ -27,14 +27,8 @@ import ( // is an adaptation of conversion's Scheme for our API objects. type Scheme struct { raw *conversion.Scheme - // Map from version and resource to the corresponding func to convert - // resource field labels in that version to internal version. - fieldLabelConversionFuncs map[string]map[string]FieldLabelConversionFunc } -// Function to convert a field selector to internal representation. -type FieldLabelConversionFunc func(label, value string) (internalLabel, internalValue string, err error) - // fromScope gets the input version, desired output version, and desired Scheme // from a conversion.Scope. func (self *Scheme) fromScope(s conversion.Scope) (inVersion, outVersion string, scheme *Scheme) { @@ -206,7 +200,7 @@ func (self *Scheme) rawExtensionToRuntimeObjectArray(in *[]RawExtension, out *[] // NewScheme creates a new Scheme. This scheme is pluggable by default. func NewScheme() *Scheme { - s := &Scheme{conversion.NewScheme(), map[string]map[string]FieldLabelConversionFunc{}} + s := &Scheme{conversion.NewScheme()} s.raw.InternalVersion = "" s.raw.MetaFactory = conversion.SimpleMetaFactory{BaseFields: []string{"TypeMeta"}, VersionField: "APIVersion", KindField: "Kind"} if err := s.raw.AddConversionFuncs( @@ -286,17 +280,6 @@ func (s *Scheme) AddConversionFuncs(conversionFuncs ...interface{}) error { return s.raw.AddConversionFuncs(conversionFuncs...) } -// AddFieldLabelConversionFunc adds a conversion function to convert field selectors -// of the given api resource from the given version to internal version representation. -func (s *Scheme) AddFieldLabelConversionFunc(version, apiResource string, conversionFunc FieldLabelConversionFunc) error { - if s.fieldLabelConversionFuncs[version] == nil { - s.fieldLabelConversionFuncs[version] = map[string]FieldLabelConversionFunc{} - } - - s.fieldLabelConversionFuncs[version][apiResource] = conversionFunc - return nil -} - // AddStructFieldConversion allows you to specify a mechanical copy for a moved // or renamed struct field without writing an entire conversion function. See // the comment in conversion.Converter.SetStructFieldCopy for parameter details. @@ -319,19 +302,6 @@ func (s *Scheme) Convert(in, out interface{}) error { return s.raw.Convert(in, out) } -// Converts the given field label and value for an apiResource field selector from -// versioned representation to an unversioned one. -func (s *Scheme) ConvertFieldLabel(version, apiResource, label, value string) (string, string, error) { - if s.fieldLabelConversionFuncs[version] == nil { - return "", "", fmt.Errorf("No conversion function found for version: %s", version) - } - conversionFunc, ok := s.fieldLabelConversionFuncs[version][apiResource] - if !ok { - return "", "", fmt.Errorf("No conversion function found for version %s and api resource %s", version, apiResource) - } - return conversionFunc(label, value) -} - // ConvertToVersion attempts to convert an input object to its matching Kind in another // version within this scheme. Will return an error if the provided version does not // contain the inKind (or a mapping by name defined with AddKnownTypeWithName). Will also diff --git a/plugin/pkg/scheduler/factory/factory.go b/plugin/pkg/scheduler/factory/factory.go index ef729490bef..e7cb6f12457 100644 --- a/plugin/pkg/scheduler/factory/factory.go +++ b/plugin/pkg/scheduler/factory/factory.go @@ -143,7 +143,7 @@ func (f *ConfigFactory) CreateFromKeys(predicateKeys, priorityKeys util.StringSe // createUnassignedPodLW returns a cache.ListWatch that finds all pods that need to be // scheduled. func (factory *ConfigFactory) createUnassignedPodLW() *cache.ListWatch { - return cache.NewListWatchFromClient(factory.Client, "pods", api.NamespaceAll, labels.Set{getHostFieldLabel(factory.Client.APIVersion()): ""}.AsSelector()) + return cache.NewListWatchFromClient(factory.Client, "pods", api.NamespaceAll, labels.Set{"DesiredState.Host": ""}.AsSelector()) } func parseSelectorOrDie(s string) labels.Selector { @@ -158,8 +158,7 @@ func parseSelectorOrDie(s string) labels.Selector { // already scheduled. // TODO: return a ListerWatcher interface instead? func (factory *ConfigFactory) createAssignedPodLW() *cache.ListWatch { - return cache.NewListWatchFromClient(factory.Client, "pods", api.NamespaceAll, - parseSelectorOrDie(getHostFieldLabel(factory.Client.APIVersion())+"!=")) + return cache.NewListWatchFromClient(factory.Client, "pods", api.NamespaceAll, parseSelectorOrDie("DesiredState.Host!=")) } // createMinionLW returns a cache.ListWatch that gets all changes to minions. @@ -232,15 +231,6 @@ func (factory *ConfigFactory) makeDefaultErrorFunc(backoff *podBackoff, podQueue } } -func getHostFieldLabel(apiVersion string) string { - switch apiVersion { - case "v1beta1", "v1beta2": - return "DesiredState.Host" - default: - return "Status.Host" - } -} - // nodeEnumerator allows a cache.Poller to enumerate items in an api.NodeList type nodeEnumerator struct { *api.NodeList