morhidi commented on code in PR #271:
URL: 
https://github.com/apache/flink-kubernetes-operator/pull/271#discussion_r901584493


##########
flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/observer/deployment/AbstractDeploymentObserver.java:
##########
@@ -252,6 +257,60 @@ private void onMissingDeployment(FlinkDeployment 
deployment) {
                 EventUtils.Component.JobManagerDeployment);
     }
 
+    /**
+     * Checks a deployment that is currently in the UPGRADING state whether it 
was already deployed
+     * but we simply miss the status information. After comparing the target 
resource generation
+     * with the one from the possible deployment if they match we update the 
status to the already
+     * DEPLOYED state.
+     *
+     * @param flinkDep Flink resource to check.
+     * @param context Context for reconciliation.
+     */
+    private void checkIfAlreadyUpgraded(FlinkDeployment flinkDep, Context 
context) {
+        Optional<Deployment> depOpt = 
context.getSecondaryResource(Deployment.class);
+        var status = flinkDep.getStatus();
+        depOpt.ifPresent(
+                deployment -> {
+                    Map<String, String> annotations = 
deployment.getMetadata().getAnnotations();
+                    if (annotations == null) {
+                        return;
+                    }
+                    Long deployedGeneration =
+                            
Optional.ofNullable(annotations.get(FlinkUtils.CR_GENERATION_LABEL))
+                                    .map(Long::valueOf)
+                                    .orElse(-1L);
+
+                    // For first deployments we get the generation from the 
metadata directly
+                    // otherwise take it simply from the lastReconciledSpec
+                    Long upgradeTargetGeneration =
+                            Optional.ofNullable(
+                                            status.getReconciliationStatus()
+                                                    
.deserializeLastReconciledSpecWithMeta())
+                                    .map(t -> 
t.f1.get("metadata").get("generation").asLong(-1L))
+                                    
.orElse(flinkDep.getMetadata().getGeneration());
+
+                    if (deployedGeneration.equals(upgradeTargetGeneration)) {
+                        logger.info(
+                                "Last reconciled generation is already 
deployed, setting reconciliation status to "
+                                        + ReconciliationState.DEPLOYED);
+
+                        var firstDeploy =

Review Comment:
   Can we move the firstDeploy out of the `if` condition and use it when 
calculating the upgradeTargetGeneration?. The logic is hidden in the lambdas, 
here, makes hard to understand the code.



-- 
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

Reply via email to