kubernetes: prefer the node SSH rule when scaling a cluster with etcd nodes by rootnomad · Pull Request #14165 · apache/cloudstack

rootnomad · 2026-09-14T18:25:20Z

Description

When a CKS cluster on an isolated network is scaled, scaleKubernetesClusterIsolatedNetworkRules revokes the SSH firewall rule found by removeSshFirewallRule and then removes the SSH port forwarding rules from port 2222 up to that rule's end port, before creating the rules again for the new node count.

removeSshFirewallRule takes the first TCP firewall rule on the source NAT IP that either starts at 2222 or has a port forwarding rule to port 22. Clusters with separate etcd nodes also have single port firewall rules (from cloud.kubernetes.etcd.node.start.port, 50000 by default) that forward to port 22, and the rules are listed without an order. After a scale up the node rule is created again, so it comes after the etcd rules, and the next scale picks an etcd rule instead. The bound for removing the port forwarding rules then becomes 2222 to 50000, which also removes the etcd node's forwarding rule, the node rule is left in place, and creating the node rule for the new size fails with a conflict, as described in the issue.

This makes removeSshFirewallRule prefer the rule that starts at 2222, which is always created for the control and worker nodes, and only fall back to the port forwarding match when there is no such rule. The destroy path uses the same method, so it now revokes the node rule there too.

Separate etcd nodes came with #9102, so this targets 4.22.

Types of changes

Breaking change (fix or feature that would cause existing functionality to change)
New feature (non-breaking change which adds functionality)
Bug fix (non-breaking change which fixes an issue)
Enhancement (improves an existing feature and functionality)
Cleanup (Code refactoring and cleanup, that may add test cases)
build/CI
test (unit or integration test code)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Major
Minor

Bug Severity

Screenshots (if appropriate):

N/A

How Has This Been Tested?

Unit tests in KubernetesClusterResourceModifierActionWorkerTest for the etcd rule listed before the node rule, the fallback to a port forwarded rule, and no SSH rule at all. The first one fails before the change and passes after it; the kubernetes-service tests pass.

Live tested on a KVM zone running a 4.22 build, with the v1.34.7 CKS ISO (it includes etcd) and a cluster with 1 control node, 1 worker node and 1 etcd node on an isolated network, cloud.kubernetes.etcd.node.start.port left at 50000. One etcd node is enough to hit it. The TCP firewall rules on the source NAT IP are shown in the order they are listed.

Before the change, after creating the cluster:

2222-2223      control and worker SSH
6443-6443      API
50000-50000    etcd SSH, forwards to 22

After scaling to 2 workers the node rule is created again, so it is now listed last:

6443-6443
50000-50000    forwards to 22
2222-2224

Scaling back to 1 worker fails:

Scaling failed for Kubernetes cluster ... unable to update network rules

The etcd rule on 50000 is revoked instead of the node rule, all SSH port forwarding rules are removed, including the etcd node's forward on 50000, 2222-2224 is left in place and the cluster goes to Alert.

With 3 etcd nodes, the layout from the issue, it is the same. After scaling to 2 workers:

6443-6443
50000-50000    forwards to 22
50001-50001    forwards to 22
50002-50002    forwards to 22
2222-2224

Scaling back to 1 worker fails with the same error. The rule on 50000 is revoked, the forwards from 2222 up to 50000 are removed (the control and worker nodes and the first etcd node), the forwards on 50001 and 50002 stay, 2222-2224 is left in place and the cluster goes to Alert.

With the change, on a new cluster with the same layout, the rules after create and after scaling to 2 workers are listed the same way as before, with the node rule after the etcd rule:

6443-6443
50000-50000    forwards to 22
2222-2224

Scaling back to 1 worker now works. The node rule is revoked and created again for the new size, and the etcd rule and its forward stay:

6443-6443
50000-50000    forwards to 22
2222-2223

port forwards: 2222 control, 2223 worker, 50000 etcd

The cluster stays Running with 1 control node, 1 worker node and 1 etcd node.

With 3 etcd nodes, scaling 1 to 2 to 1 to 2 to 1 works each time. After every scale up the node rule is listed after the etcd rules, and every scale down revokes the node rule and creates it again for the new size, while the etcd rules and their forwards stay:

after scaling to 2    6443, 50000, 50001, 50002, then 2222-2224
after scaling to 1    6443, 50000, 50001, 50002, then 2222-2223

port forwards after scaling to 1: 2222 control, 2223 worker, 50000, 50001 and 50002 etcd

The cluster stays Running with 1 control node, 1 worker node and 3 etcd nodes.

How did you try to break this feature and the system with this change?

Scaled up and down twice on the fixed build with the etcd rules listed before the node rule, which is the case that failed before. Deleted the clusters with the change in place, since the delete path also uses removeSshFirewallRule, and nothing was left behind. Created a cluster without etcd nodes and scaled it 1 to 2 to 1, the node rule went 2222-2223, 2222-2224, 2222-2223 as before. When there is no rule starting at 2222, the port forwarding match from before is still used, which the unit tests cover.

… nodes removeSshFirewallRule took the first TCP firewall rule on the source NAT IP that either starts at the node SSH start port or forwards to port 22. Clusters with separate etcd nodes also have single port rules that forward to port 22, and the rules are listed without an order. After a scale up the node SSH rule is recreated, so an etcd rule can come first and be revoked instead. The scale worker then used its end port as the bound for removing SSH port forwards, removing the etcd forwards too and failing to recreate the node rule. Prefer the rule that starts at the node SSH start port and only fall back to the port forwarding match when no such rule exists. Fixes apache#14159

codecov · 2026-09-15T06:30:04Z

Codecov Report

❌ Patch coverage is 80.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 17.94%. Comparing base (e38b7a7) to head (60ca507).
⚠️ Report is 12 commits behind head on 4.22.

Files with missing lines	Patch %	Lines
...KubernetesClusterResourceModifierActionWorker.java	80.00%	0 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff            @@
##               4.22   #14165   +/-   ##
=========================================
  Coverage     17.94%   17.94%           
- Complexity    16147    16151    +4     
=========================================
  Files          5928     5928           
  Lines        535188   535192    +4     
  Branches      65499    65502    +3     
=========================================
+ Hits          96020    96034   +14     
+ Misses       428240   428228   -12     
- Partials      10928    10930    +2

Flag	Coverage Δ
uitests	`4.02% <ø> (ø)`
unittests	`19.01% <80.00%> (+<0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

kiranchavala · 2026-09-15T06:47:18Z

@blueorangutan package

blueorangutan · 2026-09-15T06:48:04Z

@kiranchavala a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

blueorangutan · 2026-09-15T07:52:41Z

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ el10 ✔️ debian ✔️ suse15. SL-JID 19245

DaanHoogland

clgtm

DaanHoogland · 2026-09-30T12:46:22Z

@kiranchavala , were you planning to test this?

Damans227 · 2026-10-15T13:11:37Z

            }
+            if (rule == null) {
+                PortForwardingRuleVO pfRule = portForwardingRulesDao.findByNetworkAndPorts(networkId, firewallRule.getSourcePortStart(), firewallRule.getSourcePortEnd());
+                if (Objects.nonNull(pfRule) && pfRule.getDestinationPortStart() == DEFAULT_SSH_PORT) {


if the 2222 rule was deleted by hand, can this still pick an etcd rule and wipe the etcd forwarding again? the caller already falls back to a safe range when nothing is found

Good catch. You are right that with the 2222 range rule gone, the fallback matches the first rule forwarding to 22, and the etcd rules at 50000+ do, so it could grab one and the scale path would then wipe 2222..50000. Since the caller already bounds its cleanup safely on a null return, I capped the fallback below KubernetesEtcdNodeStartPort so it never crosses into the etcd range, with a test for the no-node-rule case. Pushed in 75edb36.

the cap below the etcd start port in 75edb36 covers it

Damans227 · 2026-10-15T13:11:38Z

+                }
+            }
+        }
+        if (rule != null) {


on destroy in a network the user keeps, should the etcd rules from 50000 up go too? they stay open and the next cluster with etcd nodes would clash on those ports

Agreed, the etcd firewall rules at 50000+ were not being revoked on destroy and would linger in a network the user keeps, clashing with the next etcd cluster on those ports. Fixed in the same push: the destroy path now revokes the etcd ingress rules alongside the node and API rules, with a test. 75edb36.

revoking the etcd rules on destroy in 75edb36 covers it

The SSH-rule lookup fell back to any firewall rule forwarding to port 22 when the node range rule (2222) was absent. etcd nodes also forward 50000+ to 22, so with the node rule deleted by hand the fallback could pick an etcd rule and the scale path would then wipe 2222..50000. Bound the fallback below KubernetesEtcdNodeStartPort; the caller already falls back to a safe range when no rule is returned. Also revoke the etcd ingress firewall rules on cluster destroy. They were left open in a network the user keeps, so the next cluster with etcd nodes clashed on those ports.

Damans227

thanks @rootnomad, fix looks right. LGTM

boring-cyborg Bot added the component:kubernetes label Sep 14, 2026

rootnomad marked this pull request as ready for review September 14, 2026 18:40

kiranchavala requested review from Pearl1594 and nvazquez September 15, 2026 06:21

DaanHoogland approved these changes Sep 30, 2026

View reviewed changes

DaanHoogland added the needs-testing label Sep 30, 2026

DaanHoogland linked an issue Sep 30, 2026 that may be closed by this pull request

CKS: scale-down with separate etcd revokes wrong SSH rules and fails (4.22.1.1) #14159

Open

DaanHoogland added this to the 4.22.2 milestone Sep 30, 2026

DaanHoogland assigned kiranchavala Sep 30, 2026

Damans227 reviewed Oct 15, 2026

View reviewed changes

Damans227 approved these changes Oct 15, 2026

View reviewed changes

Conversation

rootnomad commented Sep 14, 2026

Description

Types of changes

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

Bug Severity

Screenshots (if appropriate):

How Has This Been Tested?

How did you try to break this feature and the system with this change?

Uh oh!

codecov Bot commented Sep 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

kiranchavala commented Sep 15, 2026

Uh oh!

blueorangutan commented Sep 15, 2026

Uh oh!

blueorangutan commented Sep 15, 2026

Uh oh!

DaanHoogland left a comment

Choose a reason for hiding this comment

Uh oh!

DaanHoogland commented Sep 30, 2026

Uh oh!

Damans227 Oct 15, 2026

Choose a reason for hiding this comment

Uh oh!

rootnomad Oct 15, 2026

Choose a reason for hiding this comment

Uh oh!

Damans227 Oct 15, 2026

Choose a reason for hiding this comment

Uh oh!

Damans227 Oct 15, 2026

Choose a reason for hiding this comment

Uh oh!

rootnomad Oct 15, 2026

Choose a reason for hiding this comment

Uh oh!

Damans227 Oct 15, 2026

Choose a reason for hiding this comment

Uh oh!

Damans227 left a comment

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

codecov Bot commented Sep 15, 2026 •

edited

Loading