Copilot commented on code in PR #28047:
URL: https://github.com/apache/flink/pull/28047#discussion_r4565524631
##########
flink-kubernetes/src/main/java/org/apache/flink/kubernetes/kubeclient/services/ServiceType.java:
##########
@@ -109,25 +108,29 @@ public abstract Optional<Endpoint> getRestEndpoint(
*/
public abstract String getType();
- /** Get rest port from the external Service. */
+ /**
+ * Get rest port from the external Service. The rest Service is built with
exactly one port
+ * (whose name is configurable via {@link
KubernetesConfigOptions#REST_SERVICE_PORT_NAME}), so
+ * the lookup does not depend on the port name. If the Service has zero or
more than one port,
+ * this method throws — the invariant comes from {@link
#buildUpExternalRestService}.
+ */
public int getRestPortFromExternalService(Service externalService) {
- final List<ServicePort> servicePortCandidates =
- externalService.getSpec().getPorts().stream()
- .filter(x ->
x.getName().equals(Constants.REST_PORT_NAME))
- .collect(Collectors.toList());
+ final List<ServicePort> ports = externalService.getSpec().getPorts();
+ final String serviceName = externalService.getMetadata().getName();
- if (servicePortCandidates.isEmpty()) {
+ if (ports == null || ports.isEmpty()) {
throw new RuntimeException(
- "Failed to find port \""
- + Constants.REST_PORT_NAME
- + "\" in Service \""
- + externalService.getMetadata().getName()
- + "\"");
+ "Failed to find any port in Service \"" + serviceName +
"\"");
+ }
+ if (ports.size() > 1) {
Review Comment:
Endpoint discovery reads the live Kubernetes Service, so an operator-added
port now makes cluster retrieval fail even with the default configuration;
previously, the named `rest` port was selected. Preserve lookup by the
configured REST port name instead of rejecting all multi-port Services. Update
`ServiceTypeTest#testGetRestPortFromExternalServiceFailsWhenMultiplePorts` to
verify REST-port selection instead.
##########
flink-kubernetes/src/main/java/org/apache/flink/kubernetes/kubeclient/decorators/InitJobManagerDecorator.java:
##########
@@ -189,7 +189,7 @@ private List<ContainerPort> getContainerPorts() {
}
return Arrays.asList(
new ContainerPortBuilder()
- .withName(Constants.REST_PORT_NAME)
+
.withName(kubernetesJobManagerParameters.getRestServicePortName())
Review Comment:
Setting `kubernetes.rest-service.port-name` to `jobmanager-rpc` or
`blobserver` satisfies the documented naming rules but produces duplicate names
in the JobManager container, since the other two ports retain those names.
Kubernetes rejects duplicate container-port names, so deployment fails outside
host-network mode. Please reject collisions before building the Pod, or keep
the container port name independent of the configurable Service port name; the
Service already uses a numeric targetPort.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
