Skip to content

Commit 7c5d14b

Browse files
authored
Handling min.insync.replicas deletion operation when possible based on ELR version (#11857)
Signed-off-by: Paolo Patierno <ppatierno@live.com>
1 parent 8024f74 commit 7c5d14b

File tree

5 files changed

+77
-26
lines changed

5 files changed

+77
-26
lines changed

cluster-operator/src/main/java/io/strimzi/operator/cluster/operator/resource/KafkaBrokerConfigurationDiff.java

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ public class KafkaBrokerConfigurationDiff extends AbstractJsonDiff {
4242
private final Reconciliation reconciliation;
4343
private final Collection<AlterConfigOp> brokerConfigDiff;
4444
private final Map<String, ConfigModel> configModel;
45+
private final short elrVersion;
4546

4647
/**
4748
* These options are skipped because they contain placeholders
@@ -73,9 +74,11 @@ public class KafkaBrokerConfigurationDiff extends AbstractJsonDiff {
7374
* @param desired Desired configuration
7475
* @param kafkaVersion Kafka version
7576
* @param brokerNodeRef Broker node reference
77+
* @param elrVersion If Eligible Leader Replicas (ELR) is enabled on the Kafka cluster
7678
*/
77-
protected KafkaBrokerConfigurationDiff(Reconciliation reconciliation, Config brokerConfigs, String desired, KafkaVersion kafkaVersion, NodeRef brokerNodeRef) {
79+
protected KafkaBrokerConfigurationDiff(Reconciliation reconciliation, Config brokerConfigs, String desired, KafkaVersion kafkaVersion, NodeRef brokerNodeRef, short elrVersion) {
7880
this.reconciliation = reconciliation;
81+
this.elrVersion = elrVersion;
7982
this.configModel = KafkaConfiguration.readConfigModel(kafkaVersion);
8083
this.brokerConfigDiff = diff(brokerNodeRef, desired, brokerConfigs, configModel);
8184
}
@@ -215,7 +218,9 @@ private void removeProperty(Map<String, ConfigModel> configModel, Collection<Alt
215218
} else {
216219
// entry is in current, is not in desired, is not default -> it was using non-default value and was removed
217220
// if the entry was custom, it should be deleted
218-
if (!isIgnorableProperty(pathValueWithoutSlash, nodeIsController)) {
221+
if ("min.insync.replicas".equals(entry.name()) && elrVersion >= 1) {
222+
LOGGER.traceCr(reconciliation, "min.insync.replicas cannot be deleted when ELR is enabled");
223+
} else if (!isIgnorableProperty(pathValueWithoutSlash, nodeIsController)) {
219224
updatedCE.add(new AlterConfigOp(new ConfigEntry(pathValueWithoutSlash, null), AlterConfigOp.OpType.DELETE));
220225
LOGGER.infoCr(reconciliation, "{} not set in desired, unsetting back to default {}", entry.name(), "deleted entry");
221226
} else {

cluster-operator/src/main/java/io/strimzi/operator/cluster/operator/resource/KafkaRoller.java

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import org.apache.kafka.clients.admin.AlterConfigOp;
3535
import org.apache.kafka.clients.admin.AlterConfigsResult;
3636
import org.apache.kafka.clients.admin.Config;
37+
import org.apache.kafka.clients.admin.FeatureMetadata;
3738
import org.apache.kafka.common.KafkaException;
3839
import org.apache.kafka.common.KafkaFuture;
3940
import org.apache.kafka.common.config.ConfigException;
@@ -623,8 +624,12 @@ private void checkIfRestartOrReconfigureRequired(NodeRef nodeRef, boolean isCont
623624
// Always get the broker config. This request gets sent to that specific broker, so it's a proof that we can
624625
// connect to the broker and that it's capable of responding.
625626
Config brokerConfig;
627+
short elrVersion = 0;
626628
try {
627629
brokerConfig = brokerConfig(nodeRef);
630+
FeatureMetadata featureMetadata = featureMetadata();
631+
elrVersion = featureMetadata.finalizedFeatures().get("eligible.leader.replicas.version") == null ?
632+
0 : featureMetadata.finalizedFeatures().get("eligible.leader.replicas.version").maxVersionLevel();
628633
} catch (ForceableProblem e) {
629634
if (restartContext.backOff.done()) {
630635
needsRestart = true;
@@ -636,7 +641,7 @@ private void checkIfRestartOrReconfigureRequired(NodeRef nodeRef, boolean isCont
636641

637642
if (!needsRestart && allowReconfiguration) {
638643
LOGGER.traceCr(reconciliation, "Pod {}: description {}", nodeRef, brokerConfig);
639-
brokerConfigDiff = new KafkaBrokerConfigurationDiff(reconciliation, brokerConfig, kafkaConfigProvider.apply(nodeRef.nodeId()), kafkaVersion, nodeRef);
644+
brokerConfigDiff = new KafkaBrokerConfigurationDiff(reconciliation, brokerConfig, kafkaConfigProvider.apply(nodeRef.nodeId()), kafkaVersion, nodeRef, elrVersion);
640645

641646
if (brokerConfigDiff.getDiffSize() > 0) {
642647
if (brokerConfigDiff.canBeUpdatedDynamically()) {
@@ -657,6 +662,17 @@ private void checkIfRestartOrReconfigureRequired(NodeRef nodeRef, boolean isCont
657662
restartContext.brokerConfigDiff = brokerConfigDiff;
658663
}
659664

665+
/**
666+
* Returns the information about the features within the Kafka Cluster
667+
* @return information about the features
668+
*/
669+
/* test */ FeatureMetadata featureMetadata() throws ForceableProblem, InterruptedException {
670+
return await(VertxUtil.kafkaFutureToVertxFuture(reconciliation, vertx, brokerAdminClient.describeFeatures().featureMetadata()),
671+
30, TimeUnit.SECONDS,
672+
error -> new ForceableProblem("Error getting feature metadata", error)
673+
);
674+
}
675+
660676
/**
661677
* Returns a config of the given broker.
662678
* @param nodeRef The reference of the broker.

0 commit comments

Comments
 (0)