wangyang0918 commented on pull request #14629:
URL: https://github.com/apache/flink/pull/14629#issuecomment-782592922


   @xintongsong Thanks for the review. Address your concerns inline and update 
the PR.
   
   > It seems the current implementation cannot support setting the same 
template file for both JM / TM, due to the different main container names. 
Maybe it's not necessary to have different main container names for JM/TM.
   
   Yes, it does not support sharing the pod template at my first 
implementation. Because I used to believe only the JobManager needs init 
container. And this will make JobManager and TaskManager pod templates always 
different. Inspired by you and after more consideration, I think it makes sense 
to let them share a same pod template since it is easier to use and harmless. I 
have unified the main container name and introduced a common config option 
`kubernetes.pod-template-file` in this update.
   
   > It's not clear to me what kind of compatibility guarantees we provide for 
the pod templates. Would it be possible that something user defined in the 
template works well in the current Flink version and is overwritten in a new 
version? I understand it's hard to guarantee that all templates are compatible 
across versions, but maybe we can define a set of commonly used functions that 
is guaranteed (guarded by tests) compatible across versions.
   
   I think it is possible that a pod template works well in current Flink 
version and does not work in a new Flink version. This only happens when we 
introduce a new config option and overwrite(not merge) the corresponding 
Kubernetes field. This should rarely happen unless we have no other choices. 
Then we will document them and publish in the release note.
   
   We could guarantee to our users that a white list fields(e.g. annotations, 
labels, envs, init container, sidecar container, volumes) in pod template that 
will always be respected. And I have enriched the tests in 
`KubernetesJobManagerFactoryWithPodTemplateTest` and 
`KubernetesTaskManagerFactoryWithPodTemplateTest` to guide them.


----------------------------------------------------------------
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:
[email protected]


Reply via email to