Skip to content

Commit dbf3808

Browse files
authored
MINOR: Add test coverage for StorageTool format command feature validation (#20303)
### Summary Adds comprehensive test coverage for the StorageTool format command feature validation, including tests for valid feature overrides, invalid feature detection, and multiple feature specifications. Also adds debug output to help with troubleshooting format operations. ### Changes Made #### Test Coverage Improvements - **`testFormatWithReleaseVersionAndFeatureOverride()`**: Tests that feature overrides work correctly when specified with `--feature` flag - **`testFormatWithInvalidFeatureThrowsError()`**: Tests error handling for unsupported features - **`testFormatWithMultipleFeatures()`**: Tests multiple feature specifications in a single format command #### Debug Output Enhancement - **Formatter.java**: Added debug output to print bootstrap metadata during format operations - Helps with troubleshooting format issues by showing the complete bootstrap metadata being written - Improves visibility into what features and configurations are being applied #### Test Updates - **FormatterTest.java**: Updated existing tests to account for the new debug output\ ### Related - KIP-1022: [Formatting and Updating Features ](https://cwiki.apache.org/confluence/display/KAFKA/KIP-1022%3A+Formatting+and+Updating+Features) Reviewers: Kevin Wu <[email protected]>, Justine Olshan <[email protected]>
1 parent 9983331 commit dbf3808

File tree

3 files changed

+114
-28
lines changed

3 files changed

+114
-28
lines changed

core/src/test/scala/unit/kafka/tools/StorageToolTest.scala

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -385,6 +385,84 @@ Found problem:
385385
"Failed to find content in output: " + stream.toString())
386386
}
387387

