Skip to content

Commit d20a48d

Browse files
committed
test: fix test_network_extension_namespace.py
1 parent 9e3366c commit d20a48d

2 files changed

Lines changed: 59 additions & 12 deletions

File tree

extensions/network-namespace/network-namespace-wrapper.sh

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -791,14 +791,25 @@ cmd_destroy() {
791791

792792
local vsd; vsd=$(_vpc_state_dir)
793793

794-
# Remove public veth pairs tracked under VPC/net state dir
794+
# Remove public veth pairs that belong to THIS tier (guarded by .tier file).
795+
# IPs owned by other tiers are left untouched so those tiers keep working.
796+
# Backward compat: if no .tier file exists assume the IP belongs here.
795797
if [ -d "${vsd}/ips" ]; then
796798
for f in "${vsd}/ips/"*.pvlan; do
797799
[ -f "${f}" ] || continue
800+
local tier_f; tier_f="${f%.pvlan}.tier"
801+
if [ -f "${tier_f}" ]; then
802+
local owner_tier; owner_tier=$(cat "${tier_f}" 2>/dev/null || true)
803+
if [ -n "${owner_tier}" ] && [ "${owner_tier}" != "${NETWORK_ID}" ]; then
804+
log "destroy: skipping veth for $(basename "${f%.pvlan}") (owned by tier ${owner_tier})"
805+
continue
806+
fi
807+
fi
798808
local pvlan pveth_h
799809
pvlan=$(cat "${f}")
800810
pveth_h=$(pub_veth_host_name "${pvlan}" "${CHOSEN_ID}")
801811
ip link del "${pveth_h}" 2>/dev/null || true
812+
rm -f "${f}" "${f%.pvlan}" "${tier_f}" 2>/dev/null || true
802813
done
803814
fi
804815

@@ -925,6 +936,8 @@ cmd_assign_ip() {
925936
echo "${SOURCE_NAT}" > "${vsd}/ips/${PUBLIC_IP}"
926937
# Save public VLAN so add-static-nat / add-port-forward can look it up
927938
echo "${PUBLIC_VLAN}" > "${vsd}/ips/${PUBLIC_IP}.pvlan"
939+
# Save owning tier (network ID) so cmd_destroy only cleans up its own IPs
940+
echo "${NETWORK_ID}" > "${vsd}/ips/${PUBLIC_IP}.tier"
928941

929942
_dump_iptables "${NAMESPACE}"
930943
release_lock
@@ -1006,7 +1019,8 @@ cmd_release_ip() {
10061019
fi
10071020

10081021
rm -f "${vsd}/ips/${PUBLIC_IP}" \
1009-
"${vsd}/ips/${PUBLIC_IP}.pvlan"
1022+
"${vsd}/ips/${PUBLIC_IP}.pvlan" \
1023+
"${vsd}/ips/${PUBLIC_IP}.tier"
10101024

10111025
_dump_iptables "${NAMESPACE}"
10121026
release_lock

test/integration/smoke/test_network_extension_namespace.py

Lines changed: 43 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
updateNetworkServiceProvider,
4848
deleteNetworkServiceProvider,
4949
createFirewallRule,
50+
deleteFirewallRule,
5051
listPublicIpAddresses)
5152
from marvin.lib.base import (Account,
5253
Extension,
@@ -455,7 +456,7 @@ def setUp(self):
455456
self._ssh_private_key_file = None
456457

457458
def tearDown(self):
458-
self._safe_teardown()
459+
#self._safe_teardown()
459460
try:
460461
cleanup_resources(self.apiclient, self.cleanup)
461462
except Exception as e:
@@ -517,7 +518,6 @@ def _deploy_scripts(self):
517518

518519
self.mgmt_deployer = MgmtServerDeployer(self.mgtSvrDetails,
519520
logger=self.logger)
520-
# Extension path is the entrypoint file path; append .sh if omitted.
521521
self._mgmt_script_path = (self.extension_path or "").strip().rstrip('/')
522522
self.mgmt_deployer.copy_file(entry_point_src, self._mgmt_script_path)
523523
self.logger.info("network-namespace.sh deployed to mgmt at %s",
@@ -627,6 +627,18 @@ def _create_firewall_rule_for_ssh(self, ipaddressid):
627627
rule.id, ipaddressid)
628628
return rule.id
629629

630+
def _delete_firewall_rule(self, fw_rule_id):
631+
"""Delete a firewall rule by ID (best-effort, warns on failure)."""
632+
if not fw_rule_id:
633+
return
634+
cmd = deleteFirewallRule.deleteFirewallRuleCmd()
635+
cmd.id = fw_rule_id
636+
try:
637+
self.apiclient.deleteFirewallRule(cmd)
638+
self.logger.info("FW rule %s deleted", fw_rule_id)
639+
except Exception as e:
640+
self.logger.warning("Could not delete FW rule %s: %s", fw_rule_id, e)
641+
630642
def _get_source_nat_ip(self, network_id):
631643
"""Return the source NAT public IP object for *network_id*, or None."""
632644
cmd = listPublicIpAddresses.listPublicIpAddressesCmd()
@@ -1053,6 +1065,11 @@ def test_05_isolated_network_full_lifecycle(self):
10531065

10541066
# ==============================================================
10551067
# A. Static NAT
1068+
#
1069+
# StaticNATRule.enable() does NOT auto-create a firewall rule, so we
1070+
# must create one explicitly. StaticNATRule.disable() internally
1071+
# calls revokeFirewallRulesForIp(), which cascade-deletes our rule,
1072+
# so no explicit _delete_firewall_rule() is needed afterwards.
10561073
# ==============================================================
10571074
self.logger.info("--- Sub-test A: Static NAT ---")
10581075
snat_ip_obj = PublicIPAddress.create(
@@ -1070,13 +1087,15 @@ def test_05_isolated_network_full_lifecycle(self):
10701087
virtualmachineid=vm.id,
10711088
networkid=network.id)
10721089
self.logger.info("Static NAT enabled on %s", snat_ip)
1073-
1090+
# Explicit FW rule required — static NAT does not auto-create one
10741091
self._create_firewall_rule_for_ssh(snat_ip_id)
1092+
10751093
self._assert_vm_ssh_accessible(
10761094
snat_ip, 22,
10771095
"SSH via static NAT %s:22 should succeed" % snat_ip)
10781096
self.logger.info("Verified: SSH works via static NAT %s", snat_ip)
10791097

1098+
# disable() cascades revokeFirewallRulesForIp() — deletes the FW rule
10801099
StaticNATRule.disable(self.apiclient, ipaddressid=snat_ip_id)
10811100
self.logger.info("Static NAT disabled on %s", snat_ip)
10821101
self._assert_vm_ssh_not_accessible(
@@ -1087,6 +1106,12 @@ def test_05_isolated_network_full_lifecycle(self):
10871106

10881107
# ==============================================================
10891108
# B. Port forwarding
1109+
#
1110+
# NATRule.create() passes openFirewall=True (default for non-VPC),
1111+
# so CloudStack automatically creates a TCP/22 FW rule with
1112+
# relatedRuleId=pf_rule.id. pf_rule.delete() cascade-removes it.
1113+
# Do NOT call _create_firewall_rule_for_ssh() — that would conflict
1114+
# with the auto-created rule.
10901115
# ==============================================================
10911116
self.logger.info("--- Sub-test B: Port forwarding ---")
10921117
pf_ip_obj = PublicIPAddress.create(
@@ -1106,14 +1131,15 @@ def test_05_isolated_network_full_lifecycle(self):
11061131
networkid=network.id
11071132
)
11081133
self.assertIsNotNone(pf_rule)
1109-
self.logger.info("PF rule created: %s:22 → VM:22", pf_ip)
1134+
self.logger.info("PF rule created: %s:22 → VM:22 (FW rule auto-created)",
1135+
pf_ip)
11101136

1111-
self._create_firewall_rule_for_ssh(pf_ip_id)
11121137
self._assert_vm_ssh_accessible(
11131138
pf_ip, 22,
11141139
"SSH via PF %s:22 should succeed" % pf_ip)
11151140
self.logger.info("Verified: SSH works via port forwarding %s", pf_ip)
11161141

1142+
# delete() cascades revokeRelatedFirewallRule() — removes auto FW rule
11171143
pf_rule.delete(self.apiclient)
11181144
self.logger.info("PF rule deleted on %s", pf_ip)
11191145
self._assert_vm_ssh_not_accessible(
@@ -1124,6 +1150,9 @@ def test_05_isolated_network_full_lifecycle(self):
11241150

11251151
# ==============================================================
11261152
# C. Load balancer (haproxy)
1153+
#
1154+
# LoadBalancerRule.create() also uses openFirewall=True by default,
1155+
# auto-creating a TCP/22 FW rule. lb_rule.delete() cascades it.
11271156
# ==============================================================
11281157
self.logger.info("--- Sub-test C: Load balancer ---")
11291158
lb_ip_obj = PublicIPAddress.create(
@@ -1149,9 +1178,9 @@ def test_05_isolated_network_full_lifecycle(self):
11491178
)
11501179
self.assertIsNotNone(lb_rule)
11511180
lb_rule.assign(self.apiclient, vms=[vm])
1152-
self.logger.info("LB rule created, VM assigned: %s:22", lb_ip)
1181+
self.logger.info("LB rule created, VM assigned: %s:22 (FW rule auto-created)",
1182+
lb_ip)
11531183

1154-
self._create_firewall_rule_for_ssh(lb_ip_id)
11551184
self._assert_vm_ssh_accessible(
11561185
lb_ip, 22,
11571186
"SSH via LB %s:22 should succeed (haproxy required on KVM hosts)"
@@ -1165,6 +1194,10 @@ def test_05_isolated_network_full_lifecycle(self):
11651194

11661195
# ==============================================================
11671196
# D. Network restart (cleanup=True)
1197+
#
1198+
# NATRule.create() auto-creates the TCP/22 FW rule.
1199+
# After network restart both the PF rule and the FW rule must be
1200+
# re-applied by the extension, so SSH should work again.
11681201
# ==============================================================
11691202
self.logger.info("--- Sub-test D: Network restart ---")
11701203
rst_ip_obj = PublicIPAddress.create(
@@ -1183,7 +1216,6 @@ def test_05_isolated_network_full_lifecycle(self):
11831216
ipaddressid=rst_ip_id,
11841217
networkid=network.id
11851218
)
1186-
self._create_firewall_rule_for_ssh(rst_ip_id)
11871219
self._assert_vm_ssh_accessible(
11881220
rst_ip, 22,
11891221
"SSH via %s:22 should work before restart" % rst_ip)
@@ -1251,7 +1283,7 @@ def test_06_vpc_multi_tier_and_restart(self):
12511283
self.logger.info("VPC tier offering '%s' enabled", vpc_tier_offering.name)
12521284

12531285
# ---- VPC offering ----
1254-
vpc_svc = "SourceNat,StaticNat,PortForwarding,Lb,UserData"
1286+
vpc_svc = "SourceNat,StaticNat,PortForwarding,Lb,UserData,Dhcp,Dns"
12551287
_vpc_prov = {s.strip(): ext_name for s in vpc_svc.split(',')}
12561288
vpc_offering = VpcOffering.create(self.apiclient, {
12571289
"name": "ExtNet-VPC-%s" % random_gen(),
@@ -1428,7 +1460,8 @@ def test_06_vpc_multi_tier_and_restart(self):
14281460
self.cleanup = [o for o in self.cleanup if o != tier1]
14291461
self.logger.info("Tier 1 VM + network deleted")
14301462

1431-
# Tier 2 must remain accessible
1463+
# Tier 2 must remain accessible — cmd_destroy() on tier-1 must not
1464+
# delete tier-2's public veth (fixed via .tier ownership tracking).
14321465
self._assert_vm_ssh_accessible(
14331466
tier2_ip, 22,
14341467
"SSH to tier-2 VM via LB must still work after tier-1 deleted")

0 commit comments

Comments
 (0)