MTV-5530 | static IP preservation on RHEL 9/10+ after migration

[Hazanel]

On RHEL 9/10+, predictable network interface naming (e.g. enp1s0)
overrides udev NAME= rename rules to e.g eth0.
Since NM keyfiles reference interface-name=eth0, NetworkManager never activates the
connection and the VM loses its static IP.

Add generate_link_files() to network_config_util.sh which writes
systemd .link files into /etc/systemd/network/. A .link [Match]
by MAC with [Link] Name= tells systemd-udevd to rename
the interface before NetworkManager sees it, so existing NM
connections activate unchanged. The .link file is harmless on
older distros where the udev rule already works.

Ref: https://redhat.atlassian.net/browse/MTV-5530
Resolves: MTV-5530

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR adds a patch_nm_connections() function that rewrites NetworkManager connection profiles to bind interfaces by MAC address instead of interface name, integrates it into the udev rule generation pipeline, and validates the behavior with comprehensive test fixtures covering both input and expected output states.

Changes

NetworkManager MAC Address Patching

Layer / File(s)	Summary
patch_nm_connections implementation and integration pkg/virt-v2v/customize/scripts/rhel/run/network_config_util.sh	New patch_nm_connections() function (lines 526–564) scans $V2V_MAP_FILE, locates NM connection files by IP, removes interface-name= constraints, and ensures [ethernet] sections contain lowercase mac-address=. Function is called in main() (lines 598–599) before udev rule deduplication and file write.
Test validation logic and fixtures pkg/virt-v2v/customize/scripts/rhel/run/test-network_config_util.sh, pkg/virt-v2v/customize/scripts/rhel/run/networkmanager-mac-patch-test.d/root/..., pkg/virt-v2v/customize/scripts/rhel/run/networkmanager-mac-patch-test.d/expected-nm/...	Test validation (lines 66–85) iterates over expected NM profiles, verifies patched counterparts exist, and compares content via cmp. Input fixtures provide two NM connections and a MAC-to-IP mapping file; expected fixtures show desired patched state with MAC-based device binding and corresponding udev rules.

Estimated Code Review Effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly Related PRs

• kubev2v/forklift#6415: Modifies the same udev rule deduplication pipeline in network_config_util.sh—this PR adds patch_nm_connections before dedup, while the related PR changes check_dupe_hws deduplication logic.

Suggested Reviewers

• mrnold
• mnecas
• yaacov
• solenoci

Poem

