diff --git a/changelog/v0.34.3/mem-client-expression-selector.yaml b/changelog/v0.34.3/mem-client-expression-selector.yaml new file mode 100644 index 000000000..c36af59f3 --- /dev/null +++ b/changelog/v0.34.3/mem-client-expression-selector.yaml @@ -0,0 +1,5 @@ +changelog: + - type: FIX + issueLink: https://github.com/solo-io/solo-kit/issues/553 + description: >- + Add support for ExpressionSelector in memory ResourceClient. diff --git a/pkg/api/v1/clients/kube/resource_client.go b/pkg/api/v1/clients/kube/resource_client.go index d9845238b..a9816bf1a 100644 --- a/pkg/api/v1/clients/kube/resource_client.go +++ b/pkg/api/v1/clients/kube/resource_client.go @@ -7,11 +7,6 @@ import ( "strings" "time" - "github.com/solo-io/solo-kit/pkg/utils/specutils" - - "github.com/solo-io/solo-kit/pkg/utils/kubeutils" - "k8s.io/apimachinery/pkg/types" - "github.com/solo-io/go-utils/stringutils" "github.com/solo-io/solo-kit/pkg/api/shared" "github.com/solo-io/solo-kit/pkg/api/v1/clients" @@ -20,12 +15,14 @@ import ( v1 "github.com/solo-io/solo-kit/pkg/api/v1/clients/kube/crd/solo.io/v1" "github.com/solo-io/solo-kit/pkg/api/v1/resources" "github.com/solo-io/solo-kit/pkg/errors" + "github.com/solo-io/solo-kit/pkg/utils/kubeutils" + "github.com/solo-io/solo-kit/pkg/utils/specutils" "go.opencensus.io/stats" "go.opencensus.io/stats/view" "go.opencensus.io/tag" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/types" ) var ( @@ -298,7 +295,7 @@ func (rc *ResourceClient) List(namespace string, opts clients.ListOpts) (resourc return nil, err } - labelSelector, err := rc.getLabelSelector(opts) + labelSelector, err := kubeutils.ToLabelSelector(opts) if err != nil { return nil, errors.Wrapf(err, "parsing label selector") } @@ -458,16 +455,6 @@ func (rc *ResourceClient) Watch(namespace string, opts clients.WatchOpts) (<-cha return resourcesChan, errs, nil } -func (rc *ResourceClient) getLabelSelector(listOpts clients.ListOpts) (labels.Selector, error) { - // https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#set-based-requirement - if listOpts.ExpressionSelector != "" { - return labels.Parse(listOpts.ExpressionSelector) - } - - // https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#equality-based-requirement - return labels.SelectorFromSet(listOpts.Selector), nil -} - // Checks whether the group version kind of the given resource matches that of the client's underlying CRD: func (rc *ResourceClient) matchesClientGVK(resource v1.Resource) bool { return resource.GroupVersionKind().String() == rc.crd.GroupVersionKind().String() diff --git a/pkg/api/v1/clients/memory/resource_client.go b/pkg/api/v1/clients/memory/resource_client.go index 2f2f01861..6c91a3100 100644 --- a/pkg/api/v1/clients/memory/resource_client.go +++ b/pkg/api/v1/clients/memory/resource_client.go @@ -12,6 +12,7 @@ import ( "github.com/solo-io/solo-kit/pkg/api/v1/clients" "github.com/solo-io/solo-kit/pkg/api/v1/resources" "github.com/solo-io/solo-kit/pkg/errors" + "github.com/solo-io/solo-kit/pkg/utils/kubeutils" "k8s.io/apimachinery/pkg/labels" ) @@ -197,9 +198,19 @@ func (rc *ResourceClient) Delete(namespace, name string, opts clients.DeleteOpts func (rc *ResourceClient) List(namespace string, opts clients.ListOpts) (resources.ResourceList, error) { opts = opts.WithDefaults() cachedResources := rc.cache.List(rc.Prefix(namespace)) + + var labelSelector labels.Selector + var err error + if kubeutils.HasSelector(opts) { + labelSelector, err = kubeutils.ToLabelSelector(opts) + if err != nil { + return nil, errors.Wrapf(err, "parsing label selector") + } + } + var resourceList resources.ResourceList for _, resource := range cachedResources { - if labels.SelectorFromSet(opts.Selector).Matches(labels.Set(resource.GetMetadata().Labels)) { + if labelSelector == nil || labelSelector.Matches(labels.Set(resource.GetMetadata().Labels)) { clone := resources.Clone(resource) resourceList = append(resourceList, clone) } @@ -220,8 +231,9 @@ func (rc *ResourceClient) Watch(namespace string, opts clients.WatchOpts) (<-cha errs := make(chan error) updateResourceList := func() { list, err := rc.List(namespace, clients.ListOpts{ - Ctx: opts.Ctx, - Selector: opts.Selector, + Ctx: opts.Ctx, + Selector: opts.Selector, + ExpressionSelector: opts.ExpressionSelector, }) if err != nil { errs <- err diff --git a/pkg/api/v1/clients/memory/resource_client_test.go b/pkg/api/v1/clients/memory/resource_client_test.go index 8c656d92c..ca5bb024f 100644 --- a/pkg/api/v1/clients/memory/resource_client_test.go +++ b/pkg/api/v1/clients/memory/resource_client_test.go @@ -8,6 +8,7 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "github.com/onsi/gomega/gmeasure" "github.com/solo-io/solo-kit/pkg/api/v1/clients" . "github.com/solo-io/solo-kit/pkg/api/v1/clients/memory" "github.com/solo-io/solo-kit/pkg/api/v1/resources" @@ -62,33 +63,273 @@ var _ = Describe("Base", func() { Expect(listret[0]).NotTo(BeIdenticalTo(listret2[0])) }) + Context("listing resources", func() { + var ( + obj1 *v1.MockResource + obj2 *v1.MockResource + obj3 *v1.MockResource + obj4 *v1.MockResource + obj5 *v1.MockResource + ) + + BeforeEach(func() { + obj1 = &v1.MockResource{ + Metadata: &core.Metadata{ + Name: "name1", + Namespace: "ns1", + Labels: map[string]string{ + "key": "val1", + }, + }, + } + obj2 = &v1.MockResource{ + Metadata: &core.Metadata{ + Name: "name2", + Namespace: "ns2", + Labels: map[string]string{ + "key": "val2", + }, + }, + } + obj3 = &v1.MockResource{ + Metadata: &core.Metadata{ + Name: "name3", + Namespace: "ns1", + Labels: map[string]string{ + "key": "val2", + }, + }, + } + obj4 = &v1.MockResource{ + Metadata: &core.Metadata{ + Name: "name4", + Namespace: "ns2", + Labels: map[string]string{ + "key": "val3", + }, + }, + } + obj5 = &v1.MockResource{ + Metadata: &core.Metadata{ + Name: "name5", + Namespace: "ns1", + Labels: map[string]string{ + "key": "val3", + }, + }, + } + + _, err := client.Write(obj1, clients.WriteOpts{}) + Expect(err).NotTo(HaveOccurred()) + _, err = client.Write(obj2, clients.WriteOpts{}) + Expect(err).NotTo(HaveOccurred()) + _, err = client.Write(obj3, clients.WriteOpts{}) + Expect(err).NotTo(HaveOccurred()) + _, err = client.Write(obj4, clients.WriteOpts{}) + Expect(err).NotTo(HaveOccurred()) + _, err = client.Write(obj5, clients.WriteOpts{}) + Expect(err).NotTo(HaveOccurred()) + }) + + It("lists all resources when empty namespace is provided", func() { + resources, err := client.List("", clients.ListOpts{}) + Expect(err).NotTo(HaveOccurred()) + + // resources are sorted by namespace, then name + expectedResourceNames := []string{ + "name1", "name3", "name5", // ns1 + "name2", "name4", // ns2 + } + Expect(resources).To(HaveLen(len(expectedResourceNames))) + for i, r := range resources { + Expect(r.GetMetadata().GetName()).To(Equal(expectedResourceNames[i])) + } + }) + + It("lists resources in a given namespace", func() { + resources, err := client.List("ns2", clients.ListOpts{}) + Expect(err).NotTo(HaveOccurred()) + + expectedResourceNames := []string{ + "name2", "name4", + } + Expect(resources).To(HaveLen(len(expectedResourceNames))) + for i, r := range resources { + Expect(r.GetMetadata().GetName()).To(Equal(expectedResourceNames[i])) + } + }) + + It("returns empty list if namespace is invalid", func() { + resources, err := client.List("invalid-namespace", clients.ListOpts{}) + Expect(err).NotTo(HaveOccurred()) + Expect(resources).To(HaveLen(0)) + }) + + It("returns resources matching the given selector, across all namespaces", func() { + resources, err := client.List("", clients.ListOpts{ + Selector: map[string]string{ + "key": "val2", + }, + }) + Expect(err).NotTo(HaveOccurred()) + + // resources are sorted by namespace, then name + expectedResourceNames := []string{ + "name3", "name2", + } + Expect(resources).To(HaveLen(len(expectedResourceNames))) + for i, r := range resources { + Expect(r.GetMetadata().GetName()).To(Equal(expectedResourceNames[i])) + } + }) + + It("returns resources matching the given selector, in given namespace", func() { + resources, err := client.List("ns2", clients.ListOpts{ + Selector: map[string]string{ + "key": "val2", + }, + }) + Expect(err).NotTo(HaveOccurred()) + + Expect(resources).To(HaveLen(1)) + Expect(resources[0].GetMetadata().GetName()).To(Equal("name2")) + }) + + It("returns resources matching the given expression selector, across all namespaces", func() { + resources, err := client.List("", clients.ListOpts{ + ExpressionSelector: "key in (val1,val3)", + }) + Expect(err).NotTo(HaveOccurred()) + + // resources are sorted by namespace, then name + expectedResourceNames := []string{ + "name1", "name5", "name4", + } + Expect(resources).To(HaveLen(len(expectedResourceNames))) + for i, r := range resources { + Expect(r.GetMetadata().GetName()).To(Equal(expectedResourceNames[i])) + } + }) + + It("returns resources matching the given expression selector, in given namespace", func() { + resources, err := client.List("ns2", clients.ListOpts{ + ExpressionSelector: "key in (val1,val3)", + }) + Expect(err).NotTo(HaveOccurred()) + + Expect(resources).To(HaveLen(1)) + Expect(resources[0].GetMetadata().GetName()).To(Equal("name4")) + }) + + It("when both selector and expression selector are provided, uses expression selector", func() { + resources, err := client.List("ns2", clients.ListOpts{ + Selector: map[string]string{ + "key": "val2", + }, + ExpressionSelector: "key in (val1,val3)", + }) + Expect(err).NotTo(HaveOccurred()) + + Expect(resources).To(HaveLen(1)) + Expect(resources[0].GetMetadata().GetName()).To(Equal("name4")) + }) + }) + Context("Benchmarks", func() { - Measure("it should perform list efficiently", func(b Benchmarker) { - const numobjs = 10000 + const numobjs = 10000 + BeforeEach(func() { for i := 0; i < numobjs; i++ { + isEven := i%2 == 0 obj := &v1.MockResource{ Metadata: &core.Metadata{ Namespace: "ns", Name: fmt.Sprintf("n-%v", numobjs-i), + Labels: map[string]string{ + "even": fmt.Sprintf("%v", isEven), + }, }, Data: strings.Repeat("123", 1000) + fmt.Sprintf("test-%v", i), } client.Write(obj, clients.WriteOpts{}) } + }) + + It("should perform list efficiently", Serial, func() { + experiment := gmeasure.NewExperiment("list resources with no selector") + AddReportEntry(experiment.Name, experiment) + l := clients.ListOpts{} var output resources.ResourceList var err error - runtime := b.Time("runtime", func() { - output, err = client.List("ns", l) - }) - Expect(err).NotTo(HaveOccurred()) - Expect(output).To(HaveLen(numobjs)) - Expect(output[0].GetMetadata().Name).To(Equal("n-1")) - Expect(runtime.Seconds()).Should(BeNumerically("<", 0.5), "List() shouldn't take too long.") - }, 10) + experiment.Sample(func(idx int) { + experiment.MeasureDuration("list-resources", func() { + output, err = client.List("ns", l) + }) + Expect(err).NotTo(HaveOccurred()) + Expect(output).To(HaveLen(numobjs)) + Expect(output[0].GetMetadata().Name).To(Equal("n-1")) + }, gmeasure.SamplingConfig{N: 10, Duration: 10 * time.Second}) + + stats := experiment.GetStats("list-resources") + medianDuration := stats.DurationFor(gmeasure.StatMedian) + + Expect(medianDuration).To(BeNumerically("<", 500*time.Millisecond)) + }) + + It("should perform list efficiently, with equality-based selector", Serial, func() { + experiment := gmeasure.NewExperiment("list resources with equality selector") + AddReportEntry(experiment.Name, experiment) + + l := clients.ListOpts{ + Selector: map[string]string{ + "even": "true", + }, + } + var output resources.ResourceList + var err error + + experiment.Sample(func(idx int) { + experiment.MeasureDuration("list-resources-with-selector", func() { + output, err = client.List("ns", l) + }) + + Expect(err).NotTo(HaveOccurred()) + Expect(output).To(HaveLen(numobjs / 2)) + }, gmeasure.SamplingConfig{N: 10, Duration: 10 * time.Second}) + + stats := experiment.GetStats("list-resources-with-selector") + medianDuration := stats.DurationFor(gmeasure.StatMedian) + + Expect(medianDuration).To(BeNumerically("<", 500*time.Millisecond)) + }) + + It("should perform list efficiently, with set-based selector", Serial, func() { + experiment := gmeasure.NewExperiment("list resources with set selector") + AddReportEntry(experiment.Name, experiment) + + l := clients.ListOpts{ + ExpressionSelector: "even in (true,false)", + } + var output resources.ResourceList + var err error + + experiment.Sample(func(idx int) { + experiment.MeasureDuration("list-resources-with-set-selector", func() { + output, err = client.List("ns", l) + }) + + Expect(err).NotTo(HaveOccurred()) + Expect(output).To(HaveLen(numobjs)) + }, gmeasure.SamplingConfig{N: 10, Duration: 10 * time.Second}) + + stats := experiment.GetStats("list-resources-with-set-selector") + medianDuration := stats.DurationFor(gmeasure.StatMedian) + + Expect(medianDuration).To(BeNumerically("<", 500*time.Millisecond)) + }) }) }) diff --git a/pkg/utils/kubeutils/metadata.go b/pkg/utils/kubeutils/metadata.go index 11b80f1cf..5ffd30329 100644 --- a/pkg/utils/kubeutils/metadata.go +++ b/pkg/utils/kubeutils/metadata.go @@ -5,9 +5,28 @@ import ( "github.com/solo-io/solo-kit/pkg/api/v1/clients" "github.com/solo-io/solo-kit/pkg/api/v1/resources/core" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" kubetypes "k8s.io/apimachinery/pkg/types" ) +// HasSelector returns true if the ListOpts contains a selector (which may be either an equality-based selector +// or set-based expression selector). +func HasSelector(listOpts clients.ListOpts) bool { + return listOpts.ExpressionSelector != "" || len(listOpts.Selector) > 0 +} + +// ToLabelSelector converts the selector specified by the ListOpts into an apimachinery label selector. +// If both ExpressionSelector and Selector are specified in the opts, only ExpressionSelector is used. +func ToLabelSelector(listOpts clients.ListOpts) (labels.Selector, error) { + // https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#set-based-requirement + if listOpts.ExpressionSelector != "" { + return labels.Parse(listOpts.ExpressionSelector) + } + + // https://kubernetes.io/docs/concepts/overview/working-with-objects/labels/#equality-based-requirement + return labels.SelectorFromSet(listOpts.Selector), nil +} + func FromKubeMeta(meta metav1.ObjectMeta, copyOwnerReferences bool) *core.Metadata { var metaData = &core.Metadata{}