From 1059412e28a62d1c84f9b01c261e2ab0567b53df Mon Sep 17 00:00:00 2001
From: joshsiv-mulga <mrk@mulgadc.com>
Date: Fri, 24 Apr 2026 16:31:19 +1000
Subject: [PATCH] fix: localSystemID + discoverChassis use Output() to skip
 sudo PAM noise
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Follow-up to d509df7. The sudo PAM audit bug exists in two more vpcd call
sites beyond portToBr: localSystemID() and discoverChassis().

When vpcd.service's AmbientCapabilities trip sudo's PAM, stderr carries
"sudo: unable to send audit message: Operation not permitted". CombinedOutput
merges that into stdout and corrupts the parsed value:

  localSystemID → localID="sudo: ...\n\"add0104e-..."
  discoverChassis → tries to parse the stderr banner as chassis name/hostname

Downstream effect: discoverChassis skips the real local chassis as "stale",
vpcd falls back to cfg.ChassisNames (e.g. "chassis-josh-banksia") which no
chassis in SBDB actually uses, so every gateway_chassis row points at a
phantom chassis. cr-gw* chassisredirect ports never bind → no proxy-ARP
for EIPs → external ping fails with ARP timeout.

Switch both to Output() + exec.ExitError.Stderr, matching d509df7's pattern.

Found during mulga-998.b reboot drill while verifying external reachability:
chassis discovery log showed `localID="sudo: unable to send audit message...`,
`cr-gw-vpc-* chassis=[]`, and host ping to EIP returned Destination Host
Unreachable. With this fix applied, discoverChassis returns the real system-id
and (after stale gateway_chassis rows are cleared) cr-gw binds, proxy-ARP
responds, and EIP ping succeeds end-to-end.

Note: existing gateway_chassis rows written pre-fix still carry the phantom
chassis_name. Cleanup is one-shot — delete those rows and trigger an IGW
detach+attach, or run reset-dev-env.sh. vpcd does not reconcile existing
rows on startup (potential future retrofit, similar to Fix 3's
RetrofitAllExternalLocalnetOptions).
---
 spinifex/services/vpcd/vpcd.go | 26 ++++++++++++++++++++++----
 1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/spinifex/services/vpcd/vpcd.go b/spinifex/services/vpcd/vpcd.go
index 630c29761..ef510ef9d 100644
--- a/spinifex/services/vpcd/vpcd.go
+++ b/spinifex/services/vpcd/vpcd.go
@@ -183,10 +183,22 @@ var checkOVNController = func() error {
 
 // localSystemID returns the OVS external-ids:system-id, which is the chassis
 // name that the local ovn-controller registers in the Southbound DB.
+//
+// Uses Output() (stdout only) for the same reason as portToBr: vpcd.service's
+// AmbientCapabilities trips sudo's PAM into emitting audit warnings on stderr,
+// which CombinedOutput would merge into stdout and poison the system-id.
+// A corrupted localID causes discoverChassis to skip the live chassis as
+// "stale", leaving gateway_chassis pointing at a fallback name that no real
+// chassis owns → cr-gw* chassisredirect ports stay unbound → no proxy-ARP
+// for EIPs.
 var localSystemID = func() (string, error) {
-	out, err := sudoCommand("ovs-vsctl", "get", "open_vswitch", ".", "external-ids:system-id").CombinedOutput()
+	out, err := sudoCommand("ovs-vsctl", "get", "open_vswitch", ".", "external-ids:system-id").Output()
 	if err != nil {
-		return "", fmt.Errorf("ovs-vsctl get system-id: %s: %w", strings.TrimSpace(string(out)), err)
+		var stderr string
+		if exitErr, ok := err.(*exec.ExitError); ok {
+			stderr = strings.TrimSpace(string(exitErr.Stderr))
+		}
+		return "", fmt.Errorf("ovs-vsctl get system-id: %s: %w", stderr, err)
 	}
 	// ovs-vsctl wraps the value in quotes
 	return strings.Trim(strings.TrimSpace(string(out)), "\""), nil
@@ -211,9 +223,15 @@ var discoverChassis = func(sbAddr string) ([]string, error) {
 	// OVN 25.03+ removed the "list-chassis" convenience command.
 	// Use "--columns=name,hostname list Chassis" which works on all versions.
 	args = append(args, "--bare", "--columns=name,hostname", "list", "Chassis")
-	out, err := sudoCommand("ovn-sbctl", args...).CombinedOutput()
+	// Output() not CombinedOutput(): sudo PAM audit noise on stderr would
+	// otherwise be parsed as chassis name/hostname pairs.
+	out, err := sudoCommand("ovn-sbctl", args...).Output()
 	if err != nil {
-		return nil, fmt.Errorf("ovn-sbctl list Chassis: %s: %w", strings.TrimSpace(string(out)), err)
+		var stderr string
+		if exitErr, ok := err.(*exec.ExitError); ok {
+			stderr = strings.TrimSpace(string(exitErr.Stderr))
+		}
+		return nil, fmt.Errorf("ovn-sbctl list Chassis: %s: %w", stderr, err)
 	}
 
 	return parseChassisList(string(out), localID, localHostname), nil