You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2022/05/11 09:36:07 UTC

[GitHub] [flink-kubernetes-operator] Aitozi opened a new pull request, #203: [FLINK-27337] Prevent session cluster to be deleted when there are ru…

Aitozi opened a new pull request, #203:
URL: https://github.com/apache/flink-kubernetes-operator/pull/203

   …nning jobs
   
   This PR is meant to ensure all the session jobs are deleted before cleanup the session cluster. 


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

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


[GitHub] [flink-kubernetes-operator] Aitozi commented on a diff in pull request #203: [FLINK-27337] Prevent session cluster to be deleted when there are ru…

Posted by GitBox <gi...@apache.org>.
Aitozi commented on code in PR #203:
URL: https://github.com/apache/flink-kubernetes-operator/pull/203#discussion_r870165869


##########
flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/reconciler/deployment/SessionReconciler.java:
##########
@@ -134,4 +142,31 @@ protected void shutdown(FlinkDeployment deployment) {
         flinkService.deleteClusterDeployment(
                 deployment.getMetadata(), deployment.getStatus(), true);
     }
+
+    @Override
+    public DeleteControl cleanup(FlinkDeployment flinkApp, Context context) {
+        var sessionJobs =
+                informerManager
+                        .getSessionJobInformer(flinkApp.getMetadata().getNamespace())
+                        .getIndexer()
+                        .byIndex(OperatorUtils.CLUSTER_ID_INDEX, flinkApp.getMetadata().getName());
+        if (!sessionJobs.isEmpty()) {
+            var error =
+                    String.format(
+                            "The session jobs %s should be deleted first",
+                            sessionJobs.stream()
+                                    .map(job -> job.getMetadata().getName())
+                                    .collect(Collectors.toList()));
+            // TODO generate error events for this

Review Comment:
   I think events will be better here to help user know what happen. But, I have problem when generate the events here, I found that the example in `DeploymentFailedException#asEvent` can not work (It seems lack some of required parameters.)  I will improve this after figuring it out.



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

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


[GitHub] [flink-kubernetes-operator] Aitozi commented on a diff in pull request #203: [FLINK-27337] Prevent session cluster to be deleted when there are ru…

Posted by GitBox <gi...@apache.org>.
Aitozi commented on code in PR #203:
URL: https://github.com/apache/flink-kubernetes-operator/pull/203#discussion_r870094286


##########
flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/controller/FlinkControllerConfig.java:
##########
@@ -37,4 +37,9 @@ public FlinkControllerConfig(Reconciler<CR> reconciler, Set<String> watchedNames
     public Set<String> getNamespaces() {
         return watchedNamespaces;
     }
+
+    @Override
+    public Set<String> getEffectiveNamespaces() {

Review Comment:
   This PR change the behavior of the effectiveNamespaces, it will make it easy to know what is the current effective namespace direct from the config. By this, we can easily to create `InformerManger` in the webhook module and do not have to consider what is the current effective namespaces in the operator. 



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

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


[GitHub] [flink-kubernetes-operator] Aitozi commented on a diff in pull request #203: [FLINK-27337] Prevent session cluster to be deleted when there are ru…

Posted by GitBox <gi...@apache.org>.
Aitozi commented on code in PR #203:
URL: https://github.com/apache/flink-kubernetes-operator/pull/203#discussion_r870094286


##########
flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/controller/FlinkControllerConfig.java:
##########
@@ -37,4 +37,9 @@ public FlinkControllerConfig(Reconciler<CR> reconciler, Set<String> watchedNames
     public Set<String> getNamespaces() {
         return watchedNamespaces;
     }
+
+    @Override
+    public Set<String> getEffectiveNamespaces() {

Review Comment:
   This PR change the behavior of the effectiveNamespaces, it will make the webhook and the operator watch the same collection of the namespace. 
   Previously, the operator will use the internal (java-operator-sdk's) logic to generate the effective watched namespace, it may be inconsistent the webhook's.
   After this, we can easily  create `InformerManger` in the webhook module by using the watchedNamespace generated by the config or environments. 



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

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


[GitHub] [flink-kubernetes-operator] Aitozi commented on pull request #203: [FLINK-27337] Prevent session cluster to be deleted when there are ru…

Posted by GitBox <gi...@apache.org>.
Aitozi commented on PR #203:
URL: https://github.com/apache/flink-kubernetes-operator/pull/203#issuecomment-1125982252

   force pushed to solve conflicts


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

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


[GitHub] [flink-kubernetes-operator] gyfora merged pull request #203: [FLINK-27337] Prevent session cluster to be deleted when there are ru…

Posted by GitBox <gi...@apache.org>.
gyfora merged PR #203:
URL: https://github.com/apache/flink-kubernetes-operator/pull/203


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

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


[GitHub] [flink-kubernetes-operator] Aitozi commented on pull request #203: [FLINK-27337] Prevent session cluster to be deleted when there are ru…

Posted by GitBox <gi...@apache.org>.
Aitozi commented on PR #203:
URL: https://github.com/apache/flink-kubernetes-operator/pull/203#issuecomment-1123471295

   cc @gyfora @wangyang0918 


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

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