Skip to content

Commit d40c608

Browse files
scholzjscholzj
authored
Remove the old Bridge enableMetrics flag from CO (#12486)
Signed-off-by: Jakub Scholz <www@scholzj.com>
1 parent 568706e commit d40c608

5 files changed

Lines changed: 33 additions & 39 deletions

File tree

‎cluster-operator/src/main/java/io/strimzi/operator/cluster/model/KafkaBridgeCluster.java‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,6 @@ public class KafkaBridgeCluster extends AbstractModel implements SupportsLogging
119119
private KafkaBridgeAdminClientSpec kafkaBridgeAdminClient;
120120
private KafkaBridgeConsumerSpec kafkaBridgeConsumer;
121121
private KafkaBridgeProducerSpec kafkaBridgeProducer;
122-
private boolean isLegacyMetricsConfigEnabled = false;
123122
private LoggingModel logging;
124123
private MetricsModel metrics;
125124

@@ -165,7 +164,7 @@ private KafkaBridgeCluster(Reconciliation reconciliation, HasMetadata resource,
165164
* @param sharedEnvironmentProvider Shared environment provider
166165
* @return KafkaBridgeCluster instance
167166
*/
168-
@SuppressWarnings({"checkstyle:NPathComplexity", "deprecation"})
167+
@SuppressWarnings({"checkstyle:NPathComplexity"})
169168
public static KafkaBridgeCluster fromCrd(Reconciliation reconciliation,
170169
KafkaBridge kafkaBridge,
171170
SharedEnvironmentProvider sharedEnvironmentProvider) {
@@ -201,8 +200,6 @@ public static KafkaBridgeCluster fromCrd(Reconciliation reconciliation,
201200
result.metrics = new JmxPrometheusExporterModel(spec);
202201
} else if (spec.getMetricsConfig() instanceof StrimziMetricsReporter) {
203202
result.metrics = new StrimziMetricsReporterModel(spec, DEFAULT_METRICS_ALLOW_LIST);
204-
} else {
205-
result.isLegacyMetricsConfigEnabled = Boolean.TRUE.equals(spec.getEnableMetrics());
206203
}
207204

208205
result.setTls(spec.getTls() != null ? spec.getTls() : null);
@@ -579,10 +576,10 @@ public ConfigMap generateBridgeConfigMap(MetricsAndLogging metricsAndLogging) {
579576
.withKafkaConsumer(kafkaBridgeConsumer)
580577
.withHttp(http, kafkaBridgeProducer, kafkaBridgeConsumer);
581578

582-
if ((metrics instanceof JmxPrometheusExporterModel) || isLegacyMetricsConfigEnabled) {
583-
builder.withJmxPrometheusExporter((JmxPrometheusExporterModel) metrics, isLegacyMetricsConfigEnabled);
584-
} else if (metrics instanceof StrimziMetricsReporterModel) {
585-
builder.withStrimziMetricsReporter((StrimziMetricsReporterModel) metrics);
579+
if (metrics instanceof JmxPrometheusExporterModel jmxPrometheusExporterModel) {
580+
builder.withJmxPrometheusExporter(jmxPrometheusExporterModel);
581+
} else if (metrics instanceof StrimziMetricsReporterModel strimziMetricsReporterModel) {
582+
builder.withStrimziMetricsReporter(strimziMetricsReporterModel);
586583
}
587584

588585
data.put(BRIDGE_CONFIGURATION_FILENAME, builder.build());

‎cluster-operator/src/main/java/io/strimzi/operator/cluster/model/KafkaBridgeConfigurationBuilder.java‎

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -395,22 +395,14 @@ public KafkaBridgeConfigurationBuilder withStrimziMetricsReporter(StrimziMetrics
395395
* Configures the JMX Prometheus Metrics Exporter.
396396
*
397397
* @param model JMX Prometheus Metrics Exporter configuration
398-
* @param isLegacyMetricsConfigEnabled Flag which indicates whether the metrics are enabled or not in legacy mode.
399398
*
400399
* @return Returns the builder instance
401400
*/
402-
public KafkaBridgeConfigurationBuilder withJmxPrometheusExporter(
403-
JmxPrometheusExporterModel model, boolean isLegacyMetricsConfigEnabled) {
404-
if (model != null || isLegacyMetricsConfigEnabled) {
401+
public KafkaBridgeConfigurationBuilder withJmxPrometheusExporter(JmxPrometheusExporterModel model) {
402+
if (model != null) {
405403
printSectionHeader("Prometheus JMX Exporter configuration");
406404
writer.println("bridge.metrics=" + JmxPrometheusExporterMetrics.TYPE_JMX_EXPORTER);
407-
408-
// if isLegacyMetricsConfigEnabled is not used, we pass the path of the config file.
409-
// If it is used, the Bridge will use the fallback config.
410-
if (!isLegacyMetricsConfigEnabled) {
411-
writer.println("bridge.metrics.exporter.config.path="
412-
+ KAFKA_BRIDGE_CONFIG_VOLUME_MOUNT + JmxPrometheusExporterModel.CONFIG_MAP_KEY);
413-
}
405+
writer.println("bridge.metrics.exporter.config.path=" + KAFKA_BRIDGE_CONFIG_VOLUME_MOUNT + JmxPrometheusExporterModel.CONFIG_MAP_KEY);
414406

415407
writer.println();
416408
}

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,11 @@ public static KafkaBridge createKafkaBridge(String namespace, String name, Strin
181181
.withBootstrapServers(bootstrapServers)
182182
.withProducer(producer)
183183
.withConsumer(consumer)
184-
.withEnableMetrics(enableMetrics)
184+
.withNewJmxPrometheusExporterMetricsConfig()
185+
.withNewValueFrom()
186+
.withNewConfigMapKeyRef("metrics.yaml", "my-metrics-config", false)
187+
.endValueFrom()
188+
.endJmxPrometheusExporterMetricsConfig()
185189
.withHttp(http)
186190
.endSpec()
187191
.build();

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

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -664,20 +664,6 @@ public void testWithStrimziMetricsReporter() {
664664
"kafka.security.protocol=PLAINTEXT"
665665
));
666666
}
667-
668-
@Test
669-
public void testWithPrometheusJmxExporterLegacyMode() {
670-
String configuration = new KafkaBridgeConfigurationBuilder(Reconciliation.DUMMY_RECONCILIATION, BRIDGE_CLUSTER, BRIDGE_BOOTSTRAP_SERVERS)
671-
.withJmxPrometheusExporter(null, true)
672-
.build();
673-
674-
assertThat(configuration, isEquivalent(
675-
"bridge.id=my-bridge",
676-
"bridge.metrics=" + JmxPrometheusExporterMetrics.TYPE_JMX_EXPORTER,
677-
"kafka.bootstrap.servers=my-cluster-kafka-bootstrap:9092",
678-
"kafka.security.protocol=PLAINTEXT"
679-
));
680-
}
681667

682668
@Test
683669
public void testWithPrometheusJmxExporter() {
@@ -691,7 +677,7 @@ public void testWithPrometheusJmxExporter() {
691677
.build());
692678

693679
String configuration = new KafkaBridgeConfigurationBuilder(Reconciliation.DUMMY_RECONCILIATION, BRIDGE_CLUSTER, BRIDGE_BOOTSTRAP_SERVERS)
694-
.withJmxPrometheusExporter(model, false)
680+
.withJmxPrometheusExporter(model)
695681
.build();
696682

697683
assertThat(configuration, isEquivalent(

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

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
package io.strimzi.operator.cluster.operator.assembly;
66

77
import io.fabric8.kubernetes.api.model.ConfigMap;
8+
import io.fabric8.kubernetes.api.model.ConfigMapBuilder;
89
import io.fabric8.kubernetes.api.model.LabelSelector;
910
import io.fabric8.kubernetes.api.model.Service;
1011
import io.fabric8.kubernetes.api.model.apps.Deployment;
@@ -143,6 +144,8 @@ public void testCreateOrUpdateCreatesCluster(VertxTestContext context) {
143144
when(mockPdbOps.reconcile(any(), anyString(), any(), pdbCaptor.capture())).thenReturn(Future.succeededFuture());
144145

145146
when(mockCmOps.reconcile(any(), anyString(), any(), any())).thenReturn(Future.succeededFuture());
147+
// Metrics configuration map
148+
when(mockCmOps.getAsync(any(), eq("my-metrics-config"))).thenReturn(Future.succeededFuture(new ConfigMapBuilder().withData(Map.of("metrics.yaml", "metrics-config")).build()));
146149

147150
ArgumentCaptor<KafkaBridge> bridgeCaptor = ArgumentCaptor.forClass(KafkaBridge.class);
148151
when(mockBridgeOps.updateStatusAsync(any(), bridgeCaptor.capture())).thenReturn(Future.succeededFuture());
@@ -171,7 +174,7 @@ public void testCreateOrUpdateCreatesCluster(VertxTestContext context) {
171174
assertThat(dc.getMetadata().getName(), is(bridge.getComponentName()));
172175
assertThat(dc, is(bridge.generateDeployment(Map.of(
173176
Annotations.ANNO_STRIMZI_AUTH_HASH, "0",
174-
Annotations.ANNO_STRIMZI_IO_CONFIGURATION_HASH, "eeacc2f1"
177+
Annotations.ANNO_STRIMZI_IO_CONFIGURATION_HASH, "9fd6bb72"
175178
), true, null, null)));
176179

177180
// Verify PodDisruptionBudget
@@ -239,6 +242,9 @@ public void testCreateOrUpdateWithNoDiffCausesNoChanges(VertxTestContext context
239242
when(mockPdbOps.reconcile(any(), anyString(), any(), pdbCaptor.capture())).thenReturn(Future.succeededFuture());
240243

241244
when(mockCmOps.reconcile(any(), anyString(), any(), any())).thenReturn(Future.succeededFuture(ReconcileResult.created(new ConfigMap())));
245+
// Metrics configuration map
246+
when(mockCmOps.getAsync(any(), eq("my-metrics-config"))).thenReturn(Future.succeededFuture(new ConfigMapBuilder().withData(Map.of("metrics.yaml", "metrics-config")).build()));
247+
242248
KafkaBridgeAssemblyOperator ops = new KafkaBridgeAssemblyOperator(vertx,
243249
new PlatformFeaturesAvailability(true, kubernetesVersion),
244250
new MockCertManager(), new PasswordGenerator(10, "a", "a"),
@@ -317,6 +323,8 @@ public void testCreateOrUpdateUpdatesCluster(VertxTestContext context) {
317323
when(mockPdbOps.reconcile(any(), anyString(), any(), pdbCaptor.capture())).thenReturn(Future.succeededFuture());
318324

319325
when(mockCmOps.reconcile(any(), anyString(), any(), any())).thenReturn(Future.succeededFuture());
326+
// Metrics configuration map
327+
when(mockCmOps.getAsync(any(), eq("my-metrics-config"))).thenReturn(Future.succeededFuture(new ConfigMapBuilder().withData(Map.of("metrics.yaml", "metrics-config")).build()));
320328

321329
KafkaBridgeAssemblyOperator ops = new KafkaBridgeAssemblyOperator(vertx,
322330
new PlatformFeaturesAvailability(true, kubernetesVersion),
@@ -344,7 +352,7 @@ public void testCreateOrUpdateUpdatesCluster(VertxTestContext context) {
344352
assertThat(dc.getMetadata().getName(), is(compareTo.getComponentName()));
345353
assertThat(dc, is(compareTo.generateDeployment(Map.of(
346354
Annotations.ANNO_STRIMZI_AUTH_HASH, "0",
347-
Annotations.ANNO_STRIMZI_IO_CONFIGURATION_HASH, "eeacc2f1"
355+
Annotations.ANNO_STRIMZI_IO_CONFIGURATION_HASH, "9fd6bb72"
348356
), true, null, null)));
349357

350358
// Verify PodDisruptionBudget
@@ -707,15 +715,22 @@ public void testCreateOrUpdateBridgeZeroReplica(VertxTestContext context) {
707715
when(mockBridgeOps.get(kbNamespace, kbName)).thenReturn(kb);
708716
when(mockBridgeOps.getAsync(anyString(), anyString())).thenReturn(Future.succeededFuture(kb));
709717
when(mockBridgeOps.get(anyString(), anyString())).thenReturn(kb);
718+
ArgumentCaptor<KafkaBridge> bridgeCaptor = ArgumentCaptor.forClass(KafkaBridge.class);
719+
when(mockBridgeOps.updateStatusAsync(any(), bridgeCaptor.capture())).thenReturn(Future.succeededFuture());
720+
710721
when(mockServiceOps.reconcile(any(), anyString(), anyString(), any())).thenReturn(Future.succeededFuture());
722+
711723
when(mockDcOps.reconcile(any(), anyString(), anyString(), any())).thenReturn(Future.succeededFuture());
712724
when(mockDcOps.scaleUp(any(), anyString(), anyString(), anyInt(), anyLong())).thenReturn(Future.succeededFuture(42));
713725
when(mockDcOps.scaleDown(any(), anyString(), anyString(), anyInt(), anyLong())).thenReturn(Future.succeededFuture(42));
714726
when(mockDcOps.waitForObserved(any(), anyString(), anyString(), anyLong(), anyLong())).thenReturn(Future.succeededFuture());
727+
715728
when(mockPdbOps.reconcile(any(), anyString(), any(), any())).thenReturn(Future.succeededFuture());
729+
716730
when(mockCmOps.reconcile(any(), anyString(), any(), any())).thenReturn(Future.succeededFuture(ReconcileResult.created(new ConfigMap())));
717-
ArgumentCaptor<KafkaBridge> bridgeCaptor = ArgumentCaptor.forClass(KafkaBridge.class);
718-
when(mockBridgeOps.updateStatusAsync(any(), bridgeCaptor.capture())).thenReturn(Future.succeededFuture());
731+
// Metrics configuration map
732+
when(mockCmOps.getAsync(any(), eq("my-metrics-config"))).thenReturn(Future.succeededFuture(new ConfigMapBuilder().withData(Map.of("metrics.yaml", "metrics-config")).build()));
733+
719734
KafkaBridgeAssemblyOperator ops = new KafkaBridgeAssemblyOperator(vertx,
720735
new PlatformFeaturesAvailability(true, kubernetesVersion),
721736
new MockCertManager(), new PasswordGenerator(10, "a", "a"),

0 commit comments

Comments
 (0)