wangyang0918 commented on code in PR #20982:
URL: https://github.com/apache/flink/pull/20982#discussion_r1003093842
##########
flink-kubernetes/src/main/java/org/apache/flink/kubernetes/utils/KubernetesUtils.java:
##########
@@ -487,6 +489,22 @@ public static <T> String resolveUserDefinedValue(
return resolvedValue;
}
+ /**
+ * Resolve the DNS policy defined value. First check if there is a DNS
Policy override in
+ * PodTemplate, if not, check if host network is enabled.
+ *
+ * @param dnsPolicy DNS Policy defined in PodTemplate spec
+ * @param hostNetworkEnabled Host Network enabled
+ * @return the resolved value
+ */
+ public static String resolveDNSPolicy(String dnsPolicy, boolean
hostNetworkEnabled) {
Review Comment:
Given that the Flink configuration options have a higher priority, I think
`DNS_POLICY_HOSTNETWORK` need to be used if the host network enabled by
`kubernetes.hostnetwork.enabled` explicitly.
And we need a test to guard this behavior.
##########
flink-kubernetes/src/main/java/org/apache/flink/kubernetes/utils/Constants.java:
##########
@@ -25,8 +25,8 @@ public class Constants {
public static final String API_VERSION = "v1";
public static final String APPS_API_VERSION = "apps/v1";
- public static final String DNS_PLOICY_DEFAULT = "ClusterFirst";
- public static final String DNS_PLOICY_HOSTNETWORK =
"ClusterFirstWithHostNet";
+ public static final String DNS_POLICY_DEFAULT = "ClusterFirst";
+ public static final String DNS_POLICY_HOSTNETWORK =
"ClusterFirstWithHostNet";
Review Comment:
Nice catch.
##########
flink-kubernetes/src/test/java/org/apache/flink/kubernetes/kubeclient/factory/KubernetesJobManagerFactoryTest.java:
##########
@@ -468,4 +468,37 @@ void testSetJobManagerDeploymentReplicas() throws
Exception {
assertThat(kubernetesJobManagerSpecification.getDeployment().getSpec().getReplicas())
.isEqualTo(JOBMANAGER_REPLICAS);
}
+
+ @Test
Review Comment:
The new added two tests might be more appropriate to be put in
`InitJobManagerDecoratorTest`.
##########
flink-kubernetes/src/test/java/org/apache/flink/kubernetes/kubeclient/factory/KubernetesJobManagerFactoryTest.java:
##########
@@ -468,4 +468,37 @@ void testSetJobManagerDeploymentReplicas() throws
Exception {
assertThat(kubernetesJobManagerSpecification.getDeployment().getSpec().getReplicas())
.isEqualTo(JOBMANAGER_REPLICAS);
}
+
+ @Test
+ void testDNSPolicyDefaultValue() throws Exception {
+ kubernetesJobManagerSpecification =
+
KubernetesJobManagerFactory.buildKubernetesJobManagerSpecification(
+ flinkPod, kubernetesJobManagerParameters);
+
+ final PodSpec resultPodSpecWithDefaultDNSPolicy =
+ this.kubernetesJobManagerSpecification
+ .getDeployment()
+ .getSpec()
+ .getTemplate()
+ .getSpec();
+
+
assertThat(resultPodSpecWithDefaultDNSPolicy.getDnsPolicy()).isEqualTo("ClusterFirst");
Review Comment:
Maybe we could `Constants.DNS_POLICY_DEFAULT` here.
--
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]