388+
@Test
389+
def testFormatWithReleaseVersionAndFeatureOverride(): Unit = {
390+
val availableDirs = Seq(TestUtils.tempDir())
391+
val properties = new Properties()
392+
properties.putAll(defaultStaticQuorumProperties)
393+
properties.setProperty("log.dirs", availableDirs.mkString(","))
394+
val stream = new ByteArrayOutputStream()
395+
assertEquals(0, runFormatCommand(stream, properties, Seq(
396+
"--release-version", "3.7-IV0",
397+
"--feature", "share.version=1")))
398+
399+
// Verify that the feature override is applied by checking the bootstrap metadata
400+
val bootstrapMetadata = new BootstrapDirectory(availableDirs.head.toString).read
401+
402+
// Verify that the share.version feature is set to 1 as specified
403+
assertEquals(1.toShort, bootstrapMetadata.featureLevel("share.version"),
404+
"share.version should be set to 1")
405+
406+
// Verify the command output contains the expected release version
407+
assertTrue(stream.toString().contains("3.7-IV0"),
408+
"Failed to find release version in output: " + stream.toString())
409+
410+
// Verify that the format command completed successfully with features
411+
assertTrue(stream.toString().contains("Formatting metadata directory"),
412+
"Failed to find formatting message in output: " + stream.toString())
413+
}
414+
415+
@Test
416+
def testFormatWithMultipleFeatures(): Unit = {
417+
val availableDirs = Seq(TestUtils.tempDir())
418+
val properties = new Properties()
419+
properties.putAll(defaultStaticQuorumProperties)
420+
properties.setProperty("log.dirs", availableDirs.mkString(","))
421+
val stream = new ByteArrayOutputStream()
422+
assertEquals(0, runFormatCommand(stream, properties, Seq(
423+
"--release-version", "3.8-IV0",
424+
"--feature", "share.version=1",
425+
"--feature", "transaction.version=2",
426+
"--feature", "group.version=1")))
427+
428+
// Verify that all features are properly bootstrapped by checking the bootstrap metadata
429+
val bootstrapMetadata = new BootstrapDirectory(availableDirs.head.toString).read
430+
431+
// Verify that all specified features are set correctly
432+
assertEquals(1.toShort, bootstrapMetadata.featureLevel("share.version"),
433+
"share.version should be set to 1")
434+
assertEquals(2.toShort, bootstrapMetadata.featureLevel("transaction.version"),
435+
"transaction.version should be set to 2")
436+
assertEquals(1.toShort, bootstrapMetadata.featureLevel("group.version"),
437+
"group.version should be set to 1")
438+
439+
// Verify the command output contains the expected release version
440+
assertTrue(stream.toString().contains("3.8-IV0"),
441+
"Failed to find release version in output: " + stream.toString())
442+
443+
// Verify that the format command completed successfully with multiple features
444+
assertTrue(stream.toString().contains("Formatting metadata directory"),
445+
"Failed to find formatting message in output: " + stream.toString())
446+
}
447+
448+
@Test
449+
def testFormatWithInvalidFeatureThrowsError(): Unit = {
450+
val availableDirs = Seq(TestUtils.tempDir())
451+
val properties = new Properties()
452+
properties.putAll(defaultStaticQuorumProperties)
453+
properties.setProperty("log.dirs", availableDirs.mkString(","))
454+
val stream = new ByteArrayOutputStream()
455+
456+
// Test with an invalid feature that doesn't exist
457+
val exception = assertThrows(classOf[FormatterException], () => {
458+
runFormatCommand(stream, properties, Seq(
459+
"--release-version", "3.7-IV0",
460+
"--feature", "stream.version=1"))
461+
})
462+
463+
assertTrue(exception.getMessage.contains("Unsupported feature: stream.version."))
464+
}
465+
388466
@Test
389467
def testFormatWithStandaloneFlagOnBrokerFails(): Unit = {
390468
val availableDirs = Seq(TestUtils.tempDir())

metadata/src/main/java/org/apache/kafka/metadata/storage/Formatter.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -415,6 +415,7 @@ void doFormat(BootstrapMetadata bootstrapMetadata) throws Exception {
415415
if (ensemble.emptyLogDirs().isEmpty()) {
416416
printStream.println("All of the log directories are already formatted.");
417417
} else {
418+
printStream.println("Bootstrap metadata: " + bootstrapMetadata);
418419
Map<String, DirectoryType> directoryTypes = new HashMap<>();
419420
for (String emptyLogDir : ensemble.emptyLogDirs()) {
420421
DirectoryType directoryType = DirectoryType.calculate(emptyLogDir,

metadata/src/test/java/org/apache/kafka/metadata/storage/FormatterTest.java

Lines changed: 35 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ FormatterContext newFormatter() {
8989
Formatter formatter = new Formatter().
9090
setNodeId(DEFAULT_NODE_ID).
9191
setClusterId(DEFAULT_CLUSTER_ID.toString());
92-
directories.forEach(d -> formatter.addDirectory(d));
92+
directories.forEach(formatter::addDirectory);
9393
formatter.setMetadataLogDirectory(directories.get(0));
9494
return new FormatterContext(formatter);
9595
}
@@ -170,7 +170,7 @@ public void testFormatterFailsOnUnwritableDirectory() throws Exception {
170170
String expectedPrefix = "Error while writing meta.properties file";
171171
assertEquals(expectedPrefix,
172172
assertThrows(FormatterException.class,
173-
() -> formatter1.formatter.run()).
173+
formatter1.formatter::run).
174174
getMessage().substring(0, expectedPrefix.length()));
175175
}
176176
}
@@ -180,15 +180,15 @@ public void testIgnoreFormatted() throws Exception {
180180
try (TestEnv testEnv = new TestEnv(1)) {
181181
FormatterContext formatter1 = testEnv.newFormatter();
182182
formatter1.formatter.run();
183-
assertEquals("Formatting metadata directory " + testEnv.directory(0) +
184-
" with metadata.version " + MetadataVersion.latestProduction() + ".",
185-
formatter1.output().trim());
183+
assertEquals("Bootstrap metadata: " + formatter1.formatter.bootstrapMetadata() +
184+
"\nFormatting metadata directory " + testEnv.directory(0) +
185+
" with metadata.version " + MetadataVersion.latestProduction() + ".",
186+
formatter1.output().trim());
186187

187188
FormatterContext formatter2 = testEnv.newFormatter();
188189
formatter2.formatter.setIgnoreFormatted(true);
189190
formatter2.formatter.run();
190-
assertEquals("All of the log directories are already formatted.",
191-
formatter2.output().trim());
191+
assertTrue(formatter2.output().trim().contains("All of the log directories are already formatted."));
192192
}
193193
}
194194

@@ -201,7 +201,8 @@ public void testStandaloneWithIgnoreFormatted() throws Exception {
201201
formatter1.formatter
202202
.setInitialControllers(DynamicVoters.parse("1@localhost:8020:" + originalDirectoryId))
203203
.run();
204-
assertEquals("Formatting dynamic metadata voter directory " + testEnv.directory(0) +
204+
assertEquals("Bootstrap metadata: " + formatter1.formatter.bootstrapMetadata() +
205+
"\nFormatting dynamic metadata voter directory " + testEnv.directory(0) +
205206
" with metadata.version " + MetadataVersion.latestProduction() + ".",
206207
formatter1.output().trim());
207208
assertMetadataDirectoryId(testEnv, Uuid.fromString(originalDirectoryId));
@@ -244,9 +245,10 @@ public void testOneDirectoryFormattedAndOthersNotFormattedWithIgnoreFormatted()
244245
FormatterContext formatter2 = testEnv.newFormatter();
245246
formatter2.formatter.setIgnoreFormatted(true);
246247
formatter2.formatter.run();
247-
assertEquals("Formatting data directory " + testEnv.directory(1) + " with metadata.version " +
248-
MetadataVersion.latestProduction() + ".",
249-
formatter2.output().trim());
248+
assertEquals("Bootstrap metadata: " + formatter2.formatter.bootstrapMetadata() +
249+
"\nFormatting data directory " + testEnv.directory(1) + " with metadata.version " +
250+
MetadataVersion.latestProduction() + ".",
251+
formatter2.output().trim());
250252
}
251253
}
252254

@@ -256,9 +258,10 @@ public void testFormatWithOlderReleaseVersion() throws Exception {
256258
FormatterContext formatter1 = testEnv.newFormatter();
257259
formatter1.formatter.setReleaseVersion(MetadataVersion.IBP_3_5_IV0);
258260
formatter1.formatter.run();
259-
assertEquals("Formatting metadata directory " + testEnv.directory(0) +
260-
" with metadata.version " + MetadataVersion.IBP_3_5_IV0 + ".",
261-
formatter1.output().trim());
261+
assertEquals("Bootstrap metadata: " + formatter1.formatter.bootstrapMetadata() +
262+
"\nFormatting metadata directory " + testEnv.directory(0) +
263+
" with metadata.version " + MetadataVersion.IBP_3_5_IV0 + ".",
264+
formatter1.output().trim());
262265
BootstrapMetadata bootstrapMetadata =
263266
new BootstrapDirectory(testEnv.directory(0)).read();
264267
assertEquals(MetadataVersion.IBP_3_5_IV0, bootstrapMetadata.metadataVersion());
@@ -272,7 +275,7 @@ public void testFormatWithUnstableReleaseVersionFailsWithoutEnableUnstable() thr
272275
FormatterContext formatter1 = testEnv.newFormatter();
273276
formatter1.formatter.setReleaseVersion(MetadataVersion.latestTesting());
274277
assertEquals("metadata.version " + MetadataVersion.latestTesting() + " is not yet stable.",
275-
assertThrows(FormatterException.class, () -> formatter1.formatter.run()).getMessage());
278+
assertThrows(FormatterException.class, formatter1.formatter::run).getMessage());
276279
}
277280
}
278281

@@ -283,9 +286,10 @@ public void testFormatWithUnstableReleaseVersion() throws Exception {
283286
formatter1.formatter.setReleaseVersion(MetadataVersion.latestTesting());
284287
formatter1.formatter.setUnstableFeatureVersionsEnabled(true);
285288
formatter1.formatter.run();
286-
assertEquals("Formatting metadata directory " + testEnv.directory(0) +
287-
" with metadata.version " + MetadataVersion.latestTesting() + ".",
288-
formatter1.output().trim());
289+
assertEquals("Bootstrap metadata: " + formatter1.formatter.bootstrapMetadata() +
290+
"\nFormatting metadata directory " + testEnv.directory(0) +
291+
" with metadata.version " + MetadataVersion.latestTesting() + ".",
292+
formatter1.output().trim());
289293
BootstrapMetadata bootstrapMetadata =
290294
new BootstrapDirectory(testEnv.directory(0)).read();
291295
assertEquals(MetadataVersion.latestTesting(), bootstrapMetadata.metadataVersion());
@@ -318,7 +322,7 @@ public void testFormatWithScramFailsOnUnsupportedReleaseVersions() throws Except
318322
"saltedpassword=\"mT0yyUUxnlJaC99HXgRTSYlbuqa4FSGtJCJfTMvjYCE=\"]"));
319323
assertEquals("SCRAM is only supported in metadata.version 3.5-IV2 or later.",
320324
assertThrows(FormatterException.class,
321-
() -> formatter1.formatter.run()).getMessage());
325+
formatter1.formatter::run).getMessage());
322326
}
323327
}
324328

