Improvements to the network-wrapper script

Đang mở
#5 1 bình luận 0 reaction 1 người được giao Xem trên GitHub

@weizhouapache đang làm issue này rồi.

Từ ngày 8/9/2026.

Đánh giá

Issue này chưa được đánh giá.

Mô tả

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:

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

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:

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 <eth> | 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-<vlan>-<id> /
vn-<vlan>-<id> 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-<vlan>-<id>) then fails outright (Cannot find device "vn--<id>").
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/<name>.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.

Ngôn ngữ chính
Không có dữ liệu ngôn ngữ
Star
4
Fork
4
Merge trung bình
1 giờ
Pull request đã merge (30 ngày)
3

Hướng dẫn đóng góp

Chưa lập chỉ mục được hướng dẫn đóng góp cho kho mã nguồn này

Bắt đầu từ đâu

  1. Đọc hết issue, rồi đọc hướng dẫn đóng góp của dự án.
  2. Bình luận trên issue rằng bạn sẽ nhận — tránh hai người làm cùng một việc.
  3. Fork repository và làm thay đổi trên một nhánh.
  4. Mở pull request có tham chiếu số hiệu của issue.

Issue khác của apache/cloudstack-extensions

Tất cả issue của apache/cloudstack-extensions

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.