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

builder/vmware: Add vmx_remove_ethernet_interfaces #4927

Merged
merged 1 commit into from
May 25, 2017
Merged

builder/vmware: Add vmx_remove_ethernet_interfaces #4927

merged 1 commit into from
May 25, 2017

Conversation

jen20
Copy link
Collaborator

@jen20 jen20 commented May 24, 2017

This commit adds a new option, vmx_remove_ethernet_interfaces, to both of the VMWare builders. This is useful when building Vagrant boxes, since Vagrant now produces output such as:

WARNING: The VMX file for this box contains a setting that is
automatically overwritten by Vagrant when started. Vagrant will stop
overwriting this setting in an upcoming release which may pre vent
proper networking setup. Below is the detected VMX setting:

   ethernet0.pcislotnumber = "33"

If networking fails to properly configure, it may require this VMX
setting. It can be manually applied via the Vagrantfile:

   Vagrant.configure(2) do |config|
     config.vm.provider :vmware_fusion do |vmware|
       vmware.vmx["ethernet0.pcislotnumber"] = "33"
     end
   end

This can be avoided entirely by removing the ethernet adapters from the VMX file prior to packaging as a Vagrant box, in which case adapters are created as expected according to the Vagrantfile specification.

This commit adds a new option, `vmx_remove_ethernet_interfaces`, to both
of the VMWare builders. This is useful when building Vagrant boxes,
since Vagrant now produces output such as:

```
WARNING: The VMX file for this box contains a setting that is
automatically overwritten by Vagrant when started. Vagrant will stop
overwriting this setting in an upcoming release which may pre vent
proper networking setup. Below is the detected VMX setting:

   ethernet0.pcislotnumber = "33"

If networking fails to properly configure, it may require this VMX
setting. It can be manually applied via the Vagrantfile:

   Vagrant.configure(2) do |config|
     config.vm.provider :vmware_fusion do |vmware|
       vmware.vmx["ethernet0.pcislotnumber"] = "33"
     end
   end
```

This can be avoided entirely by removing the ethernet adapters from the
VMX file prior to packaging as a Vagrant box, in which case adapters are
created as expected according to the Vagrantfile specification.
@mwhooker
Copy link
Contributor

What do you think about making this part of the vagrant/vmware post processor? could call ReadVMX, modify the setting, and then rewrite the vmx

It just seems like this doesn't have any application for the vmware builder except when directly importing into vagrant.

@jen20
Copy link
Collaborator Author

jen20 commented May 25, 2017

@mwhooker I don't mind making it part of the Vagrant post-processor, particularly.

@jen20
Copy link
Collaborator Author

jen20 commented May 25, 2017

Is the idea here to add a flag specifically for VMWare in the Vagrant post-processor? If so, what do you recommend calling it?

@cbednarski
Copy link
Contributor

Hi @jen20. There is a lot of history surrounding this but I'll rehash it here. The problem here is not necessarily that Packer or Vagrant has any particular behavior. The problem is the operating system.

This issue first arose in (IIRC) Ubuntu 14.10 when they changed to systemd networking, because systemd identifies devices by their PCI slot ID (ens33) instead of the traditional eth0 identifier. When you install Ubuntu (for example) it writes boot scripts into /etc/networking that specifically identify the network card that should be initialized at boot. If this card is removed then networking does not start and your box hangs at boot, and possibly eventually passes this and just boots up with no networking.

In your proposed fix we could remove the network device but (as far I am aware) Ubuntu does not scan for new network devices on boot and will simply boot with no networking, even if a new NIC is attached. This means the box is essentially useless.

For Ubuntu specifically this might be mitigated if Ubuntu has some automatic interface detection at boot time rather than install time but I am not aware of this functionality existing. Also while Vagrant may happily reinitialize the NIC by default, other types of packer builders which may consume packer output (like VMX) do not do this.

Obviously for specific use-cases where I control the source packer build, OS, and Vagrantfiles I can supply manual overrides for this behavior in packer, vagrant, and the OS in question. But this is not a typical case with people using packer to build Vagrant boxes and share them on the internet.

The behavior described in vagrant #8095 is a compromise that allows both Packer and Vagrant to work around the OS idiosyncrasy here and makes things "just work" out of the box in as many cases as possible.

I think the "remove NICs" option is fine to add but certainly not as default behavior, since, as far as I know, it breaks all of the aforementioned scenarios. I have seen a lot of people respond to the warning in Vagrant by assuming that Packer or Vagrant is broken, but we are actually fixing a problem that the OS (Ubuntu specifically) introduced.

@jen20
Copy link
Collaborator Author

jen20 commented May 25, 2017

@cbednarski I'm not sure of the behaviour with Ubuntu here - I've specifically added this for building boxes for SmartOS, which will happily detect newly-added NICs on boot. It's not added as a default behaviour in this PR (via the false zero value of the boolean flag).

I am happy to move it to the post-processor as suggested by @mwhooker though, assuming we're ok adding provider-specific flags in there - I'd agree it's definitely more related to Vagrant VMWare processing than VMWare building, and that should remove the ambiguity around the use case.

@cbednarski
Copy link
Contributor

Sounds good to me @jen20. I figured you were using a smarter OS (no pun intended) and had not run into the same issue. The PR looks solid with default false. I'll defer to @mwhooker on the best place to put this. Thanks!

@jen20
Copy link
Collaborator Author

jen20 commented May 25, 2017

There actually doesn't appear to be a straightforward way to hook this into the postprocessor without some surgery to pass down provider-specific configuration. I'm happy to do it that way, though it may ultimately be cleaner to just leave it as is in this PR.

@mwhooker
Copy link
Contributor

Thanks everyone for discussing. This makes sense to me now

@mwhooker mwhooker merged commit ceb1806 into hashicorp:master May 25, 2017
@ghost ghost locked and limited conversation to collaborators Apr 4, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants