WIP: configure-switch #159

Closed
johnride wants to merge 18 commits from configure-switch into master
3 changed files with 269 additions and 50 deletions
Showing only changes of commit fe0501b784 - Show all commits

View File

@ -28,6 +28,7 @@ use super::PreparationOutcome;
use super::Router;
use super::Switch;
use super::SwitchError;
use super::SwitchNetworkConfig;
use super::TftpServer;
use super::Topology;
@ -280,6 +281,14 @@ impl Switch for HAClusterTopology {
) -> Result<(), SwitchError> {
todo!()
}
async fn configure_switch_network(
&self,
_host: &PhysicalHost,
_config: SwitchNetworkConfig,
) -> Result<(), SwitchError> {
todo!()
}
}
#[derive(Debug)]

View File

@ -179,8 +179,14 @@ pub trait Switch: Send + Sync {
async fn configure_host_network(
&self,
_host: &PhysicalHost,
_config: HostNetworkConfig,
host: &PhysicalHost,
config: HostNetworkConfig,
) -> Result<(), SwitchError>;
async fn configure_switch_network(
&self,
host: &PhysicalHost,
config: SwitchNetworkConfig,
) -> Result<(), SwitchError>;
}
@ -200,6 +206,16 @@ pub struct SlaveInterface {
// FIXME: Should we add speed as well? And other params
}
#[derive(Clone, Debug, PartialEq)]
pub struct SwitchNetworkConfig {
pub port_channel: PortChannel,
}
#[derive(Clone, Debug, PartialEq)]
pub struct PortChannel {
pub ports: Vec<String>,
}
#[derive(Debug, Clone, new)]
pub struct SwitchError {
msg: String,

View File

@ -8,7 +8,10 @@ use crate::{
interpret::{Interpret, InterpretError, InterpretName, InterpretStatus, Outcome},
inventory::Inventory,
score::Score,
topology::{self, Bond, HostNetworkConfig, SlaveInterface, Switch, Topology},
topology::{
self, Bond, HostNetworkConfig, PortChannel, SlaveInterface, Switch, SwitchNetworkConfig,
Topology,
},
};
#[derive(Debug, Clone, Serialize)]
@ -56,20 +59,34 @@ impl<T: Topology + Switch> Interpret<T> for HostNetworkConfigurationInterpret {
_inventory: &Inventory,
topology: &T,
) -> Result<Outcome, InterpretError> {
let host = self.score.hosts.first().unwrap();
let mac_addresses = host.get_mac_address();
let mac_address = mac_addresses.first().unwrap();
let host_network_config = HostNetworkConfig {
bond: Bond {
interfaces: vec![SlaveInterface {
mac_address: *mac_address,
}],
},
};
for host in &self.score.hosts {
let mut interfaces = vec![];
let mut ports = vec![];
let _ = topology
.configure_host_network(host, host_network_config)
.await;
for mac_address in host.get_mac_address() {
if let Some(port) = topology.get_port_for_mac_address(&mac_address).await {
interfaces.push(SlaveInterface { mac_address });
ports.push(port);
}
}
let _ = topology
.configure_host_network(
host,
HostNetworkConfig {
bond: Bond { interfaces },
},
)
.await;
let _ = topology
.configure_switch_network(
host,
SwitchNetworkConfig {
port_channel: PortChannel { ports },
},
)
.await;
}
// foreach hosts
// foreach mac addresses
@ -78,17 +95,12 @@ impl<T: Topology + Switch> Interpret<T> for HostNetworkConfigurationInterpret {
// create bond for all valid addresses (port found)
// apply network to host first, then switch (to avoid losing hosts that are already connected)
// topology.configure_host_network(host, config) <--- will create bonds
// topology.configure_switch_network(port, config) <--- will create port channels
// topology.configure_switch_network(host, config) <--- will create port channels
Ok(Outcome::success("".into()))
}
}
struct PortMapping {
port: String,
mac_address: MacAddress,
}
#[cfg(test)]
mod tests {
use assertor::*;
@ -98,8 +110,8 @@ mod tests {
use crate::{
hardware::HostCategory,
topology::{
Bond, HostNetworkConfig, PreparationError, PreparationOutcome, SlaveInterface,
SwitchError,
Bond, HostNetworkConfig, PortChannel, PreparationError, PreparationOutcome,
SlaveInterface, SwitchError, SwitchNetworkConfig,
},
};
use std::{
@ -111,15 +123,22 @@ mod tests {
lazy_static! {
pub static ref HOST_ID: Id = Id::from_str("host-1").unwrap();
pub static ref INTERFACE: MacAddress =
MacAddress::try_from("00:11:22:33:44:55".to_string()).unwrap();
pub static ref ANOTHER_HOST_ID: Id = Id::from_str("host-2").unwrap();
pub static ref EXISTING_INTERFACE: MacAddress =
MacAddress::try_from("00:00:00:00:00:00".to_string()).unwrap();
pub static ref ANOTHER_EXISTING_INTERFACE: MacAddress =
MacAddress::try_from("42:42:42:42:42:42".to_string()).unwrap();
pub static ref UNKNOWN_INTERFACE: MacAddress =
MacAddress::try_from("99:99:99:99:99:99".to_string()).unwrap();
pub static ref PORT: String = "1/0/42".into();
pub static ref ANOTHER_PORT: String = "2/0/42".into();
}
#[tokio::test]
async fn one_host_one_mac_address_should_create_bond_with_one_interface() {
let host = given_host(&HOST_ID, *INTERFACE);
async fn host_with_one_mac_address_should_create_bond_with_one_interface() {
let host = given_host(&HOST_ID, vec![*EXISTING_INTERFACE]);
let score = given_score(vec![host]);
let topology = SwitchWithPortTopology::new();
let topology = TopologyWithSwitch::new();
let _ = score.interpret(&Inventory::empty(), &topology).await;
@ -129,28 +148,161 @@ mod tests {
HostNetworkConfig {
bond: Bond {
interfaces: vec![SlaveInterface {
mac_address: *INTERFACE,
mac_address: *EXISTING_INTERFACE,
}],
},
},
)]);
}
fn given_host(id: &Id, mac_address: MacAddress) -> PhysicalHost {
#[tokio::test]
async fn host_with_one_mac_address_should_create_port_channel_with_one_port() {
let host = given_host(&HOST_ID, vec![*EXISTING_INTERFACE]);
let score = given_score(vec![host]);
let topology = TopologyWithSwitch::new();
let _ = score.interpret(&Inventory::empty(), &topology).await;
let configured_switch_networks = topology.configured_switch_networks.lock().unwrap();
assert_that!(*configured_switch_networks).contains_exactly(vec![(
HOST_ID.clone(),
SwitchNetworkConfig {
port_channel: PortChannel {
ports: vec![PORT.clone()],
},
},
)]);
}
#[tokio::test]
async fn host_with_multiple_mac_addresses_should_create_one_bond_with_all_interfaces() {
let score = given_score(vec![given_host(
&HOST_ID,
vec![*EXISTING_INTERFACE, *ANOTHER_EXISTING_INTERFACE],
)]);
let topology = TopologyWithSwitch::new();
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 {
bond: Bond {
interfaces: vec![
SlaveInterface {
mac_address: *EXISTING_INTERFACE,
},
SlaveInterface {
mac_address: *ANOTHER_EXISTING_INTERFACE,
},
],
},
},
)]);
}
#[tokio::test]
async fn multiple_hosts_should_create_one_bond_per_host() {
let score = given_score(vec![
given_host(&HOST_ID, vec![*EXISTING_INTERFACE]),
given_host(&ANOTHER_HOST_ID, vec![*ANOTHER_EXISTING_INTERFACE]),
]);
let topology = TopologyWithSwitch::new();
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 {
bond: Bond {
interfaces: vec![SlaveInterface {
mac_address: *EXISTING_INTERFACE,
}],
},
},
),
(
ANOTHER_HOST_ID.clone(),
HostNetworkConfig {
bond: Bond {
interfaces: vec![SlaveInterface {
mac_address: *ANOTHER_EXISTING_INTERFACE,
}],
},
},
),
]);
}

