gyfora commented on code in PR #561:
URL:
https://github.com/apache/flink-kubernetes-operator/pull/561#discussion_r1157007742
##########
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);
+
if (appendPod != null) {
- final ConfigOption<String> podConfigOption =
- isJM
- ? KubernetesConfigOptions.JOB_MANAGER_POD_TEMPLATE
- :
KubernetesConfigOptions.TASK_MANAGER_POD_TEMPLATE;
- effectiveConfig.setString(
- podConfigOption,
- createTempFile(
- mergePodTemplates(
- basicPod,
- appendPod,
- effectiveConfig.get(
- KubernetesOperatorConfigOptions
-
.POD_TEMPLATE_MERGE_BY_NAME))));
+ Pod mergedPodTemplate =
+ mergePodTemplates(
+ basicPodWithResource,
+ appendPod,
+ effectiveConfig.get(
+
KubernetesOperatorConfigOptions.POD_TEMPLATE_MERGE_BY_NAME));
+ effectiveConfig.setString(podConfigOption,
createTempFile(mergedPodTemplate));
+ } else {
+ effectiveConfig.setString(podConfigOption,
createTempFile(basicPodWithResource));
+ }
+ }
+
+ private static Pod applyResourceToPodTemplate(Pod podTemplate, Resource
resource) {
+ if (resource == null
+ ||
StringUtils.isNullOrWhitespaceOnly(resource.getEphemeralStorage())) {
+ return podTemplate;
}
+
+ if (podTemplate == null || podTemplate.getSpec() == null) {
+ PodSpec spec = new PodSpec();
+ spec.getContainers()
+ .add(
+ decorateContainerWithEphemeralStorage(
+ new Container(),
resource.getEphemeralStorage()));
+ Pod newPodTemplate = new Pod();
+ newPodTemplate.setSpec(spec);
+ return newPodTemplate;
Review Comment:
If only the spec is null, this logic overwrites the entire pod, that sounds
incorrect. Imagine the user only set the metadata. Please add a test to guard
against this.
--
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]
