Address initial feedback

Signed-off-by: ocorriga <[email protected]>
strimzi · Jan 17, 2025 · f943007 · f943007
1 parent a9b7788
commit f943007
Show file tree

Hide file tree

Showing 6 changed files with 5,016 additions and 11 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,7 +6,8 @@
 * Support for MirrorMaker 1 has been removed
 * Added support to configure `dnsPolicy` and `dnsConfig` using the `template` sections.
 * Store Kafka node certificates in separate Secrets, one Secret per pod.
-* Added support for Strimzi Metrics Reporter to the Kafka component.
+* Added support for Strimzi Metrics Reporter to the Kafka brokers/controllers components. 
+  Support for Kafka Connect, MirrorMaker2 and Http Bridge will be added subsequently.
 
 ### Major changes, deprecations and removals
 

diff --git a/cluster-operator/src/main/java/io/strimzi/operator/cluster/model/CruiseControl.java b/cluster-operator/src/main/java/io/strimzi/operator/cluster/model/CruiseControl.java
@@ -405,8 +405,7 @@ public Deployment generateDeployment(Map<String, String> annotations, boolean is
     protected List<EnvVar> getEnvVars() {
         List<EnvVar> varList = new ArrayList<>();
 
-        String jmxMetricsEnabled = jmxExporterMetrics != null && jmxExporterMetrics.isEnabled() ? Boolean.TRUE.toString() : Boolean.FALSE.toString();
-        varList.add(ContainerUtils.createEnvVar(ENV_VAR_CRUISE_CONTROL_JMX_EXPORTER_ENABLED, jmxMetricsEnabled));
+        varList.add(ContainerUtils.createEnvVar(ENV_VAR_CRUISE_CONTROL_JMX_EXPORTER_ENABLED, jmxExporterMetrics != null && jmxExporterMetrics.isEnabled() ? Boolean.TRUE.toString() : Boolean.FALSE.toString()));
         varList.add(ContainerUtils.createEnvVar(ENV_VAR_STRIMZI_KAFKA_BOOTSTRAP_SERVERS, KafkaResources.bootstrapServiceName(cluster) + ":" + KafkaCluster.REPLICATION_PORT));
         varList.add(ContainerUtils.createEnvVar(ENV_VAR_STRIMZI_KAFKA_GC_LOG_ENABLED, String.valueOf(gcLoggingEnabled)));
 

diff --git a/...ator/src/main/java/io/strimzi/operator/cluster/model/KafkaBrokerConfigurationBuilder.java b/...ator/src/main/java/io/strimzi/operator/cluster/model/KafkaBrokerConfigurationBuilder.java
@@ -56,6 +56,7 @@
  */
 public class KafkaBrokerConfigurationBuilder {
     private final static String CONTROL_PLANE_LISTENER_NAME = "CONTROLPLANE-9090";
+    private final static String KAFKA_METRIC_REPORTERS_CONFIG_FIELD = "metric.reporters";
     private final static String REPLICATION_LISTENER_NAME = "REPLICATION-9091";
     // Names of environment variables expanded through config providers inside the Kafka node
     private final static String PLACEHOLDER_CERT_STORE_PASSWORD = "${strimzienv:CERTS_STORE_PASSWORD}";
@@ -306,6 +307,7 @@ public KafkaBrokerConfigurationBuilder withListeners(
         ////////////////////
         // Shared configurations with values dependent on all listeners
         ////////////////////
+
         // configure OAuth principal builder for all the nodes - brokers, controllers, and mixed
         configureOAuthPrincipalBuilderIfNeeded(writer, kafkaListeners);
 
@@ -821,7 +823,7 @@ public KafkaBrokerConfigurationBuilder withUserConfiguration(KafkaConfiguration
             configProviders(userConfig);
 
             // Handle all combinations of metric.reporters
-            String metricReporters = userConfig.getConfigOption(KafkaCluster.KAFKA_METRIC_REPORTERS_CONFIG_FIELD);
+            String metricReporters = userConfig.getConfigOption(KAFKA_METRIC_REPORTERS_CONFIG_FIELD);
 
             // If the injectCcMetricsReporter / injectStrimziMetricsReporter flag is set to true, it is appended to the list of metric reporters
             if (injectCcMetricsReporter) {
@@ -832,7 +834,7 @@ public KafkaBrokerConfigurationBuilder withUserConfiguration(KafkaConfiguration
             }
             if (metricReporters != null) {
                 // update the userConfig with the new list of metric reporters
-                userConfig.setConfigOption(KafkaCluster.KAFKA_METRIC_REPORTERS_CONFIG_FIELD, metricReporters);
+                userConfig.setConfigOption(KAFKA_METRIC_REPORTERS_CONFIG_FIELD, metricReporters);
             }
 
             printSectionHeader("User provided configuration");
@@ -852,7 +854,7 @@ public KafkaBrokerConfigurationBuilder withUserConfiguration(KafkaConfiguration
                 if (injectStrimziMetricsReporter) {
                     metricReporters = appendMetricReporter(metricReporters, StrimziMetricsReporterModel.KAFKA_PROMETHEUS_METRICS_REPORTER);
                 }
-                writer.println(KafkaCluster.KAFKA_METRIC_REPORTERS_CONFIG_FIELD + "=" + metricReporters);
+                writer.println(KAFKA_METRIC_REPORTERS_CONFIG_FIELD + "=" + metricReporters);
                 writer.println();
             }
         }

diff --git a/cluster-operator/src/main/java/io/strimzi/operator/cluster/model/KafkaCluster.java b/cluster-operator/src/main/java/io/strimzi/operator/cluster/model/KafkaCluster.java
@@ -132,11 +132,6 @@ public class KafkaCluster extends AbstractModel implements SupportsMetrics, Supp
     protected static final int KAFKA_AGENT_PORT = 8443;
     protected static final String KAFKA_AGENT_PORT_NAME = "tcp-kafkaagent";
 
-    /**
-     * The configuration field name for Kafka metric reporters.
-     */
-    public static final String KAFKA_METRIC_REPORTERS_CONFIG_FIELD = "metric.reporters";
-
     /**
      * Port number used for control plane
      */