@@ -333,9 +337,10 @@ public void testFormatWithScram() throws Exception {
333337
"SCRAM-SHA-512=[name=alice,salt=\"MWx2NHBkbnc0ZndxN25vdGN4bTB5eTFrN3E=\"," +
334338
"saltedpassword=\"mT0yyUUxnlJaC99HXgRTSYlbuqa4FSGtJCJfTMvjYCE=\"]"));
335339
formatter1.formatter.run();
336-
assertEquals("Formatting metadata directory " + testEnv.directory(0) +
337-
" with metadata.version " + MetadataVersion.IBP_3_8_IV0 + ".",
338-
formatter1.output().trim());
340+
assertEquals("Bootstrap metadata: " + formatter1.formatter.bootstrapMetadata() +
341+
"\nFormatting metadata directory " + testEnv.directory(0) +
342+
" with metadata.version " + MetadataVersion.IBP_3_8_IV0 + ".",
343+
formatter1.output().trim());
339344
BootstrapMetadata bootstrapMetadata =
340345
new BootstrapDirectory(testEnv.directory(0)).read();
341346
assertEquals(MetadataVersion.IBP_3_8_IV0, bootstrapMetadata.metadataVersion());
@@ -406,7 +411,7 @@ public void testInvalidFeatureFlag() throws Exception {
406411
"are: eligible.leader.replicas.version, group.version, kraft.version, " +
407412
"share.version, streams.version, test.feature.version, transaction.version",
408413
assertThrows(FormatterException.class,
409-
() -> formatter1.formatter.run()).
414+
formatter1.formatter::run).
410415
getMessage());
411416
}
412417
}
@@ -425,6 +430,7 @@ public void testFormatWithInitialVoters(boolean specifyKRaftVersion) throws Exce
425430
formatter1.formatter.run();
426431
assertEquals((short) 1, formatter1.formatter.featureLevels.getOrDefault("kraft.version", (short) 0));
427432
assertEquals(List.of(
433+
"Bootstrap metadata: " + formatter1.formatter.bootstrapMetadata(),
428434
String.format("Formatting data directory %s with %s %s.",
429435
testEnv.directory(1),
430436
MetadataVersion.FEATURE_NAME,
@@ -459,7 +465,7 @@ public void testFormatWithInitialVotersFailsWithOlderKraftVersion() throws Excep
459465
"Cannot set kraft.version to 0 if one of the flags --standalone, --initial-controllers, or " +
460466
"--no-initial-controllers is used. For dynamic controllers support, try removing the " +
461467
"--feature flag for kraft.version.",
462-
assertThrows(FormatterException.class, () -> formatter1.formatter.run()).getMessage()
468+
assertThrows(FormatterException.class, formatter1.formatter::run).getMessage()
463469
);
464470
}
465471
}
@@ -475,7 +481,7 @@ public void testFormatWithoutInitialVotersFailsWithNewerKraftVersion() throws Ex
475481
"Cannot set kraft.version to 1 unless one of the flags --standalone, --initial-controllers, or " +
476482
"--no-initial-controllers is used. For dynamic controllers support, try using one of " +
477483
"--standalone, --initial-controllers, or --no-initial-controllers.",
478-
assertThrows(FormatterException.class, () -> formatter1.formatter.run()).getMessage()
484+
assertThrows(FormatterException.class, formatter1.formatter::run).getMessage()
479485
);
480486
}
481487
}
@@ -492,7 +498,7 @@ public void testFormatWithInitialVotersFailsWithOlderMetadataVersion() throws Ex
492498
assertEquals("kraft.version could not be set to 1 because it depends on " +
493499
"metadata.version level 21",
494500
assertThrows(IllegalArgumentException.class,
495-
() -> formatter1.formatter.run()).getMessage());
501+
formatter1.formatter::run).getMessage());
496502
}
497503
}
498504