🐰 By MAC address we bind, not by name,
NetworkManager profiles tamed with careful aim.
Interfaces patch, connections align,
udev rules and ethernet sections combine—
V2V migrations now run fine! 🌟

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The PR title directly describes the main objective: ensuring static IP preservation on RHEL 9/10+ after migration, which aligns with the core change of adding MAC address matching to NetworkManager configuration.
Description check	✅ Passed	The PR description clearly explains the problem (predictable interface naming on RHEL 9/10+ breaks NM connections) and describes the solution (add patch_nm_connections() to modify NM keyfiles by MAC address instead of interface name), which matches the changeset accurately.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@pkg/virt-v2v/customize/scripts/rhel/run/network_config_util.sh`:
- Around line 540-543: The code assumes NM_FILE from grep -El is a single path
but grep can return multiple files; update the logic around NM_FILE (the grep
-El call that searches NETWORK_CONNECTIONS_DIR for S_IP) to iterate over each
matching filename instead of treating the whole string as one quoted pathname.
Replace the single-use of NM_FILE with a loop (e.g., readarray/mapfile or while
read -r over the grep output) so each matching file is processed individually;
apply the same change to the other occurrence block referenced (the code around
lines handling NM_FILE from 548-561) to avoid failures when grep returns
multiple matches.
- Around line 553-557: The current [ethernet] patch only inserts mac-address
when missing, so update the logic in the [ethernet] handling around NM_FILE and
MAC_LOWER to always ensure the mac-address equals $MAC_LOWER: if a line matching
'^mac-address=' exists replace it with 'mac-address=$MAC_LOWER' (e.g. via sed -i
's/^mac-address=.*/mac-address='$MAC_LOWER'/' "$NM_FILE"), otherwise append the
mac-address after the '[ethernet]' header as before; apply the same
replace-or-append fix to the other similar block referenced (lines around
559-561) so any incorrect existing MAC is corrected.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 64782048-41bd-47dc-98cf-7f43adf9088c

📥 Commits

Reviewing files that changed from the base of the PR and between c77f513 and d35e6d9.

📒 Files selected for processing (8)

• pkg/virt-v2v/customize/scripts/rhel/run/network_config_util.sh
• pkg/virt-v2v/customize/scripts/rhel/run/networkmanager-mac-patch-test.d/expected-nm/eth0.nmconnection
• pkg/virt-v2v/customize/scripts/rhel/run/networkmanager-mac-patch-test.d/expected-nm/eth1.nmconnection
• pkg/virt-v2v/customize/scripts/rhel/run/networkmanager-mac-patch-test.d/expected-udev.rule
• pkg/virt-v2v/customize/scripts/rhel/run/networkmanager-mac-patch-test.d/root/etc/NetworkManager/system-connections/eth0.nmconnection
• pkg/virt-v2v/customize/scripts/rhel/run/networkmanager-mac-patch-test.d/root/etc/NetworkManager/system-connections/eth1.nmconnection
• pkg/virt-v2v/customize/scripts/rhel/run/networkmanager-mac-patch-test.d/root/tmp/macToIP
• pkg/virt-v2v/customize/scripts/rhel/run/test-network_config_util.sh

[Hazanel]

@coderabbitai - please check the fixes and in general go over and suggest whether you think there is a better approach

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
cat pkg/virt-v2v/customize/scripts/rhel/run/network_config_util.sh
echo "=== TEST SCRIPT ==="
cat pkg/virt-v2v/customize/scripts/rhel/run/test-network_config_util.sh
echo "=== INPUT eth0.nmconnection ==="
cat "pkg/virt-v2v/customize/scripts/rhel/run/networkmanager-mac-patch-test.d/root/etc/NetworkManager/system-connections/eth0.nmconnection"
echo "=== INPUT eth1.nmconnection ==="
cat "pkg/virt-v2v/customize/scripts/rhel/run/networkmanager-mac-patch-test.d/root/etc/NetworkManager/system-connections/eth1.nmconnection"
echo "=== EXPECTED eth0.nmconnection ==="
cat "pkg/virt-v2v/customize/scripts/rhel/run/networkmanager-mac-patch-test.d/expected-nm/eth0.nmconnection"
echo "=== EXPECTED eth1.nmconnection ==="
cat "pkg/virt-v2v/customize/scripts/rhel/run/networkmanager-mac-patch-test.d/expected-nm/eth1.nmconnection"
echo "=== MAP FILE ==="
cat "pkg/virt-v2v/customize/scripts/rhel/run/networkmanager-mac-patch-test.d/root/tmp/macToIP"
echo "=== EXPECTED UDEV ==="
cat "pkg/virt-v2v/customize/scripts/rhel/run/networkmanager-mac-patch-test.d/expected-udev.rule"

Length of output: 27235

---

@Hazanel — overall the fix is well-structured and takes the right approach for the problem. Here is a thorough review with the issues I found and some alternative ideas.

---

✅ What's Good

• Correct ordering: patch_nm_connections is called after udev_from_nm runs, so the interface-name= field is still present when udev rules are generated and only removed afterwards.
• Correct MAC casing: NM keyfiles expect lowercase mac-address=; the tr 'A-F' 'a-f' conversion is correct.
• Defensive guard: Early return when $NETWORK_CONNECTIONS_DIR is absent is good.
• Two-pronged strategy (udev rename + NM MAC match) is the right defensive approach — it works even if one mechanism fails on a given distro/kernel version.

---

🐛 Bugs

1. Multi-file match — NM_FILE can be multi-line

grep -El returns one path per line. If two NM files happen to contain the same IP (e.g., a gateway), $NM_FILE is a newline-delimited string and the subsequent grep -q / sed -i calls receive it as a single bad filename:

# Current — breaks if grep returns >1 path
NM_FILE=$(grep -El "address[0-9]*=.*$S_IP.*$" "$NETWORK_CONNECTIONS_DIR"/* 2>/dev/null)

Fix — take only the first match (consistent with udev_from_nm's implicit behaviour):

NM_FILE=$(grep -El "address[0-9]*=.*$S_IP.*$" "$NETWORK_CONNECTIONS_DIR"/* 2>/dev/null | head -1)

2. Existing wrong mac-address= is not corrected

If a stale/wrong mac-address= is already present in the file the function silently skips it:

if ! grep -q "^mac-address=" "$NM_FILE"; then
    sed -i "/^\[ethernet\]/a mac-address=$MAC_LOWER" "$NM_FILE"
fi

A more robust approach replaces it unconditionally:

if grep -q "^mac-address=" "$NM_FILE"; then
    sed -i "s/^mac-address=.*/mac-address=$MAC_LOWER/" "$NM_FILE"
else
    sed -i "/^\[ethernet\]/a mac-address=$MAC_LOWER" "$NM_FILE"
fi

---

⚠️ Minor Issues

3. Partial IP match (pre-existing, but also affects this function)

grep -El "address[0-9]*=.*$S_IP.*$"

IP 1.2.3.4 could match 11.2.3.4. A tighter pattern prevents this:

grep -El "address[0-9]*=.*[^0-9]${S_IP}([^0-9]|$)"

or simply anchor with a word boundary:

grep -ElP "address[0-9]*=.*\b${S_IP}\b"

4. Silent return vs. logged warning

udev_from_nm logs a warning when the directory is absent; patch_nm_connections is silent. For consistency:

if [ ! -d "$NETWORK_CONNECTIONS_DIR" ]; then
    log "Info: $NETWORK_CONNECTIONS_DIR not found, skipping NM patching."
    return 0
fi

---

🧪 Missing Test Coverage

Scenario	Covered?
No [ethernet] section (appends new section)	✅ both fixture files
[ethernet] section already present, no mac-address=	❌
[ethernet] section already present with mac-address= (idempotency)	❌
mac-address= present but wrong value (correction)	❌

The sed -i "/^\[ethernet\]/a mac-address=..." code path (when [ethernet] already exists) is entirely untested. I'd recommend adding a networkmanager-mac-patch-with-ethernet-test.d fixture to cover it.

---

💡 Alternative Approach Consideration

The current approach (edit raw .nmconnection INI files with grep/sed) is pragmatic for a shell script context without NM running. An alternative worth considering:

Use nmcli in the post-migration firstboot phase (i.e., on first boot inside the migrated VM after NM is running), which would let you do:

nmcli connection modify eth0 802-3-ethernet.mac-address "$MAC" connection.interface-name ""

However, this would require a systemd one-shot service that runs on first boot — significantly more complex than what you have. The direct file editing approach is simpler and more reliable for the virt-v2v context, so I'd keep the current approach and just address the bugs above.

---

Summary of Recommended Fixes

1. Add | head -1 to the grep -El assignment in patch_nm_connections to guard against multi-file matches.
2. Replace the mac-address= check with an unconditional sed substitution (or add an update branch) to handle stale/incorrect existing values.
3. Add a test fixture with a pre-existing [ethernet] section to cover the sed -i "/^\[ethernet\]/a ..." branch.
4. (Optional) Add a log message for the missing-directory early return for consistency.

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 10.44%. Comparing base (f1fe5d0) to head (de2a27a).
⚠️ Report is 2540 commits behind head on main.
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6418      +/-   ##
==========================================
- Coverage   15.45%   10.44%   -5.02%     
==========================================
  Files         112      521     +409     
  Lines       23377    61917   +38540     
==========================================
+ Hits         3613     6466    +2853     
- Misses      19479    54895   +35416     
- Partials      285      556     +271     

Flag	Coverage Δ
unittests	10.44% <ø> (-5.02%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Hazanel]

Thanks @coderabbitai for the thorough review, please check the fixes.

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
echo "=== network_config_util.sh ==="
cat pkg/virt-v2v/customize/scripts/rhel/run/network_config_util.sh

echo ""
echo "=== test-network_config_util.sh ==="
cat pkg/virt-v2v/customize/scripts/rhel/run/test-network_config_util.sh

echo ""
echo "=== INPUT eth0.nmconnection ==="
cat "pkg/virt-v2v/customize/scripts/rhel/run/networkmanager-mac-patch-test.d/root/etc/NetworkManager/system-connections/eth0.nmconnection"

echo ""
echo "=== INPUT eth1.nmconnection ==="
cat "pkg/virt-v2v/customize/scripts/rhel/run/networkmanager-mac-patch-test.d/root/etc/NetworkManager/system-connections/eth1.nmconnection"

echo ""
echo "=== EXPECTED eth0.nmconnection ==="
cat "pkg/virt-v2v/customize/scripts/rhel/run/networkmanager-mac-patch-test.d/expected-nm/eth0.nmconnection"

echo ""
echo "=== EXPECTED eth1.nmconnection ==="
cat "pkg/virt-v2v/customize/scripts/rhel/run/networkmanager-mac-patch-test.d/expected-nm/eth1.nmconnection"

echo ""
echo "=== MAP FILE ==="
cat "pkg/virt-v2v/customize/scripts/rhel/run/networkmanager-mac-patch-test.d/root/tmp/macToIP"

echo ""
echo "=== EXPECTED UDEV ==="
cat "pkg/virt-v2v/customize/scripts/rhel/run/networkmanager-mac-patch-test.d/expected-udev.rule"

echo ""
echo "=== Check for additional test fixtures ==="
find pkg/virt-v2v/customize/scripts/rhel/run/ -type d | sort
find pkg/virt-v2v/customize/scripts/rhel/run/ -name "*.nmconnection" | sort

Length of output: 36443

---

@Hazanel — great work on the fixes! Here's the updated status for each item:

---

✅ All Three Issues Fixed

Issue	Status
Bug 1 — multi-file grep -El result (`	head -1`)
Bug 2 — stale/wrong mac-address= silently skipped → now unconditionally replaced with sed -i "s/^mac-address=.*/..."	✅ Fixed
Minor 4 — silent early return → now logs "Info: ... not found, skipping NM patching."	✅ Fixed

The three-branch logic in patch_nm_connections is now correct:

1. No [ethernet] section → appends the whole section via printf.
2. [ethernet] present, no mac-address= → inserts via sed -i "/^\[ethernet\]/a ...".
3. [ethernet] present, mac-address= present → replaces via sed -i "s/^mac-address=.*/...".

---

⚠️ One Remaining Gap — Test Coverage for [ethernet] Already Present

Both input fixtures still have no [ethernet] section, so only branch 1 (the printf append) is exercised by the test. Branches 2 and 3 (the sed paths) remain untested.

A small additional fixture would cover this — for example a networkmanager-mac-patch-with-ethernet-test.d where the input has an existing [ethernet] section (with and without mac-address=). This isn't a blocker, but it would complete the coverage.

---

Overall the implementation is solid and ready for the happy path. The missing test scenarios are low-risk given the logic simplicity, so this is a minor polish item rather than a blocking concern. 🐇✨

[mnecas]

@Hazanel editing the source config file can be bit complicated. I would rather lean to creating .link file for the NM. https://docs.redhat.com/en/documentation/red_hat_enterprise_linux/10/html/configuring_and_managing_networking/implementing-consistent-network-interface-naming

wdyt @yaacov ?

[yaacov]

wdyt @yaacov ?

my concern is that we are solving missing support for specific OSs in virt-v2v by adding ad-hock scripts.
IIRC, we were suppose to move the missing support to virt-v2v where is can be tested and QE'd using the infrastructure they have.
when we add support using our scripts we don't have the infrastructure and CI to test out scripts using a known matrix of supported OSs to make sure we didn't miss some configuration we should support.

editing the source config file can be bit complicated.

yes, specifically for this issue, if a less intrusive fix work ( e.g. adding a separate named file ) my vote is not to edit an existing file, but to add a separate file with the correct priority.

[sonarqubecloud]

Quality Gate passed

Issues
17 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud