Skip to content

Commit ffbf70a

Browse files
im-kongePaulRMellor
im-konge
and
PaulRMellor
authored
Promote ContinueReconciliationOnManualRollingUpdateFailure FG to GA (#11323)
Signed-off-by: Lukas Kral <[email protected]> Signed-off-by: Lukáš Král <[email protected]> Co-authored-by: PaulRMellor <[email protected]>
1 parent 4aa11df commit ffbf70a

File tree

14 files changed

+74
-295
lines changed

14 files changed

+74
-295
lines changed

‎.azure/templates/jobs/system-tests/feature_gates_regression_jobs.yaml

+7-7
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ jobs:
55
display_name: 'feature-gates-regression-bundle I. - kafka + oauth'
66
profile: 'azp_kafka_oauth'
77
cluster_operator_install_type: 'bundle'
8-
strimzi_feature_gates: '-ContinueReconciliationOnManualRollingUpdateFailure'
8+
strimzi_feature_gates: '+DummyFeatureGate'
99
timeout: 360
1010
releaseVersion: '${{ parameters.releaseVersion }}'
1111
kafkaVersion: '${{ parameters.kafkaVersion }}'
@@ -16,7 +16,7 @@ jobs:
1616
display_name: 'feature-gates-regression-bundle II. - security'
1717
profile: 'azp_security'
1818
cluster_operator_install_type: 'bundle'
19-
strimzi_feature_gates: '-ContinueReconciliationOnManualRollingUpdateFailure'
19+
strimzi_feature_gates: '+DummyFeatureGate'
2020
timeout: 360
2121
releaseVersion: '${{ parameters.releaseVersion }}'
2222
kafkaVersion: '${{ parameters.kafkaVersion }}'
@@ -27,7 +27,7 @@ jobs:
2727
display_name: 'feature-gates-regression-bundle III. - dynconfig + tracing + watcher'
2828
profile: 'azp_dynconfig_listeners_tracing_watcher'
2929
cluster_operator_install_type: 'bundle'
30-
strimzi_feature_gates: '-ContinueReconciliationOnManualRollingUpdateFailure'
30+
strimzi_feature_gates: '+DummyFeatureGate'
3131
timeout: 360
3232
releaseVersion: '${{ parameters.releaseVersion }}'
3333
kafkaVersion: '${{ parameters.kafkaVersion }}'
@@ -38,7 +38,7 @@ jobs:
3838
display_name: 'feature-gates-regression-bundle IV. - operators'
3939
profile: 'azp_operators'
4040
cluster_operator_install_type: 'bundle'
41-
strimzi_feature_gates: '-ContinueReconciliationOnManualRollingUpdateFailure'
41+
strimzi_feature_gates: '+DummyFeatureGate'
4242
timeout: 360
4343
releaseVersion: '${{ parameters.releaseVersion }}'
4444
kafkaVersion: '${{ parameters.kafkaVersion }}'
@@ -49,7 +49,7 @@ jobs:
4949
display_name: 'feature-gates-regression-bundle V. - rollingupdate'
5050
profile: 'azp_rolling_update_bridge'
5151
cluster_operator_install_type: 'bundle'
52-
strimzi_feature_gates: '-ContinueReconciliationOnManualRollingUpdateFailure'
52+
strimzi_feature_gates: '+DummyFeatureGate'
5353
timeout: 360
5454
releaseVersion: '${{ parameters.releaseVersion }}'
5555
kafkaVersion: '${{ parameters.kafkaVersion }}'
@@ -60,7 +60,7 @@ jobs:
6060
display_name: 'feature-gates-regression-bundle VI. - connect + mirrormaker'
6161
profile: 'azp_connect_mirrormaker'
6262
cluster_operator_install_type: 'bundle'
63-
strimzi_feature_gates: '-ContinueReconciliationOnManualRollingUpdateFailure'
63+
strimzi_feature_gates: '+DummyFeatureGate'
6464
timeout: 360
6565
releaseVersion: '${{ parameters.releaseVersion }}'
6666
kafkaVersion: '${{ parameters.kafkaVersion }}'
@@ -71,7 +71,7 @@ jobs:
7171
display_name: 'feature-gates-regression-bundle VII. - remaining system tests'
7272
profile: 'azp_remaining'
7373
cluster_operator_install_type: 'bundle'
74-
strimzi_feature_gates: '-ContinueReconciliationOnManualRollingUpdateFailure'
74+
strimzi_feature_gates: '+DummyFeatureGate'
7575
timeout: 360
7676
releaseVersion: '${{ parameters.releaseVersion }}'
7777
kafkaVersion: '${{ parameters.kafkaVersion }}'

‎.azure/templates/jobs/system-tests/feature_gates_regression_namespace_rbac_jobs.yaml

+7-7
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ jobs:
88
cluster_operator_install_type: 'bundle'
99
timeout: 360
1010
strimzi_rbac_scope: NAMESPACE
11-
strimzi_feature_gates: '-ContinueReconciliationOnManualRollingUpdateFailure'
11+
strimzi_feature_gates: '+DummyFeatureGate'
1212
releaseVersion: '${{ parameters.releaseVersion }}'
1313
kafkaVersion: '${{ parameters.kafkaVersion }}'
1414

@@ -21,7 +21,7 @@ jobs:
2121
cluster_operator_install_type: 'bundle'
2222
timeout: 360
2323
strimzi_rbac_scope: NAMESPACE
24-
strimzi_feature_gates: '-ContinueReconciliationOnManualRollingUpdateFailure'
24+
strimzi_feature_gates: '+DummyFeatureGate'
2525
releaseVersion: '${{ parameters.releaseVersion }}'
2626
kafkaVersion: '${{ parameters.kafkaVersion }}'
2727

@@ -34,7 +34,7 @@ jobs:
3434
cluster_operator_install_type: 'bundle'
3535
timeout: 360
3636
strimzi_rbac_scope: NAMESPACE
37-
strimzi_feature_gates: '-ContinueReconciliationOnManualRollingUpdateFailure'
37+
strimzi_feature_gates: '+DummyFeatureGate'
3838
releaseVersion: '${{ parameters.releaseVersion }}'
3939
kafkaVersion: '${{ parameters.kafkaVersion }}'
4040

@@ -47,7 +47,7 @@ jobs:
4747
cluster_operator_install_type: 'bundle'
4848
timeout: 360
4949
strimzi_rbac_scope: NAMESPACE
50-
strimzi_feature_gates: '-ContinueReconciliationOnManualRollingUpdateFailure'
50+
strimzi_feature_gates: '+DummyFeatureGate'
5151
releaseVersion: '${{ parameters.releaseVersion }}'
5252
kafkaVersion: '${{ parameters.kafkaVersion }}'
5353

@@ -60,7 +60,7 @@ jobs:
6060
cluster_operator_install_type: 'bundle'
6161
timeout: 360
6262
strimzi_rbac_scope: NAMESPACE
63-
strimzi_feature_gates: '-ContinueReconciliationOnManualRollingUpdateFailure'
63+
strimzi_feature_gates: '+DummyFeatureGate'
6464
releaseVersion: '${{ parameters.releaseVersion }}'
6565
kafkaVersion: '${{ parameters.kafkaVersion }}'
6666

@@ -73,7 +73,7 @@ jobs:
7373
cluster_operator_install_type: 'bundle'
7474
timeout: 360
7575
strimzi_rbac_scope: NAMESPACE
76-
strimzi_feature_gates: '-ContinueReconciliationOnManualRollingUpdateFailure'
76+
strimzi_feature_gates: '+DummyFeatureGate'
7777
releaseVersion: '${{ parameters.releaseVersion }}'
7878
kafkaVersion: '${{ parameters.kafkaVersion }}'
7979

@@ -86,6 +86,6 @@ jobs:
8686
cluster_operator_install_type: 'bundle'
8787
timeout: 360
8888
strimzi_rbac_scope: NAMESPACE
89-
strimzi_feature_gates: '-ContinueReconciliationOnManualRollingUpdateFailure'
89+
strimzi_feature_gates: '+DummyFeatureGate'
9090
releaseVersion: '${{ parameters.releaseVersion }}'
9191
kafkaVersion: '${{ parameters.kafkaVersion }}'

‎CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
* Implicit IPv4 preference when enabling JMX has been removed, and will now use JVM defaults.
2121
This will make the cluster boot up correctly in IPv6 only environments, where IPv4 preference will break it due to lack of IPv4 addresses.
2222
* Improved the MirrorMaker2 example Grafana dashboard to set metric units and include chart descriptions.
23+
* The `ContinueReconciliationOnManualRollingUpdateFailure` feature gate moves to GA stage and is permanently enabled without the possibility to disable it.
2324

2425
### Major changes, deprecations and removals
2526

‎cluster-operator/src/main/java/io/strimzi/operator/cluster/operator/assembly/AbstractConnectOperator.java

+2-8
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,6 @@ public abstract class AbstractConnectOperator<C extends KubernetesClient, T exte
9797
private static final ReconciliationLogger LOGGER = ReconciliationLogger.create(AbstractConnectOperator.class.getName());
9898

9999
private final boolean isNetworkPolicyGeneration;
100-
private final boolean continueOnManualRUFailure;
101100
private final boolean isPodDisruptionBudgetGeneration;
102101

103102
protected final Function<Vertx, KafkaConnectApi> connectClientProvider;
@@ -159,7 +158,6 @@ public AbstractConnectOperator(Vertx vertx, PlatformFeaturesAvailability pfa, St
159158
this.versions = config.versions();
160159
this.sharedEnvironmentProvider = supplier.sharedEnvironmentProvider;
161160
this.port = port;
162-
this.continueOnManualRUFailure = config.featureGates().continueOnManualRUFailureEnabled();
163161
this.isPodDisruptionBudgetGeneration = config.isPodDisruptionBudgetGeneration();
164162
}
165163

@@ -246,12 +244,8 @@ protected Future<Void> manualRollingUpdate(Reconciliation reconciliation, KafkaC
246244
KafkaConnectRoller roller = new KafkaConnectRoller(reconciliation, connect, operationTimeoutMs, podOperations);
247245
return roller.maybeRoll(podNamesToRoll, pod -> RestartReasons.of(RestartReason.MANUAL_ROLLING_UPDATE))
248246
.recover(error -> {
249-
if (continueOnManualRUFailure) {
250-
LOGGER.warnCr(reconciliation, "Manual rolling update failed (reconciliation will be continued)", error);
251-
return Future.succeededFuture();
252-
} else {
253-
return Future.failedFuture(error);
254-
}
247+
LOGGER.warnCr(reconciliation, "Manual rolling update failed (reconciliation will be continued)", error);
248+
return Future.succeededFuture();
255249
});
256250
} else {
257251
return Future.succeededFuture();

‎cluster-operator/src/main/java/io/strimzi/operator/cluster/operator/assembly/KafkaReconciler.java

+2-9
Original file line numberDiff line numberDiff line change
@@ -158,8 +158,6 @@ public class KafkaReconciler {
158158
// marked as final, but their contents is modified during the reconciliation)
159159
private final Set<String> fsResizingRestartRequest = new HashSet<>();
160160

161-
private final boolean continueOnManualRUFailure;
162-
163161
private String logging = "";
164162
private final Map<Integer, String> brokerLoggingHash = new HashMap<>();
165163
private final Map<Integer, String> brokerConfigurationHash = new HashMap<>();
@@ -235,7 +233,6 @@ public KafkaReconciler(
235233

236234
this.adminClientProvider = supplier.adminClientProvider;
237235
this.kafkaAgentClientProvider = supplier.kafkaAgentClientProvider;
238-
this.continueOnManualRUFailure = config.featureGates().continueOnManualRUFailureEnabled();
239236
}
240237

241238
/**
@@ -390,12 +387,8 @@ protected Future<Void> manualRollingUpdate() {
390387
nodes.stream().collect(Collectors.toMap(NodeRef::nodeId, node -> Map.of())),
391388
false
392389
).recover(error -> {
393-
if (continueOnManualRUFailure) {
394-
LOGGER.warnCr(reconciliation, "Manual rolling update failed (reconciliation will be continued)", error);
395-
return Future.succeededFuture();
396-
} else {
397-
return Future.failedFuture(error);
398-
}
390+
LOGGER.warnCr(reconciliation, "Manual rolling update failed (reconciliation will be continued)", error);
391+
return Future.succeededFuture();
399392
});
400393
} else {
401394
return Future.succeededFuture();

‎cluster-operator/src/test/java/io/strimzi/operator/cluster/ClusterOperatorConfigTest.java

+3-3
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ public class ClusterOperatorConfigTest {
4343
ENV_VARS.put(ClusterOperatorConfig.STRIMZI_KAFKA_CONNECT_IMAGES, KafkaVersionTestUtils.getKafkaConnectImagesEnvVarString());
4444
ENV_VARS.put(ClusterOperatorConfig.STRIMZI_KAFKA_MIRROR_MAKER_2_IMAGES, KafkaVersionTestUtils.getKafkaMirrorMaker2ImagesEnvVarString());
4545
ENV_VARS.put(ClusterOperatorConfig.OPERATOR_NAMESPACE.key(), "operator-namespace");
46-
ENV_VARS.put(ClusterOperatorConfig.FEATURE_GATES.key(), "-ContinueReconciliationOnManualRollingUpdateFailure");
46+
ENV_VARS.put(ClusterOperatorConfig.FEATURE_GATES.key(), "+DummyFeatureGate");
4747
ENV_VARS.put(ClusterOperatorConfig.DNS_CACHE_TTL.key(), "10");
4848
ENV_VARS.put(ClusterOperatorConfig.POD_SECURITY_PROVIDER_CLASS.key(), "my.package.CustomPodSecurityProvider");
4949
ENV_VARS.put(ClusterOperatorConfig.POD_DISRUPTION_BUDGET_GENERATION.key(), "false");
@@ -100,7 +100,7 @@ public void testEnvVars() {
100100
assertThat(config.getOperationTimeoutMs(), is(30_000L));
101101
assertThat(config.getConnectBuildTimeoutMs(), is(40_000L));
102102
assertThat(config.getOperatorNamespace(), is("operator-namespace"));
103-
assertThat(config.featureGates().continueOnManualRUFailureEnabled(), is(false));
103+
assertThat(config.featureGates().dummyFeatureGateEnabled(), is(true));
104104
assertThat(config.getDnsCacheTtlSec(), is(10));
105105
assertThat(config.getPodSecurityProviderClass(), is("my.package.CustomPodSecurityProvider"));
106106
assertThat(config.isPodDisruptionBudgetGeneration(), is(false));
@@ -118,7 +118,7 @@ public void testEnvVarsDefault() {
118118
assertThat(config.getOperationTimeoutMs(), is(Long.parseLong(ClusterOperatorConfig.OPERATION_TIMEOUT_MS.defaultValue())));
119119
assertThat(config.getOperatorNamespace(), is(nullValue()));
120120
assertThat(config.getOperatorNamespaceLabels(), is(nullValue()));
121-
assertThat(config.featureGates().continueOnManualRUFailureEnabled(), is(true));
121+
assertThat(config.featureGates().dummyFeatureGateEnabled(), is(false));
122122
assertThat(config.getDnsCacheTtlSec(), is(Integer.parseInt(ClusterOperatorConfig.DNS_CACHE_TTL.defaultValue())));
123123
assertThat(config.getPodSecurityProviderClass(), is(ClusterOperatorConfig.POD_SECURITY_PROVIDER_CLASS.defaultValue()));
124124
assertThat(config.isPodDisruptionBudgetGeneration(), is(true));

‎cluster-operator/src/test/java/io/strimzi/operator/cluster/model/EntityOperatorTest.java

+3-3
Original file line numberDiff line numberDiff line change
@@ -785,14 +785,14 @@ public void testUserOperatorAndTopicOperatorNetworkPolicy() {
785785
@ParallelTest
786786
public void testFeatureGateEnvVars() {
787787
ClusterOperatorConfig config = new ClusterOperatorConfig.ClusterOperatorConfigBuilder(ResourceUtils.dummyClusterOperatorConfig(), VERSIONS)
788-
.with(ClusterOperatorConfig.FEATURE_GATES.key(), "-ContinueReconciliationOnManualRollingUpdateFailure")
788+
.with(ClusterOperatorConfig.FEATURE_GATES.key(), "+DummyFeatureGate")
789789
.build();
790790

791791
EntityOperator eo = EntityOperator.fromCrd(new Reconciliation("test", KAFKA.getKind(), KAFKA.getMetadata().getNamespace(), KAFKA.getMetadata().getName()), KAFKA, SHARED_ENV_PROVIDER, config);
792792
Deployment dep = eo.generateDeployment(Map.of(), false, null, null);
793793

794-
assertThat(dep.getSpec().getTemplate().getSpec().getContainers().get(0).getEnv().stream().filter(env -> "STRIMZI_FEATURE_GATES".equals(env.getName())).map(EnvVar::getValue).findFirst().orElseThrow(), is("-ContinueReconciliationOnManualRollingUpdateFailure"));
795-
assertThat(dep.getSpec().getTemplate().getSpec().getContainers().get(1).getEnv().stream().filter(env -> "STRIMZI_FEATURE_GATES".equals(env.getName())).map(EnvVar::getValue).findFirst().orElseThrow(), is("-ContinueReconciliationOnManualRollingUpdateFailure"));
794+
assertThat(dep.getSpec().getTemplate().getSpec().getContainers().get(0).getEnv().stream().filter(env -> "STRIMZI_FEATURE_GATES".equals(env.getName())).map(EnvVar::getValue).findFirst().orElseThrow(), is("+DummyFeatureGate"));
795+
assertThat(dep.getSpec().getTemplate().getSpec().getContainers().get(1).getEnv().stream().filter(env -> "STRIMZI_FEATURE_GATES".equals(env.getName())).map(EnvVar::getValue).findFirst().orElseThrow(), is("+DummyFeatureGate"));
796796
}
797797

798798
////////////////////

‎cluster-operator/src/test/java/io/strimzi/operator/cluster/operator/assembly/KafkaAssemblyOperatorManualRollingUpdatesTest.java

+12-30
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,6 @@
5959
import java.util.Set;
6060
import java.util.function.Function;
6161

62-
import static org.hamcrest.CoreMatchers.containsString;
6362
import static org.hamcrest.CoreMatchers.is;
6463
import static org.hamcrest.MatcherAssert.assertThat;
6564
import static org.mockito.ArgumentMatchers.any;
@@ -349,16 +348,7 @@ public void testManualPodRollingUpdateWithPodSets(VertxTestContext context) {
349348
}
350349

351350
@Test
352-
public void testManualPodRollingUpdateWithPodSetsWithError1(VertxTestContext context) {
353-
testManualPodRollingUpdateWithPodSetsWithErrorConditions(context, "-ContinueReconciliationOnManualRollingUpdateFailure", true);
354-
}
355-
356-
@Test
357-
public void testManualPodRollingUpdateWithPodSetsWithError3(VertxTestContext context) {
358-
testManualPodRollingUpdateWithPodSetsWithErrorConditions(context, "", false);
359-
}
360-
361-
private void testManualPodRollingUpdateWithPodSetsWithErrorConditions(VertxTestContext context, String featureGates, boolean expectError) {
351+
public void testManualPodRollingUpdateWithPodSetsWithErrorConditions(VertxTestContext context) {
362352
ResourceOperatorSupplier supplier = ResourceUtils.supplierWithMocks(false);
363353
Reconciliation reconciliation = new Reconciliation("test-trigger", Kafka.RESOURCE_KIND, NAMESPACE, CLUSTER_NAME);
364354

@@ -393,7 +383,7 @@ private void testManualPodRollingUpdateWithPodSetsWithErrorConditions(VertxTestC
393383
when(mockKafkaOps.get(eq(NAMESPACE), eq(CLUSTER_NAME))).thenReturn(KAFKA);
394384
when(mockKafkaOps.updateStatusAsync(any(), any())).thenReturn(Future.succeededFuture());
395385

396-
ClusterOperatorConfig config = ResourceUtils.dummyClusterOperatorConfig(featureGates);
386+
ClusterOperatorConfig config = ResourceUtils.dummyClusterOperatorConfig();
397387

398388
MockKafkaReconciler kr = new MockKafkaReconciler(
399389
reconciliation,
@@ -418,24 +408,16 @@ private void testManualPodRollingUpdateWithPodSetsWithErrorConditions(VertxTestC
418408
kr);
419409

420410
Checkpoint async = context.checkpoint();
421-
if (expectError) {
422-
kao.reconcile(reconciliation)
423-
.onComplete(context.failing(e -> context.verify(() -> {
424-
assertThat(e.getMessage(), containsString("Force failure"));
425-
async.flag();
426-
})));
427-
} else {
428-
kao.reconcile(reconciliation)
429-
.onComplete(context.succeeding(e -> context.verify(() -> {
430-
// Verify Kafka rolling updates
431-
assertThat(kr.maybeRollKafkaInvocations, is(1));
432-
assertThat(kr.kafkaNodesNeedRestart.size(), is(4));
433-
assertThat(kr.kafkaNodesNeedRestart, is(List.of("my-cluster-controllers-0", "my-cluster-controllers-1", "my-cluster-brokers-10", "my-cluster-brokers-11")));
434-
assertThat(kr.kafkaRestartReasons.apply(podWithName("anyName")), is(RestartReasons.of(RestartReason.MANUAL_ROLLING_UPDATE)));
435-
436-
async.flag();
437-
})));
438-
}
411+
kao.reconcile(reconciliation)
412+
.onComplete(context.succeeding(e -> context.verify(() -> {
413+
// Verify Kafka rolling updates
414+
assertThat(kr.maybeRollKafkaInvocations, is(1));
415+
assertThat(kr.kafkaNodesNeedRestart.size(), is(4));
416+
assertThat(kr.kafkaNodesNeedRestart, is(List.of("my-cluster-controllers-0", "my-cluster-controllers-1", "my-cluster-brokers-10", "my-cluster-brokers-11")));
417+
assertThat(kr.kafkaRestartReasons.apply(podWithName("anyName")), is(RestartReasons.of(RestartReason.MANUAL_ROLLING_UPDATE)));
418+
419+
async.flag();
420+
})));
439421
}
440422

441423
// Internal utility methods

0 commit comments

Comments
 (0)