@@ -514,12 +520,12 @@ public void testFormatElrEnabledWithMetadataVersions(MetadataVersion metadataVer
514520
formatter1.formatter.setInitialControllers(DynamicVoters.
515521
parse("1@localhost:8020:4znU-ou9Taa06bmEJxsjnw"));
516522
if (metadataVersion.isAtLeast(MetadataVersion.IBP_4_0_IV1)) {
517-
assertDoesNotThrow(() -> formatter1.formatter.run());
523+
assertDoesNotThrow(formatter1.formatter::run);
518524
} else {
519525
assertEquals("eligible.leader.replicas.version could not be set to 1 because it depends on " +
520526
"metadata.version level 23",
521527
assertThrows(IllegalArgumentException.class,
522-
() -> formatter1.formatter.run()).getMessage());
528+
formatter1.formatter::run).getMessage());
523529
}
524530
}
525531
}
@@ -539,6 +545,7 @@ public void testFormatWithNoInitialControllers(boolean specifyKRaftVersion) thro
539545
formatter1.formatter.run();
540546
assertEquals((short) 1, formatter1.formatter.featureLevels.getOrDefault("kraft.version", (short) 0));
541547
assertEquals(List.of(
548+
"Bootstrap metadata: " + formatter1.formatter.bootstrapMetadata(),
542549
String.format("Formatting data directory %s with %s %s.",
543550
testEnv.directory(1),
544551
MetadataVersion.FEATURE_NAME,

0 commit comments

Comments
 (0)