Skip to content
This repository has been archived by the owner on Dec 4, 2023. It is now read-only.

*DRAFT* Initial proposal of Kafka address space #2710

Closed
wants to merge 1 commit into from
Closed

Conversation

lulf
Copy link
Member

@lulf lulf commented Apr 24, 2019

No description provided.

@lulf lulf changed the title Initial proposal of Kafka address space *DRAFT* Initial proposal of Kafka address space Apr 24, 2019
----

Creating this resource will not create a Kafka cluster, but it will prepare the `address-space-controller` with the configuration needed to create one when a tenant creates an `AddressSpace` of the `kafka` type. Moreover, settings that are applied on a per-address-space basis will be ignored and set based on the `AddressSpace` definition instead. The following settings are ignored/overridden:

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This mean that all Kafka address spaces share the same Kakfa cluster, right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, that will come at a later point. I will try to clarify this in the doc, but in the same way we don't create the router and broker when you create the *InfraConfig, we would not create the Kafka cluster when you create the KafkaConfig. The cluster will be created when the AddressSpace is created.

I think longer term, sharing a cluster would be ideal, but it requires modifications to Kafka itself or some proxy in the middle to make it transparent.

@k-wall
Copy link
Member

k-wall commented Apr 30, 2019

How would this proposal live alongside #2306 (bridging)

@lulf
Copy link
Member Author

lulf commented Apr 30, 2019

@k-wall In the first version I think bridging should not be supported. However, we would need to support it at some point, and I imagine it would be in a similar form as for the brokered space, by deploying an amqp-kafka bridge.

spec:
type: kafka
plan: kafka-small
endpoints:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would that include an AMQP 1.0 endpoint?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the first iteration I think AMQP 1.0 is out of scope. I would like to see this be part of the next iteration, and this design doc should updated when preparing for that.

spec:
type: topic
plan: kafka-topic-small
properties:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I don't like with the "bag of properties" concept here, is that you no real way to validate that. I would suggest to follow the pattern of Kubernetes instead: create dedicated types for the different choices: see https://github.com/kubernetes/api/blob/master/core/v1/types.go#L49

Copy link
Member Author

@lulf lulf Apr 30, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ctron, I’m not sure I understand the reasoning behind distinct types leading to better validation.

The proposal is not describing a “bag of properties” without a schema. Each field within the properties object may have different and possibly nested types.

This should provide the same level of validation of each property as you would from having separate objects for different address types.

Moreover, any such object would need to allow any property to be omitted and have a default in order to simplify configuration. In addition, the implementation would need to allow unknown properties in order to handle forward and backward compatibility.

With the above requirements I think either direction will give the same benefits.

A challenge in having separate fields would be that we have the same types (I.e topic) defines in multiple address space types, so i don’t think distinct properties fields for each type makes sense.

For instance, if I understand you correctly, you want to have something like this:

kafkaTopicProperties:
  partitions: 2
  replicas: 3
  retention.ms: 7200000
brokeredTopicProperties:
  retention.ms: 7200000
standardQueueProperties:
  partitions: 2

I think we would end up with a much more complex schema, without any validation benefits of having distinct fields for the reasons I've stated above.

I can update the proposal to clarify this bit though. I think we would in any case benefit make sure that if a property is allowed for different address types in different address space types, they should mean the same thing. We could for instance prefix the properties that are specific to the address space type, i.e. kafka.partitions to ensure they are distinct from broker.partitions in the standard address space.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I think the conclusion is to use the properties field name, but provide OpenAPI spec to ensure that only a valid set of properties are specified together. I will update this proposal to reflect that, does that sound ok @ctron ?

@lulf
Copy link
Member Author

lulf commented Oct 2, 2019

@k-wall Do you want to use this document as a basis for the 'new' design or should I just close this PR?

@k-wall
Copy link
Member

k-wall commented Oct 7, 2019

We should keep the branch so that we can reuse your work later. It'd be nice to close the PR to keep the PR queue short/clean.

@lulf lulf closed this Oct 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants