[adestis-mf]

Description

This PR fixes scaling a CKS cluster on an isolated network that has external nodes (VMs added with addNodesToKubernetesCluster).

When the cluster is scaled, scaleKubernetesClusterIsolatedNetworkRules rebuilds the SSH rules on the source NAT IP: it revokes the firewall rule of the SSH port range (removeSshFirewallRule), removes the port forwarding rules in that range and calls setupKubernetesClusterIsolatedNetworkRules. That creates the range for the control and worker nodes that CKS deploys, 2222 to 2222+n-1 for n such nodes, and after it a single-port firewall rule and a port forwarding rule for each external node. The existing rules of the external nodes were not removed first:

• Scale-up: the new range overlaps the firewall rule of the first external node. FirewallManagerImpl.detectRulesConflict rejects it, because the ports overlap and protocol and source CIDR (0.0.0.0/0) are the same, and the scale fails with "unable to update network rules", caused by "Failed to provision firewall rules for SSH access" (CKS: Scaling of Kubernetes Cluster fails if there is external worker node is added #12633). By then the SSH firewall rule and the SSH port forwarding rules of the other nodes are removed, and they are not recreated.
• Scale-down: the external node gets new rules on its new port, and its old firewall and port forwarding rules stay on the old port.

Changes:

• removeExternalNodeSshRules runs before the rules are rebuilt. For each external node it revokes the port forwarding rules on the cluster's public IP from a port of the node SSH port range (2222 up to the SSH ports of the etcd nodes, cloud.kubernetes.etcd.node.start.port) to port 22, each together with the firewall rule for that port. The existing code then recreates them after the new range. The firewall rule is revoked first, so that if a revoke fails, a retry still finds it through the port forwarding rule. Both failures are reported as ManagementServerException, which the scale worker already handles.
• On scale-down the node VMs are now passed to the rebuild in the same order as on scale-up: the nodes deployed by CKS first, then the external nodes (getNodeVmIdsForSshRules). That is the order in which addFirewallRulesForNodes and provisionSshPortForwardingRules assign the SSH ports. Until now scale-down passed the VM map order. Once a scale-up has added a node after an external node, and a node before the external node in the VM map is then removed (by nodeids), that order put the external node before the remaining new node, and the port forwarding rules no longer matched the firewall rules. A later scale then failed on the same conflict.

As a result the SSH port of an external node moves with the range on every scale, as the existing code intends, and its old port is closed.

Clusters without external nodes get the same rules as before. The VPC tier path is only affected by the order of the node VMs on scale-down: with external nodes, their port forwarding rules now get the ports after the other nodes, as on scale-up. Shared networks are not affected.

Fixes: #12633

#14673 describes the same problem and is a duplicate of #12633.

This targets 4.22, where the bug exists (#12633 is on the 4.22.3 milestone). The commit merges forward to main without conflicts.

Scaling with nodeids that include an external node, which is what the node list in the UI does (#12641), failed with the same error: the range stays the same, and the recreated rule of the external node conflicts with its existing rule. Scaling never removes external nodes (scaleDownKubernetesClusterSize filters them out), but the node count was still lowered by the number of nodeids. With the conflict fixed, such a request would succeed without removing anything, so scaleKubernetesCluster now rejects external nodes in nodeids up front (validateKubernetesClusterScaleNodesToRemove); they are removed with removeNodesFromKubernetesCluster. The UI still offers the delete action for external nodes in the node list, which now gets this error; changing the UI to use removeNodesFromKubernetesCluster there (#12641) is left for a separate change.

#14670 (open, against main) adds the same SSH firewall rules on VPC tiers whose VPC has the Firewall service and rebuilds them the same way on scale. Once this change is merged forward, I'll call the new method in that path as well.

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):

How Has This Been Tested?

New unit tests in KubernetesClusterScaleWorkerTest and KubernetesClusterManagerImplTest. Three of them run scaleKubernetesClusterIsolatedNetworkRules with the actual removeSshFirewallRule, removePortForwardingRules, addFirewallRulesForNodes and provisionSshPortForwardingRules. The rules are kept in lists; a new firewall rule that overlaps an existing one is rejected, which models what detectRulesConflict does for these rules.

• Scale-up with two external nodes and an etcd node: 2222-2224 for the nodes deployed by CKS, 2225 and 2226 for the external nodes, each port forwarded to its node; the etcd rule is not touched. The rules of the external nodes are listed before the range rule, so removeSshFirewallRule would pick one of them if they were not removed first.
• Scale-up, then scale-down removing a worker that comes before the external node in the VM map, then scale-up again: after each step one rule per port and each port forwarded to the right node, no rule left on an old port.
• Scale-up without external nodes: the same rules as before, and no port forwarding rule is looked up or revoked by VM.
• getNodeVmIdsForSshRules: the external nodes come last, etcd nodes are left out.
• removeExternalNodeSshRules revokes an old SSH rule of an external node as well, and keeps its other rules: to another private port, from a port below 2222 or in the etcd range, and on another public IP.
• A failed port forwarding revoke is reported as ManagementServerException, and a second call removes what is left. A firewall revoke that returns false is reported, and the port forwarding rule stays.
• KubernetesClusterManagerImplTest: validateKubernetesClusterScaleNodesToRemove accepts nodes deployed by CKS and rejects a list with an external node.

Without the new call in scaleKubernetesClusterIsolatedNetworkRules, the two scale tests with external nodes fail with "Failed to provision firewall rules for SSH access": the new range 2222-2224 overlaps the old rule of an external node on 2224. When the scale-down passes the node VMs in VM map order, as before, it gives the external node port 2223 and the worker port 2224, while the single-port firewall rule on 2224 is the one created for the external node; the following scale-up then fails with the same error. If the external node rules are removed after removeSshFirewallRule instead of before, the scale-up test fails as well.

mvn -pl plugins/integrations/kubernetes-service -am test -Dtest='**/kubernetes/**/*Test' \
    -Dsurefire.failIfNoSpecifiedTests=false -DfailIfNoTests=false

Result with this change on top of 4.22 (b56bbc2), JDK 11 / Maven 3.10: BUILD SUCCESS, 87 tests in the kubernetes-service module, 0 failures, 0 errors; checkstyle 0 violations. The commit also applies on top of main (1881dc6): 137 tests, 0 failures, 0 errors; checkstyle 0 violations.

Live, on 4.23.0.0 without this change (KVM, isolated network created by CKS, v1.35.4, 1 control node, 1 worker and an external node from the CKS-ready Ubuntu 22.04 template; the code paths involved are the same as on 4.22):

• Scale-up to size=3 failed with "unable to update network rules": the new range 2222-2224 conflicted with the rule of the external node on 2224. Afterwards only the rules of the external node and 6443 were left, and the cluster was back in Running.
• Doing by hand what this change does, deleting the firewall and port forwarding rule of the external node, and scaling up again gave 2222-2224 for the control and worker nodes and 2225 for the external node. Each port reached the expected node, and all nodes were Ready.
• Removing the first worker by nodeids then forwarded 2223 to the external node and 2224 to the new worker, and left the old rules of the external node on 2225. The next scale-up failed again: removeSshFirewallRule revoked the old rule of the external node on 2225 instead of the range rule, and the new range conflicted with the old one.

The patched build itself has not been deployed to a live management server.

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

• Cluster without external nodes: nothing is revoked by the new method, and the rules are rebuilt as before.
• Only rules that match what CKS creates for an external node are revoked: a port forwarding rule of the external node on the cluster's public IP from a port of the node SSH port range to port 22, and the firewall rule for exactly that port. KubernetesClusterRemoveWorker finds the SSH port of a node in a similar way. A rule that a user created with exactly this pattern for an external node is revoked too; one from a port outside that range, to another port or on another IP is kept.
• A failed revoke stops the scale through the existing failure handling instead of leaving the cluster in Scaling, and a retry finds the remaining rules.
• removeSshFirewallRule is unchanged; kubernetes: prefer the node SSH rule when scaling a cluster with etcd nodes #14165 changes it, and this PR does not touch those lines.
• Any client that passes an external node in nodeids to scaleKubernetesCluster, such as the delete action in the UI's node list, now gets an error. The request fails with "External nodes can not be removed by scaling a cluster, use removeNodesFromKubernetesCluster", before any change to the cluster. Before, the same request failed later on isolated networks, after the SSH port forwarding rules of the other nodes had been removed, or, on VPC tiers, lowered the node count without removing the node (both from reading the code). Requests with only nodes deployed by CKS are validated as before.
• scaleKubernetesClusterIsolatedNetworkRules is now protected for the unit tests.