kubernetes: prefer the node SSH rule when scaling a cluster with etcd nodes

[nagaboinaramgopal]

Description

Fixes #14159

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

• BLOCKER
• Critical
• Major
• Minor
• Trivial

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.

[codecov]

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

@blueorangutan package

[blueorangutan]

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

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