[Bug]: cruiseControl.config.sample.store.topic.replication.factor seem to have no effect

jbnjohnathan commented 11 months ago

Bug Description

When creating a kafka cluster using Strimzi there are two cruise control topics created. I want them to have an initial replication factor of 3.

NAME                                                                                               CLUSTER   PARTITIONS   REPLICATION FACTOR   READY
strimzi.cruisecontrol.modeltrainingsamples                                 kafka                   32                          2                           True
strimzi.cruisecontrol.partitionmetricsamples                                kafka                   32                         2                           True

I have set the value cruiseControl.config.sample.store.topic.replication.factor: 3 in the kafka cr manifest. Then I create a new kafka cluster using strimzi. However the replication factor is still set to 2.

According to https://github.com/linkedin/cruise-control/wiki/Configurations#kafkasamplestore-configurations the KafkaSampleStore has a property with name sample.store.topic.replication.factor with a default value of 2.

Since there is another issue indicating that some cruseControl settings live under kafka.config I also tried to set it here

kafka:
  config:
    cruise.control.sample.store.topic.replication.factor: 3

But still the two topics are created with a replication factor of 2

Steps to reproduce

Deploy strimzi 0.35.1
Deploy the kafka CR from "Configuration files and logs" below
Check the replication factor with "kubectl get kafkatopic" for topic
- strimzi.cruisecontrol.modeltrainingsamples
- strimzi.cruisecontrol.partitionmetricsamples

Expected behavior

Expected the replication factor for the two topics to be 3 but it is 2

Strimzi version

0.35.1

Kubernetes version

1.25

Installation method

Helm

Infrastructure

OpenShift on premise

Configuration files and logs

