bgeng777 commented on a change in pull request #131:
URL:
https://github.com/apache/flink-kubernetes-operator/pull/131#discussion_r840381900
##########
File path:
flink-kubernetes-operator/src/test/java/org/apache/flink/kubernetes/operator/controller/FlinkDeploymentControllerTest.java
##########
@@ -397,7 +402,59 @@ public void testUpgradeNotReadyCluster() {
testUpgradeNotReadyCluster(appCluster, false);
}
- public void testUpgradeNotReadyCluster(FlinkDeployment appCluster, boolean
allowUpgrade) {
+ @Test
+ public void verifyReconcileWithBadConfig() {
+
+ FlinkDeployment appCluster = TestUtils.buildApplicationCluster();
+ UpdateControl<FlinkDeployment> updateControl;
+ // Override rest port, and it should be saved in lastReconciledSpec
once a successful
+ // reconcile() finishes.
+
appCluster.getSpec().getFlinkConfiguration().put(RestOptions.PORT.key(),
"8088");
+ updateControl = testController.reconcile(appCluster, context);
+ assertTrue(updateControl.isUpdateStatus());
+ assertEquals(
+ JobManagerDeploymentStatus.DEPLOYING,
+ appCluster.getStatus().getJobManagerDeploymentStatus());
+
+ // Check when the bad config is applied, observe() will change the
cluster state correctly
+ appCluster.getSpec().getJobManager().setReplicas(-1);
+ // Next reconcile will set error msg and observe with previous
validated config
+ updateControl = testController.reconcile(appCluster, context);
+ assertEquals(
+ "JobManager replicas should not be configured less than one.",
+ appCluster.getStatus().getReconciliationStatus().getError());
+ assertTrue(updateControl.isUpdateStatus());
+ assertEquals(
+ JobManagerDeploymentStatus.DEPLOYED_NOT_READY,
+ appCluster.getStatus().getJobManagerDeploymentStatus());
+
+ // Check the exception
+ appCluster.getSpec().getJobManager().setReplicas(1);
+ appCluster.getSpec().getJob().setJarURI(null);
+ assertDoesNotThrow(
Review comment:
Not sure if I understand correctly: you are right that the
`assertDoesNotThrow` is redundant here.
The idea of this part is that when `setJarURI(null)`, if we get effective
config before validation, the test will fail with exception due to
`getEffectiveConfig()`. But as long as it can successfully finish, we guarantee
that validation is done before `getEffectiveConfig()`.
I removed extra `assertDoesNotThrow`. Let me know if that is enough.
--
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]
