DaanHoogland commented on a change in pull request #4461:
URL: https://github.com/apache/cloudstack/pull/4461#discussion_r521278137



##########
File path: 
plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterDestroyWorker.java
##########
@@ -239,7 +240,10 @@ public boolean destroy() throws CloudRuntimeException {
                 }
             } else {
                 try {
-                    deleteKubernetesClusterNetworkRules();
+                    NetworkVO kubernetesClusterNetwork = 
networkDao.findById(kubernetesCluster.getNetworkId());
+                    if (kubernetesClusterNetwork != null && 
kubernetesClusterNetwork.getGuestType() != Network.GuestType.Shared) {
+                        deleteKubernetesClusterNetworkRules();
+                    }

Review comment:
       I know I'm not a nice guy, but can we please have new code in separate 
methods:
   ```
   private void checkForRulesToDelete(){
       NetworkVO kubernetesClusterNetwork = 
networkDao.findById(kubernetesCluster.getNetworkId());
       if (kubernetesClusterNetwork != null && 
kubernetesClusterNetwork.getGuestType() != Network.GuestType.Shared) {
           deleteKubernetesClusterNetworkRules();
       }
   }
   ```
   and then
   ```suggestion
                       checkForRulesToDelete();
   ```
   




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

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to