Skip to content

Commit

Permalink
Add a hidden persist-nic-names command
Browse files Browse the repository at this point in the history
xref https://issues.redhat.com//browse/OCPBUGS-10787

In RHEL8 to RHEL9 upgrades, some NIC names may change; we saw
this in the wild with `mlx5` based interfaces at least.
RHEL Leapp has logic to handle this, but adapting it to work
with rpm-ostree/coreos systems would be a heavy lift.

This logic is designed to be executed from a privileged container
*before* the host upgrades, which fits in well with current
OCP logic in the Machine Config Operator.

Specifically, openshift/machine-config-operator#3650
will run this command.

Co-authored-by: Gris Ge <fge@redhat.com>
Signed-off-by: Colin Walters <walters@verbum.org>

service: Add a generated by comment

This is very useful for future introspection.

Signed-off-by: Colin Walters <walters@verbum.org>
  • Loading branch information
cathay4t authored and cgwalters committed Apr 4, 2023
1 parent e5956c1 commit 6ab48ac
Show file tree
Hide file tree
Showing 8 changed files with 330 additions and 6 deletions.
1 change: 1 addition & 0 deletions .dockerignore
@@ -0,0 +1 @@
rust/target
42 changes: 42 additions & 0 deletions .github/workflows/container.yml
@@ -0,0 +1,42 @@
name: Create and publish a container image

on:
push:
branches: ["main"]

env:
REGISTRY: ghcr.io
IMAGE_NAME: ${{ github.repository }}

jobs:
build-and-push-image:
runs-on: ubuntu-latest
permissions:
contents: read
packages: write

steps:
- name: Checkout repository
uses: actions/checkout@v3

- name: Log in to the Container registry
uses: docker/login-action@f054a8b539a109f9f41c372932f1ae047eff08c9
with:
registry: ${{ env.REGISTRY }}
username: ${{ github.actor }}
password: ${{ secrets.GITHUB_TOKEN }}

- name: Extract metadata (tags, labels) for Docker
id: meta
uses: docker/metadata-action@98669ae865ea3cffbcbaa878cf57c20bbf1c6c38
with:
images: ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}

- name: Build and push Docker image
uses: docker/build-push-action@ad44023a93711e3deb337508980b4b5e9bcdc5dc
with:
context: .
file: packaging/Dockerfile.c8s-nmstate-bin
push: true
tags: ${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:latest
labels: ${{ steps.meta.outputs.labels }}
9 changes: 9 additions & 0 deletions packaging/Dockerfile.c8s-nmstate-bin
@@ -0,0 +1,9 @@
FROM quay.io/centos/centos:stream8 as builder
WORKDIR /build
COPY rust .
RUN dnf -y install --setopt=install_weak_deps=False \
binutils git make rust-toolset rpm-build python3 python3-devel && \
dnf clean all && cargo build --release && strip target/release/nmstatectl

FROM quay.io/centos/centos:stream8
COPY --from=builder /build/target/release/nmstatectl /usr/bin/nmstatectl
3 changes: 2 additions & 1 deletion rust/src/cli/Cargo.toml
Expand Up @@ -29,6 +29,7 @@ uuid = { version = "1.1", features = ["v4"] }
chrono = "0.4"

[features]
default = ["query_apply", "gen_conf"]
default = ["query_apply", "gen_conf", "persist_nic"]
query_apply = ["nmstate/query_apply", "ctrlc"]
gen_conf = ["nmstate/gen_conf"]
persist_nic = []
59 changes: 57 additions & 2 deletions rust/src/cli/ncl.rs
Expand Up @@ -8,6 +8,8 @@ mod error;
mod format;
#[cfg(feature = "gen_conf")]
mod gen_conf;
#[cfg(feature = "persist_nic")]
pub(crate) mod persist_nic;
#[cfg(feature = "query_apply")]
mod policy;
#[cfg(feature = "query_apply")]
Expand Down Expand Up @@ -49,6 +51,7 @@ const SUB_CMD_EDIT: &str = "edit";
const SUB_CMD_VERSION: &str = "version";
const SUB_CMD_AUTOCONF: &str = "autoconf";
const SUB_CMD_SERVICE: &str = "service";
const SUB_CMD_PERSIST_NIC_NAMES: &str = "persist-nic-names";
const SUB_CMD_POLICY: &str = "policy";
const SUB_CMD_FORMAT: &str = "format";

Expand All @@ -61,7 +64,7 @@ fn main() {
print_result_and_exit(autoconf(&argv[1..]));
}

let matches = clap::Command::new(APP_NAME)
let mut app = clap::Command::new(APP_NAME)
.version(clap::crate_version!())
.author("Gris Ge <fge@redhat.com>")
.about("Command line of nmstate")
Expand Down Expand Up @@ -317,7 +320,40 @@ fn main() {
.subcommand(
clap::Command::new(SUB_CMD_VERSION)
.about("Show version")
).get_matches();
);
if cfg!(feature = "persist_nic") {
app = app.subcommand(
clap::Command::new(SUB_CMD_PERSIST_NIC_NAMES)
.about("Generate .link files which persist active network interfaces to their current names")
.arg(
clap::Arg::new("DRY_RUN")
.long("dry-run")
.takes_value(false)
.help(
"Only output changes that would be made",
),
)
.arg(
clap::Arg::new("INSPECT")
.long("inspect")
.takes_value(false)
.help(
"Print the state of any persisted nics",
),
)
.arg(
clap::Arg::new("ROOT")
.long("root")
.short('r')
.required(false)
.takes_value(true)
.default_value("/")
.help("Target root filesystem for writing state"),
)
// We don't want to expose this outside of OCP yet
.hide(true));
};
let matches = app.get_matches();
let (log_module_filters, log_level) =
match matches.occurrences_of("verbose") {
0 => (vec!["nmstate", "nm_dbus"], LevelFilter::Info),
Expand Down Expand Up @@ -374,6 +410,25 @@ fn main() {
print_result_and_exit(state_edit(matches));
} else if let Some(matches) = matches.subcommand_matches(SUB_CMD_SERVICE) {
print_result_and_exit(ncl_service(matches));
} else if let Some(matches) =
matches.subcommand_matches(SUB_CMD_PERSIST_NIC_NAMES)
{
#[cfg(feature = "persist_nic")]
{
let action =
if matches.try_contains_id("DRY_RUN").unwrap_or_default() {
persist_nic::PersistAction::DryRun
} else if matches.try_contains_id("INSPECT").unwrap_or_default()
{
persist_nic::PersistAction::Inspect
} else {
persist_nic::PersistAction::Save
};
print_result_and_exit(crate::persist_nic::run_persist_immediately(
matches.value_of("ROOT").unwrap(),
action,
));
}
} else if let Some(matches) = matches.subcommand_matches(SUB_CMD_POLICY) {
print_result_and_exit(policy(matches));
} else if let Some(matches) = matches.subcommand_matches(SUB_CMD_FORMAT) {
Expand Down
216 changes: 216 additions & 0 deletions rust/src/cli/persist_nic.rs
@@ -0,0 +1,216 @@
//! # Handling writing .link files for NICs
//!
//! This module implements logic for generating systemd [`.link`] files
//! based on active networking state.
//!
//! The logic currently is:
//!
//! - Iterate over all active NICs
//! - Skip any that don't have a MAC address (hence are likely non-physical)
//! - Skip any that are using DHCP
//!
//! ## Known broken cases
//!
//! - bond devices over DHCP
//!
//! [`.link`]: https://www.freedesktop.org/software/systemd/man/systemd.link.html
use std::path::Path;

use nmstate::{InterfaceType, NetworkState};

use crate::error::CliError;

/// Comment added into our generated link files
const PERSIST_GENERATED_BY: &str = "# Generated by nmstate";
/// The file prefix for our generated persisted NIC names.
/// 98 here is important as it should be invoked after others but before
/// 99-default.link
const PERSIST_FILE_PREFIX: &str = "98-nmstate";
/// See https://www.freedesktop.org/software/systemd/man/systemd.link.html
const SYSTEMD_NETWORK_LINK_FOLDER: &str = "etc/systemd/network";
/// File which if present signals that we have already performed NIC name persistence.
const NMSTATE_PERSIST_STAMP: &str = ".nmstate-persist.stamp";

/// The action to take
pub(crate) enum PersistAction {
/// Persist NIC name state
Save,
/// Print what we would do in Save mode
DryRun,
/// Output any persisted state
Inspect,
}

fn gather_state() -> Result<NetworkState, CliError> {
let mut state = NetworkState::new();
state.set_kernel_only(true);
state.set_running_config_only(true);
state.retrieve()?;
Ok(state)
}

fn process_interfaces<F>(state: &NetworkState, mut f: F) -> Result<(), CliError>
where
F: FnMut(&nmstate::Interface, &str) -> Result<(), CliError>,
{
for iface in state
.interfaces
.iter()
.filter(|i| i.iface_type() == InterfaceType::Ethernet)
{
let iface_name = iface.name();
let mac = match iface.base_iface().mac_address.as_ref() {
Some(c) => c,
None => continue,
};
let base_iface = iface.base_iface();
let ipv4_manual = base_iface
.ipv4
.as_ref()
.map(|ip| ip.is_static())
.unwrap_or_default();
let ipv6_manual = base_iface
.ipv6
.as_ref()
.map(|ip| ip.is_static())
.unwrap_or_default();
let ip_manual = ipv4_manual || ipv6_manual;
if !ip_manual {
log::info!("Skipping interface {iface_name} as no static IP addressing was found");
continue;
}

f(iface, mac.as_str())?;
}
Ok(())
}

/// For all active interfaces, write a systemd .link file which persists the currently
/// active name.
pub(crate) fn run_persist_immediately(
root: &str,
action: PersistAction,
) -> Result<String, CliError> {
let dry_run = match action {
PersistAction::Save => false,
PersistAction::DryRun => true,
PersistAction::Inspect => return inspect(root),
};

let stamp_path = Path::new(root)
.join(SYSTEMD_NETWORK_LINK_FOLDER)
.join(NMSTATE_PERSIST_STAMP);
if stamp_path.exists() {
log::info!("{} exists; nothing to do", stamp_path.display());
return Ok("".to_string());
}

let state = gather_state()?;
let mut changed = false;
process_interfaces(&state, |iface, mac| {
let iface_name = iface.name();
let action = if dry_run {
"Would persist"
} else {
"Persisting"
};
log::info!(
"{action} the interface with MAC {mac} to \
interface name {iface_name}"
);
if !dry_run {
changed |=
persist_iface_name_via_systemd_link(root, mac, iface.name())?;
}
Ok(())
})?;

if !changed {
log::info!("No changes.");
}

if !dry_run {
std::fs::write(stamp_path, b"")?;
}

Ok("".to_string())
}

pub(crate) fn inspect(root: &str) -> Result<String, CliError> {
let netdir = Path::new(root).join(SYSTEMD_NETWORK_LINK_FOLDER);
let stamp_path = netdir.join(NMSTATE_PERSIST_STAMP);
if !stamp_path.exists() {
log::info!(
"{} does not exist, no prior persisted state",
stamp_path.display()
);
return Ok("".to_string());
}

let mut n = 0;
for e in netdir.read_dir()? {
let e = e?;
let name = e.file_name();
let name = if let Some(n) = name.to_str() {
n
} else {
continue;
};
if !name.ends_with(".link") {
continue;
}
if !name.starts_with(PERSIST_FILE_PREFIX) {
continue;
}
log::info!("Found persisted NIC file: {name}");
n += 1;
}
if n == 0 {
log::info!("No persisted NICs found");
}

let state = gather_state()?;
process_interfaces(&state, |iface, mac| {
let iface_name = iface.name();
log::info!(
"NOTE: would persist the interface with MAC {mac} to interface name {iface_name}"
);
Ok(())
})?;

Ok("".to_string())
}

fn persist_iface_name_via_systemd_link(
root: &str,
mac: &str,
iface_name: &str,
) -> Result<bool, CliError> {
let link_dir = Path::new(root).join(SYSTEMD_NETWORK_LINK_FOLDER);

let file_path =
link_dir.join(format!("{PERSIST_FILE_PREFIX}-{iface_name}.link"));
if file_path.exists() {
log::info!("Network link file {} already exists", file_path.display());
return Ok(false);
}

if !link_dir.exists() {
std::fs::create_dir(&link_dir)?;
}

let content =
format!("{PERSIST_GENERATED_BY}\n[Match]\nMACAddress={mac}\n\n[Link]\nName={iface_name}\n");

std::fs::write(&file_path, content.as_bytes()).map_err(|e| {
CliError::from(format!(
"Failed to store captured states to file {}: {e}",
file_path.display()
))
})?;
log::info!(
"Systemd network link file created at {}",
file_path.display()
);
Ok(true)
}
2 changes: 1 addition & 1 deletion rust/src/cli/service.rs
Expand Up @@ -89,7 +89,7 @@ fn get_config_files(folder: &str) -> Result<Vec<PathBuf>, CliError> {
}

// rename file by adding a suffix `.applied`.
fn relocate_file(file_path: &Path) -> Result<(), CliError> {
pub(crate) fn relocate_file(file_path: &Path) -> Result<(), CliError> {
let new_path = file_path.with_extension(RELOCATE_FILE_EXTENTION);
std::fs::rename(file_path, &new_path)?;
log::info!(
Expand Down

0 comments on commit 6ab48ac

Please sign in to comment.