Damans227 commented on code in PR #14165:
URL: https://github.com/apache/cloudstack/pull/14165#discussion_r4284837591


##########
plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterResourceModifierActionWorker.java:
##########
@@ -544,16 +544,23 @@ protected FirewallRule removeApiFirewallRule(final 
IpAddress publicIp) {
     }
 
     protected FirewallRule removeSshFirewallRule(final IpAddress publicIp, 
final long networkId) {
-        FirewallRule rule = null;
+        FirewallRuleVO rule = null;
         List<FirewallRuleVO> firewallRules = 
firewallRulesDao.listByIpPurposeProtocolAndNotRevoked(publicIp.getId(), 
FirewallRule.Purpose.Firewall, NetUtils.TCP_PROTO);
         for (FirewallRuleVO firewallRule : firewallRules) {
-            PortForwardingRuleVO pfRule = 
portForwardingRulesDao.findByNetworkAndPorts(networkId, 
firewallRule.getSourcePortStart(), firewallRule.getSourcePortEnd());
-            if (Objects.equals(firewallRule.getSourcePortStart(), 
CLUSTER_NODES_DEFAULT_START_SSH_PORT) || (Objects.nonNull(pfRule) && 
pfRule.getDestinationPortStart() == DEFAULT_SSH_PORT) ) {
+            if (Objects.equals(firewallRule.getSourcePortStart(), 
CLUSTER_NODES_DEFAULT_START_SSH_PORT)) {
                 rule = firewallRule;
-                firewallService.revokeIngressFwRule(firewallRule.getId(), 
true);
-                logger.debug("The SSH firewall rule {} with the id {} was 
revoked", firewallRule.getName(), firewallRule.getId());
                 break;
             }
+            if (rule == null) {
+                PortForwardingRuleVO pfRule = 
portForwardingRulesDao.findByNetworkAndPorts(networkId, 
firewallRule.getSourcePortStart(), firewallRule.getSourcePortEnd());
+                if (Objects.nonNull(pfRule) && 
pfRule.getDestinationPortStart() == DEFAULT_SSH_PORT) {

Review Comment:
   the cap below the etcd start port in 75edb36 covers it



##########
plugins/integrations/kubernetes-service/src/main/java/com/cloud/kubernetes/cluster/actionworkers/KubernetesClusterResourceModifierActionWorker.java:
##########
@@ -544,16 +544,23 @@ protected FirewallRule removeApiFirewallRule(final 
IpAddress publicIp) {
     }
 
     protected FirewallRule removeSshFirewallRule(final IpAddress publicIp, 
final long networkId) {
-        FirewallRule rule = null;
+        FirewallRuleVO rule = null;
         List<FirewallRuleVO> firewallRules = 
firewallRulesDao.listByIpPurposeProtocolAndNotRevoked(publicIp.getId(), 
FirewallRule.Purpose.Firewall, NetUtils.TCP_PROTO);
         for (FirewallRuleVO firewallRule : firewallRules) {
-            PortForwardingRuleVO pfRule = 
portForwardingRulesDao.findByNetworkAndPorts(networkId, 
firewallRule.getSourcePortStart(), firewallRule.getSourcePortEnd());
-            if (Objects.equals(firewallRule.getSourcePortStart(), 
CLUSTER_NODES_DEFAULT_START_SSH_PORT) || (Objects.nonNull(pfRule) && 
pfRule.getDestinationPortStart() == DEFAULT_SSH_PORT) ) {
+            if (Objects.equals(firewallRule.getSourcePortStart(), 
CLUSTER_NODES_DEFAULT_START_SSH_PORT)) {
                 rule = firewallRule;
-                firewallService.revokeIngressFwRule(firewallRule.getId(), 
true);
-                logger.debug("The SSH firewall rule {} with the id {} was 
revoked", firewallRule.getName(), firewallRule.getId());
                 break;
             }
+            if (rule == null) {
+                PortForwardingRuleVO pfRule = 
portForwardingRulesDao.findByNetworkAndPorts(networkId, 
firewallRule.getSourcePortStart(), firewallRule.getSourcePortEnd());
+                if (Objects.nonNull(pfRule) && 
pfRule.getDestinationPortStart() == DEFAULT_SSH_PORT) {
+                    rule = firewallRule;
+                }
+            }
+        }
+        if (rule != null) {

Review Comment:
   revoking the etcd rules on destroy in 75edb36 covers it



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

Reply via email to