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.

Sign up for GitHub

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

Jump to bottom

Fix root volume resizing on EC2 KVM instances (M5, C5, etc) #39164

Merged
merged 1 commit into from Apr 20, 2018
Merged

Fix root volume resizing on EC2 KVM instances (M5, C5, etc) #39164

merged 1 commit into from Apr 20, 2018

Conversation

ngortheone
Copy link
Contributor

@ngortheone ngortheone commented Apr 19, 2018 •
edited

Fixes:
#33417
#33092

Root volume resizing was broken on new KVM instances because of the way partition name was computed

rootDevice="$(readlink -f "$rootDevice")"
        parentDevice="$(lsblk -npo PKNAME "$rootDevice")"
        TMPDIR=/run sh $(type -P growpart) "$parentDevice" "''${rootDevice#$parentDevice}"

"''${rootDevice#$parentDevice}" results in p1 instead of 1 when root device is nvme disk

resulting in small root partition

# lsblk
NAME        MAJ:MIN RM SIZE RO TYPE MOUNTPOINT
nvme0n1     259:0    0  32G  0 disk
└─nvme0n1p1 259:2    0  2G  0 part /

Suggested solution borrowed from here:
https://git.launchpad.net/cloud-initramfs-tools/tree/growroot/scripts/local-bottom/growroot
(lines 38 - 49)

Performed testing on m4 and m5 instances and confirmed that fix works

@ngortheone ngortheone changed the title Fix #33092 Fix root volume resizing on EC2 KVM instances (M5, C5, etc) Apr 19, 2018
growPartition: fix volume resizing on EC2 NVME instances
3a47c7e 
The previous code for this accidentally picked up a "p" when computing the partition number.
This logic should be more robust
copumpkin
copumpkin approved these changes Apr 19, 2018
View reviewed changes
Copy link
Member

@copumpkin copumpkin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, thanks!

We should probably also backport this to 18.03 and regenerate the AMIs, @edolstra, right?

@copumpkin
Copy link
Member

cc @greglearns @polynomial @hakujin who have also expressed interest in the related tickets

@hakujin
Copy link
Contributor

hakujin commented Apr 19, 2018

Great news. Thanks!

@copumpkin copumpkin merged commit ec569fc into NixOS:master Apr 20, 2018
This was referenced Apr 20, 2018
Closed
Closed
This was referenced Apr 25, 2018
Merged
Closed
@hakujin
Copy link
Contributor

hakujin commented May 1, 2018

@copumpkin @edolstra how can I help get this fix into new AMIs? New to nix but happy to contribute if someone will point me in the correct direction.

@copumpkin
Copy link
Member

We've discovered one remaining bug that makes the new instance types fail if you use large root EBS volumes with NixOS. Hoping to get a fix to that in today, then whenever @edolstra has time to make the AMIs, I think we'll be set.

@zeme-wana
Copy link

May we have an estimate for when the new AMIs will be published? I tried to build them myself (on NixOS 18.03) with create-amis.sh, but it crashes with

error: attribute 'version' in selection path 'lib.version' not found

@copumpkin
Copy link
Member

I think all the relevant fixes should be in the 18.03 branch and the AMIs should be ready for @edolstra to publish

@edolstra
Copy link
Member

New AMIs are up!

@copumpkin
Copy link
Member

Thanks 👍

@zeme-wana
Copy link

Thank you @edolstra

@cristiam86
Copy link

cristiam86 commented Nov 8, 2018 •
edited

I found the solution here

TLDR: if your partition name is nvme0n1p1, then your growpart command should be like this
sudo growpart /dev/nvme0n1 1 (without the letter 'p')

@copumpkin
Copy link
Member

@cristiam86 sorry, the solution to what? is this not working for you?

@cristiam86
Copy link

I mean that I wasn't able to understand the solution given by @ngortheone. So I looked a little bit further and found that StackOverflow post were the problem was explained in a more simple way.

@copumpkin
Copy link
Member

Oh, if you have a simpler one that still works a PR would be very nice!

@cristiam86
Copy link

cristiam86 commented Nov 8, 2018 •
edited

Oh no, sorry. My problem wasn't linked with this project, the problem was resizing the partition manually but looking for solutions I stumble across this repo and it helped me to reach the final solution.
Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@copumpkin copumpkin copumpkin approved these changes

Assignees
No one assigned
Labels
6.topic: nixos 8.has: module (update) 10.rebuild-darwin: 0 10.rebuild-linux: 0 12. first-time contribution
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
8 participants
@ngortheone @copumpkin @hakujin @zeme-wana @edolstra @cristiam86 @GrahamcOfBorg @Janik-Haag