[EnMasse] REST API changes continued...

[Ulf Lilleengen]

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