[GitHub] [flink-kubernetes-operator] wangyang0918 commented on a change in pull request #103: [FLINK-26830] Transition to SUSPEND before redeploying on job upgrades

[GitBox <gi...@apache.org>]

wangyang0918 commented on a change in pull request #103:
URL: https://github.com/apache/flink-kubernetes-operator/pull/103#discussion_r833874394



##########
File path: flink-kubernetes-operator/src/test/java/org/apache/flink/kubernetes/operator/controller/FlinkDeploymentControllerTest.java
##########
@@ -403,8 +425,18 @@ public void testUpgradeNotReadyCluster(FlinkDeployment appCluster, boolean allow
 
             flinkService.setPortReady(true);
             testController.reconcile(appCluster, context);
-            testController.reconcile(appCluster, context);
-
+            if (appCluster.getSpec().getJob() != null

Review comment:
       All the tests could pass without this `if...else...` change.  Do I miss something?

##########
File path: flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/reconciler/ReconciliationUtils.java
##########
@@ -93,6 +101,26 @@ public static void updateForReconciliationError(FlinkDeployment flinkApp, String
         } else {
             updateControl = UpdateControl.noUpdate();
         }
-        return updateControl;
+
+        if (!reschedule) {
+            return updateControl;
+        }
+
+        ReconciliationStatus reconciliationStatus = current.getStatus().getReconciliationStatus();
+        if (current.getSpec().getJob() != null
+                && current.getSpec().getJob().getState() == JobState.RUNNING
+                && reconciliationStatus != null
+                && reconciliationStatus.isSuccess()
+                && reconciliationStatus.getLastReconciledSpec().getJob().getState()
+                        == JobState.SUSPENDED) {
+            return updateControl.rescheduleAfter(0);

Review comment:
       Even though, we schedule a ZERO delay reconciliation, we still have a chance other CR changes sneak in and then applied together with next reconciliation. Maybe this is not a problem since we are always orienting toward the final state.




-- 
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: commits-unsubscribe@flink.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org