spec:
  clientsCa:
    generateCertificateAuthority: true
    renewalDays: 30
    validityDays: 730
  clusterCa:
    generateCertificateAuthority: true
    renewalDays: 30
    validityDays: 1460
  cruiseControl:
    brokerCapacity:
      inboundNetwork: 10000KB/s
      outboundNetwork: 10000KB/s
    config:
      cpu.balance.threshold: 1.1
      cpu.capacity.threshold: 0.7
      cpu.low.utilization.threshold: 0
      default.goals: com.linkedin.kafka.cruisecontrol.analyzer.goals.NetworkInboundCapacityGoal,
        com.linkedin.kafka.cruisecontrol.analyzer.goals.NetworkOutboundCapacityGoal,
        com.linkedin.kafka.cruisecontrol.analyzer.goals.CpuCapacityGoal, com.linkedin.kafka.cruisecontrol.analyzer.goals.MinTopicLeadersPerBrokerGoal,
        com.linkedin.kafka.cruisecontrol.analyzer.goals.ReplicaCapacityGoal, com.linkedin.kafka.cruisecontrol.analyzer.goals.DiskCapacityGoal,
        com.linkedin.kafka.cruisecontrol.analyzer.goals.ReplicaDistributionGoal, com.linkedin.kafka.cruisecontrol.analyzer.goals.PotentialNwOutGoal,
        com.linkedin.kafka.cruisecontrol.analyzer.goals.DiskUsageDistributionGoal,
        com.linkedin.kafka.cruisecontrol.analyzer.goals.NetworkInboundUsageDistributionGoal,
        com.linkedin.kafka.cruisecontrol.analyzer.goals.NetworkOutboundUsageDistributionGoal,
        com.linkedin.kafka.cruisecontrol.analyzer.goals.CpuUsageDistributionGoal,
        com.linkedin.kafka.cruisecontrol.analyzer.goals.TopicReplicaDistributionGoal,
        com.linkedin.kafka.cruisecontrol.analyzer.goals.LeaderReplicaDistributionGoal,
        com.linkedin.kafka.cruisecontrol.analyzer.goals.LeaderBytesInDistributionGoal,
        com.linkedin.kafka.cruisecontrol.analyzer.goals.PreferredLeaderElectionGoal
      disk.balance.threshold: 1.1
      disk.capacity.threshold: 0.8
      disk.low.utilization.threshold: 0
      goal.balancedness.priority.weight: 1.1
      goal.balancedness.strictness.weight: 1.5
      goal.violation.distribution.threshold.multiplier: 1
      goals: com.linkedin.kafka.cruisecontrol.analyzer.goals.NetworkInboundCapacityGoal,
        com.linkedin.kafka.cruisecontrol.analyzer.goals.NetworkOutboundCapacityGoal,
        com.linkedin.kafka.cruisecontrol.analyzer.goals.CpuCapacityGoal, com.linkedin.kafka.cruisecontrol.analyzer.goals.MinTopicLeadersPerBrokerGoal,
        com.linkedin.kafka.cruisecontrol.analyzer.goals.ReplicaCapacityGoal, com.linkedin.kafka.cruisecontrol.analyzer.goals.DiskCapacityGoal,
        com.linkedin.kafka.cruisecontrol.analyzer.goals.ReplicaDistributionGoal, com.linkedin.kafka.cruisecontrol.analyzer.goals.PotentialNwOutGoal,
        com.linkedin.kafka.cruisecontrol.analyzer.goals.DiskUsageDistributionGoal,
        com.linkedin.kafka.cruisecontrol.analyzer.goals.NetworkInboundUsageDistributionGoal,
        com.linkedin.kafka.cruisecontrol.analyzer.goals.NetworkOutboundUsageDistributionGoal,
        com.linkedin.kafka.cruisecontrol.analyzer.goals.CpuUsageDistributionGoal,
        com.linkedin.kafka.cruisecontrol.analyzer.goals.TopicReplicaDistributionGoal,
        com.linkedin.kafka.cruisecontrol.analyzer.goals.LeaderReplicaDistributionGoal,
        com.linkedin.kafka.cruisecontrol.analyzer.goals.LeaderBytesInDistributionGoal,
        com.linkedin.kafka.cruisecontrol.analyzer.goals.PreferredLeaderElectionGoal
      hard.goals: com.linkedin.kafka.cruisecontrol.analyzer.goals.NetworkInboundCapacityGoal,
        com.linkedin.kafka.cruisecontrol.analyzer.goals.NetworkOutboundCapacityGoal,
        com.linkedin.kafka.cruisecontrol.analyzer.goals.CpuCapacityGoal
      leader.replica.count.balance.threshold: 1.1
      max.replicas.per.broker: 10000
      min.topic.leaders.per.broker: 1
      num.proposal.precompute.threads: 1
      overprovisioned.max.replicas.per.broker: 1500
      overprovisioned.min.brokers: 3
      overprovisioned.min.extra.racks: 2
      proposal.expiration.ms: 900000
      sample.store.topic.replication.factor: 3
      topic.replica.count.balance.max.gap: 40
      topic.replica.count.balance.min.gap: 2
      topic.replica.count.balance.threshold: 3
    livenessProbe:
      initialDelaySeconds: 30
      timeoutSeconds: 5
    metricsConfig:
      type: jmxPrometheusExporter
      valueFrom:
        configMapKeyRef:
          key: metrics-config.yml
          name: cruise-control-metrics
    readinessProbe:
      initialDelaySeconds: 30
      timeoutSeconds: 5
    resources:
      limits:
        cpu: 250m
        memory: 512Mi
      requests:
        cpu: 250m
        memory: 512Mi
  entityOperator:
    tlsSidecar:
      resources:
        limits:
          cpu: 250m
          memory: 128Mi
        requests:
          cpu: 200m
          memory: 64Mi
    topicOperator:
      resources:
        limits:
          cpu: 250m
          memory: 512Mi
        requests:
          cpu: 250m
          memory: 512Mi
    userOperator:
      resources:
        limits:
          cpu: 250m
          memory: 512Mi
        requests:
          cpu: 250m
          memory: 512Mi
  kafka:
    authorization:
      type: simple
    config:
      auto.create.topics.enable: "false"
      cruise.control.metrics.topic.replication.factor: 3
      cruise.control.sample.store.topic.replication.factor: 3
      default.replication.factor: 3
      delete.topic.enable: "true"
      inter.broker.protocol.version: "3.4"
      min.insync.replicas: 2
      offsets.topic.replication.factor: 3
      transaction.state.log.min.isr: 2
      transaction.state.log.replication.factor: 3
    jvmOptions:
      -Xms: 256m
      -Xmx: 256m
    listeners:
    - authentication:
        type: tls
      name: tls
      port: 9093
      tls: true
      type: internal
    - authentication:
        type: tls
      configuration:
        bootstrap:
          host: XXX
        brokers:
        - broker: 0
          host: XXX
        - broker: 1
          host: kXXX
        - broker: 2
          host: XXX
      name: external
      port: 9094
      tls: true
      type: route
    livenessProbe:
      initialDelaySeconds: 15
      timeoutSeconds: 5
    metricsConfig:
      type: jmxPrometheusExporter
      valueFrom:
        configMapKeyRef:
          key: kafka-metrics-config.yml
          name: kafka-metrics
    readinessProbe:
      initialDelaySeconds: 15
      timeoutSeconds: 5
    replicas: 3
    resources:
      limits:
        cpu: 500m
        memory: 1Gi
      requests:
        cpu: 250m
        memory: 512Mi
    storage:
      class: thin-csi
      deleteClaim: true
      size: 12Gi
      type: persistent-claim
    template:
      clusterCaCert:
        metadata:
          annotations:
            contact: XXX
            kafkaCert: "True"
      pod:
        affinity:
          podAntiAffinity:
            preferredDuringSchedulingIgnoredDuringExecution:
            - podAffinityTerm:
                labelSelector:
                  matchExpressions:
                  - key: app.kubernetes.io/name
                    operator: In
                    values:
                    - kafka
                topologyKey: kubernetes.io/hostname
              weight: 100
    version: 3.4.0
  kafkaExporter:
    resources:
      limits:
        cpu: 250m
        memory: 128Mi
      requests:
        cpu: 200m
        memory: 64Mi
  zookeeper:
    jvmOptions:
      -Xms: 256m
      -Xmx: 256m
    replicas: 1
    resources:
      limits:
        cpu: "1"
        memory: 1Gi
      requests:
        cpu: 250m
        memory: 512Mi
    storage:
      class: thin-csi
      deleteClaim: true
      size: 10Gi
      type: persistent-claim
    template:
      pod:
        affinity:
          podAntiAffinity:
            preferredDuringSchedulingIgnoredDuringExecution:
            - podAffinityTerm:
                labelSelector:
                  matchExpressions:
                  - key: app.kubernetes.io/name
                    operator: In
                    values:
                    - zookeeper
                topologyKey: kubernetes.io/hostname
              weight: 100
