[FLINK-37893][k8s] Make REST service port name configurable#28047
[FLINK-37893][k8s] Make REST service port name configurable#280471fanwang wants to merge 3 commits intoapache:masterfrom
Conversation
The Kubernetes REST service exposed by the JobManager hardcodes its port name to "rest" (Constants.REST_PORT_NAME). Operators in environments that enforce port-naming policies (e.g., service meshes that route by port name, or organizations with strict port-name conventions) cannot align Flink with those policies. This change adds a new config option kubernetes.rest-service.port-name (string, default "rest") that controls the port name on the REST Service. The default preserves existing behavior, making this a purely additive change. ServiceType#getRestPortFromExternalService no longer filters ports by name. Flink always builds the rest Service with a single port, so the lookup can simply return that port; this lets the reader work for any configured port name without plumbing the name through the abstract getRestEndpoint signature on ServiceType. KubernetesResourceManagerDriver#updateKubernetesServiceTargetPortIfNecessary reads the configured port name from flinkConfig when updating the rest service's target port in host-network mode.
fc792bc to
43d6396
Compare
…CE_PORT_NAME The new kubernetes.rest-service.port-name option was missing from the generated config docs, which made ConfigOptionsDocsCompletenessITCase fail in the flink-docs surefire integration-tests step.
Efrat19
left a comment
There was a problem hiding this comment.
Thank you for this contribution
LGTM % minor notes
| final List<ServicePort> ports = externalService.getSpec().getPorts(); | ||
|
|
||
| if (servicePortCandidates.isEmpty()) { | ||
| if (ports == null || ports.isEmpty()) { |
There was a problem hiding this comment.
Good catch — added a check that throws when the Service has more than one port (the invariant comes from buildUpExternalRestService building exactly one). New test testGetRestPortFromExternalServiceFailsWhenMultiplePorts covers it. 0cf59c1
| .withSelector(kubernetesJobManagerParameters.getSelectors()) | ||
| .addNewPort() | ||
| .withName(Constants.REST_PORT_NAME) | ||
| .withName(kubernetesJobManagerParameters.getRestServicePortName()) |
There was a problem hiding this comment.
Could you update container port name as well?
InitJobManagerDecorator.java#L192
There was a problem hiding this comment.
Done — InitJobManagerDecorator#getContainerPorts now reads the same configured name. I broadened the option's description to reflect that it covers both the Service port and the container port. New test testMainContainerRestPortNameWithCustomConfig covers it. 0cf59c1
…Service Address review feedback on the FLINK-37893 PR: - Make `kubernetes.rest-service.port-name` also apply to the JobManager container's rest port (InitJobManagerDecorator), not just the Service. Service meshes and policy engines that inspect port names end-to-end (svc -> pod -> container) need both to align. - `ServiceType#getRestPortFromExternalService` now rejects Services with more than one port. The invariant that the rest Service has exactly one port comes from `buildUpExternalRestService`; throwing on violation makes the contract explicit instead of silently picking ports.get(0). - Broaden the option's description to reflect the wider scope. - Add tests: - `InitJobManagerDecoratorTest#testMainContainerRestPortNameWithCustomConfig` - `ServiceTypeTest#testGetRestPortFromExternalServiceFailsWhenMultiplePorts` 108/108 tests pass under `./mvnw -pl flink-kubernetes -Dtest='*KubeClient*,*ServiceType*,*ServiceDecorator*,*JobManagerDecorator*,KubernetesResourceManagerDriverTest,KubernetesUtilsTest' test`.
What is the purpose of the change
The Kubernetes REST service exposed by the JobManager hardcodes its port name to
"rest"(Constants.REST_PORT_NAME). Operators in environments that enforce port-naming policies — for example, service meshes that route by port name, or organizations with strict port-name conventions — cannot align Flink's rest service with those policies.This change adds a new config option
kubernetes.rest-service.port-name(string, default"rest") that controls the port name on the REST Service. The default preserves existing behavior, making this a purely additive change.Brief change log
kubernetes.rest-service.port-nameinKubernetesConfigOptions(default"rest")KubernetesJobManagerParameters#getRestServicePortName()ServiceType#buildUpExternalRestServiceuses the configured name instead ofConstants.REST_PORT_NAMEServiceType#getRestPortFromExternalServiceno longer filters ports by name. Flink always builds the rest Service with a single port, so the lookup can simply return that port; this lets the reader work for any configured name without plumbing it through the abstractgetRestEndpointsignature onServiceType(which would change a public surface and require updating all subclasses).KubernetesResourceManagerDriver#updateKubernetesServiceTargetPortIfNecessaryreads the configured port name fromflinkConfigwhen updating the rest service's target port in host-network modeVerifying this change
This change added tests and can be verified as follows:
ExternalServiceDecoratorTest#testDefaultRestServicePortName— verifies the default port name"rest"is used when the option is unsetExternalServiceDecoratorTest#testCustomRestServicePortName— verifies a custom port name ("flink-rest") propagates to the built ServiceServiceTypeTest#testGetRestPortFromExternalServiceWithDefaultName— verifies the reader works for a Service with the default port name (back-compat)ServiceTypeTest#testGetRestPortFromExternalServiceWithCustomName— verifies the reader works for a Service with a custom port nameServiceTypeTest#testGetRestPortFromExternalServiceFailsWhenNoPorts— verifies the error path when the Service has no portsmvn -pl flink-kubernetes -Dtest='*KubeClient*,*ServiceType*,*ServiceDecorator*,*JobManagerDecorator*,KubernetesResourceManagerDriverTest,KubernetesUtilsTest' test)Does this pull request potentially affect one of the following parts:
@Public(Evolving): yes —KubernetesConfigOptionsis@PublicEvolving. Adds a new option (additive).KubernetesResourceManagerDriver. No behavior change at default.Documentation
withDescription(auto-generated into the Flink config docs)Was generative AI tooling used to co-author this PR?