vishesh92 commented on PR #11800:
URL: https://github.com/apache/cloudstack/pull/11800#issuecomment-5566437000
Thanks @mosys0815, I confirmed the change fixes #10996: on a multipath
default route with `src`, the new command returns the interface holding the
source IP (`hostip` in your setup), where the old command returned `32`.
However, I found a regression while reviewing using AI. The new command only
works when the default route carries a `src`/`prefsrc` attribute. When it
doesn't, the inner `jq` prints `null`, the `select()` matches nothing, and the
command returns an empty string with exit code 0. `getDefaultEthDevice()` then
returns null, `getAllDefaultNicIps()` returns an empty list.
**When does the default route have no `src`?**
Whenever the route was installed by a static configuration rather than a
DHCP client. I verified these on Ubuntu 24.04, Rocky 9 and openSUSE Leap 15.6
docker container:
| How the gateway was configured | Resulting default route | PR command
returns |
|---|---|---|
| `ip route add default via GW dev eth0` | `default via GW dev eth0` | empty
|
| ifupdown static (`/etc/network/interfaces`, `gateway`) | `default via GW
dev eth0 onlink` | empty |
| systemd-networkd / netplan static (`Gateway=`) | `default via GW dev eth0
proto static` | empty |
| NetworkManager / nmcli `ipv4.method manual` + `ipv4.gateway` (EL9) |
`default via GW dev eth0 proto static metric 550` | empty |
| wicked static (`/etc/sysconfig/network/routes`, SUSE) | `default via GW
dev eth0` | empty |
| isc dhclient-script | `ip -4 route add default via ${router} dev
${interface}`, no `src` | empty |
| systemd-networkd or NetworkManager **DHCP** lease | `default via GW dev
eth0 proto dhcp src IP metric ...` | eth0 |
DHCP-managed hosts are fine, which is presumably why the Trillian run
passed. Production management servers with a static IP are the ones that hit
this.
**`jq` dependency**
As @weizhouapache noted, `jq` is not in `Requires` of any spec file nor in
`debian/control`. Nothing in the management server's Java code shells out to
`jq` today; the only user in the repo is the optional Proxmox extension script
under `extensions/`. This would be the first core code path that needs it, and
when it is absent the pipeline fails with `jq: command not found` and we end up
in the same empty-result path (with a WARN in the log this time).
**Reproduction**
Save this as `repro.sh`. It builds dummy interfaces and the three route
layouts in an isolated network stack, and prints what the old and new commands
return for each:
```bash
PR() { ip -j a | jq -r '.[] | .addr_info | map(select(.local == "'`ip -j r
s default | jq -r '.[0] | .prefsrc'`'")) | .[].label'; }
OLD() { ip route show default 0.0.0.0/0 | head -1 | awk '{print $5}'; }
show() { echo " old=[$(OLD | tr '\n' ' ')] PR=[$(PR 2>&1 | tr '\n' ' ')]";
}
ip link add d0 type dummy; ip link set d0 up; ip addr add 10.0.0.2/24 dev d0
ip link add d1 type dummy; ip link set d1 up; ip addr add 10.0.1.2/24 dev d1
ip link add hostip type dummy; ip link set hostip up; ip addr add
10.72.44.3/32 dev hostip
ip route del default 2>/dev/null
echo "1. default route WITHOUT src"
ip route add default via 10.0.0.1 dev d0; show
echo "2. default route WITH src"
ip route replace default via 10.0.0.1 dev d0 src 10.0.0.2; show
echo "3. multipath default route with src (#10996)"
ip route del default
ip route add default src 10.72.44.3 metric 32 nexthop via 10.0.0.1 dev d0
weight 1 nexthop via 10.0.1.1 dev d1 weight 1
ip route add default via 10.0.0.1 dev d0 src 10.0.0.2 metric 1024 onlink;
show
```
Run it in a throwaway container (or in a network namespace on a host: `sudo
ip netns add t && sudo ip netns exec t bash repro.sh && sudo ip netns del t`):
```bash
docker run --rm --cap-add NET_ADMIN -v "$PWD/repro.sh:/repro.sh:ro"
ubuntu:24.04 \
bash -c 'apt-get -qq update >/dev/null && apt-get -qq install -y iproute2
jq >/dev/null; bash /repro.sh'
```
Output:
```
1. default route WITHOUT src
old=[d0 ] PR=[]
2. default route WITH src
old=[d0 ] PR=[d0 ]
3. multipath default route with src (#10996)
old=[32 ] PR=[hostip ]
```
Case 1 is the regression, case 3 is the bug this PR fixes. To see it end to
end, start a management server on a host whose default route has no `src` and
check the SAN of the served certificate; it will contain only DNS entries:
```bash
openssl s_client -connect <mgmt-ip>:9090 </dev/null 2>/dev/null | openssl
x509 -noout -ext subjectAltName
```
**Suggested fix**
Keep the `src`-first lookup, since it picks the right interface for the L3
case, but fall back to the route's `dev` when `src` is absent, and drop `jq` so
no new dependency is needed. This returns `d0`, `d0`, `hostip` for the three
cases above, `d0` for a multipath route without `src`, and empty when there is
no default route, on both Ubuntu 24.04 and Rocky 9:
```bash
r=$(ip -o route show default | head -1)
s=$(grep -oP '\bsrc \K\S+' <<<"$r")
if [ -n "$s" ]; then
ip -o addr show to "$s" | awk '{print $2}' | head -1
else
grep -oP '\bdev \K\S+' <<<"$r" | head -1
fi
```
Alternatively parse the `ip -o route show default` line in Java and only
shell out for `ip -o addr show to <src>`, which would make the logic
unit-testable. If you'd rather keep `jq`, it needs to be added to the RPM specs
and `debian/control` before this can merge.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
