apache / apache/cloudstack

Bug: agent, vm cleanup - iptables/ipset chain existence not checked before deletion, causing misleading error logs

Abierto
#12,770 6 comentarios 0 reacciones 1 asignado Reclamado por @dheeraj12347 Ver en GitHub
component:kvm good-first-issue type:improvement
Lenguaje dominante
Java
Estrellas
3.1k
Forks
1.4k
Merge medio
6 d 19 h
PR fusionados (30 d)
32

Descripción

## Title

**Cleanup: iptables/ipset chain existence not checked before deletion, causing misleading error logs**

##### ISSUE TYPE
* Bug Report

##### COMPONENT NAME
`scripts/vm/network/security_group.py`

##### CLOUDSTACK VERSION
main branch (4.22)

##### SUMMARY
When VM cleanup runs, the `destroy_network_rules_for_vm()` function attempts to flush and delete iptables chains and ipsets without first checking if they exist. The `execute()` function logs `CalledProcessError` at ERROR level with full traceback before re-raising, causing confusing duplicate log entries when chains don't exist. Exit code 1 from iptables/ipset is normal when a chain/set doesn't exist (idempotent cleanup), not a real error.

##### STEPS TO REPRODUCE
1. Create a VM with security groups in CloudStack
2. Delete the VM
3. Observe management server logs for the VM cleanup output

##### EXPECTED RESULTS
- Cleanup should be idempotent - safe to run multiple times
- "Chain doesn't exist" should be logged at DEBUG level
- Only real errors should be logged at ERROR level
- No duplicate logging of the same issue

##### ACTUAL RESULTS
```
2026-03-09 09:46:52,842 ERROR [c.c.v.s.security_group] (Thread-123) Command exited non-zero: iptables -X i-6-526-VM-eg
Traceback (most recent call last):
File "/usr/share/cloudstack-common/scripts/vm/network/security_group.py", line 53, in execute
return check_output(cmd, shell=True).decode()
File "/usr/lib/python3.13/subprocess.py", line 472, in check_output
return run(*popenargs, stdout=PIPE, timeout=timeout, check=True, ...)
File "/usr/lib/python3.13/subprocess.py", line 577, in run
raise CalledProcessError(retcode, process.args, output=stdout, stderr=stderr)
subprocess.CalledProcessError: Command 'iptables -X i-6-526-VM-eg' returned non-zero exit status 1.

2026-03-09 09:46:52,842 DEBUG Ignoring failure to delete chain: i-6-526-VM-eg
```

The ERROR level log with full traceback is misleading - this is expected behavior when chains don't exist.

##### PROPOSED FIX

The fix should check if chains exist before attempting deletion. When `iptables -S chain` returns exit code 1, the chain doesn't exist - this is normal, not a real error. Only real errors (other exit codes) should be logged.

Add helper functions:

```python
def iptables_chain_exists(chain):
"""Check if iptables chain exists"""
try:
execute("iptables -S %s 2>/dev/null" % chain)
return True
except CalledProcessError as e:
if e.returncode == 1:
return False # Chain not found - not an error
raise # Real error (other exit code)

def ipset_exists(setname):
"""Check if ipset exists"""
try:
execute("ipset list %s 2>/dev/null" % setname)
return True
except CalledProcessError as e:
if e.returncode == 1:
return False # Set not found - not an error
raise # Real error (other exit code)
```

Update `destroy_network_rules_for_vm()` to check existence before deletion:

```python
for chain in [_f for _f in chains if _f]:
if iptables_chain_exists(chain):
try:
execute("iptables -F " + chain)
execute("iptables -X " + chain)
except Exception as e:
logging.error("Failed to flush/delete chain %s: %s", chain, str(e))
else:
logging.debug("Chain %s does not exist, skipping", chain)

for ipset in [vm_ipsetname, vm_ipsetname + '-6']:
if ipset_exists(ipset):
try:
execute('ipset -F ' + ipset)
execute('ipset -X ' + ipset)
except Exception as e:
logging.error("Failed to flush/delete ipset %s: %s", ipset, str(e))
else:
logging.debug("Ipset %s does not exist, skipping", ipset)
```

This approach:
- Makes cleanup idempotent (safe to run multiple times)
- Provides clear logging: "chain doesn't exist" at DEBUG, real errors at ERROR
- Distinguishes between "not found" (exit code 1, expected) and real errors (other exit codes)
- No duplicate logging

Guía de contribución

Abrir la guía de contribución

Evaluación

Este issue todavía no se ha evaluado.

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.