Skip to content

agent: Avoid needlessly re-applying an unchanged NVUE configuration #1965

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
DrewBloechl wants to merge 2 commits into
base: main
Choose a base branch
from
+84 −22
Open

agent: Avoid needlessly re-applying an unchanged NVUE configuration #1965

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3cb5007
nvue-client: Add hash method to NvueConfig
DrewBloechl May 27, 2026
4899766
agent: Avoid re-applying unchanged NVUE config
DrewBloechl May 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 60 additions & 8 deletions crates/agent/src/ethernet_virtualization.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ use carbide_network::ip::prefix::Ipv4Net;
use carbide_network::virtualization::{VpcVirtualizationType, build_dual_stack_list};
use eyre::WrapErr;
use mac_address::MacAddress;
use nvue_client::client::NvueClientError;
use nvue_client::{NvueClient, NvueConfig};
use serde::Deserialize;
use tokio::process::Command as TokioCommand;
Expand Down Expand Up @@ -145,8 +146,54 @@ struct PostAction {
}

pub enum NvueUpdateFlavor<'a> {
StartupFile { hbn_root: &'a Path, skip_post: bool },
RestApi { nvue_client: &'a NvueClient },
StartupFile {
hbn_root: &'a Path,
skip_post: bool,
},
RestApi {
nvue_context: &'a mut NvueClientContext,
},
}

/// The NVUE client and other information associated with it.
pub struct NvueClientContext {
pub nvue_client: NvueClient,
pub last_applied_hash: Option<u64>,
}

impl NvueClientContext {
pub fn new(nvue_client: NvueClient) -> Self {
let last_applied_hash = None;
Self {
nvue_client,
last_applied_hash,
}
}

// Wrap the inner nvue_client's `push_config()` and try to avoid re-applying
// a configuration we're already using. Returns Ok(Some(revision_id)) on
// a change, Ok(None) if the config was unchanged, and otherwise passes
// through errors from the inner client.
pub async fn update_config(
&mut self,
config: &NvueConfig,
) -> Result<Option<String>, NvueClientError> {
let new_hash = config.u64_hash();
Copy link
Copy Markdown
Contributor

@chet chet May 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we instead lean on the ConfigVersion that we got from carbide-api for the config? i.e. if the ConfigVersion hasn't changed, don't write anything?

Copy link
Copy Markdown
Contributor Author

@DrewBloechl DrewBloechl May 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't currently have a way to link a given NVUE config back to the ConfigVersion it relates to, so while I'd like to do that at some point, it seems awkward under the current architecture.

Copy link
Copy Markdown
Contributor Author

@DrewBloechl DrewBloechl May 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose we could do that further up in the logic, but this is more or less mirroring what the HBN startup-file code does.


if let Some(last_applied_hash) = self.last_applied_hash
&& new_hash == last_applied_hash
{
Ok(None)
} else {
self.nvue_client
.push_config(config)
.await
.map(|revision_id| {
self.last_applied_hash.replace(new_hash);
Some(revision_id)
})
}
}
}

/// Converts an RPC routing profile into the NVUE renderer model.
Expand Down Expand Up @@ -197,7 +244,8 @@ pub async fn update_nvue(
) -> eyre::Result<bool> {
let hbn_version = match update_flavor {
NvueUpdateFlavor::StartupFile { .. } => hbn::read_version().await?,
NvueUpdateFlavor::RestApi { nvue_client } => nvue_client
NvueUpdateFlavor::RestApi { ref nvue_context } => nvue_context
.nvue_client
.system_build_info()
.await
.map_err(|e| eyre::eyre!("Couldn't get HBN version from NVUE: {e}"))
Expand Down Expand Up @@ -570,15 +618,19 @@ pub async fn update_nvue(
}
Ok(true)
}
NvueUpdateFlavor::RestApi { nvue_client } => {
NvueUpdateFlavor::RestApi { nvue_context } => {
let config = NvueConfig::from_yaml(&next_contents)
.map_err(|e| eyre::eyre!("Couldn't parse NVUE config as YAML: {e}"))?;
let revision_id = nvue_client
.push_config(&config)
let revision_id = nvue_context
.update_config(&config)
.await
.map_err(|e| eyre::eyre!("Couldn't push new config to NVUE server: {e}"))?;
tracing::debug!(revision_id, "Applied NVUE config via REST API");
Ok(true)
if let Some(revision_id) = revision_id {
tracing::debug!(revision_id, "Applied NVUE config via REST API");
Ok(true)
} else {
Ok(false)
}
}
}
}
Expand Down
28 changes: 15 additions & 13 deletions crates/agent/src/main_loop.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ use crate::dpu::interface::Interface;
use crate::dpu::route::{DpuRoutePlan, IpRoute, Route};
use crate::duppet::{SummaryFormat, SyncOptions};
use crate::ethernet_virtualization::{
InterfaceTranslationMode, NvueUpdateFlavor, ServiceAddresses,
InterfaceTranslationMode, NvueClientContext, NvueUpdateFlavor, ServiceAddresses,
};
use crate::fmds_client::FmdsUpdater;
use crate::health::HealthCheckParams;
Expand Down Expand Up @@ -221,11 +221,12 @@ pub async fn setup_and_run(
// We have eight cores. Letting ovs_vswitchd have one is OK.
};

let nvue_client = match options.hbn_config_mode {
let nvue_context = match options.hbn_config_mode {
HbnConfigMode::ContainerExec => None,
HbnConfigMode::NvueRest => {
let nvue_client = nvue_client::NvueClient::new_https_from_env()?;
Some(nvue_client)
let nvue_context = NvueClientContext::new(nvue_client);
Some(nvue_context)
}
};

Expand Down Expand Up @@ -376,7 +377,7 @@ pub async fn setup_and_run(
close_sender,
network_monitor_handle,
extension_service_manager,
nvue_client,
nvue_context,
dhcp_interface_translation_mode,
};

Expand Down Expand Up @@ -409,7 +410,7 @@ struct MainLoop {
network_monitor_handle: Option<JoinHandle<()>>,
close_sender: watch::Sender<bool>,
extension_service_manager: extension_services::ExtensionServiceManager,
nvue_client: Option<nvue_client::NvueClient>,
nvue_context: Option<NvueClientContext>,
dhcp_interface_translation_mode: Option<InterfaceTranslationMode>,
}

Expand Down Expand Up @@ -558,10 +559,11 @@ impl MainLoop {
if self.is_hbn_up {
// First thing is to read the existing HBN version and properly set the hbn device names
// associated with that version.
let hbn_version = match self.nvue_client.as_mut() {
let hbn_version = match self.nvue_context.as_mut() {
None => hbn::read_version().await?,
Some(nvue_client) => {
let nvue_system_build = nvue_client.system_build_info().await?;
Some(nvue_context) => {
let nvue_system_build =
nvue_context.nvue_client.system_build_info().await?;
match nvue_system_build.strip_prefix("HBN ") {
Some(hbn_version) => Ok(hbn_version.into()),
None => Err(eyre::format_err!(
Expand Down Expand Up @@ -680,8 +682,8 @@ impl MainLoop {
};

if bridging_result.is_ok() {
let update_flavor = match self.nvue_client.as_ref() {
Some(nvue_client) => NvueUpdateFlavor::RestApi { nvue_client },
let update_flavor = match self.nvue_context.as_mut() {
Some(nvue_context) => NvueUpdateFlavor::RestApi { nvue_context },
None => NvueUpdateFlavor::StartupFile {
hbn_root: &self.agent_config.hbn.root_dir,
skip_post: self.agent_config.hbn.skip_reload,
Expand Down Expand Up @@ -783,7 +785,7 @@ impl MainLoop {
match ethernet_virtualization::interfaces(
&conf,
self.factory_mac_address,
self.nvue_client.as_ref(),
self.nvue_context.as_ref().map(|c| &c.nvue_client),
)
.await
{
Expand Down Expand Up @@ -826,7 +828,7 @@ impl MainLoop {
current_instance_config_version = status_out.instance_config_version.clone();
current_instance_id = status_out.instance_id.as_ref().map(|id| id.to_string());

let health_report = match self.nvue_client.as_ref() {
let health_report = match self.nvue_context.as_ref() {
None => {
health::health_check(HealthCheckParams {
hbn_root: &self.agent_config.hbn.root_dir,
Expand All @@ -840,7 +842,7 @@ impl MainLoop {
})
.await
}
Some(nvue_client) => health::nvue_api_health(nvue_client).await,
Some(nvue_context) => health::nvue_api_health(&nvue_context.nvue_client).await,
};
is_healthy = !health_report.successes.is_empty() && health_report.alerts.is_empty();
self.is_hbn_up = health::is_up(&health_report);
Expand Down
10 changes: 9 additions & 1 deletion crates/nvue-client/src/config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,11 @@
* limitations under the License.
*/

use std::hash::{DefaultHasher, Hash, Hasher};

use crate::client::NvueClientError;

#[derive(Clone, Debug, serde::Deserialize, serde::Serialize)]
#[derive(Clone, Debug, Hash, serde::Deserialize, serde::Serialize)]
#[serde(transparent)]
pub struct NvueConfig {
// FIXME: Replace this with a more strongly typed inner representation
Expand Down Expand Up @@ -67,6 +69,12 @@ impl NvueConfig {
Ok(None)
}
}

pub fn u64_hash(&self) -> u64 {
let mut h = DefaultHasher::new();
self.hash(&mut h);
h.finish()
}
}

#[derive(Clone, Debug, serde::Deserialize, serde::Serialize)]
Expand Down
Loading