From 42aee3ac5e81621e4c3cfc300ae5ac16cc4eb767 Mon Sep 17 00:00:00 2001 From: Wojciech Tyczynski Date: Fri, 19 Aug 2016 09:27:53 +0200 Subject: [PATCH 1/2] Allow for selector creation without validation --- pkg/labels/labels.go | 9 +++++++++ pkg/labels/selector.go | 16 ++++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/pkg/labels/labels.go b/pkg/labels/labels.go index 637a45fd38d..00139714be2 100644 --- a/pkg/labels/labels.go +++ b/pkg/labels/labels.go @@ -61,6 +61,15 @@ func (ls Set) AsSelector() Selector { return SelectorFromSet(ls) } +// ValidatedAsSelector converts labels into a selector, but +// assumes that labels are already validated and thus don't +// preform any validation. +// According to our measurements this is significantly faster +// in codepaths that matter at high sccale. +func (ls Set) AsSelectorPreValidated() Selector { + return SelectorFromValidatedSet(ls) +} + // FormatLables convert label map into plain string func FormatLabels(labelMap map[string]string) string { l := Set(labelMap).String() diff --git a/pkg/labels/selector.go b/pkg/labels/selector.go index 6551aba6016..703cf7aa1bf 100644 --- a/pkg/labels/selector.go +++ b/pkg/labels/selector.go @@ -796,6 +796,22 @@ func SelectorFromSet(ls Set) Selector { return internalSelector(requirements) } +// SelectorFromValidatedSet returns a Selector which will match exactly the given Set. +// A nil and empty Sets are considered equivalent to Everything(). +// It assumes that Set is already validated and doesn't do any validation. +func SelectorFromValidatedSet(ls Set) Selector { + if ls == nil { + return internalSelector{} + } + var requirements internalSelector + for label, value := range ls { + requirements = append(requirements, Requirement{key: label, operator: selection.Equals, strValues: sets.NewString(value)}) + } + // sort to have deterministic string representation + sort.Sort(ByKey(requirements)) + return internalSelector(requirements) +} + // ParseToRequirements takes a string representing a selector and returns a list of // requirements. This function is suitable for those callers that perform additional // processing on selector requirements. From e9d5be628afef4ee03b7f6dc4a311a36740c8a23 Mon Sep 17 00:00:00 2001 From: Wojciech Tyczynski Date: Fri, 19 Aug 2016 09:36:55 +0200 Subject: [PATCH 2/2] Don't validate selector that is already validated --- pkg/client/cache/listers.go | 7 +++---- pkg/client/cache/listers_test.go | 8 ++++---- pkg/controller/controller_utils.go | 2 +- pkg/controller/endpoint/endpoints_controller.go | 2 +- pkg/controller/replicaset/replica_set.go | 2 +- pkg/controller/replication/replication_controller.go | 11 +++++------ pkg/labels/labels.go | 2 +- plugin/pkg/scheduler/algorithm/listers.go | 4 ++-- 8 files changed, 18 insertions(+), 20 deletions(-) diff --git a/pkg/client/cache/listers.go b/pkg/client/cache/listers.go index 099c85646ff..6041744477f 100644 --- a/pkg/client/cache/listers.go +++ b/pkg/client/cache/listers.go @@ -269,11 +269,10 @@ func (s *StoreToReplicationControllerLister) GetPodControllers(pod *api.Pod) (co for _, m := range items { rc = *m.(*api.ReplicationController) - labelSet := labels.Set(rc.Spec.Selector) - selector = labels.Set(rc.Spec.Selector).AsSelector() + selector = labels.Set(rc.Spec.Selector).AsSelectorPreValidated() // If an rc with a nil or empty selector creeps in, it should match nothing, not everything. - if labelSet.AsSelector().Empty() || !selector.Matches(labels.Set(pod.Labels)) { + if selector.Empty() || !selector.Matches(labels.Set(pod.Labels)) { continue } controllers = append(controllers, rc) @@ -513,7 +512,7 @@ func (s *StoreToServiceLister) GetPodServices(pod *api.Pod) (services []api.Serv // services with nil selectors match nothing, not everything. continue } - selector = labels.Set(service.Spec.Selector).AsSelector() + selector = labels.Set(service.Spec.Selector).AsSelectorPreValidated() if selector.Matches(labels.Set(pod.Labels)) { services = append(services, service) } diff --git a/pkg/client/cache/listers_test.go b/pkg/client/cache/listers_test.go index a63f06ee93e..f592dc89c96 100644 --- a/pkg/client/cache/listers_test.go +++ b/pkg/client/cache/listers_test.go @@ -143,7 +143,7 @@ func TestStoreToReplicationControllerLister(t *testing.T) { }, }, list: func(lister StoreToReplicationControllerLister) ([]api.ReplicationController, error) { - return lister.ReplicationControllers(api.NamespaceAll).List(labels.Set{}.AsSelector()) + return lister.ReplicationControllers(api.NamespaceAll).List(labels.Set{}.AsSelectorPreValidated()) }, outRCNames: sets.NewString("hmm", "foo"), }, @@ -158,7 +158,7 @@ func TestStoreToReplicationControllerLister(t *testing.T) { }, }, list: func(lister StoreToReplicationControllerLister) ([]api.ReplicationController, error) { - return lister.ReplicationControllers("hmm").List(labels.Set{}.AsSelector()) + return lister.ReplicationControllers("hmm").List(labels.Set{}.AsSelectorPreValidated()) }, outRCNames: sets.NewString("hmm"), }, @@ -715,7 +715,7 @@ func TestStoreToPodLister(t *testing.T) { spl := StoreToPodLister{store} // Verify that we can always look up by Namespace. - defaultPods, err := spl.Pods(api.NamespaceDefault).List(labels.Set{}.AsSelector()) + defaultPods, err := spl.Pods(api.NamespaceDefault).List(labels.Set{}.AsSelectorPreValidated()) if err != nil { t.Errorf("Unexpected error: %v", err) } else if e, a := 1, len(defaultPods); e != a { @@ -725,7 +725,7 @@ func TestStoreToPodLister(t *testing.T) { } for _, id := range ids { - got, err := spl.List(labels.Set{"name": id}.AsSelector()) + got, err := spl.List(labels.Set{"name": id}.AsSelectorPreValidated()) if err != nil { t.Errorf("Unexpected error: %v", err) continue diff --git a/pkg/controller/controller_utils.go b/pkg/controller/controller_utils.go index 0a851c23f81..0baa16e71e3 100644 --- a/pkg/controller/controller_utils.go +++ b/pkg/controller/controller_utils.go @@ -472,7 +472,7 @@ func (r RealPodControl) createPods(nodeName, namespace string, template *api.Pod if len(nodeName) != 0 { pod.Spec.NodeName = nodeName } - if labels.Set(pod.Labels).AsSelector().Empty() { + if labels.Set(pod.Labels).AsSelectorPreValidated().Empty() { return fmt.Errorf("unable to create pods, no labels") } if newPod, err := r.KubeClient.Core().Pods(namespace).Create(pod); err != nil { diff --git a/pkg/controller/endpoint/endpoints_controller.go b/pkg/controller/endpoint/endpoints_controller.go index fbc53141a20..56a5dddf79f 100644 --- a/pkg/controller/endpoint/endpoints_controller.go +++ b/pkg/controller/endpoint/endpoints_controller.go @@ -358,7 +358,7 @@ func (e *EndpointController) syncService(key string) { } glog.V(5).Infof("About to update endpoints for service %q", key) - pods, err := e.podStore.Pods(service.Namespace).List(labels.Set(service.Spec.Selector).AsSelector()) + pods, err := e.podStore.Pods(service.Namespace).List(labels.Set(service.Spec.Selector).AsSelectorPreValidated()) if err != nil { // Since we're getting stuff from a local cache, it is // basically impossible to get this error. diff --git a/pkg/controller/replicaset/replica_set.go b/pkg/controller/replicaset/replica_set.go index 3994aec597c..2efd4a2358a 100644 --- a/pkg/controller/replicaset/replica_set.go +++ b/pkg/controller/replicaset/replica_set.go @@ -659,7 +659,7 @@ func (rsc *ReplicaSetController) syncReplicaSet(key string) error { // part of the filteredPods. fullyLabeledReplicasCount := 0 readyReplicasCount := 0 - templateLabel := labels.Set(rs.Spec.Template.Labels).AsSelector() + templateLabel := labels.Set(rs.Spec.Template.Labels).AsSelectorPreValidated() for _, pod := range filteredPods { if templateLabel.Matches(labels.Set(pod.Labels)) { fullyLabeledReplicasCount++ diff --git a/pkg/controller/replication/replication_controller.go b/pkg/controller/replication/replication_controller.go index a1276732c94..da06830b546 100644 --- a/pkg/controller/replication/replication_controller.go +++ b/pkg/controller/replication/replication_controller.go @@ -291,11 +291,10 @@ func isControllerMatch(pod *api.Pod, rc *api.ReplicationController) bool { if rc.Namespace != pod.Namespace { return false } - labelSet := labels.Set(rc.Spec.Selector) - selector := labels.Set(rc.Spec.Selector).AsSelector() + selector := labels.Set(rc.Spec.Selector).AsSelectorPreValidated() // If an rc with a nil or empty selector creeps in, it should match nothing, not everything. - if labelSet.AsSelector().Empty() || !selector.Matches(labels.Set(pod.Labels)) { + if selector.Empty() || !selector.Matches(labels.Set(pod.Labels)) { return false } return true @@ -662,7 +661,7 @@ func (rm *ReplicationManager) syncReplicationController(key string) error { rm.queue.Add(key) return err } - cm := controller.NewPodControllerRefManager(rm.podControl, rc.ObjectMeta, labels.Set(rc.Spec.Selector).AsSelector(), getRCKind()) + cm := controller.NewPodControllerRefManager(rm.podControl, rc.ObjectMeta, labels.Set(rc.Spec.Selector).AsSelectorPreValidated(), getRCKind()) matchesAndControlled, matchesNeedsController, controlledDoesNotMatch := cm.Classify(pods) for _, pod := range matchesNeedsController { err := cm.AdoptPod(pod) @@ -694,7 +693,7 @@ func (rm *ReplicationManager) syncReplicationController(key string) error { return aggregate } } else { - pods, err := rm.podStore.Pods(rc.Namespace).List(labels.Set(rc.Spec.Selector).AsSelector()) + pods, err := rm.podStore.Pods(rc.Namespace).List(labels.Set(rc.Spec.Selector).AsSelectorPreValidated()) if err != nil { glog.Errorf("Error getting pods for rc %q: %v", key, err) rm.queue.Add(key) @@ -716,7 +715,7 @@ func (rm *ReplicationManager) syncReplicationController(key string) error { // matching pods must be part of the filteredPods. fullyLabeledReplicasCount := 0 readyReplicasCount := 0 - templateLabel := labels.Set(rc.Spec.Template.Labels).AsSelector() + templateLabel := labels.Set(rc.Spec.Template.Labels).AsSelectorPreValidated() for _, pod := range filteredPods { if templateLabel.Matches(labels.Set(pod.Labels)) { fullyLabeledReplicasCount++ diff --git a/pkg/labels/labels.go b/pkg/labels/labels.go index 00139714be2..822b137a9e7 100644 --- a/pkg/labels/labels.go +++ b/pkg/labels/labels.go @@ -61,7 +61,7 @@ func (ls Set) AsSelector() Selector { return SelectorFromSet(ls) } -// ValidatedAsSelector converts labels into a selector, but +// AsSelectorPreValidated converts labels into a selector, but // assumes that labels are already validated and thus don't // preform any validation. // According to our measurements this is significantly faster diff --git a/plugin/pkg/scheduler/algorithm/listers.go b/plugin/pkg/scheduler/algorithm/listers.go index 3aea446c308..7887614535d 100644 --- a/plugin/pkg/scheduler/algorithm/listers.go +++ b/plugin/pkg/scheduler/algorithm/listers.go @@ -85,7 +85,7 @@ func (f FakeServiceLister) GetPodServices(pod *api.Pod) (services []api.Service, if service.Namespace != pod.Namespace { continue } - selector = labels.Set(service.Spec.Selector).AsSelector() + selector = labels.Set(service.Spec.Selector).AsSelectorPreValidated() if selector.Matches(labels.Set(pod.Labels)) { services = append(services, service) } @@ -134,7 +134,7 @@ func (f FakeControllerLister) GetPodControllers(pod *api.Pod) (controllers []api if controller.Namespace != pod.Namespace { continue } - selector = labels.Set(controller.Spec.Selector).AsSelector() + selector = labels.Set(controller.Spec.Selector).AsSelectorPreValidated() if selector.Matches(labels.Set(pod.Labels)) { controllers = append(controllers, controller) }