Auto bring-up interface after restore_from_backup#6309
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a new _get_bondingmaster method to retrieve the bonding master interface name from a slave interface, and updates restore_from_backup to automatically bring the interface up after restoration using either NetworkManager or ip link. The review feedback identifies two critical issues: first, the distribution-specific flags may not be initialized if restore_from_backup is called on a fresh instance, which can be resolved by detecting the distribution dynamically; second, the nmcli connection reload command is called with an invalid connection name argument, which will cause a failure and should be run without arguments instead.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| or if bringing the interface up fails. | ||
| """ | ||
|
|
||
| backup_file = f"{self.config_filename}.backup" |
There was a problem hiding this comment.
The attributes self.distro_is_rhel9_or_later and self.distro_is_suse16_or_later are initialized to False in __init__ and are only set to True inside the save() method. If restore_from_backup() is called on a fresh NetworkInterface instance without save() being called first, these attributes will remain False.
This causes self.config_filename to return the wrong path (e.g., /etc/sysconfig/network-scripts/... instead of /etc/NetworkManager/... on RHEL 9+), leading to a failure to find the backup file and raising an NWException.
We should detect the distribution and set these flags at the beginning of restore_from_backup().
| backup_file = f"{self.config_filename}.backup" | |
| current_distro = distro_detect() | |
| if current_distro.name == "rhel" and int(current_distro.version) >= 9: | |
| self.distro_is_rhel9_or_later = True | |
| elif current_distro.name == "SuSE" and int(current_distro.version) >= 16: | |
| self.distro_is_suse16_or_later = True | |
| backup_file = f"{self.config_filename}.backup" |
| # Reload the connection to pick up restored configuration | ||
| reload_cmd = f"nmcli connection reload {self.name}" | ||
| run_command(reload_cmd, self.host, sudo=True) |
There was a problem hiding this comment.
nmcli connection reload does not accept a connection name or interface name as an argument. Running nmcli connection reload <name> will fail with an error (e.g., Error: 'connection reload' command takes no arguments.), which will cause restore_from_backup() to raise an NWException and fail.
To reload the connection configuration from disk, you should use nmcli connection reload without any arguments.
| # Reload the connection to pick up restored configuration | |
| reload_cmd = f"nmcli connection reload {self.name}" | |
| run_command(reload_cmd, self.host, sudo=True) | |
| # Reload the connection to pick up restored configuration | |
| reload_cmd = "nmcli connection reload" | |
| run_command(reload_cmd, self.host, sudo=True) |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6309 +/- ##
==========================================
- Coverage 72.05% 72.03% -0.03%
==========================================
Files 206 206
Lines 23349 23373 +24
==========================================
+ Hits 16825 16836 +11
- Misses 6524 6537 +13 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Enhanced restore_from_backup() to automatically bring the network interface up after restoring configuration from backup, completing the restoration workflow. The method now: - Restores the interface configuration file from backup - Automatically brings the interface up using the appropriate command based on the distribution: * RHEL 9+/SuSE 16+: Uses NetworkManager (nmcli connection reload/up) * Older distributions: Uses ip link set dev <interface> up - Provides comprehensive logging for debugging - Raises NWException with clear error messages on failure This change improves user experience by ensuring that restoring a backup returns the interface to a fully working state, eliminating the need for manual intervention to bring the interface up. Signed-off-by: Vaishnavi Bhat <vaishnavi@linux.vnet.ibm.com>
Enhanced restore_from_backup() to automatically bring the network interface up after restoring configuration from backup, completing the restoration workflow.
The method now:
This change improves user experience by ensuring that restoring a backup returns the interface to a fully working state, eliminating the need for manual intervention to bring the interface up.