wangyang0918 commented on a change in pull request #11715: [FLINK-16598][k8s] 
Respect the rest-port exposed by the external Service when retrieving Endpoint
URL: https://github.com/apache/flink/pull/11715#discussion_r409364554
 
 

 ##########
 File path: 
flink-kubernetes/src/main/java/org/apache/flink/kubernetes/kubeclient/Fabric8FlinkKubeClient.java
 ##########
 @@ -264,17 +263,32 @@ private void setOwnerReference(Deployment deployment, 
List<HasMetadata> resource
        }
 
        /**
-        * To get nodePort of configured ports.
+        * Get rest port from the external Service.
         */
-       private int getServiceNodePort(Service service, ConfigOption<Integer> 
configPort) {
-               final int port = this.flinkConfig.getInteger(configPort);
-               if (service.getSpec() != null && service.getSpec().getPorts() 
!= null) {
-                       for (ServicePort p : service.getSpec().getPorts()) {
-                               if (p.getPort() == port) {
-                                       return p.getNodePort();
-                               }
-                       }
+       private int getRestPortFromExternalService(Service externalService) {
+               final List<ServicePort> servicePortCandidates = 
externalService.getSpec().getPorts()
+                       .stream()
+                       .filter(x -> 
x.getName().equals(Constants.REST_PORT_NAME))
+                       .collect(Collectors.toList());
+
+               if (servicePortCandidates.isEmpty()) {
+                       throw new RuntimeException("Failed to find port \"" + 
Constants.REST_PORT_NAME + "\" in Service \"" +
+                               
KubernetesUtils.getRestServiceName(this.clusterId) + "\"");
+               }
+
+               final ServicePort externalServicePort = 
servicePortCandidates.get(0);
+
+               final KubernetesConfigOptions.ServiceExposedType 
externalServiceType =
+                       
KubernetesConfigOptions.ServiceExposedType.valueOf(externalService.getSpec().getType());
+
+               switch (externalServiceType) {
+                       case ClusterIP:
+                       case LoadBalancer:
 
 Review comment:
   Glad to hear that you will create a new ticket to rework the 
`getRestEndpoint` according to `LoadBalacer` or `NodePort`. I think this change 
makes sense to me.
   
   Currently, the problem in the unmanaged K8s cluster without LD is Flink 
client print a wrong JobManager url, which is in such schema `K8s_Master:8081`, 
it should be `K8s_Master:NodePort`. I think this could be fixed in your 
upcoming tickets.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

Reply via email to