ipset: Better error handling and messages

- Clearer messages.
- Make sure logs are showed in order.
- When cleaning after an error on startup,
  do not try to undo an action that failed.
This commit is contained in:
ppom 2026-03-02 12:00:00 +01:00
commit 3ca54c6c43
No known key found for this signature in database
3 changed files with 62 additions and 50 deletions

View file

@ -1,4 +1,4 @@
use std::{fmt::Debug, u32};
use std::{fmt::Debug, u32, usize};
use reaction_plugin::{Exec, shutdown::ShutdownToken, time::parse_duration};
use remoc::rch::mpsc as remocMpsc;
@ -173,7 +173,7 @@ impl Set {
}
}
pub async fn init(&self, ipset: &mut IpSet) -> Result<(), String> {
pub async fn init(&self, ipset: &mut IpSet) -> Result<(), (usize, String)> {
for (set, version) in [
(&self.sets.ipv4, Version::IPv4),
(&self.sets.ipv6, Version::IPv6),
@ -186,44 +186,42 @@ impl Set {
version,
timeout: self.timeout,
}))
.await?;
.await
.map_err(|err| (0, err.to_string()))?;
// insert set in chains
for chain in &self.chains {
for (i, chain) in self.chains.iter().enumerate() {
ipset
.order(Order::InsertSet(SetChain {
set: set.clone(),
chain: chain.clone(),
target: self.target.clone(),
}))
.await?;
.await
.map_err(|err| (i + 1, err.to_string()))?;
}
}
}
Ok(())
}
pub async fn destroy(&self, ipset: &mut IpSet) {
for (set, version) in [
(&self.sets.ipv4, Version::IPv4),
(&self.sets.ipv6, Version::IPv6),
] {
pub async fn destroy(&self, ipset: &mut IpSet, until: Option<usize>) {
for set in [&self.sets.ipv4, &self.sets.ipv6] {
if let Some(set) = set {
for chain in &self.chains {
if let Err(err) = ipset
for chain in self
.chains
.iter()
.take(until.map(|until| until - 1).unwrap_or(usize::MAX))
{
let _ = ipset
.order(Order::RemoveSet(SetChain {
set: set.clone(),
chain: chain.clone(),
target: self.target.clone(),
}))
.await
{
eprintln!(
"ERROR while removing {version} set {set} from chain {chain}: {err}"
);
}
.await;
}
if let Err(err) = ipset.order(Order::DestroySet(set.clone())).await {
eprintln!("ERROR while destroying {version} set {set}: {err}");
if until.is_none_or(|until| until != 0) {
let _ = ipset.order(Order::DestroySet(set.clone())).await;
}
}
}

View file

@ -72,7 +72,7 @@ impl IpSet {
pub enum IpSetError {
Thread(String),
IpSet(String),
IpSet(()),
}
impl Display for IpSetError {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
@ -81,7 +81,7 @@ impl Display for IpSetError {
"{}",
match self {
IpSetError::Thread(err) => err,
IpSetError::IpSet(err) => err,
IpSetError::IpSet(()) => "ipset error",
}
)
}
@ -90,12 +90,12 @@ impl From<IpSetError> for String {
fn from(value: IpSetError) -> Self {
match value {
IpSetError::Thread(err) => err,
IpSetError::IpSet(err) => err,
IpSetError::IpSet(()) => "ipset error".to_string(),
}
}
}
pub type OrderType = (Order, oneshot::Sender<Result<(), String>>);
pub type OrderType = (Order, oneshot::Sender<Result<(), ()>>);
struct Set {
session: Session<HashNet>,
@ -121,7 +121,7 @@ impl IPsetManager {
}
}
fn handle_order(&mut self, order: Order) -> Result<(), String> {
fn handle_order(&mut self, order: Order) -> Result<(), ()> {
match order {
Order::CreateSet(CreateSet {
name,
@ -139,7 +139,7 @@ impl IPsetManager {
};
builder.with_ipv6(version == Version::IPv6)?.build()
})
.map_err(|err| format!("Could not create set {name}: {err}"))?;
.map_err(|err| eprintln!("ERROR Could not create set {name}: {err}"))?;
self.sessions.insert(name, Set { session, version });
}
@ -149,7 +149,7 @@ impl IPsetManager {
session
.session
.destroy()
.map_err(|err| format!("Could not destroy set {set}: {err}"))?;
.map_err(|err| eprintln!("ERROR Could not destroy set {set}: {err}"))?;
}
}
@ -162,9 +162,13 @@ impl IPsetManager {
Ok(())
}
fn insert_remove_ip(&mut self, set: String, ip: String, insert: bool) -> Result<(), String> {
fn insert_remove_ip(&mut self, set: String, ip: String, insert: bool) -> Result<(), ()> {
self._insert_remove_ip(set, ip, insert)
.map_err(|err| eprintln!("ERROR {err}"))
}
fn _insert_remove_ip(&mut self, set: String, ip: String, insert: bool) -> Result<(), String> {
let session = self.sessions.get_mut(&set).ok_or(format!(
"No set handled by us with this name: {set}. This likely is a bug."
"No set handled by this plugin with this name: {set}. This likely is a bug."
))?;
let mut net_data = NetDataType::new(Ipv4Addr::LOCALHOST, 0);
@ -182,24 +186,28 @@ impl IPsetManager {
Ok(())
}
fn insert_remove_set(&self, options: SetChain, insert: bool) -> Result<(), String> {
let SetChain {
set,
chain,
target: action,
} = options;
fn insert_remove_set(&self, options: SetChain, insert: bool) -> Result<(), ()> {
self._insert_remove_set(options, insert)
.map_err(|err| eprintln!("ERROR {err}"))
}
fn _insert_remove_set(&self, options: SetChain, insert: bool) -> Result<(), String> {
let SetChain { set, chain, target } = options;
let version = self
.sessions
.get(&set)
.ok_or(format!("No set managed by us with this name: {set}"))?
.ok_or(format!(
"No set managed by this plugin with this name: {set}"
))?
.version;
if insert {
eprintln!("INFO inserting {version} set {set} in chain {chain}");
let (verb, verbing, from) = if insert {
("insert", "inserting", "in")
} else {
eprintln!("INFO removing {version} set {set} from chain {chain}");
}
("remove", "removing", "from")
};
eprintln!("INFO {verbing} {version} set {set} {from} chain {chain}");
let command = match version {
Version::IPv4 => "iptables",
@ -217,20 +225,20 @@ impl IPsetManager {
&set,
"src",
"-j",
&action,
&target,
])
.spawn()
.map_err(|err| format!("Could not insert ipset {set} in chain {chain}: {err}"))?;
.map_err(|err| format!("Could not {verb} ipset {set} {from} chain {chain}: Could not execute {command}: {err}"))?;
let exit = child
.wait()
.map_err(|err| format!("Could not insert ipset: {err}"))?;
.map_err(|err| format!("Could not {verb} ipset {set} {from} chain {chain}: {err}"))?;
if exit.success() {
Ok(())
} else {
Err(format!(
"Could not insert ipset: exit code {}",
"Could not {verb} ipset: exit code {}",
exit.code()
.map(|c| c.to_string())
.unwrap_or_else(|| "<unknown>".to_string())

View file

@ -109,15 +109,21 @@ impl PluginInfo for Plugin {
let mut first_error = None;
for (i, set) in self.sets.iter().enumerate() {
// Retain if error
if let Err(err) = set.init(&mut self.ipset).await {
first_error = Some((i, RemoteError::Plugin(err)));
if let Err((failed_step, err)) = set.init(&mut self.ipset).await {
first_error = Some((i, failed_step, RemoteError::Plugin(err)));
break;
}
}
// Destroy initialized sets if error
if let Some((i, err)) = first_error {
for set in self.sets.iter().take(i + 1) {
let _ = set.destroy(&mut self.ipset).await;
if let Some((last_set, failed_step, err)) = first_error {
eprintln!("DEBUG last_set: {last_set} failed_step: {failed_step} err: {err}");
for (curr_set, set) in self.sets.iter().enumerate().take(last_set + 1) {
let until = if last_set == curr_set {
Some(failed_step)
} else {
None
};
let _ = set.destroy(&mut self.ipset, until).await;
}
return Err(err);
}
@ -148,6 +154,6 @@ impl PluginInfo for Plugin {
async fn destroy_sets_at_shutdown(mut ipset: IpSet, sets: Vec<Set>, shutdown: ShutdownToken) {
shutdown.wait().await;
for set in sets {
set.destroy(&mut ipset).await;
set.destroy(&mut ipset, None).await;
}
}