diff --git a/changelog/v0.30.10/sharedrefclient.yaml b/changelog/v0.30.10/sharedrefclient.yaml new file mode 100644 index 000000000..9c77a8599 --- /dev/null +++ b/changelog/v0.30.10/sharedrefclient.yaml @@ -0,0 +1,6 @@ +changelog: + - type: NEW_FEATURE + resolvesIssue: false + issueLink: https://github.com/solo-io/gloo/issues/7166 + description: > + Add an extension of the InMemoryResourceClient, called SharedRefResourceClient that does not clone resources on reads/writes. \ No newline at end of file diff --git a/pkg/api/v1/clients/factory/resource_client_factory.go b/pkg/api/v1/clients/factory/resource_client_factory.go index 78b4480b4..ad0b71950 100644 --- a/pkg/api/v1/clients/factory/resource_client_factory.go +++ b/pkg/api/v1/clients/factory/resource_client_factory.go @@ -110,6 +110,8 @@ func newResourceClient(ctx context.Context, factory ResourceClientFactory, param return file.NewResourceClient(opts.RootDir, resourceType), nil case *MemoryResourceClientFactory: return memory.NewResourceClient(opts.Cache, resourceType), nil + case *SharedRefMemoryResourceClientFactory: + return memory.NewSharedRefResourceClient(opts.Cache, resourceType), nil case *KubeConfigMapClientFactory: if opts.Cache == nil { return nil, errors.Errorf("invalid opts, configmap client requires a kube core cache") @@ -193,6 +195,14 @@ func (f *MemoryResourceClientFactory) NewResourceClient(ctx context.Context, par return newResourceClient(ctx, f, params) } +type SharedRefMemoryResourceClientFactory struct { + Cache memory.InMemoryResourceCache +} + +func (f *SharedRefMemoryResourceClientFactory) NewResourceClient(ctx context.Context, params NewResourceClientParams) (clients.ResourceClient, error) { + return newResourceClient(ctx, f, params) +} + type KubeConfigMapClientFactory struct { Clientset kubernetes.Interface Cache cache.KubeCoreCache diff --git a/pkg/api/v1/clients/memory/resource_client.go b/pkg/api/v1/clients/memory/resource_client.go index 2f2f01861..4ead86f54 100644 --- a/pkg/api/v1/clients/memory/resource_client.go +++ b/pkg/api/v1/clients/memory/resource_client.go @@ -105,6 +105,9 @@ type ResourceClient struct { resourceType resources.Resource cache InMemoryResourceCache } +type SharedRefResourceClient struct { + ResourceClient +} func NewResourceClient(cache InMemoryResourceCache, resourceType resources.Resource) *ResourceClient { return &ResourceClient{ @@ -113,6 +116,15 @@ func NewResourceClient(cache InMemoryResourceCache, resourceType resources.Resou } } +func NewSharedRefResourceClient(cache InMemoryResourceCache, resourceType resources.Resource) *SharedRefResourceClient { + return &SharedRefResourceClient{ + ResourceClient{ + cache: cache, + resourceType: resourceType, + }, + } +} + var _ clients.ResourceClient = &ResourceClient{} func (rc *ResourceClient) Kind() string { @@ -141,7 +153,6 @@ func (rc *ResourceClient) Read(namespace, name string, opts clients.ReadOpts) (r clone := resources.Clone(resource) return clone, nil } - func (rc *ResourceClient) Write(resource resources.Resource, opts clients.WriteOpts) (resources.Resource, error) { opts = opts.WithDefaults() if err := resources.Validate(resource); err != nil { @@ -267,3 +278,66 @@ func newOrIncrementResourceVer(resourceVersion string) string { } return fmt.Sprintf("%v", curr+1) } + +func (rc *SharedRefResourceClient) Read(namespace, name string, opts clients.ReadOpts) (resources.Resource, error) { + if err := resources.ValidateName(name); err != nil { + return nil, errors.Wrapf(err, "validation error") + } + opts = opts.WithDefaults() + resource, ok := rc.cache.Get(rc.key(namespace, name)) + if !ok { + return nil, errors.NewNotExistErr(namespace, name) + } + + return resource, nil +} + +func (rc *SharedRefResourceClient) List(namespace string, opts clients.ListOpts) (resources.ResourceList, error) { + opts = opts.WithDefaults() + cachedResources := rc.cache.List(rc.Prefix(namespace)) + var resourceList resources.ResourceList + for _, resource := range cachedResources { + if labels.SelectorFromSet(opts.Selector).Matches(labels.Set(resource.GetMetadata().Labels)) { + resourceList = append(resourceList, resource) + } + } + + sort.Stable(resourceList) + + return resourceList, nil +} + +func (rc *SharedRefResourceClient) Write(resource resources.Resource, opts clients.WriteOpts) (resources.Resource, error) { + opts = opts.WithDefaults() + if err := resources.Validate(resource); err != nil { + return nil, errors.Wrapf(err, "validation error") + } + + key := rc.key(resource.GetMetadata().GetNamespace(), resource.GetMetadata().GetName()) + + original, err := rc.Read( + resource.GetMetadata().GetNamespace(), + resource.GetMetadata().GetName(), + clients.ReadOpts{}, + ) + if original != nil && err == nil { + if !opts.OverwriteExisting { + return nil, errors.NewExistErr(resource.GetMetadata()) + } + if resource.GetMetadata().GetResourceVersion() != original.GetMetadata().GetResourceVersion() { + return nil, errors.NewResourceVersionErr( + resource.GetMetadata().GetNamespace(), + resource.GetMetadata().GetName(), + resource.GetMetadata().GetResourceVersion(), + original.GetMetadata().GetResourceVersion(), + ) + } + } + + // initialize or increment resource version + resource.GetMetadata().ResourceVersion = newOrIncrementResourceVer(resource.GetMetadata().GetResourceVersion()) + + rc.cache.Set(key, resource) + + return resource, nil +} diff --git a/pkg/api/v1/clients/memory/resource_client_test.go b/pkg/api/v1/clients/memory/resource_client_test.go index 9b5e8f735..2af7adaae 100644 --- a/pkg/api/v1/clients/memory/resource_client_test.go +++ b/pkg/api/v1/clients/memory/resource_client_test.go @@ -18,77 +18,124 @@ import ( ) var _ = Describe("Base", func() { - var ( - client *ResourceClient - ) - BeforeEach(func() { - client = NewResourceClient(NewInMemoryResourceCache(), &v1.MockResource{}) - }) - AfterEach(func() { - }) - It("CRUDs resources", func() { - selector := map[string]string{ - helpers.TestLabel: helpers.RandString(8), - } - generic.TestCrudClient("ns1", "ns2", client, clients.WatchOpts{ - Selector: selector, - Ctx: context.TODO(), - RefreshRate: time.Minute, + Context("Standard in memory client", func() { + var ( + client *ResourceClient + ) + BeforeEach(func() { + client = NewResourceClient(NewInMemoryResourceCache(), &v1.MockResource{}) }) - }) - It("should not return pointer to internal object", func() { - obj := &v1.MockResource{ - Metadata: &core.Metadata{ - Namespace: "ns", - Name: "n", - }, - Data: "test", - } - client.Write(obj, clients.WriteOpts{}) - ret, err := client.Read("ns", "n", clients.ReadOpts{}) - Expect(err).NotTo(HaveOccurred()) - Expect(ret).NotTo(BeIdenticalTo(obj)) + AfterEach(func() { + }) + It("CRUDs resources", func() { + selector := map[string]string{ + helpers.TestLabel: helpers.RandString(8), + } + generic.TestCrudClient("ns1", "ns2", client, clients.WatchOpts{ + Selector: selector, + Ctx: context.TODO(), + RefreshRate: time.Minute, + }) + }) + It("should not return pointer to internal object", func() { + obj := &v1.MockResource{ + Metadata: &core.Metadata{ + Namespace: "ns", + Name: "n", + }, + Data: "test", + } + client.Write(obj, clients.WriteOpts{}) + ret, err := client.Read("ns", "n", clients.ReadOpts{}) + Expect(err).NotTo(HaveOccurred()) + Expect(ret).NotTo(BeIdenticalTo(obj)) - ret2, err := client.Read("ns", "n", clients.ReadOpts{}) - Expect(err).NotTo(HaveOccurred()) - Expect(ret).NotTo(BeIdenticalTo(ret2)) + ret2, err := client.Read("ns", "n", clients.ReadOpts{}) + Expect(err).NotTo(HaveOccurred()) + Expect(ret).NotTo(BeIdenticalTo(ret2)) - listret, err := client.List("ns", clients.ListOpts{}) - Expect(err).NotTo(HaveOccurred()) - Expect(listret[0]).NotTo(BeIdenticalTo(obj)) + listret, err := client.List("ns", clients.ListOpts{}) + Expect(err).NotTo(HaveOccurred()) + Expect(listret[0]).NotTo(BeIdenticalTo(obj)) - listret2, err := client.List("ns", clients.ListOpts{}) - Expect(err).NotTo(HaveOccurred()) - Expect(listret[0]).NotTo(BeIdenticalTo(listret2[0])) - }) + listret2, err := client.List("ns", clients.ListOpts{}) + Expect(err).NotTo(HaveOccurred()) + Expect(listret[0]).NotTo(BeIdenticalTo(listret2[0])) + }) - Context("Benchmarks", func() { - Measure("it should perform list efficiently", func(b Benchmarker) { - const numobjs = 10000 + Context("Benchmarks", func() { + Measure("it should perform list efficiently", func(b Benchmarker) { + const numobjs = 10000 - for i := 0; i < numobjs; i++ { - obj := &v1.MockResource{ - Metadata: &core.Metadata{ - Namespace: "ns", - Name: fmt.Sprintf("n-%v", numobjs-i), - }, - Data: strings.Repeat("123", 1000) + fmt.Sprintf("test-%v", i), + for i := 0; i < numobjs; i++ { + obj := &v1.MockResource{ + Metadata: &core.Metadata{ + Namespace: "ns", + Name: fmt.Sprintf("n-%v", numobjs-i), + }, + Data: strings.Repeat("123", 1000) + fmt.Sprintf("test-%v", i), + } + client.Write(obj, clients.WriteOpts{}) } - client.Write(obj, clients.WriteOpts{}) + 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) + + }) + }) + + Context("Shared ref memory client", func() { + var ( + client *SharedRefResourceClient + ) + BeforeEach(func() { + client = NewSharedRefResourceClient(NewInMemoryResourceCache(), &v1.MockResource{}) + }) + AfterEach(func() { + }) + It("CRUDs resources", func() { + selector := map[string]string{ + helpers.TestLabel: helpers.RandString(8), } - l := clients.ListOpts{} - var output resources.ResourceList - var err error - runtime := b.Time("runtime", func() { - output, err = client.List("ns", l) + generic.TestCrudClient("ns1", "ns2", client, clients.WatchOpts{ + Selector: selector, + Ctx: context.TODO(), + RefreshRate: time.Minute, }) + }) + It("should return pointer to internal object", func() { + obj := &v1.MockResource{ + Metadata: &core.Metadata{ + Namespace: "ns", + Name: "n", + }, + Data: "test", + } + client.Write(obj, clients.WriteOpts{}) + ret, err := client.Read("ns", "n", clients.ReadOpts{}) Expect(err).NotTo(HaveOccurred()) - Expect(output).To(HaveLen(numobjs)) - Expect(output[0].GetMetadata().Name).To(Equal("n-1")) + Expect(ret).To(BeIdenticalTo(obj)) - Expect(runtime.Seconds()).Should(BeNumerically("<", 0.5), "List() shouldn't take too long.") - }, 10) + ret2, err := client.Read("ns", "n", clients.ReadOpts{}) + Expect(err).NotTo(HaveOccurred()) + Expect(ret).To(BeIdenticalTo(ret2)) - }) + listret, err := client.List("ns", clients.ListOpts{}) + Expect(err).NotTo(HaveOccurred()) + Expect(listret[0]).To(BeIdenticalTo(obj)) + listret2, err := client.List("ns", clients.ListOpts{}) + Expect(err).NotTo(HaveOccurred()) + Expect(listret[0]).To(BeIdenticalTo(listret2[0])) + }) + }) }) diff --git a/test/tests/generic/test_crud_client.go b/test/tests/generic/test_crud_client.go index 00619e9d1..d8b6c6ee0 100644 --- a/test/tests/generic/test_crud_client.go +++ b/test/tests/generic/test_crud_client.go @@ -44,12 +44,12 @@ func TestCrudClient(namespace1, namespace2 string, client ResourceClient, opts c ExpectWithOffset(testOffset, err).NotTo(HaveOccurred()) ExpectWithOffset(testOffset, list).To(BeEmpty()) - r1, err := client.Write(input, clients.WriteOpts{}) + r1, err := client.Write(resources.Clone(input), clients.WriteOpts{}) ExpectWithOffset(testOffset, err).NotTo(HaveOccurred()) postWrite(callbacks, r1) _, err = client.Write(input, clients.WriteOpts{}) - ExpectWithOffset(testOffset, err).To(HaveOccurred()) + ExpectWithOffset(testOffset, err).To(HaveOccurred(), "Allowed overwriting resource without OverwriteExisting") ExpectWithOffset(testOffset, errors.IsExist(err)).To(BeTrue()) ExpectWithOffset(testOffset, r1).To(BeAssignableToTypeOf(&v1.MockResource{})) @@ -62,7 +62,7 @@ func TestCrudClient(namespace1, namespace2 string, client ResourceClient, opts c _, err = client.Write(input, clients.WriteOpts{ OverwriteExisting: true, }) - ExpectWithOffset(testOffset, err).To(HaveOccurred()) + ExpectWithOffset(testOffset, err).To(HaveOccurred(), "Wrote resource without changed version") resources.UpdateMetadata(input, func(meta *core.Metadata) { meta.ResourceVersion = r1.GetMetadata().ResourceVersion @@ -99,6 +99,7 @@ func TestCrudClient(namespace1, namespace2 string, client ResourceClient, opts c }, } r2, err := client.Write(input, clients.WriteOpts{}) + r2 = resources.Clone(r2) ExpectWithOffset(testOffset, err).NotTo(HaveOccurred()) // with labels @@ -125,7 +126,7 @@ func TestCrudClient(namespace1, namespace2 string, client ResourceClient, opts c meta.ResourceVersion = "" }) _, err = client.Write(r2, clients.WriteOpts{OverwriteExisting: true}) - ExpectWithOffset(testOffset, err).To(HaveOccurred()) + ExpectWithOffset(testOffset, err).To(HaveOccurred(), "Did not get version error") err = client.Delete(namespace1, "adsfw", clients.DeleteOpts{}) ExpectWithOffset(testOffset, err).To(HaveOccurred())