If I understand correctly this has the correct behavior. It is perfectly normal for a host to have mac addresses that are not connected to the switch. Most of them do have disconnected 1gb interfaces.

It is also normal to find connected interfaces in the switch that are not part of the cluster. If an admin connects to the switch for debugging, or the switch is used for other purposes than only this cluster.

If I understand correctly this has the correct behavior. It is perfectly normal for a host to have mac addresses that are not connected to the switch. Most of them do have disconnected 1gb interfaces. It is also normal to find connected interfaces in the switch that are not part of the cluster. If an admin connects to the switch for debugging, or the switch is used for other purposes than only this cluster.

in that case what should we do if we realize that not a single mac addresses were found on the switch? should we simply ignore it (current solution) or should we clear existing configurations to a clean slate?

and actually the question will be similar when we'll approach the "update config" scenario: what to do with existing bonds? and port-channels? should we just clear everything before and just apply the new config? or a smart way to update existing bonds/port-channels?

in that case what should we do if we realize that not a single mac addresses were found on the switch? should we simply ignore it (current solution) or should we clear existing configurations to a clean slate? and actually the question will be similar when we'll approach the "update config" scenario: what to do with existing bonds? and port-channels? should we just clear everything before and just apply the new config? or a smart way to update existing bonds/port-channels?

