ijuma commented on code in PR #18845:
URL: https://github.com/apache/kafka/pull/18845#discussion_r1952730662
##########
metadata/src/test/java/org/apache/kafka/controller/FeatureControlManagerTest.java:
##########
@@ -349,28 +344,27 @@ public void testCanUseUnsafeDowngradeIfMetadataChanged() {
public void testCanUseSafeDowngradeIfMetadataDidNotChange() {
FeatureControlManager manager = new FeatureControlManager.Builder().
setQuorumFeatures(features(MetadataVersion.FEATURE_NAME,
- MetadataVersion.IBP_3_0_IV1.featureLevel(),
MetadataVersion.IBP_3_3_IV1.featureLevel())).
- setMetadataVersion(MetadataVersion.IBP_3_1_IV0).
- setMinimumBootstrapVersion(MetadataVersion.IBP_3_0_IV1).
+ MetadataVersion.MINIMUM_VERSION.featureLevel(),
MetadataVersion.IBP_3_6_IV0.featureLevel())).
+ setMetadataVersion(MetadataVersion.IBP_3_5_IV0).
build();
assertEquals(ControllerResult.of(Collections.emptyList(),
ApiError.NONE),
manager.updateFeatures(
- singletonMap(MetadataVersion.FEATURE_NAME,
MetadataVersion.IBP_3_0_IV1.featureLevel()),
+ singletonMap(MetadataVersion.FEATURE_NAME,
MetadataVersion.IBP_3_4_IV0.featureLevel()),
singletonMap(MetadataVersion.FEATURE_NAME,
FeatureUpdate.UpgradeType.SAFE_DOWNGRADE),
true));
}
@Test
- public void testCannotDowngradeBefore3_3_IV0() {
+ public void testCannotDowngradeBeforeMinimumKraftVersion() {
FeatureControlManager manager = new FeatureControlManager.Builder().
setQuorumFeatures(features(MetadataVersion.FEATURE_NAME,
- MetadataVersion.IBP_3_0_IV1.featureLevel(),
MetadataVersion.IBP_3_3_IV3.featureLevel())).
- setMetadataVersion(MetadataVersion.IBP_3_3_IV0).
+ MetadataVersionTestUtils.IBP_3_0_IV1_FEATURE_LEVEL,
MetadataVersion.MINIMUM_VERSION.featureLevel())).
Review Comment:
The current code tests a slightly different path than the one you're
suggesting. The error is thrown in a different place. I looked a bit more
deeply though and perhaps your suggestion is closer to what will happen in
production:
```java
public static Map<String, VersionRange> defaultSupportedFeatureMap(boolean
enableUnstable) {
Map<String, VersionRange> features = new HashMap<>(1);
features.put(MetadataVersion.FEATURE_NAME, VersionRange.of(
MetadataVersion.MINIMUM_VERSION.featureLevel(),
enableUnstable ?
MetadataVersion.latestTesting().featureLevel() :
MetadataVersion.latestProduction().featureLevel()));
for (Feature feature : Feature.PRODUCTION_FEATURES) {
short maxVersion = enableUnstable ? feature.latestTesting() :
feature.latestProduction();
if (maxVersion > 0) {
features.put(feature.featureName(),
VersionRange.of(feature.minimumProduction(), maxVersion));
}
}
return features;
}
```
Changing it to your suggestion.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
