wangyang0918 commented on a change in pull request #16674:
URL: https://github.com/apache/flink/pull/16674#discussion_r681431919
##########
File path:
flink-kubernetes/src/test/java/org/apache/flink/kubernetes/kubeclient/factory/KubernetesJobManagerFactoryTest.java
##########
@@ -139,6 +140,12 @@ public void testDeploymentMetadata() throws IOException {
expectedLabels.putAll(userLabels);
assertEquals(expectedLabels,
resultDeployment.getMetadata().getLabels());
+ for (Map.Entry<String, String> entry : userAnnotations.entrySet()) {
Review comment:
Maybe we could use
`assertThat(resultDeployment.getMetadata().getAnnotations(),
equalTo(userAnnotations));` instead.
##########
File path:
flink-kubernetes/src/main/java/org/apache/flink/kubernetes/kubeclient/factory/KubernetesJobManagerFactory.java
##########
@@ -106,6 +106,7 @@ private static Deployment createJobManagerDeployment(
.withName(
KubernetesUtils.getDeploymentName(
kubernetesJobManagerParameters.getClusterId()))
+
.addToAnnotations(kubernetesJobManagerParameters.getAnnotations())
Review comment:
Could we use `withAnnotations` here? AFAIK, we should only set the
`kubernetesJobManagerParameters.getAnnotations()` to the deployment.
##########
File path:
flink-kubernetes/src/test/java/org/apache/flink/kubernetes/kubeclient/factory/KubernetesJobManagerFactoryTest.java
##########
@@ -161,6 +168,12 @@ public void testDeploymentSpec() throws IOException {
assertEquals(expectedLabels,
resultDeploymentSpec.getTemplate().getMetadata().getLabels());
assertEquals(expectedLabels,
resultDeploymentSpec.getSelector().getMatchLabels());
+ for (Map.Entry<String, String> entry : userAnnotations.entrySet()) {
Review comment:
Same as above.
##########
File path:
flink-kubernetes/src/test/java/org/apache/flink/kubernetes/kubeclient/factory/KubernetesJobManagerFactoryTest.java
##########
@@ -203,6 +213,10 @@ public void testPodSpec() throws IOException {
assertEquals(3, resultedMainContainer.getArgs().size());
assertEquals(3, resultedMainContainer.getVolumeMounts().size());
+
Review comment:
I do not think we need to add such verification 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: issues-unsubscr...@flink.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
