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

Add thin disk provisioning & unit number to solve error #1

Merged
merged 1 commit into from
May 24, 2022

Conversation

0dragosh
Copy link
Contributor

This PR adds thin disk provisioning as optional and disk unit number to solve this issue.

@0dragosh
Copy link
Contributor Author

@ilpozzd I don't know how open you are to receiving PRs but I have just started using your module with talos and vsphere, it's great 👍🏻
I thought I'd contribute since I may have some specific needs & already have terraform experience, hope this is ok.

@ilpozzd ilpozzd merged commit 6cb12ce into ilpozzd:main May 24, 2022
@ilpozzd
Copy link
Owner

ilpozzd commented May 24, 2022

@0dragosh, thanks for PR! I will update this module in the coming days. The Talos configuration transformation will be moved to a separate module so that the resulting userdata can be used in any cloud that supports it.

@0dragosh
Copy link
Contributor Author

@ilpozzd awesome! Cheers!

@ilpozzd
Copy link
Owner

ilpozzd commented May 24, 2022

@0dragosh, I tested your solution and unfortunately setting the thin_provisioned and eagerly_scrub parameters has no effect, because when cloning a virtual machine from OVF, the parameters that are specified there are used. In addition, setting these parameters will lead to the fact that when trying to update the infrastructure, a false re-creation of the machine will be caused (since the thin_provisioned and eagerly_scrub parameters are specified in the code alone, while others were created from OVF). So you need to specify statically the parameters so that they match those in OVF (thin_provisioned = false, eagerly_scrub = false)

@0dragosh
Copy link
Contributor Author

@ilpozzd did you test with a second disk that's not in the OVF? My use case is a second data disk for storage.

@ilpozzd
Copy link
Owner

ilpozzd commented May 24, 2022

@0dragosh, I can change the data structure by adding one block for a basic fixed disk (like in OVF) and a dynamic block for all others

@0dragosh
Copy link
Contributor Author

@ilpozzd That's what I was trying to achieve but I'm not that great with VMware. Thanks!

@ilpozzd
Copy link
Owner

ilpozzd commented May 24, 2022

@0dragosh, I found the better solution :)

thin_provisioned = index(var.disks, disk.value) == 0 ? false : lookup(disk.value, "thin_provisioned", false)

ilpozzd added a commit that referenced this pull request May 24, 2022
@0dragosh
Copy link
Contributor Author

Looks good!

@0dragosh 0dragosh deleted the add-thin-disk branch May 24, 2022 18:47
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.

2 participants