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

[bugfix] cd_rom.iso_id in resource_vm.go is an optional parameter #17

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Xiritianming
Copy link

No description provided.

@Xiritianming Xiritianming marked this pull request as ready for review August 29, 2024 04:21
@Xiritianming Xiritianming changed the title [bugfix] cd_rom.iso_id in resource_vm.go is a optional parameter [bugfix] cd_rom.iso_id in resource_vm.go is an optional parameter Aug 29, 2024
@Sczlog
Copy link
Collaborator

Sczlog commented Nov 11, 2024

I am not sure if we should change this to optional, imo a field optional and unconfigured in terraform schema mean it won't be managed by terraform.
If we only change the spec's optional/required field, will make something unexpected happen, for example, in spec we have two cd_rom, appy with iso_id configured, then update iso_id to unconfigured, expected behavior should be change nothing but in current implementation, it will unmount 2 iso.
For now, I prefer to keep it as required and use empty string to indicate cd_rom unmounted.

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