Ok, I think I figured it out and it's not as complicated as it seems.

The problem is not only specific replacement logic, it is also idempotency. Does the current code currently support detecting that the mac address is connected to a port that is already in a port channel with the correct configuration (correct mac + ports assigned to it) ?

I think the correct behavior here is :

  • You have a list of mac addresses that can be bonded together (the entire list for a single host)
  • Find all the port numbers in the switch stack that match the host
  • Check if any of those are already in a port channel
  • If more than one port channel found, crash with a clear error message that the status of the switch configuration is not supported with the actual, wrong config clearly outlined.
  • Ensure the port channel ends up with the correct config (all found ports and only found ports, no foreign port allowed)

This behavior should be quite easily testable too 😁

Ok, I think I figured it out and it's not as complicated as it seems. The problem is not only specific replacement logic, it is also idempotency. Does the current code currently support detecting that the mac address is connected to a port that is already in a port channel with the correct configuration (correct mac + ports assigned to it) ? I think the correct behavior here is : - You have a list of mac addresses that can be bonded together (the entire list for a single host) - Find all the port numbers in the switch stack that match the host - Check if any of those are already in a port channel - If more than one port channel found, crash with a clear error message that the status of the switch configuration is not supported with the actual, wrong config clearly outlined. - Ensure the port channel ends up with the correct config (all found ports and only found ports, no foreign port allowed) This behavior should be quite easily testable too 😁
#[tokio::test]
async fn multiple_hosts_should_create_one_port_channel_per_host() {
let score = given_score(vec![
given_host(&HOST_ID, vec![*EXISTING_INTERFACE]),
given_host(&ANOTHER_HOST_ID, vec![*ANOTHER_EXISTING_INTERFACE]),
]);
let topology = TopologyWithSwitch::new();
let _ = score.interpret(&Inventory::empty(), &topology).await;
let configured_switch_networks = topology.configured_switch_networks.lock().unwrap();
assert_that!(*configured_switch_networks).contains_exactly(vec![
(
HOST_ID.clone(),
SwitchNetworkConfig {
port_channel: PortChannel {
ports: vec![PORT.clone()],
},
},
),
(
ANOTHER_HOST_ID.clone(),
SwitchNetworkConfig {
port_channel: PortChannel {
ports: vec![ANOTHER_PORT.clone()],
},
},
),
]);
}
#[tokio::test]
async fn port_not_found_for_mac_address_should_not_configure_interface() {
// FIXME: Should it still configure an empty bond/port channel?
let score = given_score(vec![given_host(&HOST_ID, vec![*UNKNOWN_INTERFACE])]);
let topology = TopologyWithSwitch::new_port_not_found();
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 {
bond: Bond { interfaces: vec![] },
},
)]);
let configured_switch_networks = topology.configured_switch_networks.lock().unwrap();
assert_that!(*configured_switch_networks).contains_exactly(vec![(
HOST_ID.clone(),
SwitchNetworkConfig {
port_channel: PortChannel { ports: vec![] },
},
)]);
}
fn given_score(hosts: Vec<PhysicalHost>) -> HostNetworkConfigurationScore {
HostNetworkConfigurationScore { hosts }
}
fn given_host(id: &Id, mac_addresses: Vec<MacAddress>) -> PhysicalHost {
let network = mac_addresses.iter().map(|m| given_interface(*m)).collect();
PhysicalHost {
id: id.clone(),
category: HostCategory::Server,
network: vec![NetworkInterface {
name: "interface-1".into(),
mac_address,
speed_mbps: None,
is_up: true,
mtu: 1,
ipv4_addresses: vec![],
ipv6_addresses: vec![],
driver: "driver".into(),
firmware_version: None,
}],
network,
storage: vec![],
labels: vec![],
memory_modules: vec![],
@ -158,24 +310,46 @@ mod tests {
}
}
fn given_score(hosts: Vec<PhysicalHost>) -> HostNetworkConfigurationScore {
HostNetworkConfigurationScore { hosts }
fn given_interface(mac_address: MacAddress) -> NetworkInterface {
NetworkInterface {
name: format!("{mac_address}"),
mac_address,
speed_mbps: None,
is_up: true,
mtu: 1,
ipv4_addresses: vec![],
ipv6_addresses: vec![],
driver: "driver".into(),
firmware_version: None,
}
}
struct SwitchWithPortTopology {
struct TopologyWithSwitch {
available_ports: Vec<String>,
configured_host_networks: Arc<Mutex<Vec<(Id, HostNetworkConfig)>>>,
configured_switch_networks: Arc<Mutex<Vec<(Id, SwitchNetworkConfig)>>>,
}
impl SwitchWithPortTopology {
impl TopologyWithSwitch {
fn new() -> Self {
Self {
available_ports: vec![PORT.clone(), ANOTHER_PORT.clone()],
configured_host_networks: Arc::new(Mutex::new(vec![])),
configured_switch_networks: Arc::new(Mutex::new(vec![])),
}
}
fn new_port_not_found() -> Self {
Self {
available_ports: vec![],
configured_host_networks: Arc::new(Mutex::new(vec![])),
configured_switch_networks: Arc::new(Mutex::new(vec![])),
}
}
}
#[async_trait]
impl Topology for SwitchWithPortTopology {
impl Topology for TopologyWithSwitch {
fn name(&self) -> &str {
"SwitchWithPortTopology"
}
@ -186,9 +360,18 @@ mod tests {
}
#[async_trait]
impl Switch for SwitchWithPortTopology {
async fn get_port_for_mac_address(&self, mac_address: &MacAddress) -> Option<String> {
Some("1/0/42".into())
impl Switch for TopologyWithSwitch {
async fn get_port_for_mac_address(&self, _mac_address: &MacAddress) -> Option<String> {
if self.available_ports.is_empty() {
return None;
}
Some(
self.available_ports
.get(self.configured_host_networks.lock().unwrap().len() % 2)
.unwrap()
.clone(),
)
}
async fn configure_host_network(
@ -201,5 +384,16 @@ mod tests {
Ok(())
}
async fn configure_switch_network(
&self,
host: &PhysicalHost,
config: SwitchNetworkConfig,
) -> Result<(), SwitchError> {
let mut configured_switch_networks = self.configured_switch_networks.lock().unwrap();
configured_switch_networks.push((host.id.clone(), config.clone()));
Ok(())
}
}
}