Unspecified `min.insync.replicas` in `Kafka.spac.kafka.config` leads to a never-ending rolling update

[scholzj]

When a Kafka cluster is created without configuring the min.insync.replicas in .spec.kafka.config, it seems to end in a never-ending rolling update, where the brokers are rolled again and again. This seems to be related to the ELR changes.

2025-09-10 16:14:36 INFO  KafkaBrokerConfigurationDiff:220 - Reconciliation #112(watch) Kafka(myproject/my-cluster): min.insync.replicas not set in desired, unsetting back to default deleted entry
2025-09-10 16:14:36 ERROR KafkaRoller:706 - Reconciliation #112(watch) Kafka(myproject/my-cluster): Error updating cluster-wide configuration
org.apache.kafka.common.errors.InvalidConfigurationException: Cluster-level min.insync.replicas cannot be removed while ELR is enabled.
2025-09-10 16:14:36 INFO  KafkaRoller:449 - Reconciliation #112(watch) Kafka(myproject/my-cluster): Rolling Pod my-cluster-bodymoor-3000/3000 due to []
2025-09-10 16:14:37 INFO  PodOperator:54 - Reconciliation #112(watch) Kafka(myproject/my-cluster): Rolling pod my-cluster-bodymoor-3000

This is likely a blocker for 0.48.0 release.

[ppatierno]

This is the reason why it's happening.

2025-09-10 17:26:53 INFO  KafkaBrokerConfigurationDiff:220 - Reconciliation #66(watch) Kafka(myproject/my-cluster): min.insync.replicas not set in desired, unsetting back to default deleted entry
2025-09-10 17:26:53 INFO  KafkaRoller:694 - Reconciliation #66(watch) Kafka(myproject/my-cluster): Updating cluster wide configuration with {ConfigResource(type=BROKER, name='')=[AlterConfigOp{opType=DELETE, configEntry=ConfigEntry(name=min.insync.replicas, value=null, source=UNKNOWN, isSensitive=false, isReadOnly=false, synonyms=[], type=UNKNOWN, documentation=null)}]}
2025-09-10 17:26:53 INFO  KafkaRoller:695 - Reconciliation #66(watch) Kafka(myproject/my-cluster): Updating broker configuration my-cluster-broker-1/1
2025-09-10 17:26:53 INFO  KafkaRoller:696 - Reconciliation #66(watch) Kafka(myproject/my-cluster): Updating broker configuration my-cluster-broker-1/1 with {ConfigResource(type=BROKER, name='1')=[]}
2025-09-10 17:26:53 ERROR KafkaRoller:706 - Reconciliation #66(watch) Kafka(myproject/my-cluster): Error updating cluster-wide configuration
org.apache.kafka.common.errors.InvalidConfigurationException: Cluster-level min.insync.replicas cannot be removed while ELR is enabled.
2025-09-10 17:26:53 INFO  KafkaRoller:449 - Reconciliation #66(watch) Kafka(myproject/my-cluster): Rolling Pod my-cluster-broker-1/1 due to []
2025-09-10 17:26:53 INFO  PodOperator:54 - Reconciliation #66(watch) Kafka(myproject/my-cluster): Rolling pod my-cluster-broker-1

A missing min.insync.replicas is determining an alter configuration with a DELETE operation for it, and it's not allowed.
It comes from the KafkaBrokerConfigurationDiff. When reading the current config for the broker, the Kafka Admin API seems to return a value for min.insync.replicas which is set by default by Kafka (when not set on creation by the user) but of course it's not present in the provided configuration.
Then of course, because of the failed update, the KafkaRoller return a ForceableProblem which leads to a rolling.

[ppatierno]

This is the entry representing min.insync.replicas when we get the broker config from the Kafka Admin API in the operator:

ConfigEntry(name=min.insync.replicas, value=1, source=DYNAMIC_DEFAULT_BROKER_CONFIG, isSensitive=false, isReadOnly=false, synonyms=[], type=INT, documentation=null)

I was wondering if it's valid that for all the DYNAMIC_DEFAULT_BROKER_CONFIG entries we should not issue a DELETE operation if it's not in the desired config.

Within Kafka it's definition is like ...

DYNAMIC_DEFAULT_BROKER_CONFIG,  // dynamic broker config that is configured as default for all brokers in the cluster

[ppatierno]

I had an offline chat with @showuon. The Kafka documentation states that The removal of min.insync.replicas config at the cluster-level is not allowed. (see https://kafka.apache.org/documentation/#eligible_leader_replicas_tool) so it means that the operator can't issue a DELETE operation as it does for other missing config parameters.
The only solution seems to be to handle min.insync.replicas as a special config and not issuing a DELETE operation for it.

[ppatierno]

Looking at the KafkaBrokerConfigurationDiff.removeProperty() method we are actually already skipping properties which are in current configuration but not desired (so not under the .spec.kafka.config) and they are using default values https://github.com/strimzi/strimzi-kafka-operator/blob/main/cluster-operator/src/main/java/io/strimzi/operator/cluster/operator/resource/KafkaBrokerConfigurationDiff.java#L209

The problem is that currently Kafka implementation of the isDefault() https://github.com/apache/kafka/blob/trunk/clients/src/main/java/org/apache/kafka/clients/admin/ConfigEntry.java#L103 is like this:

public boolean isDefault() {
        return source == ConfigSource.DEFAULT_CONFIG;
    }

So it's actually ignoring that a default source could even be DYNAMIC_DEFAULT_BROKER_CONFIG as it happens for min.insync.replicas. The current fix would be to add a new condition in the operator like this:

} else if (entry.isDefault() || ConfigEntry.ConfigSource.DYNAMIC_DEFAULT_BROKER_CONFIG.equals(entry.source())) {
... // DON'T ISSUE A DELETE OPERATION
}

@showuon is there any reason why the ConfigEntry.isDefault() doesn't take DYNAMIC_DEFAULT_BROKER_CONFIG into account?

@tinaselenge as KafkaRoller expert wdyt from an operator perspective?

[tinaselenge]

@ppatierno , I think what you suggested make sense to me. My understanding is that our intention is not to change anything if configuration is not specified in the Kafka CR and config is set to default.

Maybe there should be a method like ConfigEntry.isDynamicDefault() in the upstream Kafka.

[ppatierno]

Maybe there should be a method like ConfigEntry.isDynamicDefault() in the upstream Kafka.

That's why I asked @showuon if there is any reason DYNAMIC_DEFAULT_BROKER_CONFIG isn't taken into account. If no specific reason I can contribute to it by adding to the isDefault or with a new isDynamicDefault as per your suggestion.

[showuon]

I feel the isDefault is more like: Has this config been explicitly set, even if it has the same value.
So maybe you can add one more isDefaultValue method to it.

[ppatierno]

So maybe you can add one more isDefaultValue method to it.

During my testing, even if you start without min.insync.replicas (so it uses default 1), then you change to 2, the corresponding ConfigEntry has always DYNAMIC_DEFAULT_BROKER_CONFIG with value 2, so it doesn't look to me it's saying "it's using default value". It's meaning is a little bit cryptic tbh. So I am not sure a new isDefaultValue checking DYNAMIC_DEFAULT_BROKER_CONFIG makes much sense. Or did I misunderstand what you meant?