Fix DHCPCD --deprovision bug #1920
Conversation
This is a pretty clean commit. We're adding a umount method to `fileutil.py. Similar to how `rm_file` works, if you pass the umount functiona a path that doesn't exist or isn't mounted then nothing happens. Then as part of the `del_dhcp_lease` deprovision workflow we're adding a step which removes dirs mounted by DHCPCD. On all Arch based systems, DHCPCD mounts files which cannot be deleted, even via a `sudo rm`. In turn this causes `waagent --deprovision` to fail with a stack trace when it trys to clear out cached DHCPCD client leases. This commit addresses that issue by umounting dhcpcd related files. For more info see github issue: Azure#1890
*Fix umount *Kill DHCPCD service w/systemctl
Bug-Reaper
requested review from
kevinclark19a,
larohra,
narrieta,
pgombar and
ZhidongPeng
as code owners
|
|
Codecov Report
@@ Coverage Diff @@
## develop #1920 +/- ##
===========================================
- Coverage 69.60% 69.59% -0.02%
===========================================
Files 85 85
Lines 11921 11931 +10
Branches 1670 1673 +3
===========================================
+ Hits 8298 8303 +5
- Misses 3252 3257 +5
Partials 371 371
Continue to review full report at Codecov.
|
| # find all possible mounts | ||
| for path in glob.glob(paths): |
There was a problem hiding this comment.
Unnecessary indentation here after the comment.
There was a problem hiding this comment.
Yep, the indentation is unnecessary here. I'll amend that.
| command=["umount",path] | ||
| shellutil.run_command(command) |
There was a problem hiding this comment.
Is the usage of the command var necessary here? Looks like a single-use variable, consider combining this into a single line.
There was a problem hiding this comment.
I agree, we can shorten this to one line.
|
|
||
| # For Arch based systems. Kill DHCPCD service. Needed to clear cached leases. | ||
| shellutil.run("systemctl stop dhcpcd", chk_err=False) | ||
|
|
||
| # For Arch based systems. Files mounted by DHCPCD can't be deleted whilst mounted, not even by root. | ||
| dirs_to_umount = ["/var/lib/dhcpcd/run/systemd/journal", "/var/lib/dhcpcd/run/udev", "/var/lib/dhcpcd/sys", "/var/lib/dhcpcd/dev", "/var/lib/dhcpcd/proc"] | ||
| actions.append(DeprovisionAction(fileutil.umount, dirs_to_umount)) |
There was a problem hiding this comment.
If this is Arch-specific logic, then it should go into the ArchDeprovisionHandler.
There was a problem hiding this comment.
So perhaps I should rephrase my comment here. While it's true this is for Arch systems, it's also for any *Nix distro that's running the latest version of dhcpcd. A quick check on wiki shows a large number of distros can be configured with systemd/dhcpcd.
The systems that can be configured with systemd/dhcpcd include: Arch Linux, CentOS, CoreOS, Fedora, Mageia, Manjaro Linux, openSUSE, SUSE Linux Enterprise Server, Red Hat Enterprise Linux, Solus, Linux Mint, Ubuntu and Debian.
Among those, many use systemd/dhcpcd by default.
I believe this is the correct place to run this, but I can update my comment to make it clear this is a DHCPCD specific issue and not an Arch one.
|
Initial testing is covered in the issue. Testing methodology for this PR has gone something like this:
|
Description
Issue #1890
#1890
Fixes bug where
waagent --deprovisionwould error/crash when trying to delete cached DHCPCD leases.This error occurs because DHCPCD mounts files onto sub-directories of
/var/lib/dhcpcd. These mounted DHCPCD files cannot be deleted whilst mounted (even with the all mighty power ofsudo rm). This causeswaagent --deprovisionto crash when attempting to delete cached DHCPCD leases.This PR adds the new method
umount(path)to common/utils/fileutil.py. If thepathargument does not pass os.path.ismount() then nothing happens. Otherwise the argumentpathis unmounted.Right before we delete cached DHCPCD leases in the
del_dhcp_lease, we call the methodfileutil.umount()against paths of known DHCPCD mountpoints that cause this error. Because some mountpoints cannot be unmounted while DHCPCD is in use, we also first make sure that the DHCPCD service is not running. (pa/deprovision/default.py)In doing so this PR address the crash/error and has been tested to work.
PR information
Quality of Code and Contribution Guidelines