status:
  clusterId: XXX
  conditions:
  - lastTransitionTime: "2023-10-21T09:48:38.518267170Z"
    status: "True"
    type: Ready
  listeners:
  - addresses:
    - host: XXX
      port: 9093
    bootstrapServers: XXX
    certificates:
    - |
      -----BEGIN CERTIFICATE-----
      XXX
      -----END CERTIFICATE-----
    name: tls
    type: tls
  - addresses:
    - host: XXX
      port: 443
    bootstrapServers: XXX
    certificates:
    - |
      -----BEGIN CERTIFICATE-----
      XXX
      -----END CERTIFICATE-----
    name: external
    type: external
  observedGeneration: 1

Additional context

According to https://github.com/strimzi/strimzi-kafka-operator/issues/4268 setting sample.store.topic.replication.factor under cruiseControl.config should produce the expected result.

scholzj commented 11 months ago

You should configure the _cample config you shared as code, otherwise it is unreadable.

jbnjohnathan commented 11 months ago

You should configure the _cample config you shared as code, otherwise it is unreadable.

Doh, thanks. Fixed now

scholzj commented 11 months ago

I don't think this should be in .spec.kafka.config. There should be only the configuration metrics reporter and I do not think that is provided by it.

@kyguy Any idea?

jbnjohnathan commented 11 months ago

I don't think this should be in .spec.kafka.config. There should be only the configuration metrics reporter and I do not think that is provided by it.

@kyguy Any idea?

No I didn't think so either. Just to be clear, I first tried only with spec.cruiseControl.config.sample.store.topic.replication.factor: 3 When that didn't work I also tried adding it to spec.kafka.config just to try even if I didn't think this would make any difference.

kyguy commented 11 months ago

@jbnjohnathan

spec.cruiseControl.config.sample.store.topic.replication.factor

This is the correct place. That being said, the replication factor of the topic cannot be updated after initial creation. So if you create a cluster without this config in the right place, those topics will be created with the default replication factor value of 2.

When you have existing topics:

strimzi.cruisecontrol.modeltrainingsamples
strimzi.cruisecontrol.partitionmetricsamples

and want to update their replication factor,

Set spec.cruiseControl.config.sample.store.topic.replication.factor to desired value
Delete the strimzi.cruisecontrol.modeltrainingsamples and strimzi.cruisecontrol.partitionmetricsamples topics.

Cruise Control will then create them with the value specified in spec.cruiseControl.config.sample.store.topic.replication.factor.

jbnjohnathan commented 11 months ago

@jbnjohnathan
spec.cruiseControl.config.sample.store.topic.replication.factor
This is the correct place. That being said, the replication factor of the topic cannot be updated after initial creation. So if you create a cluster without this config in the right place, those topics will be created with the default replication factor value of 2.

When you have existing topics:
strimzi.cruisecontrol.modeltrainingsamples
strimzi.cruisecontrol.partitionmetricsamples
and want to update their replication factor,

Set spec.cruiseControl.config.sample.store.topic.replication.factor to desired value

Delete the strimzi.cruisecontrol.modeltrainingsamples and strimzi.cruisecontrol.partitionmetricsamples topics.

Cruise Control will then create them with the value specified in spec.cruiseControl.config.sample.store.topic.replication.factor.

Yeah exactly. I hope it was clear in my issue description that I did set it in spec.cruiseControl.config.sample.store.topic.replication.factor and I did it before creating the cluster.

kyguy commented 11 months ago

Yeah exactly. I hope it was clear in my issue description that I did set it in spec.cruiseControl.config.sample.store.topic.replication.factor and I did it before creating the cluster.

Did a cluster exist in the same namespace where the new cluster was created? If so, were the KafkaTopic resources used by that cluster deleted explicitly as well?

Deleting a Kafka resource does not delete the KafkaTopics created by that cluster. So if the KafkaTopic resources were not deleted explicitly, the new cluster is going to use the existing KafkaTopic resources (which have the old/default replication factor configuration)

jbnjohnathan commented 11 months ago

@kyguy You were correct, even though I removed the cluster in between testing the kafkaTopic remained and this is why it never changed. Thanks!

strimzi / strimzi-kafka-operator