Skip to content

Commit

Permalink
Derive bind address from advertised address if not set (#2200)
Browse files Browse the repository at this point in the history
  • Loading branch information
aradwann authored Nov 14, 2024
1 parent 73f3779 commit 484a0df
Show file tree
Hide file tree
Showing 6 changed files with 273 additions and 47 deletions.
17 changes: 14 additions & 3 deletions crates/local-cluster-runner/src/node/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ pub struct Node {
#[mutator(requires = [base_dir])]
pub fn with_node_socket(self) {
let node_socket: PathBuf = PathBuf::from(self.base_config.node_name()).join("node.sock");
self.base_config.common.bind_address = BindAddress::Uds(node_socket.clone());
self.base_config.common.bind_address = Some(BindAddress::Uds(node_socket.clone()));
self.base_config.common.advertised_address = AdvertisedAddress::Uds(node_socket);
}
Expand Down Expand Up @@ -207,8 +207,19 @@ impl Node {
if let BindAddress::Uds(file) = &mut self.base_config.metadata_store.bind_address {
*file = base_dir.join(&*file)
}
if let BindAddress::Uds(file) = &mut self.base_config.common.bind_address {
*file = base_dir.join(&*file)

if self.base_config.common.bind_address.is_none() {
// Derive bind_address from advertised_address
self.base_config.common.bind_address = Some(
self.base_config
.common
.advertised_address
.derive_bind_address(),
);
}

if let Some(BindAddress::Uds(file)) = &mut self.base_config.common.bind_address {
*file = base_dir.join(&*file);
}
if let AdvertisedAddress::Uds(file) = &mut self.base_config.common.advertised_address {
*file = base_dir.join(&*file)
Expand Down
2 changes: 1 addition & 1 deletion crates/node/src/network_server/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ impl NetworkServer {
);

server_builder
.run(node_health, axum_router, &options.bind_address)
.run(node_health, axum_router, &options.bind_address.unwrap())
.await?;

Ok(())
Expand Down
16 changes: 13 additions & 3 deletions crates/types/src/config/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,11 @@ pub struct CommonOptions {
#[serde(flatten)]
pub metadata_store_client: MetadataStoreClientOptions,

/// Address to bind for the Node server. Default is `0.0.0.0:5122`
/// Address to bind for the Node server. Derived from the advertised address, defaulting
/// to `0.0.0.0:$PORT` (where the port will be inferred from the URL scheme).
#[serde(default, skip_serializing_if = "Option::is_none")]
#[cfg_attr(feature = "schemars", schemars(with = "String"))]
pub bind_address: BindAddress,
pub bind_address: Option<BindAddress>,

/// Address that other nodes will use to connect to this node. Default is `http://127.0.0.1:5122/`
#[cfg_attr(feature = "schemars", schemars(with = "String"))]
Expand Down Expand Up @@ -316,6 +318,14 @@ impl CommonOptions {
.expect("number of cpu cores fits in u32"),
)
}

/// set derived values if they are not configured to reduce verbose configurations
pub fn set_derived_values(&mut self) {
// Only derive bind_address if it is not explicitly set
if self.bind_address.is_none() {
self.bind_address = Some(self.advertised_address.derive_bind_address());
}
}
}

impl Default for CommonOptions {
Expand All @@ -337,7 +347,7 @@ impl Default for CommonOptions {
allow_bootstrap: true,
base_dir: None,
metadata_store_client: MetadataStoreClientOptions::default(),
bind_address: "0.0.0.0:5122".parse().unwrap(),
bind_address: None,
advertised_address: AdvertisedAddress::from_str("http://127.0.0.1:5122/").unwrap(),
bootstrap_num_partitions: NonZeroU16::new(24).unwrap(),
histogram_inactivity_timeout: None,
Expand Down
4 changes: 3 additions & 1 deletion crates/types/src/config_loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,9 @@ impl ConfigLoader {
figment = figment.merge(Figment::from(Serialized::defaults(cli_overrides)))
}

let config: Configuration = figment.extract()?;
let mut config: Configuration = figment.extract()?;

config.common.set_derived_values();
Ok(config.apply_cascading_values())
}

Expand Down
Loading

0 comments on commit 484a0df

Please sign in to comment.