josenk / terraform-provider-esxi

Terraform-provider-esxi plugin
GNU General Public License v3.0
538 stars 154 forks source link

default nic type not pulled from clone #176

Closed craigers521 closed 2 years ago

craigers521 commented 2 years ago

Describe the bug Cloned VM had a nic type of vmxnet3. I did not specify nic_type in network_interfaces for the resource assuming it would clone nic_type but instead it defaulted to e1000. This causes netplan to apply incorrectly as ubuntu names the interface something different based on nic type.

To Reproduce spin up ubuntu vm with nictype vmxnet3, clone that vm without specifying nic type in tf

Expected behavior nic type would clone from VM, resulting interface would be named correctly to match cloud-init config for netplan

Terraform files

  network_interfaces {
    virtual_network = "VMs"
  }

Desktop (please complete the following information):

Additional context Jonathon - this issue is very minor and was easily corrected by declaring the nic_type. The provider was even able to update nic_type in place without destroying and re-creating the VM. I am only reporting the issue for your awareness, but i also just really wanted to thank you for this amazing provider and your contribution to the community, awesome job man!

josenk commented 2 years ago

Thanks for the comments!

Nics are a complicated part of all this... There are many use cases and each user may have different expectations to the results. So, right from the beginning I decided wipe nic information from any source and build it based on defaults and what's specified in the tf files.

If you declare a default, when should they get overwritten? In my opinion, they should be overwritten from the settings in the tf files, not by the source.

Another issue is that this would be a breaking change. How many people do not define their nic type because the current expectation is e1000 and that default works for them. If I change the default to now be e1000 if nothing in the source, otherwise it will be ???? depending on what's defined as the 'first' nic in the source. This change would break things, so it would need to bump the major version. BTW: How do you even define what is the first nic anyways??? It's not possible.

The best solution to your problem as you pointed out is simply define your settings as needed, since the default does not work for you.

Thanks again for the feedback.

craigers521 commented 2 years ago

Yeah after that everything mostly worked great. The only problem I ran into was renaming host name, and I had an issue with DHCP where I had to reset the machine-ID in Ubuntu. Otherwise it thought all the clones were the same machine. I think these things could be fixed with some startup scripts and/or ansible tho.

Thanks again.

On Fri, May 6, 2022, 1:55 PM Jonathan Senkerik @.***> wrote:

Thanks for the comments!

Nics are a complicated part of all this... There are many use cases and each user may have different expectations to the results. So, right from the beginning I decided wipe nic information from any source and build it based on defaults and what's specified in the tf files.

If you declare a default, when should they get overwritten? In my opinion, they should be overwritten from the settings in the tf files, not by the source.

  • For example, if I say the default number of CPU's should be 1, then what do you do when the source image has 2 cpus and nothing is specified in the TF files? In my opinion it should be 1. Otherwise, you basically don't have a default any longer...

Another issue is that this would be a breaking change. How many people do not define their nic type because the current expectation is e1000 and that default works for them. If I change the default to now be e1000 if nothing in the source, otherwise it will be ???? depending on what's defined as the 'first' nic in the source. This change would break things, so it would need to bump the major version. BTW: How do you even define what is the first nic anyways??? It's not possible.

The best solution to your problem as you pointed out is simply define your settings as needed, since the default does not work for you.

Thanks again for the feedback.

— Reply to this email directly, view it on GitHub https://github.com/josenk/terraform-provider-esxi/issues/176#issuecomment-1119924593, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADX4HWR5PLQUFPTA3IV6VQDVIVTLNANCNFSM5UWFUOYQ . You are receiving this because you authored the thread.Message ID: @.***>