apache / apache/cloudstack-extensions
Improvements to the network-wrapper script
- Langage dominant
- Aucune donnée de langage
- Étoiles
- 4
- Forks
- 4
- Merge moyen
- 1 h
- PR mergées (30 j)
- 3
Description
Multiple bugs in `Network-Namespace/network-namespace-wrapper.sh` prevent it from working out of the box
## Summary
Following the setup instructions in the CloudStack `NetworkExtension` protocol doc
(`framework/extensions/.../network/README.md` in apache/cloudstack) to register this
reference implementation as a `NetworkOrchestrator` extension (CloudStack 4.23,
KVM, physical network registered with `guest.network.device`/`public.network.device`),
I hit six distinct issues before a VM could successfully deploy onto a network backed
by this script. Filing them together since they were found in one debugging pass and
several are related.
**Environment:** Apache CloudStack 4.23.0, KVM hosts on Ubuntu 24.04, `network-namespace`
branch, script fetched fresh via the documented raw.githubusercontent.com URL.
---
## 1. `ensure_host_bridge()`'s return-via-stdout leaks into the extension's real stdout
`ensure_host_bridge()` returns its result the usual bash way — `echo "${br}"` — meant to
be captured with `br=$(ensure_host_bridge ...)`. But at **four call sites** it's invoked
as a bare statement, with the actual bridge name recomputed separately on the next line
via `host_bridge_name()`:
- `cmd_implement_network()` (guest bridge): `ensure_host_bridge "${GUEST_ETH}" "${VLAN}"`
- `cmd_assign_ip()` (public bridge): `ensure_host_bridge "${PUB_ETH}" "${PUBLIC_VLAN}"`
- two more identical public-bridge call sites in the VPC source-NAT / source-NAT-IP-update paths
Since CloudStack's `NetworkExtensionElement` treats the command's stdout as the payload
to parse, this leaked bridge name (e.g. `breth0-600`, or later `cloudbr0` once VLAN was
`untagged`) corrupts the real output and gets logged as `Ignoring non-object script output: ...`
or worse, treated as the entire (invalid) response.
**Fix:** redirect all four calls to `/dev/null`:
```bash
ensure_host_bridge "${GUEST_ETH}" "${VLAN}" >/dev/null
...
ensure_host_bridge "${PUB_ETH}" "${PUBLIC_VLAN}" >/dev/null
```
---
## 2. `implement-network` never emits the required `network.broadcast_uri` / `network.broadcast_domain_type` JSON
Per the protocol doc, whenever the extension has the detail
`network.isolation.method=NetworkExtension` set (which selects
`NetworkExtensionGuestNetworkGuru` in CloudStack core), `implement-network`'s stdout
**must** be a JSON object containing `network.broadcast_domain_type` and
`network.broadcast_uri` — enforced unconditionally by
`NetworkExtensionElement.applyNetworkUpdateFromScriptOutput()` in CloudStack core.
`cmd_implement_network()` never emits these keys anywhere in the script, so every
`implement-network` call fails with:
```
Script output is missing required network properties 'network.broadcast_uri' and
'network.broadcast_domain_type' for network ...
```
**Fix:** emit the JSON at the end of `cmd_implement_network()`, e.g.:
```bash
if [ -n "${VLAN}" ]; then
printf '{"network.broadcast_domain_type":"Vlan","network.broadcast_uri":"vlan://%s"}\n' "${VLAN}"
fi
```
---
## 3. No handling for an `untagged` public VLAN
When a zone's public IP range has no VLAN tag, CloudStack passes the literal string
`"untagged"` as `payload.public_vlan`. `ensure_host_bridge()`/`host_bridge_name()` pass
this straight through to:
```bash
ip link add link "${eth}" name "${vif}" type vlan id "${vlan}"
```
which fails immediately since VLAN IDs must be numeric:
```
Error: argument "untagged" is wrong: id is invalid
```
This is a completely normal, common CloudStack zone configuration (public network
without 802.1q tagging), not an edge case.
**Fix:** special-case `vlan == "untagged"` in both functions to reuse the NIC's
*existing* bridge (e.g. `cloudbr0`, detected via `ip -o link show | grep -o 'master [^ ]*'`)
instead of trying to create a fresh VLAN sub-interface + bridge.
---
## 4. Related to #3 — `teardown_host_bridge_if_unused()` would delete the host's shared bridge
Once #3 is fixed so that `host_bridge_name()` can return an *existing*, host-critical
bridge like `cloudbr0` for the untagged case, `teardown_host_bridge_if_unused()` — which
runs `ip link del "${br}"` whenever it thinks the bridge has no other members — becomes
dangerous: it did not create `cloudbr0` and has no business deleting it, but nothing
in the current code prevents it from trying once that bridge is "unused" by this
script's own veth accounting.
**Fix:** short-circuit `teardown_host_bridge_if_unused()` to a no-op whenever
`vlan == "untagged"`.
---
## 5. `pub_veth_host_name()` / `pub_veth_ns_name()` don't apply the existing 15-char shortening logic
The **guest**-side equivalents (`veth_host_name()`, `veth_ns_name()`) already shorten
the generated interface name via `shorten_id()` when `vh--` /
`vn--` would exceed the Linux `IFNAMSIZ` limit (15 chars). The **public**-side
functions never got the same treatment — they just do `echo "vph-${pvlan}-${id}"` /
`echo "vpn-${pvlan}-${id}"` unconditionally.
This is latent for short numeric VLANs but fails as soon as `pvlan` is `"untagged"`
(8 chars) combined with even a short numeric id, e.g. network id `222`:
`vpn-untagged-222` = 17 characters →
```
Error: argument "vpn-untagged-222" is wrong: "name" not a valid ifname
```
**Fix:** apply the same three-tier shortening (`shorten_id()`, then hard-truncate to 15)
already used in `veth_host_name()`/`veth_ns_name()`.
---
## 6. Design gap: the script requires the caller to pre-supply a VLAN; it never self-allocates one
`NetworkExtensionGuestNetworkGuru` deliberately skips CloudStack's normal auto-VLAN
pool — the protocol is designed for the extension's `implement-network` handler to
generate **its own** segment identifier and report it back (the doc's own example uses
`{"network.broadcast_domain_type":"Lswitch","network.broadcast_uri":"ovn://cs-net-42"}`,
no VLAN involved at all).
This script instead just echoes back whatever `payload.vlan` it was given and never
allocates anything itself, so a plain auto-VLAN Isolated network offering (no
`specifyvlan`) always reaches `implement-network` with an empty VLAN, and the script's
device-naming logic (`vn--`) then fails outright (`Cannot find device "vn--"`).
In practice this forces every offering built on this extension to use
`specifyvlan=true` plus an explicit `vlan=` on every `createNetwork` call, which isn't
mentioned anywhere in the README's "CloudStack Setup Steps".
**Suggested fix:** either (a) have `implement-network` fall back to self-generating a
stable identifier (e.g. derived from the network/VPC id) when `payload.vlan` is empty,
consistent with the protocol's intent, or (b) at minimum, document the
`specifyvlan=true` + explicit `vlan=` requirement explicitly in the README's setup
steps.
---
## 7. (Minor / docs) Missing host packages fail silently as far as CloudStack is concerned
`cmd_restore_network()` hard-requires `dnsmasq`, `apache2`/`httpd`, and `haproxy` on the
KVM host via `_require_binary`, but `die()` in this script only logs to the wrapper's
own local log file (`/tmp/cloudstack-extensions/.log` on the KVM host) — nothing
reaches stdout, so CloudStack (and the mgmt-server SSH proxy's exit-code-3 convention
for "remote script returned non-zero") only ever sees an **empty** error message. This
makes a missing-package issue on a KVM host very hard to diagnose from the CloudStack
side alone.
**Suggested fix:** either have `_require_binary`'s `die()` path also print to stdout
(so it surfaces in the CloudStack-side log), or list `dnsmasq`, `apache2`, `haproxy` as
explicit prerequisites in the README's KVM host setup section.
Guide de contribution
Aucun guide de contribution indexé pour ce dépôt
Évaluation
Cette issue n'a pas encore été évaluée.