lsst-sqre / strimzi-registry-operator

A Kubernetes Operator for running the Confluent Schema Registry with a Strimzi-based Kafka cluster
MIT License
79 stars 16 forks source link

DM-35597: Improve handling of both v1beta1 and v1beta2 Strimzi APIs #53

Closed jonathansick closed 2 years ago

jonathansick commented 2 years ago

This PR attempts to clarify the listener type and name terminology, and indicate to the user that we're interested in listener names for the modern Strimzi usage.

How the Strimzi listener specification has changed

In old versions of Strimzi, three types of listeners were available: plain, tls, and external.

apiVersion: kafka.strimzi.io/v1beta1
kind: Kafka
spec:
  kafka:
    # ...
    listeners:
      plain: {}
      tls:
        authentication:
          type: "tls"
      external: {}

strimzi-registry-operator uses the status.listeners field on the Kafka resource to resolve the bootstrap server. In older versions of Strimzi, this looked like:

apiVersion: kafka.strimzi.io/v1beta1
kind: Kafka
status:
  listeners:
    - addresses:
      - host events-kafka-bootstrap.events.svc
        port: 9093
    type: tls

Again, all listeners had types in the original v1beta1 Strimzi API.

When the Strimzi v1beta2 API came out, the listener schema changed significantly. Now spec.kafka.listeners became an array type, and you could name a listener, in addition to giving it a type. An example from the Strimzi 0.29.0 docs:

apiVersion: kafka.strimzi.io/v1beta2
kind: Kafka
metadata:
  name: my-cluster
spec:
  kafka:
    #...
    listeners:
      - name: plain
        port: 9092
        type: internal
        tls: false
      - name: tls
        port: 9093
        type: internal
        tls: true
        authentication:
          type: tls
      - name: external1
        port: 9094
        type: route
        tls: true
      - name: external2
        port: 9095
        type: ingress
        tls: true
        authentication:
          type: tls
        configuration:
          bootstrap:
            host: bootstrap.myingress.com
          brokers:
          - broker: 0
            host: broker-0.myingress.com
          - broker: 1
            host: broker-1.myingress.com
          - broker: 2
            host: broker-2.myingress.com
    #...

So whereas a listeners type (tls, plain, or external) was what we expected a StrimziSchemaRegistry operator user to configure in Strimzi v1beta1, in v1beta2 we expect the user to configure the listener's name.

To make things slightly confusing, in v1beta2, type came to be used for a slightly different purpose. Types can be internal, route, loadbalancer, nodeport, or ingress. There's no longer a plain or tls type because those are authentication details configured separately.

Further, in initial versions of Strimzi using the v1beta2 API, the status.listeners field now listed listeners and only their type, but not their name:

apiVersion: kafka.strimzi.io/v1beta2
kind: Kafka
status:
  listeners:
  - addresses:
    - host: alert-broker-kafka-bootstrap.strimzi.svc
      port: 9092
    bootstrapServers: alert-broker-kafka-bootstrap.strimzi.svc:9092
    certificates:
    - "..."
    type: internal
  - addresses:
    - host: 10.106.209.159
      port: 9094
    bootstrapServers: 10.106.209.159:9094
    certificates:
    - "..."
    type: external

To deal with this listener information, Strimzi Registry Operator needs to look at spec.listeners and find the type associated with the named listener.

More recently, Strimzi has started to include the listener name in status.listeners, which is ideal:

apiVersion: kafka.strimzi.io/v1beta2
kind: Kafka
status:
  listeners:
    - addresses:
        - host: sasquatch-kafka-bootstrap.sasquatch.svc
          port: 9092
      bootstrapServers: 'sasquatch-kafka-bootstrap.sasquatch.svc:9092'
      name: plain
      type: plain
    - addresses:
        - host: sasquatch-kafka-bootstrap.sasquatch.svc
          port: 9093
      bootstrapServers: 'sasquatch-kafka-bootstrap.sasquatch.svc:9093'
      certificates:
        - ""
      name: tls
      type: tls
jonathansick commented 2 years ago

@afausti Could you take a look at this and see if it makes sense from your perspective? If you could also try it in a test environment with the tickets-DM-35597 image tag that'd be great.

I'm going to see if I can set up minikube or similar to do an integration test of the operator in GitHub Actions.

jonathansick commented 2 years ago

This is all working well in Minikube in #57 . I've also got more changes stacked up in #59 and #60 . My plan is to merge all these and then create a 0.5.0rc1 release.