Skip to content

Commit

Permalink
fix: Revert "feat: [NODE-1327] Replace mac_address with `determinis…
Browse files Browse the repository at this point in the history
…tic_ips` (#2785)

This reverts commit 245e13e (#2757)

The commit seems to introduce issues in nested tests.
  • Loading branch information
nmattia authored Nov 23, 2024
1 parent efa77e7 commit 8869fff
Show file tree
Hide file tree
Showing 38 changed files with 632 additions and 322 deletions.
40 changes: 17 additions & 23 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -126,18 +126,18 @@ members = [
"rs/http_endpoints/public",
"rs/http_endpoints/xnet",
"rs/http_utils",
"rs/ic_os/deterministic_ips",
"rs/ic_os/dev_test_tools/deterministic_ips",
"rs/ic_os/build_tools/dflate",
"rs/ic_os/build_tools/diroid",
"rs/ic_os/config",
"rs/ic_os/config_types",
"rs/ic_os/fstrim_tool",
"rs/ic_os/metrics_tool",
"rs/ic_os/os_tools/guestos_tool",
"rs/ic_os/os_tools/hostos_tool",
"rs/ic_os/build_tools/inject_files",
"rs/ic_os/dev_test_tools/launch-single-vm",
"rs/ic_os/network",
"rs/ic_os/network/mac_address",
"rs/ic_os/nft_exporter",
"rs/ic_os/nss_icos",
"rs/ic_os/dev_test_tools/setupos-inject-configuration",
Expand Down
2 changes: 1 addition & 1 deletion ic-os/dev-tools/bare_metal_deployment/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ The csv file contains secrets which should _not_ be submitted via the command li

deploy.py requires a CSV file with the information to deploy to multiple BMC's. Include the BMC info _for each BMC_ where each row is "ip address, username, password".

Each row optionally includes a final parameter - the GuestOS ipv6 address. This is used to check if the resulting machine has deployed successfully. This is calculated deterministically. See bazel target /rs/ic_os/deterministic_ips to calculate.
Each row optionally includes a final parameter - the GuestOS ipv6 address. This is used to check if the resulting machine has deployed successfully. This is calculated deterministically. See bazel target /rs/ic_os/dev_test_tools/deterministic_ips to calculate.

This file is plaintext readable - make it readable only by the current user.

Expand Down
8 changes: 2 additions & 6 deletions rs/ic_os/config/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,7 @@ package(default_visibility = ["//rs:ic-os-pkg"])

DEPENDENCIES = [
# Keep sorted.
"//rs/ic_os/config_types",
"//rs/ic_os/deterministic_ips",
"//rs/ic_os/network/mac_address",
"//rs/ic_os/utils",
"//rs/types/types",
"@crate_index//:anyhow",
Expand Down Expand Up @@ -45,10 +44,7 @@ rust_binary(
crate_name = "config",
edition = "2021",
proc_macro_deps = MACRO_DEPENDENCIES,
deps = [
":config_lib",
"//rs/ic_os/network",
] + DEPENDENCIES,
deps = [":config_lib"] + DEPENDENCIES,
)

rust_test(
Expand Down
4 changes: 1 addition & 3 deletions rs/ic_os/config/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,7 @@ serde_json = { workspace = true }
serde = { workspace = true }
serde_with = "1.6.2"
regex = { workspace = true }
config_types = { path = "../config_types" }
deterministic_ips = { path = "../deterministic_ips" }
network = { path = "../network" } # Only required by bin
mac_address = { path = "../network/mac_address" }

[dev-dependencies]
once_cell = "1.8"
Expand Down
6 changes: 3 additions & 3 deletions rs/ic_os/config/src/config_ini.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
use regex::Regex;
use std::collections::HashMap;
use std::fs::read_to_string;
use std::net::{Ipv4Addr, Ipv6Addr};
use std::path::Path;

use config_types::ConfigMap;

use anyhow::bail;
use anyhow::{Context, Result};
use regex::Regex;

pub type ConfigMap = HashMap<String, String>;
pub struct ConfigIniSettings {
pub ipv6_prefix: String,
pub ipv6_prefix_length: u8,
Expand Down
20 changes: 11 additions & 9 deletions rs/ic_os/config/src/generate_testnet_config.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
use anyhow::Result;
use deterministic_ips::{Deployment, HwAddr};
use mac_address::mac_address::FormattedMacAddress;
use std::net::{Ipv4Addr, Ipv6Addr};
use std::path::PathBuf;
use url::Url;

use crate::serialize_and_write_config;
use config_types::*;
use crate::types::*;

#[derive(Default)]
pub struct GenerateTestnetConfigArgs {
Expand All @@ -23,8 +23,8 @@ pub struct GenerateTestnetConfigArgs {

// ICOSSettings arguments
pub node_reward_type: Option<String>,
pub mgmt_mac: Option<HwAddr>,
pub deployment_environment: Option<Deployment>,
pub mgmt_mac: Option<String>,
pub deployment_environment: Option<String>,
pub elasticsearch_hosts: Option<String>,
pub elasticsearch_tags: Option<String>,
pub use_nns_public_key: Option<bool>,
Expand Down Expand Up @@ -164,12 +164,14 @@ fn create_guestos_config(config: GenerateTestnetConfigArgs) -> Result<GuestOSCon
let node_reward_type = node_reward_type.unwrap_or_else(|| "type3.1".to_string());

let mgmt_mac = match mgmt_mac {
Some(mac) => mac,
// Use a dummy MAC address
None => "00:00:00:00:00:00".parse()?,
Some(mac_str) => FormattedMacAddress::try_from(mac_str.as_str())?,
None => {
// Use a dummy MAC address
FormattedMacAddress::try_from("00:00:00:00:00:00")?
}
};

let deployment_environment = deployment_environment.unwrap_or(Deployment::Testnet);
let deployment_environment = deployment_environment.unwrap_or_else(|| "testnet".to_string());

let logging = Logging {
elasticsearch_hosts: elasticsearch_hosts.unwrap_or_else(|| "".to_string()),
Expand Down Expand Up @@ -277,7 +279,7 @@ mod tests {
fn test_valid_configuration() {
let args = GenerateTestnetConfigArgs {
ipv6_config_type: Some(Ipv6ConfigType::RouterAdvertisement),
mgmt_mac: Some("00:11:22:33:44:55".parse().unwrap()),
mgmt_mac: Some("00:11:22:33:44:55".to_string()),
nns_urls: Some(vec!["https://example.com".to_string()]),
..Default::default()
};
Expand Down
24 changes: 15 additions & 9 deletions rs/ic_os/config/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
pub mod config_ini;
pub mod deployment_json;
pub mod generate_testnet_config;
pub mod types;

use anyhow::{Context, Result};
use serde::{Deserialize, Serialize};
Expand Down Expand Up @@ -44,11 +45,12 @@ pub fn deserialize_config<T: for<'de> Deserialize<'de>, P: AsRef<Path>>(file_pat

#[cfg(test)]
mod tests {
use config_types::*;
use deterministic_ips::Deployment;
use super::*;
use mac_address::mac_address::FormattedMacAddress;
use types::*;

#[test]
fn test_serialize_and_deserialize() {
fn test_serialize_and_deserialize() -> Result<(), Box<dyn std::error::Error>> {
let ipv6_config = Ipv6Config::Deterministic(DeterministicIpv6Config {
prefix: "2a00:fb01:400:200".to_string(),
prefix_length: 64_u8,
Expand All @@ -72,8 +74,8 @@ mod tests {
let icos_dev_settings = ICOSDevSettings::default();
let icos_settings = ICOSSettings {
node_reward_type: Some("type3.1".to_string()),
mgmt_mac: "ec:2a:72:31:a2:0c".parse().unwrap(),
deployment_environment: Deployment::Mainnet,
mgmt_mac: FormattedMacAddress::try_from("ec:2a:72:31:a2:0c")?,
deployment_environment: "Mainnet".to_string(),
logging,
use_nns_public_key: true,
nns_urls: vec!["http://localhost".parse().unwrap()],
Expand Down Expand Up @@ -136,6 +138,8 @@ mod tests {
serialize_and_deserialize(&setupos_config_struct);
serialize_and_deserialize(&hostos_config_struct);
serialize_and_deserialize(&guestos_config_struct);

Ok(())
}

// Test config version 1.0.0
Expand Down Expand Up @@ -255,19 +259,21 @@ mod tests {
"#;

#[test]
fn test_deserialize_hostos_config_v1_0_0() {
let config: HostOSConfig = serde_json::from_str(HOSTOS_CONFIG_JSON_V1_0_0).unwrap();
fn test_deserialize_hostos_config_v1_0_0() -> Result<(), Box<dyn std::error::Error>> {
let config: HostOSConfig = serde_json::from_str(HOSTOS_CONFIG_JSON_V1_0_0)?;
assert_eq!(config.config_version, "1.0.0");
assert_eq!(config.hostos_settings.vm_cpu, "kvm");
Ok(())
}

#[test]
fn test_deserialize_guestos_config_v1_0_0() {
let config: GuestOSConfig = serde_json::from_str(GUESTOS_CONFIG_JSON_V1_0_0).unwrap();
fn test_deserialize_guestos_config_v1_0_0() -> Result<(), Box<dyn std::error::Error>> {
let config: GuestOSConfig = serde_json::from_str(GUESTOS_CONFIG_JSON_V1_0_0)?;
assert_eq!(config.config_version, "1.0.0");
assert_eq!(
config.icos_settings.mgmt_mac.to_string(),
"ec:2a:72:31:a2:0c"
);
Ok(())
}
}
23 changes: 16 additions & 7 deletions rs/ic_os/config/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,15 @@ use clap::{Args, Parser, Subcommand};
use config::config_ini::{get_config_ini_settings, ConfigIniSettings};
use config::deployment_json::get_deployment_settings;
use config::serialize_and_write_config;
use deterministic_ips::{Deployment, HwAddr};
use network::resolve_mgmt_mac;
use mac_address::mac_address::{get_ipmi_mac, FormattedMacAddress};
use regex::Regex;
use std::fs::File;
use std::path::{Path, PathBuf};

use config::generate_testnet_config::{
generate_testnet_config, GenerateTestnetConfigArgs, Ipv6ConfigType,
};
use config_types::*;
use config::types::*;

#[derive(Subcommand)]
#[allow(clippy::large_enum_variant)]
Expand Down Expand Up @@ -91,9 +90,9 @@ pub struct GenerateTestnetConfigClapArgs {
#[arg(long)]
pub node_reward_type: Option<String>,
#[arg(long)]
pub mgmt_mac: Option<HwAddr>,
pub mgmt_mac: Option<String>,
#[arg(long)]
pub deployment_environment: Option<Deployment>,
pub deployment_environment: Option<String>,
#[arg(long)]
pub elasticsearch_hosts: Option<String>,
#[arg(long)]
Expand Down Expand Up @@ -199,7 +198,17 @@ pub fn main() -> Result<()> {
elasticsearch_tags: None,
};

let mgmt_mac = resolve_mgmt_mac(deployment_json_settings.deployment.mgmt_mac)?;
let mgmt_mac = match deployment_json_settings.deployment.mgmt_mac {
Some(config_mac) => {
let mgmt_mac = FormattedMacAddress::try_from(config_mac.as_str())?;
println!(
"Using mgmt_mac address found in deployment.json: {}",
mgmt_mac
);
mgmt_mac
}
None => get_ipmi_mac()?,
};

let node_reward_type = node_reward_type.expect("Node reward type is required.");

Expand All @@ -214,7 +223,7 @@ pub fn main() -> Result<()> {
let icos_settings = ICOSSettings {
node_reward_type: Some(node_reward_type),
mgmt_mac,
deployment_environment: deployment_json_settings.deployment.name.parse()?,
deployment_environment: deployment_json_settings.deployment.name,
logging,
use_nns_public_key,
nns_urls: deployment_json_settings.nns.url.clone(),
Expand Down
Loading

0 comments on commit 8869fff

Please sign in to comment.