WIP: configure-switch #159

Closed
johnride wants to merge 18 commits from configure-switch into master
7 changed files with 171 additions and 65 deletions
Showing only changes of commit f2f55d98d4 - Show all commits

View File

@ -1,4 +1,5 @@
use std::{
borrow::Cow,
fmt::{self, Display},
sync::Arc,
};
@ -6,8 +7,11 @@ use std::{
use async_trait::async_trait;
use harmony_types::net::{IpAddress, MacAddress};
use log::{debug, info};
use russh::client::{Handle, Handler};
use russh_keys::key;
use russh::{
client::{Handle, Handler},
kex::DH_G1_SHA1,
};
use russh_keys::key::{self, SSH_RSA};
use std::str::FromStr;
#[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Clone)]
@ -19,6 +23,30 @@ pub struct MacAddressEntry {
pub struct BrocadeClient {
client: Handle<Client>,
options: BrocadeOptions,
}
#[derive(Default, Clone, Debug)]
pub struct BrocadeOptions {
pub dry_run: bool,
pub ssh: SshOptions,
}
#[derive(Clone, Debug)]
pub struct SshOptions {
pub preferred_algorithms: russh::Preferred,
}
impl Default for SshOptions {
fn default() -> Self {
Self {
preferred_algorithms: russh::Preferred {
kex: Cow::Borrowed(&[DH_G1_SHA1]),
key: Cow::Borrowed(&[SSH_RSA]),
..Default::default()
},
}
}
}
impl BrocadeClient {
@ -26,14 +54,25 @@ impl BrocadeClient {
ip_addresses: &[IpAddress],
username: &str,
password: &str,
options: Option<BrocadeOptions>,
) -> Result<Self, Error> {
let ip = ip_addresses[0]; // FIXME: Find a better way to get master switch IP address
if ip_addresses.is_empty() {
return Err(Error::ConfigurationError(
"No IP addresses provided".to_string(),
));
}
let config = russh::client::Config::default();
let ip = ip_addresses[0]; // FIXME: Find a better way to get master switch IP address
let options = options.unwrap_or_default();
let config = russh::client::Config {
preferred: options.ssh.preferred_algorithms.clone(),
..Default::default()
};
let mut client = russh::client::connect(Arc::new(config), (ip, 22), Client {}).await?;
match client.authenticate_password(username, password).await? {
true => Ok(Self { client }),
true => Ok(Self { client, options }),
false => Err(Error::AuthenticationError(
"ssh authentication failed".to_string(),
)),
@ -41,7 +80,7 @@ impl BrocadeClient {
}
pub async fn show_mac_address_table(&self) -> Result<Vec<MacAddressEntry>, Error> {
let output = self.run_command("show mac-address-table").await?;
let output = self.run_command("show mac-address").await?;
let mut entries = Vec::new();
// The Brocade output usually has a header and then one entry per line.
@ -104,7 +143,7 @@ impl BrocadeClient {
pub async fn find_available_channel_id(&self) -> Result<u8, Error> {
debug!("[Brocade] Finding next available channel id...");
let output = self.run_command("show port-channel summary").await?;
let output = self.run_command("show lag").await?;
let mut used_ids = Vec::new();
// Sample output line: "3 Po3(SU) LACP Eth Yes 128/128 active "
@ -121,7 +160,7 @@ impl BrocadeClient {
// Sort the used IDs to find the next available number.
used_ids.sort();
let mut next_id = 1;
let mut next_id = 0;
for &id in &used_ids {
if id == next_id {
next_id += 1;
@ -136,6 +175,11 @@ impl BrocadeClient {
}
async fn run_command(&self, command: &str) -> Result<String, Error> {
if !command.starts_with("show") && self.options.dry_run {
info!("[Brocade] Dry-run mode enabled, skipping command: {command}");
return Ok("".into());
}
debug!("[Brocade] Running command: '{command}'...");
let mut channel = self.client.channel_open_session().await?;
@ -161,9 +205,13 @@ impl BrocadeClient {
)));
}
}
russh::ChannelMsg::Success
| russh::ChannelMsg::WindowAdjusted { .. }
| russh::ChannelMsg::Eof => {}
russh::ChannelMsg::Eof => {
channel.close().await?;
}
russh::ChannelMsg::Close => {
break;
}
russh::ChannelMsg::Success | russh::ChannelMsg::WindowAdjusted { .. } => {}
_ => {
return Err(Error::UnexpectedError(format!(
"Russh got unexpected msg {msg:?}"
@ -172,20 +220,25 @@ impl BrocadeClient {
}
}
channel.close().await?;
let output = String::from_utf8(output).expect("Output should be UTF-8 compatible");
debug!("[Brocade] Command output:\n{output}");
Ok(output)
}
async fn run_commands(&self, commands: Vec<String>) -> Result<(), Error> {
let mut channel = self.client.channel_open_session().await?;
// Execute commands sequentially and check for errors immediately.
for command in commands {
if !command.starts_with("show") && self.options.dry_run {
info!("[Brocade] Dry-run mode enabled, skipping command: {command}");
continue;
}
debug!("[Brocade] Running command: '{command}'...");
let mut channel = self.client.channel_open_session().await?;
let mut output = Vec::new();
let mut close_received = false;
channel.exec(true, command.as_str()).await?;
loop {
@ -206,12 +259,30 @@ impl BrocadeClient {
)));
}
}
_ => {} // Ignore other messages like success or EOF for now.
russh::ChannelMsg::Eof => {
channel.close().await?;
}
russh::ChannelMsg::Close => {
close_received = true;
break;
}
russh::ChannelMsg::Success | russh::ChannelMsg::WindowAdjusted { .. } => {}
_ => {
return Err(Error::UnexpectedError(format!(
"Russh got unexpected msg {msg:?}"
)));
}
}
}
channel.close().await?;
if !close_received {
return Err(Error::UnexpectedError(format!(
"Channel closed without receiving a final CLOSE message for command: {}",
command
)));
}
}
Ok(())
}
}

View File

@ -12,11 +12,11 @@ pub type FirewallGroup = Vec<PhysicalHost>;
pub struct PhysicalHost {
pub id: Id,
pub category: HostCategory,
pub network: Vec<NetworkInterface>,
pub storage: Vec<StorageDrive>,
pub network: Vec<NetworkInterface>, // FIXME: Don't use harmony_inventory_agent::NetworkInterface
Review

I understand we don't want to use the "inventory agent" data structures, but I think it's OK to have one base data structure that we try to reuse across crates for basic components like NetworkInterface and these few others.

IMO, the FIXME should be about refactoring the type into a shared types crate or something like that.

Why would that not be correct?

I understand we don't want to use the "inventory agent" data structures, but I think it's OK to have one base data structure that we try to reuse across crates for basic components like NetworkInterface and these few others. IMO, the FIXME should be about refactoring the type into a shared types crate or something like that. Why would that not be correct?
Review

that's one of the options, yes 😉 the fixme is just stating the problem: we should not use directly this type from harmony_inventory_agent

possible solutions are:

  1. (what you suggested) move it to harmony_types and both harmony_inventory_agent and harmony uses it
  2. keep the harmony_inventory_agent::NetworkInterface as is and have a different structure in harmony and map from one to the other

As of now I'm personally 50/50: I have a feeling that the needs from harmony_inventory_agent might evolve differently from harmony thus the NetworkInterface might evolve differently. So going with option 1. wouldn't be adequate. But on the other hand for now they are indeed very similar so why maintain 2 structs? 🤷

Anyway for now it's not really a big issue and won't be hard to fix. So it's more like a note for the future to be aware of.

that's one of the options, yes 😉 the `fixme` is just stating the problem: we should not use directly this type from `harmony_inventory_agent` possible solutions are: 1. (what you suggested) move it to `harmony_types` and both `harmony_inventory_agent` and `harmony` uses it 2. keep the `harmony_inventory_agent::NetworkInterface` as is and have a different structure in `harmony` and map from one to the other As of now I'm personally 50/50: I have a feeling that the needs from `harmony_inventory_agent` might evolve differently from `harmony` thus the `NetworkInterface` might evolve differently. So going with option 1. wouldn't be adequate. But on the other hand for now they are indeed very similar so why maintain 2 structs? 🤷 Anyway for now it's not really a big issue and won't be hard to fix. So it's more like a note for the future to be aware of.
pub storage: Vec<StorageDrive>, // FIXME: Don't use harmony_inventory_agent::StorageDrive
pub labels: Vec<Label>,
pub memory_modules: Vec<MemoryModule>,
pub cpus: Vec<CPU>,
pub memory_modules: Vec<MemoryModule>, // FIXME: Don't use harmony_inventory_agent::MemoryModule
pub cpus: Vec<CPU>, // FIXME: Don't use harmony_inventory_agent::CPU
}
impl PhysicalHost {

View File

@ -1,4 +1,5 @@
use async_trait::async_trait;
use brocade::BrocadeOptions;
use harmony_macros::ip;
use harmony_secret::SecretManager;
use harmony_types::net::MacAddress;
@ -305,7 +306,12 @@ impl HAClusterTopology {
// FIXME: We assume Brocade switches
let switches: Vec<IpAddr> = self.switch.iter().map(|s| s.ip).collect();
let client = BrocadeSwitchClient::init(&switches, &auth.username, &auth.password)
let brocade_options = Some(BrocadeOptions {
dry_run: *crate::config::DRY_RUN,
..Default::default()
});
let client =
BrocadeSwitchClient::init(&switches, &auth.username, &auth.password, brocade_options)
.await
.map_err(|e| SwitchError::new(format!("Failed to connect to switch: {e}")))?;
@ -506,14 +512,13 @@ impl HttpServer for HAClusterTopology {
#[async_trait]
impl Switch for HAClusterTopology {
async fn get_port_for_mac_address(&self, mac_address: &MacAddress) -> Option<String> {
let client = self.get_switch_client().await;
let Ok(client) = client else {
return None;
};
client.find_port(mac_address).await
async fn get_port_for_mac_address(
&self,
mac_address: &MacAddress,
) -> Result<Option<String>, SwitchError> {
let client = self.get_switch_client().await?;
let port = client.find_port(mac_address).await?;
Ok(port)
}
async fn configure_host_network(

View File

@ -175,7 +175,10 @@ impl FromStr for DnsRecordType {
#[async_trait]
pub trait Switch: Send + Sync {
async fn get_port_for_mac_address(&self, mac_address: &MacAddress) -> Option<String>;
async fn get_port_for_mac_address(
&self,
mac_address: &MacAddress,
) -> Result<Option<String>, SwitchError>;
async fn configure_host_network(
&self,
@ -218,7 +221,7 @@ impl Error for SwitchError {}
#[async_trait]
pub trait SwitchClient: Send + Sync {
async fn find_port(&self, mac_address: &MacAddress) -> Option<String>;
async fn find_port(&self, mac_address: &MacAddress) -> Result<Option<String>, SwitchError>;
async fn configure_port_channel(&self, switch_ports: Vec<String>) -> Result<u8, SwitchError>;
}

View File

@ -1,5 +1,5 @@
use async_trait::async_trait;
use brocade::BrocadeClient;
use brocade::{BrocadeClient, BrocadeOptions};
use harmony_secret::Secret;
use harmony_types::net::{IpAddress, MacAddress};
use serde::{Deserialize, Serialize};
@ -15,23 +15,26 @@ impl BrocadeSwitchClient {
ip_addresses: &[IpAddress],
username: &str,
password: &str,
options: Option<BrocadeOptions>,
) -> Result<Self, brocade::Error> {
let brocade = BrocadeClient::init(ip_addresses, username, password).await?;
let brocade = BrocadeClient::init(ip_addresses, username, password, options).await?;
Ok(Self { brocade })
}
}
#[async_trait]
impl SwitchClient for BrocadeSwitchClient {
async fn find_port(&self, mac_address: &MacAddress) -> Option<String> {
let Ok(table) = self.brocade.show_mac_address_table().await else {
return None;
};
async fn find_port(&self, mac_address: &MacAddress) -> Result<Option<String>, SwitchError> {
let table = self
.brocade
.show_mac_address_table()
.await
.map_err(|e| SwitchError::new(format!("Failed to get mac address table: {e}")))?;
table
Ok(table
.iter()
.find(|entry| entry.mac_address == *mac_address)
.map(|entry| entry.port_name.clone())
.map(|entry| entry.port_name.clone()))
}
async fn configure_port_channel(&self, switch_ports: Vec<String>) -> Result<u8, SwitchError> {

View File

@ -1,5 +1,6 @@
use async_trait::async_trait;
use harmony_types::id::Id;
use log::{debug, info, warn};
use serde::Serialize;
use crate::{
@ -56,13 +57,21 @@ impl<T: Topology + Switch> Interpret<T> for HostNetworkConfigurationInterpret {
_inventory: &Inventory,
topology: &T,
) -> Result<Outcome, InterpretError> {
info!(
"Started network configuration for {} host(s)...",
self.score.hosts.len()
);
let mut configured_host_count = 0;
for host in &self.score.hosts {
let mut switch_ports = vec![];
for network_interface in &host.network {
let mac_address = network_interface.mac_address;
if let Some(port_name) = topology.get_port_for_mac_address(&mac_address).await {
match topology.get_port_for_mac_address(&mac_address).await {
Ok(Some(port_name)) => {
switch_ports.push(SwitchPort {
interface: NetworkInterface {
name: network_interface.name.clone(),
@ -73,14 +82,27 @@ impl<T: Topology + Switch> Interpret<T> for HostNetworkConfigurationInterpret {
port_name,
});
}
Ok(None) => debug!("No port found for host '{}', skipping", host.id),
Err(e) => warn!("Failed to get port for host '{}': {}", host.id, e),
}
}
if !switch_ports.is_empty() {
configured_host_count += 1;
let _ = topology
.configure_host_network(host, HostNetworkConfig { switch_ports })
.await;
}
}
Ok(Outcome::success("".into()))
if configured_host_count > 0 {
Ok(Outcome::success(format!(
"Configured {configured_host_count}/{} host(s)",
self.score.hosts.len()
)))
} else {
Ok(Outcome::noop("No hosts configured".into()))
}
}
}
@ -221,12 +243,7 @@ mod tests {
let _ = score.interpret(&Inventory::empty(), &topology).await;
let configured_host_networks = topology.configured_host_networks.lock().unwrap();
assert_that!(*configured_host_networks).contains_exactly(vec![(
HOST_ID.clone(),
HostNetworkConfig {
switch_ports: vec![],
},
)]);
assert_that!(*configured_host_networks).is_empty();
}
fn given_score(hosts: Vec<PhysicalHost>) -> HostNetworkConfigurationScore {
@ -297,12 +314,15 @@ mod tests {
#[async_trait]
impl Switch for TopologyWithSwitch {
async fn get_port_for_mac_address(&self, _mac_address: &MacAddress) -> Option<String> {
async fn get_port_for_mac_address(
&self,
_mac_address: &MacAddress,
) -> Result<Option<String>, SwitchError> {
let mut ports = self.available_ports.lock().unwrap();
if ports.is_empty() {
return None;
return Ok(None);
}
Some(ports.remove(0))
Ok(Some(ports.remove(0)))
}
async fn configure_host_network(

View File

@ -54,6 +54,9 @@ struct DeployArgs {
#[arg(long = "profile", short = 'p', default_value = "dev")]
harmony_profile: HarmonyProfile,
#[arg(long = "dry-run", short = 'd', default_value = "false")]
dry_run: bool,
}
#[derive(Args, Clone, Debug)]
@ -178,6 +181,7 @@ async fn main() {
command
.env("HARMONY_USE_LOCAL_K3D", format!("{use_local_k3d}"))
.env("HARMONY_PROFILE", format!("{}", args.harmony_profile))
.env("HARMONY_DRY_RUN", format!("{}", args.dry_run))
.arg("-y")
.arg("-a");