gyfora commented on code in PR #561:
URL:
https://github.com/apache/flink-kubernetes-operator/pull/561#discussion_r1157015140
##########
flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/config/FlinkConfigBuilder.java:
##########
@@ -391,31 +399,87 @@ private static void setResource(
}
private static void setPodTemplate(
- Pod basicPod, Pod appendPod, Configuration effectiveConfig,
boolean isJM)
+ Pod basicPod,
+ Pod appendPod,
+ Resource resource,
+ Configuration effectiveConfig,
+ boolean isJM)
throws IOException {
- if (basicPod == null && appendPod == null) {
- return;
- }
-
// Avoid to create temporary pod template files for JobManager and
TaskManager if it is not
// configured explicitly via .spec.JobManagerSpec.podTemplate or
// .spec.TaskManagerSpec.podTemplate.
+ final ConfigOption<String> podConfigOption =
+ isJM
+ ? KubernetesConfigOptions.JOB_MANAGER_POD_TEMPLATE
+ : KubernetesConfigOptions.TASK_MANAGER_POD_TEMPLATE;
+
+ Pod basicPodWithResource = applyResourceToPodTemplate(basicPod,
resource);
Review Comment:
Honestly I don't think this logic will work like this due to how Pod merging
works by default. I recommend testing the following example which is likely to
be broken currently:
1. Top level (common) pod template empty.
2. TM/JM level pod Template define 2 containers: [init, main-container] , in
this order
I think the result will not be what you expect.
--
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]
