Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make use of partx instead of partprobe #1846

Merged

Conversation

davidcassany
Copy link
Contributor

This commit makes use of partx to notify the kernel on partition updates. This fixes the installation over loop devices on some distributions (e.g. Leap 15.5).

@davidcassany davidcassany requested a review from a team as a code owner October 18, 2023 15:42
@davidcassany davidcassany marked this pull request as draft October 18, 2023 15:42
@davidcassany davidcassany force-pushed the fix_installation_to_loop_devices branch 2 times, most recently from 03503d6 to 70adc18 Compare October 18, 2023 15:55
@davidcassany
Copy link
Contributor Author

I am facing the same or very similar issue as it was reported in #1841 reported while trying to install to a loop device. So far I noted the process does not work on Leap 15.5 hosts, feels like there is something updated in parted or some related tooling. Feels like it doesn't notify the kernel anymore for /dev/loop devices 🤷🏽‍♂️ Interestingly enough I can't see such an issue on my main host (running Leap 15.3, yes I should update it...).

This PR works fine on both Leap 15.3 and Leap 15.5, moreover relies on partx which is part of util-linux. This opens the door to actually completely remove parted from our dependency set and use sgdisk instead.

@davidcassany davidcassany force-pushed the fix_installation_to_loop_devices branch 2 times, most recently from 6230117 to c1566a6 Compare October 18, 2023 16:08
This commit makes use of partx to notify the kernel on partition
updates. This fixes the installation over loop devices on some
distributions (e.g. Leap 15.5).

Signed-off-by: David Cassany <[email protected]>
@davidcassany davidcassany force-pushed the fix_installation_to_loop_devices branch from c1566a6 to e59c794 Compare October 18, 2023 17:03
@codecov-commenter
Copy link

Codecov Report

Attention: 296 lines in your changes are missing coverage. Please review.

Comparison is base (4c84315) 75.78% compared to head (e59c794) 75.33%.
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1846      +/-   ##
==========================================
- Coverage   75.78%   75.33%   -0.46%     
==========================================
  Files          63       67       +4     
  Lines        5789     6814    +1025     
==========================================
+ Hits         4387     5133     +746     
- Misses       1092     1311     +219     
- Partials      310      370      +60     
Files Coverage Δ
pkg/action/install.go 80.54% <100.00%> (ø)
pkg/action/reset.go 71.73% <100.00%> (ø)
pkg/action/upgrade.go 61.96% <100.00%> (ø)
pkg/config/config.go 96.34% <100.00%> (+0.88%) ⬆️
pkg/constants/constants.go 87.09% <ø> (+4.83%) ⬆️
pkg/mocks/cloud-init-runner_mock.go 88.46% <100.00%> (+2.74%) ⬆️
pkg/partitioner/parted.go 97.81% <100.00%> (+0.13%) ⬆️
pkg/utils/grub.go 58.36% <100.00%> (+0.17%) ⬆️
pkg/partitioner/partitioner.go 75.00% <75.00%> (ø)
pkg/cloudinit/cloudinit.go 90.00% <70.00%> (-5.00%) ⬇️
... and 10 more

... and 2 files with indirect coverage changes

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

@davidcassany davidcassany marked this pull request as ready for review October 18, 2023 17:52
@davidcassany davidcassany merged commit fd92278 into rancher:main Oct 19, 2023
11 checks passed
@davidcassany davidcassany deleted the fix_installation_to_loop_devices branch October 19, 2023 05:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants