Skip to content

Commit

Permalink
Remove the external service name method from KafkaResources (#11023)
Browse files Browse the repository at this point in the history 
Signed-off-by: Jakub Scholz <[email protected]>
  • Loading branch information
@scholzj
scholzj authored Jan 11, 2025
1 parent 1f7df2a commit 8032536
Show file tree
Hide file tree
Showing 4 changed files with 11 additions and 27 deletions.
14 changes: 0 additions & 14 deletions api/src/main/java/io/strimzi/api/kafka/model/kafka/KafkaResources.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,20 +127,6 @@ public static String brokersServiceName(String clusterName) {
return clusterName + "-kafka-brokers";
}

/**
* Returns the name of the external bootstrap {@code Service} for a {@code Kafka} cluster of the given name.
* This {@code Service} will only exist if {@code Kafka.spec.kafka.listeners.external} is configured for a
* loadbalancer or NodePort in the {@code Kafka} resource with the given name. This is used only for the backwards
* compatible service names (listener name has to be `external` and port has to be 9094).
*
* @param clusterName The {@code metadata.name} of the {@code Kafka} resource.
*
* @return The name of the corresponding bootstrap {@code Service}.
*/
public static String externalBootstrapServiceName(String clusterName) {
return clusterName + "-kafka-external-bootstrap";
}

/**
* Returns the name of the Kafka metrics and log {@code ConfigMap} for a {@code Kafka} cluster of the given name.
* @param clusterName The {@code metadata.name} of the {@code Kafka} resource.
Expand Down
14 changes: 7 additions & 7 deletions ...r-operator/src/test/java/io/strimzi/operator/cluster/model/KafkaClusterListenersTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -769,7 +769,7 @@ public void testExternalRoutes() {
// Check external bootstrap service
List<Service> bootstrapServices = kc.generateExternalBootstrapServices();
assertThat(bootstrapServices.size(), is(1));
assertThat(bootstrapServices.get(0).getMetadata().getName(), is(KafkaResources.externalBootstrapServiceName(CLUSTER)));
assertThat(bootstrapServices.get(0).getMetadata().getName(), is(CLUSTER + "-kafka-external-bootstrap"));
assertThat(bootstrapServices.get(0).getSpec().getType(), is("ClusterIP"));
assertThat(bootstrapServices.get(0).getSpec().getSelector(), is(expectedBrokerSelectorLabels()));
assertThat(bootstrapServices.get(0).getSpec().getPorts().size(), is(1));
Expand Down Expand Up @@ -811,7 +811,7 @@ public void testExternalRoutes() {
assertThat(bootstrapRoutes.get(0).getMetadata().getName(), is(KafkaResources.bootstrapServiceName(CLUSTER)));
assertThat(bootstrapRoutes.get(0).getSpec().getTls().getTermination(), is("passthrough"));
assertThat(bootstrapRoutes.get(0).getSpec().getTo().getKind(), is("Service"));
assertThat(bootstrapRoutes.get(0).getSpec().getTo().getName(), is(KafkaResources.externalBootstrapServiceName(CLUSTER)));
assertThat(bootstrapRoutes.get(0).getSpec().getTo().getName(), is(CLUSTER + "-kafka-external-bootstrap"));
assertThat(bootstrapRoutes.get(0).getSpec().getPort().getTargetPort(), is(new IntOrString(9094)));
TestUtils.checkOwnerReference(bootstrapRoutes.get(0), KAFKA);

Expand Down Expand Up @@ -999,7 +999,7 @@ public void testExternalLoadBalancers() {
List<Service> bootstrapServices = kc.generateExternalBootstrapServices();
assertThat(bootstrapServices.size(), is(1));

assertThat(bootstrapServices.get(0).getMetadata().getName(), is(KafkaResources.externalBootstrapServiceName(CLUSTER)));
assertThat(bootstrapServices.get(0).getMetadata().getName(), is(CLUSTER + "-kafka-external-bootstrap"));
assertThat(bootstrapServices.get(0).getMetadata().getFinalizers(), is(List.of()));
assertThat(bootstrapServices.get(0).getSpec().getType(), is("LoadBalancer"));
assertThat(bootstrapServices.get(0).getSpec().getSelector(), is(expectedBrokerSelectorLabels()));
Expand Down Expand Up @@ -1420,7 +1420,7 @@ public void testExternalNodePorts() {
List<Service> bootstrapServices = kc.generateExternalBootstrapServices();
assertThat(bootstrapServices.size(), is(1));

assertThat(bootstrapServices.get(0).getMetadata().getName(), is(KafkaResources.externalBootstrapServiceName(CLUSTER)));
assertThat(bootstrapServices.get(0).getMetadata().getName(), is(CLUSTER + "-kafka-external-bootstrap"));
assertThat(bootstrapServices.get(0).getSpec().getType(), is("NodePort"));
assertThat(bootstrapServices.get(0).getSpec().getSelector(), is(expectedBrokerSelectorLabels()));
assertThat(bootstrapServices.get(0).getSpec().getPorts().size(), is(1));
Expand Down Expand Up @@ -1606,7 +1606,7 @@ public void testExternalNodePortOverrides() {

// Check external bootstrap service
Service ext = kc.generateExternalBootstrapServices().get(0);
assertThat(ext.getMetadata().getName(), is(KafkaResources.externalBootstrapServiceName(CLUSTER)));
assertThat(ext.getMetadata().getName(), is(CLUSTER + "-kafka-external-bootstrap"));
assertThat(ext.getSpec().getType(), is("NodePort"));
assertThat(ext.getSpec().getSelector(), is(expectedBrokerSelectorLabels()));
assertThat(ext.getSpec().getPorts().size(), is(1));
Expand Down Expand Up @@ -1889,7 +1889,7 @@ public void testExternalIngress() {
List<Service> bootstrapServices = kc.generateExternalBootstrapServices();
assertThat(bootstrapServices.size(), is(1));

assertThat(bootstrapServices.get(0).getMetadata().getName(), is(KafkaResources.externalBootstrapServiceName(CLUSTER)));
assertThat(bootstrapServices.get(0).getMetadata().getName(), is(CLUSTER + "-kafka-external-bootstrap"));
assertThat(bootstrapServices.get(0).getSpec().getType(), is("ClusterIP"));
assertThat(bootstrapServices.get(0).getSpec().getSelector(), is(expectedBrokerSelectorLabels()));
assertThat(bootstrapServices.get(0).getSpec().getPorts().size(), is(1));
Expand Down Expand Up @@ -1939,7 +1939,7 @@ public void testExternalIngress() {
assertThat(bootstrapIngresses.get(0).getSpec().getRules().get(0).getHost(), is("my-kafka-bootstrap.com"));
assertThat(bootstrapIngresses.get(0).getSpec().getRules().get(0).getHttp().getPaths().size(), is(1));
assertThat(bootstrapIngresses.get(0).getSpec().getRules().get(0).getHttp().getPaths().get(0).getPath(), is("/"));
assertThat(bootstrapIngresses.get(0).getSpec().getRules().get(0).getHttp().getPaths().get(0).getBackend().getService().getName(), is(KafkaResources.externalBootstrapServiceName(CLUSTER)));
assertThat(bootstrapIngresses.get(0).getSpec().getRules().get(0).getHttp().getPaths().get(0).getBackend().getService().getName(), is(CLUSTER + "-kafka-external-bootstrap"));
assertThat(bootstrapIngresses.get(0).getSpec().getRules().get(0).getHttp().getPaths().get(0).getBackend().getService().getPort().getNumber(), is(9094));
TestUtils.checkOwnerReference(bootstrapIngresses.get(0), KAFKA);

Expand Down
7 changes: 3 additions & 4 deletions systemtest/src/test/java/io/strimzi/systemtest/kafka/listeners/ListenersST.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -545,7 +545,7 @@ void testNodePort() {
if (listenerStatus.getName().equals(TestConstants.EXTERNAL_LISTENER_DEFAULT_NAME)) {
List<String> listStatusAddresses = listenerStatus.getAddresses().stream().map(ListenerAddress::getHost).sorted(Comparator.comparing(String::toString)).toList();
List<Integer> listStatusPorts = listenerStatus.getAddresses().stream().map(ListenerAddress::getPort).toList();
Integer nodePort = kubeClient(testStorage.getNamespaceName()).getService(testStorage.getNamespaceName(), KafkaResources.externalBootstrapServiceName(testStorage.getClusterName())).getSpec().getPorts().get(0).getNodePort();
Integer nodePort = kubeClient(testStorage.getNamespaceName()).getService(testStorage.getNamespaceName(), testStorage.getClusterName() + "-kafka-external-bootstrap").getSpec().getPorts().get(0).getNodePort();

List<String> nodeIPsBrokers = kubeClient(testStorage.getNamespaceName()).listPods(testStorage.getBrokerSelector())
.stream().map(pods -> pods.getStatus().getHostIP()).distinct().sorted(Comparator.comparing(String::toString)).toList();
Expand Down Expand Up @@ -643,9 +643,8 @@ void testOverrideNodePortConfiguration() {
.endSpec()
.build());

LOGGER.info("Checking nodePort to {} for bootstrap service {}", clusterBootstrapNodePort,
KafkaResources.externalBootstrapServiceName(testStorage.getClusterName()));
assertThat(kubeClient(testStorage.getNamespaceName()).getService(testStorage.getNamespaceName(), KafkaResources.externalBootstrapServiceName(testStorage.getClusterName()))
LOGGER.info("Checking nodePort to {} for bootstrap service {}", clusterBootstrapNodePort, testStorage.getClusterName() + "-kafka-external-bootstrap");
assertThat(kubeClient(testStorage.getNamespaceName()).getService(testStorage.getNamespaceName(), testStorage.getClusterName() + "-kafka-external-bootstrap")
.getSpec().getPorts().get(0).getNodePort(), is(clusterBootstrapNodePort));
String firstExternalService = KafkaResource.getStrimziPodSetName(testStorage.getClusterName(), KafkaNodePoolResource.getBrokerPoolName(testStorage.getClusterName())) + "-" + TestConstants.EXTERNAL_LISTENER_DEFAULT_NAME + "-" + 0;
LOGGER.info("Checking nodePort to {} for kafka-broker service {}", brokerNodePort, firstExternalService);
Expand Down
3 changes: 1 addition & 2 deletions systemtest/src/test/java/io/strimzi/systemtest/operators/CustomResourceStatusST.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@
import java.util.Map;
import java.util.stream.Collectors;

import static io.strimzi.api.kafka.model.kafka.KafkaResources.externalBootstrapServiceName;
import static io.strimzi.systemtest.TestTags.BRIDGE;
import static io.strimzi.systemtest.TestTags.CONNECT;
import static io.strimzi.systemtest.TestTags.CONNECTOR_OPERATOR;
Expand Down Expand Up @@ -432,7 +431,7 @@ void assertKafkaStatus(long expectedObservedGeneration, String internalAddress)
case TestConstants.EXTERNAL_LISTENER_DEFAULT_NAME:
Service extBootstrapService = kubeClient(Environment.TEST_SUITE_NAMESPACE).getClient().services()
.inNamespace(Environment.TEST_SUITE_NAMESPACE)
.withName(externalBootstrapServiceName(sharedTestStorage.getClusterName()))
.withName(sharedTestStorage.getClusterName() + "-kafka-external-bootstrap")
.get();
assertThat("External bootstrap has incorrect port", listener.getAddresses().get(0).getPort(), is(extBootstrapService.getSpec().getPorts().get(0).getNodePort()));
assertThat("External bootstrap has incorrect host", listener.getAddresses().get(0).getHost() != null);
Expand Down

0 comments on commit 8032536

Please sign in to comment.