growpart: Fix behaviour for ZFS datasets #5169
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
igalic
commented
Apr 11, 2024
| LOG.debug("Failed: zpool get size %s", zpool) | ||
| return None | ||
| return int(size.strip()) | ||
|
|
||
| def devent2dev(devent): |
There was a problem hiding this comment.
might wanna rename this function now that it returns more, maybe deventinfo?
There was a problem hiding this comment.
might wanna rename this function now that it returns more, maybe deventinfo?
Agreed, how about a snake case device_entry_info() or dev_entry_info() or something?
There was a problem hiding this comment.
sounds good, I'll change that tomorrow or the day after.
igalic
force-pushed
the
fix/zfs_growpart
branch
5 times, most recently
from
5589b3e
to
05f7702
Compare
igalic
force-pushed
the
fix/zfs_growpart
branch
3 times, most recently
from
4850aec
to
587c5ea
Compare
…cal#5169) First off, the recursion was bogus, because it called the wrong function, but more importantly, it would lead to infinite recursion anyway, even if we called the right one. Throw a ValueError instead, because we've most likely been called with a bogus value. Add a test to show that. Sponsored by: The FreeBSD Foundation
igalic
added a commit
to igalic/cloud-init
that referenced
this pull request
Apr 29, 2024
on FreeBSD, ZFS datasets aren't exposed directly as disks devices, so stats will fail. Currently, this means that growpart will be skipped for FreeBSD ZFS volumes, even tho we are perfectly capable of resizing ZFS. - Return fs from devent2dev(), which is now inappropriately named - Add get_zfs_size() helper, and extend get_size() to accept an `fs` parameter. - Add _call_resizer() helper, to avoid an indent on `if fs == zfs` - Add a unittest to show the behaviour Sponsored by: The FreeBSD Foundation
igalic
force-pushed
the
fix/zfs_growpart
branch
from
587c5ea
to
a277569
Compare
igalic
added a commit
to igalic/cloud-init
that referenced
this pull request
Apr 29, 2024
on FreeBSD, ZFS datasets aren't exposed directly as disks devices, so stats will fail. Currently, this means that growpart will be skipped for FreeBSD ZFS volumes, even tho we are perfectly capable of resizing ZFS. - Return fs from devent2dev(), which is now inappropriately named - Add get_zfs_size() helper, and extend get_size() to accept an `fs` parameter. - Add _call_resizer() helper, to avoid an indent on `if fs == zfs` - Add a unittest to show the behaviour Sponsored by: The FreeBSD Foundation
igalic
force-pushed
the
fix/zfs_growpart
branch
from
a277569
to
457438e
Compare
on FreeBSD, ZFS datasets aren't exposed directly as disks devices, so stats will fail. Currently, this means that growpart will be skipped for FreeBSD ZFS volumes, even tho we are perfectly capable of resizing ZFS. - Return fs from devent2dev(), which is now inappropriately named - Add get_zfs_size() helper, and extend get_size() to accept an `fs` parameter. - Add _call_resizer() helper, to avoid an indent on `if fs == zfs` - Add a unittest to show the behaviour Sponsored by: The FreeBSD Foundation
igalic
force-pushed
the
fix/zfs_growpart
branch
from
457438e
to
ff2bb74
Compare
|
tested on aws: Still works! |
Nice, thanks for this. Busy at a conference now but I'll try to review (and land it if ready) in the next few days. |
|
Hello! Thank you for this proposed change to cloud-init. This pull request is now marked as stale as it has not seen any activity in 14 days. If no activity occurs within the next 7 days, this pull request will automatically close. If you are waiting for code review and you are seeing this message, apologies! Please reply, tagging TheRealFalcon, and he will ensure that someone takes a look soon. (If the pull request is closed and you would like to continue working on it, please do tag TheRealFalcon to reopen it.) |
github-actions
bot
added
the
stale-pr
holmanb
pushed a commit
that referenced
this pull request
May 31, 2024
First off, the recursion was bogus, because it called the wrong function, but more importantly, it would lead to infinite recursion anyway, even if we called the right one. Throw a ValueError instead, because we've most likely been called with a bogus value. Add a test to show that. Sponsored by: The FreeBSD Foundation
holmanb
pushed a commit
that referenced
this pull request
Jun 28, 2024
First off, the recursion was bogus, because it called the wrong function, but more importantly, it would lead to infinite recursion anyway, even if we called the right one. Throw a ValueError instead, because we've most likely been called with a bogus value. Add a test to show that. Sponsored by: The FreeBSD Foundation
holmanb
pushed a commit
that referenced
this pull request
Jun 28, 2024
on FreeBSD, ZFS datasets aren't exposed directly as disks devices, so stats will fail. Currently, this means that growpart will be skipped for FreeBSD ZFS volumes, even tho we are perfectly capable of resizing ZFS. - Return fs from devent2dev(), which is now inappropriately named - Add get_zfs_size() helper, and extend get_size() to accept an `fs` parameter. - Add _call_resizer() helper, to avoid an indent on `if fs == zfs` - Add a unittest to show the behaviour Sponsored by: The FreeBSD Foundation
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Proposed Commit Message
Additional Context
Test Steps
Checklist
Merge type