Centralise IP address object address retrieval and existence checks.

Following illumos 16677, the error message that indicates that an address object does not exists has been changed to be consistent, whereas it was previously one of two different messages or sometimes even completely blank. A couple of places in omicron relied on the specific error message so this change centralises the check, and updates it to cater for illumos pre- and post-16677. Longer term this could be replaced by bindings to libipadm.
citrus-it · Aug 1, 2024 · 48a7cfd · 48a7cfd
1 parent 9d811b7
commit 48a7cfd
Show file tree

Hide file tree

Showing 2 changed files with 82 additions and 106 deletions.
diff --git a/illumos-utils/src/ipadm.rs b/illumos-utils/src/ipadm.rs
@@ -13,7 +13,11 @@ use std::net::Ipv6Addr;
 pub struct Ipadm {}
 
 /// Expected error message contents when showing an addrobj that doesn't exist.
-const ADDROBJ_NOT_FOUND_ERR: &str = "Address object not found";
+// The message changed to be consistent regardless of the state of the
+// system in illumos 16677. It is now always `ERR1` below. Prior to that, it
+// would most often be `ERR2` but could sometimes be blank or `ERR1`.
+const ADDROBJ_NOT_FOUND_ERR1: &str = "address: Object not found";
+const ADDROBJ_NOT_FOUND_ERR2: &str = "Address object not found";
 
 /// Expected error message when an interface already exists.
 const INTERFACE_ALREADY_EXISTS: &str = "Interface already exists";
@@ -37,6 +41,34 @@ impl Ipadm {
         }
     }
 
