[EnMasse] REST API changes continued...

Thu May 17 09:48:28 UTC 2018

Hi,

regarding a), it looks like the same size limitations as for ConfigMaps (1MB, see [1]) also apply here, since the limitation is in etcd ([2]).
An address space CRD with 1MB size limit would only allow for roughly 4k addresses at the most.

This would result in having to use API server aggregation with a different persistence layer in order to support more addresses per address space.

Best regards,
Carsten

[1] https://github.com/kubernetes/kubernetes/issues/19781
[2] https://coreos.com/etcd/docs/latest/dev-guide/limit.html#request-size-limit

> Hi,
> 
> After some discussions with Rob and Gordon yesterday, I think there is an
> agreement that we can go with (2) and leave (1) on the drawing table.
> 
> For completeness, however, I'd like to hear your opinions on an alternative
> approach for (1), which I think would give a reasonable compromise in the strimzi
> integration, and remove at least 2447 lines of java code (and possible even more
> since the api server does not support watches etc. at present).
> 
> If we define a custom resource definition for the address space definition with
> inlined addresses, we could remove the api-server alltogether. The reason I think
> this is more attractive is that there are a lot of features provided out-of-the-box like
> watches, json-patch, merge-patch, builtin authn and authz, finalizers etc...(see
> "Common Features" in [1])
> 
> For the strimzi case, the address-space-controller would have to create/delete the
> strimzi configmaps for the topics based on the address space definition.
> 
> For brokered and standard, it could temporarily write the address configmap until
> agent/console/standard-controller is using the address space crd.
> 
> Some questions:
> 
> a) how well does a CRD perform? Is it ok to have 100k addresses in a single CRD?
> b) it is ok to loose the ability to choose the underlying persistence for the config?
> c) can we live without the ability to delete single addresses using json patch?
> d) for strimzi, is it important that we reflect changes to the strimzi configmaps back
> into the address space definition?
> 
> The reason I like this approach is that we can remove a component from our
> system in addition to the reduced complexity in agent. So to me, the
> disadvantages of not being able to delete a single address using json patch is
> outweighed by the huge reduction in complexity.
> 
> [1]
> https://kubernetes.io/docs/concepts/api-extension/custom-
> resources/#customresourcedefinitions
> 
> Best regards,
> 
> Ulf
> 
> On 05/15/2018 12:03 PM, Ulf Lilleengen wrote:
> > Hi all,
> >
> > The initial work on the REST API to enable CRD support for address
> > spaces will soon be merged.
> >
> > I'd like to continue the discussion around addresses and CRDs. The
> > goals (I think) are to "Allow users to create/get/update/delete
> > addresses using oc or kubectl".
> >
> > So far, there have been 2 proposals from the community, not mutually
> > exclusive. I have most comments around the first proposal though.
> >
> > 1) Add address definitions to the address space definition. I'll call
> > this 'inlined addresses'. One example of that is:
> >
> > apiVersion: enmasse.io/v1
> > kind: AddressSpace
> > metadata:
> >     name: myspace
> > spec:
> >     type: standard
> >     plan: small-standard
> >     addresses:
> >       - address: myqueue
> >         type: queue
> >         plan: small-queue
> >       - ...
> > status:
> >     isReady: true
> >     addresses:
> >       - address: myqueue
> >         phase: Configuring
> >         isReady: false
> >         messages:
> >           - "Autolink not present on router"
> >           - "Address not present on router"
> >       - ...
> >
> > I have a few concerns with our current implementations ability to
> > support both per-address APIs and inlined addresses.
> >
> > Today, concurrent modifications to addresses and address spaces are
> > handled by using a resourceVersion (which reflects the resource
> > version in the configmap) in the metadata of the resource (in the same
> > way as other k8s resources). If 2 clients update a resource at the
> > same time, one of them will fail and should retry.
> >
> > If we expose both the per-address REST API and alternative (2) to
> > modify individual addresses, _and_ allowing the same addresses to be
> > changed through the address space definition, how do we prevent one
> > client to overwrite the changes made by another?
> >
> > Moreover, if a client wants to watch an address space for changes, if
> > we continue to use 1 configmap per address, then the api server will
> > have to handle 1 watch for the address space and 1 for the addresses
> > which complicates the implementation (although should be doable).
> >
> > To me, the easiest solution to the above questions is changing the
> > implementation to store everything in the same configmap, and then use
> > the resourceVersion of the address space configmap in the per-address
> > resource definition exposed in the REST API and in (2). I think this
> > would work for the standard address space type (and brokered address
> > space type as long as we create addresses 'via' the configmap and not
> > directly in the broker).
> >
> > However, integrating with strimzi will be harder the way strimzi works
> > today. For instance, the logical way to integrate with strimzi would
> > be to have the REST API proxy requests to the Strimzi API. I.e. when
> > creating an address for the 'kafka' address space, it would (until
> > strimzi supports custom resources) create a configmap per address that
> > the strimzi topic-controller would use to configure kafka. The same
> > issues as raised above with the combination of 'inline' addresses with
> > per-address configmaps would be the same.
> >
> > Inlined addresses seems to set some limitations on the granularity of
> > how we persist the data and also requires the controllers for
> > different address spaces to use the enmasse API for reading address
> information.
> > Storing all address information in a single object feels wrong to me,
> > and I'm a little worried about how this will scale.
> >
> > If the goals is that the user should be able to create addresses
> > declaratively, I think that is satisfied with (2). However, if the
> > majority still thinks the address inlining is the way forward, and we
> > can find a good way to integrate with strimzi, I won't stand in the
> > way for it.
> >
> >
> >
> > 2) Introduce a top-level API for modifying addresses (for CRDs)
> >
> > The idea is to support the following path for
> > creating/reading/updating/deleting addresses:
> > /apis/enmasse.io/v1/namespaces/[:namespace]/addresses
> >
> > To ensure that addresses from different address spaces do not
> > conflict, the proposal is to prefix the address name with the address
> > space name and introduce a separator character which cannot be part of
> > the address space name.
> >
> > One proposal is to use the '.' as the separator. This would cause the
> > address definition that is created to be:
> >
> > apiVersion: enmasse.io/v1
> > kind: Address
> > metadata:
> >      name: myspace.addr1
> > spec:
> >      type: queue
> >      plan: small-queue
> >      address: addr_1
> >
> > When using the command line tool, the address would show up as:
> >
> > $ oc get addresses
> > NAME               AGE
> > myspace.addr1      12s
> > myspace.addr2      11s
> > otherspace.addr1   6s
> >
> > Should we remove the currently required addressSpace field from
> > metadata as in the above example?
> >
> > Other concerns? I think this proposal can be quickly implemented as is.
> >
> >
> > Best regards,
> >
> > Ulf
> >
> 
> --
> Ulf
> 
> _______________________________________________
> enmasse mailing list
> enmasse at redhat.com
> https://www.redhat.com/mailman/listinfo/enmasse