wangyang0918 commented on a change in pull request #18854:
URL: https://github.com/apache/flink/pull/18854#discussion_r825573905
##########
File path:
flink-kubernetes/src/test/java/org/apache/flink/kubernetes/kubeclient/Fabric8FlinkKubeClientTest.java
##########
@@ -250,6 +251,41 @@ public void testCreateFlinkTaskManagerPod() throws
Exception {
resultTaskManagerPod.getMetadata().getOwnerReferences().get(0).getUid());
}
+ @Test
+ public void testCreateTwoTaskManagerPods() throws Exception {
+ Fabric8FlinkKubeClient localClient =
+ new Fabric8FlinkKubeClient(
+ flinkConfig,
+ kubeClient,
+ Executors.newFixedThreadPool(2, new
ExecutorThreadFactory("Testing-IO")));
+
localClient.createJobManagerComponent(this.kubernetesJobManagerSpecification);
+ final KubernetesPod kubernetesPod1 =
+ new KubernetesPod(
+ new PodBuilder()
+ .editOrNewMetadata()
+ .withName("mock-task-manager-pod1")
+ .endMetadata()
+ .editOrNewSpec()
+ .endSpec()
+ .build());
Review comment:
This could be factor out into a new method `private KubernetesPod
buildTaskManagerPod(String name)` and deduplicate all other similar use cases.
##########
File path:
flink-kubernetes/src/test/java/org/apache/flink/kubernetes/kubeclient/Fabric8FlinkKubeClientTest.java
##########
@@ -250,6 +251,41 @@ public void testCreateFlinkTaskManagerPod() throws
Exception {
resultTaskManagerPod.getMetadata().getOwnerReferences().get(0).getUid());
}
+ @Test
+ public void testCreateTwoTaskManagerPods() throws Exception {
Review comment:
I suggest to introduce the
`KubernetesClientTestBase#mockGetDeploymentWithError` to avoid exposing the
internal state of `Fabric8FlinkKubeClient`.
```
protected void mockGetDeploymentWithError(String deploymentId) {
final String path =
String.format(
"/apis/apps/v1/namespaces/%s/deployments/%s",
NAMESPACE, deploymentId);
server.expect()
.get()
.withPath(path)
.andReturn(500, "Expected error")
.always();
}
```
After then, the test could be simplified like following. Please make sure
the test failed before this PR and pass after this PR.
```
@Test
public void testCreateTwoTaskManagerPods() throws Exception {
flinkKubeClient.createJobManagerComponent(this.kubernetesJobManagerSpecification);
flinkKubeClient.createTaskManagerPod(buildTaskManagerPod("mock-task-manager-pod1")).get();
mockGetDeploymentWithError(CLUSTER_ID);
try {
flinkKubeClient
.createTaskManagerPod(buildTaskManagerPod("mock-task-manager-pod2"))
.get();
} catch (Throwable throwable) {
fail("Should not get the JobManager deployment more than once");
}
}
```
--
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]
