This commit refactors the freebsd networking to:
1. Use predictable network naming
2. Properly handle DHCP vs static networks on up and reload [GH-5852]
3. Perform all networking configuration in a single command to prevent
partial configuration.
This updates the guest capabilities to run in as few communicator
commands as possible. Additionally, it fixes a number of issues around
hostname and idempotency.
This patch was tested against:
- puphpet/debian75-x64
- debian/jessie64
- debian/wheezy64
with custom networking, custom hostname, and rsync shared folders.
This commit does a few things:
1. Make the hostname update idempotent with `grep -w`
2. Add the given hostname to `/etc/hosts` as recommended by the docs
3. Add missing tests
This commit updates the procedure for configuring arch networks to occur
in a single command. Previously, each network was configured
independently. If, for some reason, one of the networks destroyed the
SSH connection, the box would be irrecoverable. This commit does not
alleviate that behavior, but attempts to mitigate it by running all
network-related configuration commands in a single communicator (SSH)
session.
The new procedure looks like this:
1. Upload a temp file to /tmp/vagrant-network-id... for each interface
on the guest.
2. Compile a commands array (of bash) to execute after all network
configurations have been uploaded.
3. Concatenate all the commands together in a single communicator
session.
This was tested against `terrywant/archlinux` using the following Vagrantfile:
```ruby
Vagrant.configure(2) do |config|
config.vm.box = "terrywang/archlinux"
config.vm.hostname = "banana-ramama.example.com"
config.vm.network "private_network", type: "dhcp"
config.vm.network "private_network", ip: "33.33.33.10"
config.vm.provision "file", source: "Vagrantfile", destination: "/tmp/vf"
config.vm.provision "shell", inline: "echo hi"
end
```
This commit updates the procedure for changing the hostname on arch
guests to occur in a single command. Previously, setting the hostname
and adding the value of the hostname to the /etc/hosts file was done in
two different uploads. This reduces the cycle to a single upload, making
provisioning a bit faster.
Additionally, this changes the behavior of the /etc/hosts file to:
1. Not remove localhost as an alias of 127.0.0.1
2. Prepend our custom hostname before localhost
The resulting /etc/hosts file will look something like:
127.0.0.1 my-host.example.com my-host
127.0.0.1 localhost.mydomain localhost
Tested against `terrywang/archlinux` using the following Vagrantfile:
```ruby
Vagrant.configure(2) do |config|
config.vm.box = "terrywang/archlinux"
config.vm.hostname = "banana-ramama.example.com"
config.vm.network "private_network", type: "dhcp"
config.vm.network "private_network", ip: "33.33.33.10"
config.vm.provision "file", source: "Vagrantfile", destination: "/tmp/vf"
config.vm.provision "shell", inline: "echo hi"
end
```
With this change, the presence of Ansible configuration files (like
playbook file, inventory path, galaxy role file, etc.) is no longer
performed by the `config` classes, but by the `provisioner` classes
(at the beginning of the provision command).
This change fixes several issues:
- Resolve#6984 as `provision` method are only executed when remote
(ssh) communication with the guest machine is possible.
- Resolve#6763 in a better way than 4e451c6 initially did.
- Improve the general provisioner speed since the `config` checks are
actually triggered by many vagrant actions (e.g. `destroy`,...), and
can also be triggered multiple times during a vagrant run (e.g. on
callback request made by the machine provider).
Unlike the former `config`-based checks, the provision action won't
collect all the invalid options, but only report the first invalid
option found and abort the execution.
Some unit tests were not implemented yet to save my scarce "open source
contribution time" for other important issues, but they should be done
at last via GH-6633.
This adds a new core command, `docker-exec`, which allows the user to
exec into an already-running container.
- Fixes#6566
- Fixes#5193
- Fixes#4904
- Fixes#4057
- Fixes#4179
- Fixes#4903
This commit changes Vagrant::Util::Platform to cache the result of some
common operations. These values are highly unlikely to change over the
course of a single Vagrant run and they are only cached for that run.
This commit changes the way ports are aggregated in the Docker provider.
Previously ports were aggregated by their "number", but that is not a
truly unique representation. Instead, the protocol is now taken into
account when generating the port map.
Fixes GH-5527
Changes:
- Add "config" unit tests for `ansible_local` (guest)
- Share some "config" examples between both ansible provisioners
- Move `config_host.rb` specific examples to `config/host.rb`
- Add a requirement to "../helpers" in `config/guest.rb` in order to be
able to run the related unit tests
References:
- This is the first part of GH-6633 resolution
- This change is a handy prerequisite for GH-6570
Not addressed yet:
- FIXME (guest.rb): Some test-double stubs are currently not working as
expected, and the related checks are commented out for the moment
(no idea why, but this is not urgent to be fixed because of GH-7335
rejection. See also GH-6984)
- FIXME (shared.rb): The guest-based config should actually NOT return
an error when the extra_vars file cannot be found, but only display a
a warning (similarly to the changes done for GH-6763, see 4e451c6)
This fixes a fairly large tempfile leak. Vagrant uses a template
renderer to write network configuration files locally to disk. Then,
that temporarily file is uploaded to the remote host and moved into
place. Since Vagrant is such a short-lived process, GC never came along
and cleaned up those tempfiles, resulting in many temporary files being
created through regular Vagrant usage.
The Util::Tempfile class uses a block to ensure the temporary file is
deleted when the block finishes. This API required small tweaks to the
usage, but provides more safety to ensure the files are deleted.
This commit attempts to uniquely identify the temporary files and
directories that are created during test runs. Where it was a quick
fix, this commit also removes the temporary files and directories.
There are still a ton of temporary files due to calls to
.isolated_environment in the tests without an easy API an easy way
to provide a closer to that function.
With this change, the `raw_arguments` and `raw_ssh_args` options are:
- STILL automatically converted as an Array when they are set a String
(no behaviour change)
- rejected if they are not of Array data type otherwise
Additional Notes:
- the 'as_array' tiny helper has been removed since it was no longer
used.
- there is for now no deeper validation (i.e. verifying that the Array
elements are only *String* objects)
With cb80286a4a, the helper function
stringify_ansible_playbook_command was also applied on the
`raw_arguments` content, which is not wanted. Given that users have used
the `raw_arguments` option as a workaround to avoid the bug GH-6726,
this new change ensure that any `--extra-vars` option passed as a raw
argument won't be additonally enquoted by the ansible_local
provisioner.
This change also improves the ansible remote provisioner verbose output,
but has no impact on its behaviour, which was already correct.
Note that this refactoring introduces some code duplications that are not
very elegant (see ansible_playbook_command_for_shell_execution in
host.rb and execute_ansible_playbook_from_host in base.rb). I hope we
can find a better implementation later, but it is good enough for now
since all these parts are covered by corresponding unit tests (the
`ansible_local` stuff being tested via the verbose output of the ansible
remote provisioner).
Before this minor change, the '--limit' and '--start-at-task'
ansible-playbook command line arguments were enclosed into single
quotes. Using double quotes adds a bit more flexibility, especially
about the task name referred by `start_at_task` option.
It also aligns with the handling of the '--extra-vars' parameter
(see cb80286).
Without this change, the JSON string generated from the `extra_vars`
Ruby hash is passed without enclosing quotes and is then not parseable
by the ansible-playbook command when exectuted in a usual shell context.
In this changeset, the ansible (remote) unit test coverage is improved
to cover both usage of `extra_vars` (ansible_local unit tests are still
missing).
Additional Notes:
- Double quotes are favored to single quotes in order to allow usage of
any character for the variable values. For this reason additional
escaping is appended to JSON-inner double quotes and backslashes.
- This problem was not affecting the `ansible` remote provisioner
(which is running the ansible-playbook command via the childprocess
Ruby library). But with this change, the `verbose` output will also
now be correct for a copy-paste reuse.
- After this change, all the "--extra-vars" arguments (also a var
file passed with the @-syntax or anything coming via the
`raw_arguments` option) are "blindly" and systematically enclosed
in double quoted and double-escaped.
This is not optimal and can potentially break with peculiar values
(e.g. a double quote character (") cannot be used in a json value
when using `raw_arguments`). That said, I think that the current
solution is a reasonable trade-off, since the official `extra_vars`
option should now be able to cover a great majority of use cases.
Fix#6726
Previously the default channel was "current", but after discussion with
@coderanger on GH-6979, it seems like this was a poor design decision.
Instead, we should use the stable channel and allow users to opt-in to
prerelease versions.
Fixes GH-6979
In cd93721, I relied on a suprising combination of quotes to protect ssh
execution to strip the quoted path to the private key file.
Since any ssh command line argument can be passed via
`ANSIBLE_SSH_ARGS`, it is quite more readable and easy to rely on the
`-i` argument, which is not affected like `-o IdentityFile=...` and also
supports multiple occurences.
See also http://sourceforge.net/p/fuse/mailman/message/30498048/
Finally fix#6671
Note that I decided to not squash both commits for better
documentation and traceability.
Surprisingly (to me at least), a simple quote enclosure was not enough
to fix the problem.
Caveat: the stringified ansible-playbook command logged in verbose mode
is wrongly formatted (no quotes are escaped).
Fix#6671
Set the IPv6 adapter IP to be <prefix>::1. Otherwise, guest to host
communication over IPv6 is not routed correctly. This means that
consumers should not specify <prefix>::1 IP addresses to VirtualBox,
which should be a reasonable restriction.
Fixes#6658
Vagrant should only consider the host-only interfaces used by the
virtual machine in the IPv6 fixup code. There may be other interfaces
present on the system with IPv6 addresses that for various reasons
would fail the routing check (for example, an interface with no
machines attached).
The patch changes the behavior to not scan all of the host-only
interfaces and adds a unit test for the behavior (that the correct IP
is validated).
Lastly, there is a small fix here that may not be an issue for most
people where the IPv6 prefix was asummed to be a multiple of 16 for
the purposes of constructing the UDP probe datagram. This assumption
has been removed.
Fixes#6586
With the introduction of inventory variables, group members provided as
String are not splitted (by ' ') into an array (instead of
auto-conversion to a single-item array).
String and Symbol types are different when used as a Hash key. By
default the Vagrant machine names are set in Symbol format, but users
may write their `host_vars` entries with String keys. This is a very
simple way to ensure smooth experience, without having to coerce the
data types during the config validation (e.g. with a library like
Hashie, which is currently not in the Vagrant dependencies)
See also:
- https://bugs.ruby-lang.org/issues/5964#note-17
- https://github.com/intridea/hashie#keyconversion
This helps with some confusion caused in GH-2538, since the output says:
> Running cleanup tasks for 'shell' provisioner...
But that's actually not true. It is running the cleanup tasks iff the
provisioner defined a cleanup task. This commit changes the
provisioner_cleanup middleware to only run cleanup tasks if the subclass
defines a cleanup task.
The reason we can't just check if the provisioner `respond_to?` the
`cleanup` method is because the parent provisioner base class (which
all provisioners inherit from) defines a blank cleanup method. This is
important because it means we never risk calling an unimplemented
cleanup function, and it also helps define the public API for a
provisioner.
At the moment, the vagrant ssh username is used as default username when
force_remote_user option is disabled, even for winrm-communiating
machines. This could be improved in the future, but people hitting this
problem can easily work around it by syncing `config.ssh.unsername` and
`config.winrm.username` in their Vagrantfile.
ref #5086
This is required because the Chef Server almost always needs a node name to
interact. This will default to the hostname, but that's always going to be
`vagrant.vm`, which will collide easily.
This generates a random hostname with `vagrant-` as the prefix and stores the
result in the machine's data directory.
This fixes GH-6395 by only appending the access_token once. It also fixes a
bug that was never reported. If a user supplied an access_token for a box URL,
Vagrant would silently overwrite it.
After this commit, Vagrant only appends an access_token to the URL if no
value exists at the key.
As described in /etc/sysconfig/network/ifcfg.template
Static template was already using the right one, but the dhcp configuration seems
to be copied from a Fedora/Redhat template.
This fixes the issue that the interface does not come up after reboot.
With this change, the existing host-based Ansible provisioner is
refactored to share a maximum of code with this new guest-based Ansible
provisioner.
At this stage of development, the existing unit tests are intentionally
modified as little as possible, to keep safe the existing funtionalities.
Other issues resolved by this changeset:
- Display a warning when running from a Windows host [GH-5292]
- Do not run `ansible-playbook` in verbose mode when the `verbose` option
is set to an empty string.
The benefits of the following "breaking change" are the following:
- default behaviour naturally fits with most common usage (i.e. always
connect with Vagrant SSH settings)
- the autogenerated inventory is more consistent by providing both the
SSH username and private key.
- no longer needed to explain how to override Ansible `remote_user` parameters
Important: With the `force_remote_user` option, people still can fall
back to the former behavior (prior to Vagrant 1.8.0), which means that
Vagrant integration capabilities are still quite open and flexible.
Here we implement a naive solution to #5605 which catches the case that
a provided source contains an object which cannot be inspected, because
an object contained within in has an #inspect string that returns a
string that is incompatible with the encoding in
`Encoding.default_external` or a string which cannot be downcast to
7-bit ascii.
The Ruby VM implementation of "#inspect" implements this checking on
these lines of code: http://git.io/vZYNS. A Ruby level override of
this method does not cause this problem. For example:
```ruby
class Foo
def inspect
"😍".encode("UTF-16LE")
end
```
will not cause the problem, because that's a Ruby implementation and the
VM's checks don't occur.
However, if we have an Object which **does** use the VM implementation
of inspect, that contains an object that has an inspect string which
returns non-ascii, we encounter the bug. For example:
```ruby
class Bar
def inspect
"😍".encode("UTF-16LE")
end
end
class Foo
def initialize
@bar = Bar.new
end
end
Foo.new.inspect
```
Will cause the issue.
The solution this patch provides basically catches the encoding error
and inserts a string which attempts to help the user work out which
object was provided without blowing up. Most likely, this was caused
by a user having a weird encoding coming out of one of the sources
passed in, but without a full repro case, it's not clear whether a patch
should be applied to a different object in the system.
Closes#5605.
We gained a ton of improvemnts to WinRM error handling in
https://github.com/mitchellh/vagrant/pull/4943, but we also got one bug.
The new code raises an exception when `winrm_info` does not return right
away. This was preventing us from catching the retry/timout logic that's
meant to wait until boot_timeout for the WinRM communicator to be ready.
This restores the proper behavior by rescuing the WinRMNotReady
exception and continuing to retry until the surrounding timeout fires.
With this change, the `Vagrant::plugins_enabled?` is now false when the
embedded Bundler is not available.
Resolve the broken RSpec unit tests after
479323f1e8.
When provisioning multiple machines in sequence (the default vagrant
behaviour), it doesn't make sense to require to provide the private ssh
key(s) via the custom ansible inventory script/file.
To align with the handling of multiple ssh keys per machine, we won't
rely any longer on `--private-key` command line argument, but only pass
the keys via `ANSIBLE_SSH_ARGS` environment variable.
Note that when vagrant generates the ansible inventory and that only one
key is associated to a VM, this step would be redundant, and therefore
won't be applied.
This change fixes the breaking change introduced by 3d62a91.
Revert 1c884fa4e5 which introduced the
following bug:
Instead of allowing to dump the `ansible-playbook` command details when
VAGRANT_LOG=debug was defined, it was then impossible to disable this
console output when VAGRANT_LOG was undefined (in such case,
``@logger.debug? systematically returns `true`)
In order to keep things simple and focused, it is preferable to drop the
bad idea to mix Ansible verbosity and Vagrant log level.
Fix#5803
After #5532 (e745436df3), it was no longer
possible to enable ansible colorized output. Even though
`ANSIBLE_NOCOLOR` has no effect *at the moment* in vagrant+ansible
integration, I agree to keep it for clarity and consistence.
The new `--no-color` behaviour (bug fix#5531) is now covered by a unit
test.
//cc @marsam, @sethvargo
Use of $stdin, $stdout, and $stderr globals makes testing difficult. By
exposing the IO objects as writable attributes, input/output can be more
easily simulated using StringIO or doubles.
This should fix the cleaning up of the default VirtualBox dhcpserver,
which we've been fighting with for ages over in #3083. We were checking
for a structure _including_ a netmask, but the driver was not populating
netmask.
This change helps to avoid troubles like reported in #5018 and #4860.
Note that for sake of configuration simplicity, no new `ansible.timeout`
option has been added. The users who want to set a different value can
rely on `ansible.raw_arguments`.
This SSH option is always set, except when Vagrant is running from an
operating system fo the Solaris-family, as this parameter is not
supported by SunSSH. Logic taken from
bed1f8335f/lib/vagrant/util/ssh.rb (L116-L121)Fix#5017
/cc @sethvargo - Some weirdness here but overall should work fine. I'm
not sure if there was a GH issue this should be attached to or close. To
explain:
We just use the first machine with the default provider. A
Vagrant::Environment guarantees there is at least one machine, so
`env.machine_names.first` will always work. And we can just use the
default provider because we don't really care. Finally, it can be any
old machine we pass in because we just want the "global" config to
validate and there is no way to say "don't validate machine-specific
configs", so we might as well just pick the first machine to validate.
Seth Vargo
David O'Rourke
Gilles Cornu
Kierran McPherson
Ryan Heffernan
Matt Wrock
Mark Mickan
Mitchell Hashimoto
Luke Bakken
Hasyimi Bahrudin
Timur Alperovich
Kamjar Gerami
Mitchell Hashimoto
Christian Henz
Shawn Neal
Seng Lin Shee
Duncan Mac-Vicar P
matthewcodes
Dan Dunckel
Brian Dwyer
tomfanning
Sam Phippen
Paul Hinze
Jeremy Roberts
konomae
Ievgen Prokhorenko
Luis Pabón
Francesco
Tiago Mendes-Costa
Fabio Rapposelli
Jean-Francois Bibeau
Jeff Johnson
Volodymyr Babchynskyy
Mario Rodas
Dan Duvall
Max Lincoln
Michael Lopez
Richard Guin
Simon Vetter
Paul Hinze
mbrodala
Torben Knerr