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

Custom LXC Root Volume Size #89

Closed
DacoDev opened this issue Jul 9, 2022 · 12 comments · Fixed by #207
Closed

Custom LXC Root Volume Size #89

DacoDev opened this issue Jul 9, 2022 · 12 comments · Fixed by #207
Labels
in-progress The issue is being worked on ✨ enhancement New feature or request

Comments

@DacoDev
Copy link

DacoDev commented Jul 9, 2022

Is your feature request related to a problem? Please describe.
Root volumes for LXC containers are made at the basic size that the image was created at

Describe the solution you'd like
Ability to specify a custom size root volume

Describe alternatives you've considered
Creating volumes after creation and attaching them
Making a custom container template image based on a stock one and cloning

Additional context
Has been requested previously on the pre-fork danitso repo: danitso/terraform-provider-proxmox#86

@DacoDev DacoDev added the ✨ enhancement New feature or request label Jul 9, 2022
@DacoDev
Copy link
Author

DacoDev commented Jul 25, 2022

Howdy, just FYI, I don't know GO directly but am learning as I go and forked to start working on this: https://github.com/DacoDev/terraform-provider-proxmox-dacofork

From digging around, it looks like some of the resources that will be needed for this already exist. Just don't want to duplicate efforts or if you have any suggestions the repo is public.

@bpg
Copy link
Owner

bpg commented Jul 26, 2022

Awesome! I'll try to take a look in the next few days.

@bpg
Copy link
Owner

bpg commented Jul 26, 2022

@DacoDev Thanks for the code!
I've made a few changes on top of your main branch, here is the patch: Fix_RootFS_parameters_handling.patch.zip
(or if you add me as a contributor to your repo I can make a PR there)

The changes seems to be working, here is an example container from my test env:
Screen Shot 2022-07-26 at 6 35 36 PM

The only remaining part is documentation update -- add rootfs_size param under disk in docs/resources/virtual_environment_container.md.

Then feel free to open a PR!

@DacoDev
Copy link
Author

DacoDev commented Jul 27, 2022

Thanks for checking it out! I applied the patch and added you as a contributor - really great reference for what I can improve on and conventions to follow. I'll do a bit more testing, I'm a little concerned around the default value set to 4G. I'm not quite sure how LXC images are created/deployed but if the base image to deploy is larger than 4G we may hit a snag so I'll see what I can do around that or if it's an issue that we run in to. I based that just off the LXCs that I've created from defaults they were all 4G.

Then I'll update the docs in either direction that goes and PR!

@bpg bpg added this to the 0.6.0 milestone Jul 30, 2022
@DacoDev
Copy link
Author

DacoDev commented Jul 31, 2022

Sorry for the delay on getting this back, I ran into issues while spinning up a container, it wasn't actually receiving or recognizing the custom volume size. I believe that there are/were some components of creating the rootfs URL arguments missing that piggybacking on the Disk section was causing. So since there are many other options you can pass for the rootFS anyway I'm working on adding it in as a new TF stanza; so instead of being nested under disk it'd be its own rootFS section

@bpg
Copy link
Owner

bpg commented Aug 2, 2022

Yeah, I think having a separate node in the model for rootfs params is a good idea.

@bpg bpg removed this from the 0.6.0 milestone Aug 12, 2022
@bpg
Copy link
Owner

bpg commented Dec 29, 2022

Hey @DacoDev! Are you still working on this feature? If not, I can try finishing it...

@bpg bpg added the in-progress The issue is being worked on label Dec 29, 2022
@DacoDev
Copy link
Author

DacoDev commented Dec 29, 2022

Hey @bpg , I’m sorry but no, I got lost in the sauce then school started (going back as an adult) and I ended up scrapping my ProxMox host in favor of a few smaller NUCs as a k8s cluster. I made a few commits after your patch but I’m not sure if any of them would be useful, I think you’re a collaborator on my repo but let me know if you need me to push anything over!

@bpg
Copy link
Owner

bpg commented Dec 29, 2022

No worries, I'll take a peek, thanks!

@numkem
Copy link

numkem commented Jan 13, 2023

I'd love to see this merged soon as it's pretty much the only thing that limits functionality vs the other provider.

@bpg
Copy link
Owner

bpg commented Jan 13, 2023

Hopefully this weekend 🤞

@bpg
Copy link
Owner

bpg commented Jan 16, 2023

The DiskSize field in RootFS struct is read-only, so can't be used to change the FS size. However, there is a special syntax for the volume field that allows to specify a different rootfs size at creation time. More details in https://pve.proxmox.com/pve-docs/chapter-pct.html#_storage_backed_mount_points.

So I basically started from scratch and implemented it like:

  disk {
    datastore_id = "local"
    size         = 10
  }

Note that the datastore_id field is mandatory if you want to customize the size.
Also, this feature can't be used (as of now) to resize the rootfs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in-progress The issue is being worked on ✨ enhancement New feature or request
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants