search

vatesfr / terraform-provider-xenorchestra

Xen Orchestra provider for Terraform
MIT License
150 stars 33 forks source link

Removed mac statefunc #233

Closed ravager-dk closed 1 year ago

ravager-dk commented 1 year ago

Fix for issue #232

StateFunc is removed for mac adresses, so they work for unknown values during plan phase

ddelnano commented 1 year ago

Sorry for seeing this after #235 and thanks again for contributing!

As mentioned on that PR, this statefunc is required for existing tests. It also would be a breaking change since supporting dashed mac addresses was added as a feature enhancement in https://github.com/terra-farm/terraform-provider-xenorchestra/issues/114#issuecomment-768825955. Removing this statefunc isn't impossible, but it would require documentation updates (and potentially other changes) if the breaking change is deemed acceptable.

Can you please explain the steps to reproduce the error you are seeing? I tried to run a VM acceptance test that didn't set a mac address in addition to one that only performed a terraform plan. In both cases, I didn't see any errors after adding the statefunc back in.

Being able to reproduce what you are seeing would be helpful to understand what options we have available to address #235.

ravager-dk commented 1 year ago

I understand what you mean about dashed mac. Let's close this PR for now and I will paste the example in the issue. Probably I can figure out a different way to handle it in a new PR.