Skip to content

KAFKA-19719: --no-initial-controllers should not assume kraft.version=1 #20551

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jsancio merged 18 commits into from
Sep 25, 2025
Merged

KAFKA-19719: --no-initial-controllers should not assume kraft.version=1 #20551

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
dcc3668
--no-intial-controllers should not assume kraft.version=1
kevin-wu24 Sep 17, 2025
e0ad6c0
refactor kafkaclustertestkit to match semantics of format tool regard…
kevin-wu24 Sep 18, 2025
1720ad7
fix build
kevin-wu24 Sep 18, 2025
a905fc8
fix test
kevin-wu24 Sep 18, 2025
e671a39
absence of static voters config determines kraft version at format ti…
kevin-wu24 Sep 18, 2025
ae21d0e
cleanup
kevin-wu24 Sep 18, 2025
8917f9d
formatter should not check kraft.version against release version
kevin-wu24 Sep 18, 2025
9864633
cleanup
kevin-wu24 Sep 18, 2025
951b874
fixing test
kevin-wu24 Sep 18, 2025
c99a253
code review
kevin-wu24 Sep 19, 2025
c087a97
cleanup
kevin-wu24 Sep 22, 2025
8c0271b
code review and removing duplicate tests
kevin-wu24 Sep 22, 2025
cf4a00e
fix error msg
kevin-wu24 Sep 22, 2025
f8926de
remove references to setting kraft.version explicitly in docs
kevin-wu24 Sep 23, 2025
6e23ed8
cleanup storage help output
kevin-wu24 Sep 23, 2025
d1e3b65
Merge branch 'trunk' into KAFKA-19719
kevin-wu24 Sep 24, 2025
14fabc4
code review + removing KRaftVersion dependency on MV
kevin-wu24 Sep 24, 2025
989796a
fix test
kevin-wu24 Sep 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 6 additions & 9 deletions core/src/main/scala/kafka/tools/StorageTool.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,8 +149,9 @@ object StorageTool extends Logging {
})
val initialControllers = namespace.getString("initial_controllers")
val isStandalone = namespace.getBoolean("standalone")
if (!config.quorumConfig.voters().isEmpty &&
(Option(initialControllers).isDefined || isStandalone)) {
val staticVotersEmpty = config.quorumConfig.voters().isEmpty
formatter.setStaticVotersEmpty(staticVotersEmpty)
if (!staticVotersEmpty && (Option(initialControllers).isDefined || isStandalone)) {

@chia7712 chia7712 Sep 19, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I still feel it is weird that users configure controller.quorum.voters with --no-initial-controllers, but you are right that we should keep the compatibility.

Should we add another warning to remind users that --no-initial-controllers should not be used with static voters even though it is no-op?

@kevin-wu24 kevin-wu24 Sep 19, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Should we add another warning to remind users that --no-initial-controllers should not be used with static voters even though it is no-op?

I can add it to the flag's help output.

throw new TerseFailure("You cannot specify " +
QuorumConfig.QUORUM_VOTERS_CONFIG + " and format the node " +
"with --initial-controllers or --standalone. " +
Expand All @@ -163,16 +164,12 @@ object StorageTool extends Logging {
if (isStandalone) {
formatter.setInitialControllers(createStandaloneDynamicVoters(config))

@chia7712 chia7712 Sep 24, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It seems that --initial-controllers becomes a no-op when --standalone is defined. Should we add a warning for this case?

@kevin-wu24 kevin-wu24 Sep 24, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

If you look at the dynamic quorum arguments, you cannot specify multiple at the same time:

val reconfigurableQuorumOptions = formatParser.addMutuallyExclusiveGroup()

}
if (namespace.getBoolean("no_initial_controllers")) {
formatter.setNoInitialControllersFlag(true)
} else {
if (config.processRoles.contains(ProcessRole.ControllerRole)) {
if (config.quorumConfig.voters().isEmpty && formatter.initialVoters().isEmpty) {
if (!namespace.getBoolean("no_initial_controllers") &&
config.processRoles.contains(ProcessRole.ControllerRole) &&
staticVotersEmpty && formatter.initialVoters().isEmpty) {

@jsancio jsancio Sep 22, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would add a newline after && and before formatter.initialVoters().isEmpt.

throw new TerseFailure("Because " + QuorumConfig.QUORUM_VOTERS_CONFIG +
" is not set on this controller, you must specify one of the following: " +
"--standalone, --initial-controllers, or --no-initial-controllers.");
}
}
}
Option(namespace.getList("add_scram")).
foreach(scramArgs => formatter.setScramArguments(scramArgs.asInstanceOf[util.List[String]]))
Expand Down
51 changes: 34 additions & 17 deletions core/src/test/java/kafka/server/ReconfigurableQuorumIntegrationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,8 @@ public void testCreateAndDestroyReconfigurableCluster() throws Exception {
new TestKitNodes.Builder().
setNumBrokerNodes(1).
setNumControllerNodes(1).
setFeature(KRaftVersion.FEATURE_NAME, KRaftVersion.KRAFT_VERSION_1.featureLevel()).

@chia7712 chia7712 Sep 19, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

the method setFeature is useless now. Could you please remove it?

build()
).build()) {
).setStandalone(true).build()) {
cluster.format();
cluster.startup();
try (Admin admin = Admin.create(cluster.clientProperties())) {
Expand All @@ -108,13 +107,23 @@ static Map<Integer, Uuid> findVoterDirs(Admin admin) throws Exception {

@Test
public void testRemoveController() throws Exception {
try (KafkaClusterTestKit cluster = new KafkaClusterTestKit.Builder(
new TestKitNodes.Builder().
setNumBrokerNodes(1).
setNumControllerNodes(3).
setFeature(KRaftVersion.FEATURE_NAME, KRaftVersion.KRAFT_VERSION_1.featureLevel()).
build()
).build()) {
final var nodes = new TestKitNodes.Builder().
setNumBrokerNodes(1).
setNumControllerNodes(3).
build();

final Map<Integer, Uuid> initialVoters = new HashMap<>();
for (final var controllerNode : nodes.controllerNodes().values()) {
initialVoters.put(
controllerNode.id(),
controllerNode.metadataDirectoryId()
);
}

try (KafkaClusterTestKit cluster = new KafkaClusterTestKit.Builder(nodes).
setInitialVoterSet(initialVoters).
build()
) {
cluster.format();
cluster.startup();
try (Admin admin = Admin.create(cluster.clientProperties())) {
Expand All @@ -133,12 +142,22 @@ public void testRemoveController() throws Exception {

@Test
public void testRemoveAndAddSameController() throws Exception {
try (KafkaClusterTestKit cluster = new KafkaClusterTestKit.Builder(
new TestKitNodes.Builder().
setNumBrokerNodes(1).
setNumControllerNodes(4).
setFeature(KRaftVersion.FEATURE_NAME, KRaftVersion.KRAFT_VERSION_1.featureLevel()).
build()).build()
final var nodes = new TestKitNodes.Builder().
setNumBrokerNodes(1).
setNumControllerNodes(4).
build();

final Map<Integer, Uuid> initialVoters = new HashMap<>();
for (final var controllerNode : nodes.controllerNodes().values()) {
initialVoters.put(
controllerNode.id(),
controllerNode.metadataDirectoryId()
);
}

try (KafkaClusterTestKit cluster = new KafkaClusterTestKit.Builder(nodes).
setInitialVoterSet(initialVoters).
build()
) {
cluster.format();
cluster.startup();
Expand Down Expand Up @@ -173,7 +192,6 @@ public void testControllersAutoJoinStandaloneVoter() throws Exception {
final var nodes = new TestKitNodes.Builder().
setNumBrokerNodes(1).
setNumControllerNodes(3).
setFeature(KRaftVersion.FEATURE_NAME, KRaftVersion.KRAFT_VERSION_1.featureLevel()).
build();
try (KafkaClusterTestKit cluster = new KafkaClusterTestKit.Builder(nodes).
setConfigProp(QuorumConfig.QUORUM_AUTO_JOIN_ENABLE_CONFIG, true).
Expand All @@ -199,7 +217,6 @@ public void testNewVoterAutoRemovesAndAdds() throws Exception {
final var nodes = new TestKitNodes.Builder().
setNumBrokerNodes(1).
setNumControllerNodes(3).
setFeature(KRaftVersion.FEATURE_NAME, KRaftVersion.KRAFT_VERSION_1.featureLevel()).
build();

// Configure the initial voters with one voter having a different directory ID.
Expand Down
3 changes: 1 addition & 2 deletions core/src/test/scala/integration/kafka/server/KRaftClusterTest.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1035,8 +1035,7 @@ class KRaftClusterTest {
val cluster = new KafkaClusterTestKit.Builder(
new TestKitNodes.Builder().
setNumBrokerNodes(1).
setNumControllerNodes(1).
setFeature(KRaftVersion.FEATURE_NAME, 1.toShort).build()).build()
setNumControllerNodes(1).build()).setStandalone(true).build()
try {
cluster.format()
cluster.startup()
Expand Down
9 changes: 2 additions & 7 deletions core/src/test/scala/unit/kafka/tools/StorageToolTest.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -605,19 +605,14 @@ Found problem:
Seq("--release-version", "3.9-IV0"))).getMessage)
}

@ParameterizedTest
@ValueSource(booleans = Array(false, true))
def testFormatWithNoInitialControllersSucceedsOnController(setKraftVersionFeature: Boolean): Unit = {
@Test
def testFormatWithNoInitialControllersSucceedsOnController(): Unit = {
val availableDirs = Seq(TestUtils.tempDir())
val properties = new Properties()
properties.putAll(defaultDynamicQuorumProperties)
properties.setProperty("log.dirs", availableDirs.mkString(","))
val stream = new ByteArrayOutputStream()
val arguments = ListBuffer[String]("--release-version", "3.9-IV0", "--no-initial-controllers")
if (setKraftVersionFeature) {
arguments += "--feature"
arguments += "kraft.version=1"
}
assertEquals(0, runFormatCommand(stream, properties, arguments.toSeq))
assertTrue(stream.toString().
contains("Formatting metadata directory %s".format(availableDirs.head)),
Expand Down
26 changes: 13 additions & 13 deletions metadata/src/main/java/org/apache/kafka/metadata/storage/Formatter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ public class Formatter {
* The initial KIP-853 voters.
*/
private Optional<DynamicVoters> initialControllers = Optional.empty();
private boolean noInitialControllersFlag = false;
private boolean staticVotersEmpty = false;

public Formatter setPrintStream(PrintStream printStream) {
this.printStream = printStream;
Expand Down Expand Up @@ -217,8 +217,8 @@ public Formatter setInitialControllers(DynamicVoters initialControllers) {
return this;
}

public Formatter setNoInitialControllersFlag(boolean noInitialControllersFlag) {
this.noInitialControllersFlag = noInitialControllersFlag;
public Formatter setStaticVotersEmpty(boolean staticVotersEmpty) {
this.staticVotersEmpty = staticVotersEmpty;
return this;
}

Expand All @@ -227,7 +227,7 @@ public Optional<DynamicVoters> initialVoters() {
}

boolean hasDynamicQuorum() {
return initialControllers.isPresent() || noInitialControllersFlag;
return staticVotersEmpty;
}

public BootstrapMetadata bootstrapMetadata() {
Expand Down Expand Up @@ -337,7 +337,7 @@ Map<String, Short> calculateEffectiveFeatureLevels() {
/**
* Calculate the effective feature level for kraft.version. In order to keep existing
* command-line invocations of StorageTool working, we default this to 0 if no dynamic
* voter quorum arguments were provided. As a convenience, if dynamic voter quorum arguments
* voter quorum arguments were provided. As a convenience, if --standalone or --initial-voters
* were passed, we set the latest kraft.version. (Currently there is only 1 non-zero version).
*
* @param configuredKRaftVersionLevel The configured level for kraft.version
Expand All @@ -348,20 +348,20 @@ private short effectiveKRaftFeatureLevel(Optional<Short> configuredKRaftVersionL
if (configuredKRaftVersionLevel.get() == 0) {
if (hasDynamicQuorum()) {
throw new FormatterException(
"Cannot set kraft.version to " +
configuredKRaftVersionLevel.get() +
" if one of the flags --standalone, --initial-controllers, or --no-initial-controllers is used. " +
"Cannot set kraft.version to " + configuredKRaftVersionLevel.get() +

@chia7712 chia7712 Sep 24, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Would you mind using 0 instead of configuredKRaftVersionLevel.get()? for example:

                    throw new FormatterException(
                        "Cannot set kraft.version to 0 if controller.quorum.voters is empty and one of the flags --standalone, " +
                        "--initial-controllers, or --no-initial-controllers is used. For dynamic controllers support, " +
                        "try removing the --feature flag for kraft.version."
                    );

" if controller.quorum.voters is empty and one of the flags --standalone, " +
"--initial-controllers, or --no-initial-controllers is used. " +

@jsancio jsancio Sep 22, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This this true? Doesn't the tool allow --no-initial-controller when controller.quorum.voters is specified?

@kevin-wu24 kevin-wu24 Sep 22, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, this needs to be fixed.

@kevin-wu24 kevin-wu24 Sep 22, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sorry for the confusion, but the original error message should be correct. If the static voters is not specified, --no-intial-controllers is set, and kraft.version is set to 0, the formatter should fail. The message says if controller.quorum.voters is empty AND one of the KIP-853 flags is set.

If static voters is specified, --no-initial-controllers is set, and kraft.version is set to 0, the tool does not fail. The hasDynamicQuorum boolean is true when static voters is empty and false otherwise.

"For dynamic controllers support, try removing the --feature flag for kraft.version."
);
}
} else {
if (!hasDynamicQuorum()) {
throw new FormatterException(
"Cannot set kraft.version to " +
configuredKRaftVersionLevel.get() +
" unless one of the flags --standalone, --initial-controllers, or --no-initial-controllers is used. " +
"For dynamic controllers support, try using one of --standalone, --initial-controllers, or " +
"--no-initial-controllers."
"Cannot set kraft.version to " + configuredKRaftVersionLevel.get() +
" unless controller.quorum.voters is empty and one of the flags --standalone, " +
"--initial-controllers, or -no-initial-controllers is used. " +

@FrankYang0529 FrankYang0529 Sep 21, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: -no-initial-controllers -> --no-initial-controllers.

@kevin-wu24 kevin-wu24 Sep 22, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sounds good, will fix

"For dynamic controllers support, try using one of --standalone, --initial-controllers, " +
"or --no-initial-controllers and removing controller.quorum.voters."
);
}
}
Expand Down
Loading
Loading