+    pub fn addrobj_addr(
+        addrobj: &str,
+    ) -> Result<Option<String>, ExecutionError> {
+        // Note that additional privileges are not required to list address
+        // objects, and so there is no `pfexec` here.
+        let mut cmd = std::process::Command::new(IPADM);
+        let cmd = cmd.args(&["show-addr", "-po", "addr", addrobj]);
+        match execute(cmd) {
+            Err(ExecutionError::CommandFailure(info))
+                if [ADDROBJ_NOT_FOUND_ERR1, ADDROBJ_NOT_FOUND_ERR2]
+                    .iter()
+                    .any(|&ss| info.stderr.contains(ss)) =>
+            {
+                // The address object does not exist.
+                Ok(None)
+            }
+            Err(e) => Err(e),
+            Ok(output) => {
+                let stdout = String::from_utf8_lossy(&output.stdout);
+                Ok(stdout.trim().lines().next().map(|s| s.to_owned()))
+            }
+        }
+    }
+
+    pub fn addrobj_exists(addrobj: &str) -> Result<bool, ExecutionError> {
+        Ok(Self::addrobj_addr(addrobj)?.is_some())
+    }
+
     // Set MTU to 9000 on both IPv4 and IPv6
     pub fn set_interface_mtu(datalink: &str) -> Result<(), ExecutionError> {
         let mut cmd = std::process::Command::new(PFEXEC);
@@ -71,77 +103,53 @@ impl Ipadm {
         datalink: &str,
         listen_addr: &Ipv6Addr,
     ) -> Result<(), ExecutionError> {
-        // Create auto-configured address on the IP interface if it doesn't already exist
+        // Create auto-configured address on the IP interface if it doesn't
+        // already exist
         let addrobj = format!("{}/{}", datalink, IPV6_LINK_LOCAL_ADDROBJ_NAME);
-        let mut cmd = std::process::Command::new(PFEXEC);
-        let cmd = cmd.args(&[IPADM, "show-addr", &addrobj]);
-        match execute(cmd) {
-            Err(ExecutionError::CommandFailure(info))
-                if info.stderr.contains(ADDROBJ_NOT_FOUND_ERR) =>
-            {
-                let mut cmd = std::process::Command::new(PFEXEC);
-                let cmd = cmd.args(&[
-                    IPADM,
-                    "create-addr",
-                    "-t",
-                    "-T",
-                    "addrconf",
-                    &addrobj,
-                ]);
-                execute(cmd)?;
-            }
-            Err(other) => return Err(other),
-            Ok(_) => (),
-        };
+        if !Self::addrobj_exists(&addrobj)? {
+            let mut cmd = std::process::Command::new(PFEXEC);
+            let cmd = cmd.args(&[
+                IPADM,
+                "create-addr",
+                "-t",
+                "-T",
+                "addrconf",
+                &addrobj,
+            ]);
+            execute(cmd)?;
+        }
 
         // Create static address on the IP interface if it doesn't already exist
         let addrobj = format!("{}/{}", datalink, IPV6_STATIC_ADDROBJ_NAME);
-        let mut cmd = std::process::Command::new(PFEXEC);
-        let cmd = cmd.args(&[IPADM, "show-addr", &addrobj]);
-        match execute(cmd) {
-            Err(ExecutionError::CommandFailure(info))
-                if info.stderr.contains(ADDROBJ_NOT_FOUND_ERR) =>
-            {
-                let mut cmd = std::process::Command::new(PFEXEC);
-                let cmd = cmd.args(&[
-                    IPADM,
-                    "create-addr",
-                    "-t",
-                    "-T",
-                    "static",
-                    "-a",
-                    &listen_addr.to_string(),
-                    &addrobj,
-                ]);
-                execute(cmd).map(|_| ())
-            }
-            Err(other) => Err(other),
-            Ok(_) => Ok(()),
+        if !Self::addrobj_exists(&addrobj)? {
+            let mut cmd = std::process::Command::new(PFEXEC);
+            let cmd = cmd.args(&[
+                IPADM,
+                "create-addr",
+                "-t",
+                "-T",
+                "static",
+                "-a",
+                &listen_addr.to_string(),
+                &addrobj,
+            ]);
+            execute(cmd)?;
         }
+
+        Ok(())
     }
 
     // Create gateway on the IP interface if it doesn't already exist
     pub fn create_opte_gateway(
         opte_iface: &String,
     ) -> Result<(), ExecutionError> {
         let addrobj = format!("{}/public", opte_iface);
-        let mut cmd = std::process::Command::new(PFEXEC);
-        let cmd = cmd.args(&[IPADM, "show-addr", &addrobj]);
-        match execute(cmd) {
-            Err(_) => {
-                let mut cmd = std::process::Command::new(PFEXEC);
-                let cmd = cmd.args(&[
-                    IPADM,
-                    "create-addr",
-                    "-t",
-                    "-T",
-                    "dhcp",
-                    &addrobj,
-                ]);
-                execute(cmd)?;
-            }
-            Ok(_) => (),
-        };
+        if !Self::addrobj_exists(&addrobj)? {
+            let mut cmd = std::process::Command::new(PFEXEC);
+            let cmd =
+                cmd.args(&[IPADM, "create-addr", "-t", "-T", "dhcp", &addrobj]);
+            execute(cmd)?;
+        }
         Ok(())
     }
 }
diff --git a/sled-agent/src/bin/zone-bundle.rs b/sled-agent/src/bin/zone-bundle.rs
@@ -247,53 +247,21 @@ async fn fetch_underlay_address() -> anyhow::Result<Ipv6Addr> {
     #[cfg(target_os = "illumos")]
     {
         const EXPECTED_ADDR_OBJ: &str = "underlay0/sled6";
-        let output = Command::new("ipadm")
-            .arg("show-addr")
-            .arg("-p")
-            .arg("-o")
-            .arg("addr")
-            .arg(EXPECTED_ADDR_OBJ)
-            .output()
-            .await?;
-        // If we failed because there was no such interface, then fall back to
-        // localhost.
-        if !output.status.success() {
-            match std::str::from_utf8(&output.stderr) {
-                Err(_) => bail!(
-                    "ipadm command failed unexpectedly, stderr:\n{}",
-                    String::from_utf8_lossy(&output.stderr)
-                ),
-                Ok(out) => {
-                    if out.contains("Address object not found") {
-                        eprintln!(
-                            "Expected addrobj '{}' not found, using localhost",
-                            EXPECTED_ADDR_OBJ,
-                        );
-                        return Ok(Ipv6Addr::LOCALHOST);
-                    } else {
-                        bail!(
-                            "ipadm subcommand failed unexpectedly, stderr:\n{}",
-                            String::from_utf8_lossy(&output.stderr),
-                        );
-                    }
-                }
-            }
+        match Ipadm::addrobj_addr(EXPECTED_ADDR_OBJ) {
+            // If we failed because there was no such interface, then fall back
+            // to localhost.
+            Ok(None) => return Ok(Ipv6Addr::LOCALHOST),
+            Ok(Some(addr)) => addr
+                .trim()
+                .split_once('/')
+                .context("expected a /64 subnet")?
+                .0
+                .parse()
+                .context("invalid IPv6 address"),
+            Err(e) => bail!(
+                "failed to get address for addrobj {EXPECTED_ADDR_OBJ}: {e}",
+            ),
         }
-        let out = std::str::from_utf8(&output.stdout)
-            .context("non-UTF8 output in ipadm")?;
-        let lines: Vec<_> = out.trim().lines().collect();
-        anyhow::ensure!(
-            lines.len() == 1,
-            "No addresses or more than one address on expected interface '{}'",
-            EXPECTED_ADDR_OBJ
-        );
-        lines[0]
-            .trim()
-            .split_once('/')
-            .context("expected a /64 subnet")?
-            .0
-            .parse()
-            .context("invalid IPv6 address")
     }
 }