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.
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).
The rationale here is to make it so existing Vagrantfile
configurations remain unchanged.
While benh57's solution worked for me, I had to add a
".to_yaml" to the VagrantFile line that implemented puppet.facter
and this would mean existing Vagrant configurations that use Puppet
would produce an error if that was not present.
Additionally, without this change, the Vagrant file also needed a
"require('yaml')" declaration to make the ".to_yaml" conversion
possible.
When updating the inventory, write to a temp file and replace the
original once writing is complete, to allow for an atomic replacement
of the contents.
Ensures that ansible reading an inventory file will get either the old
or new contents, but never the truncated version of the file that
appears should you open it with 'w' mode set to replace the contents.
Solves the 'provided hosts list is empty' error, which is emitted by
ansible should it manage to be reading the inventory file just as it
was truncated, but before the new contents were flushed to disk.
Partially-Fixes: #6526
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
Before this change, the detection of a non-existing path on the guest
machine was considered as an error and lead to interrupt the current vagrant
action. This was actually a mistake to do so, since the config checks
are performed before many other vagrant actions than `provision`.
The config.validate phase is also intended to primarily check the options
sanity, but it cannot be too strict with the guest state (which can easily
get "out of automatic control").
With this change, we still apply these checks (when possible), but only warn
about possible configuration problems. This way, the subsequent
statements will happen anyway (e.g. ansible commands will be
executed, vagrant machine will be destroyed, etc.)
At least for ansible 2.0.0.1 the command `ansible-galaxy --help` is inappropriate for testing if ansible is installed, as it yields an error:
```
vagrant@vagrant-ubuntu-trusty-64:~$ ansible-galaxy --help && echo "OK"
Usage: ansible-galaxy [delete|import|info|init|install|list|login|remove|search|setup] [--help] [options] ...
Options:
-h, --help show this help message and exit
-v, --verbose verbose mode (-vvv for more, -vvvv to enable connection
debugging)
--version show program's version number and exit
ERROR! Missing required action
```
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
Like in the (remote) `ansible` provisioner, it is preferred to pass the
directory that contains the generated inventory file. This way, advanced
inventory usages can be achieved by adding more inventory files into the
same directory.
Related to #2103 and #6500
[ci skip]
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
I missed to rename the refactored exceptions as AnsibleCommandFailed in
the guest-based parts. The lack of unit tests for these parts hurts...
on my agenda, I swear!
See c1f3d114f5
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.
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 current docker installer attempt to install the linux-image-extra-`uname -r` DEB package on Debian systems.
This package may not exist, for example on custom kernels (e.g., Linode servers).
If this happens, Vagrant halts the provisioning. However, this package is not really needed in newer Debian releases (such as Ubuntu 14.04).
This small patch checks if the linux-image-extra-`uname -r` package exists, and it will install it if it does. In either case, it will continue provisioning.
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.
Starting with vagrant 1.7.3
(commit 1152b4e1df) we don't
save the command to be executed in the file anymore, but we send
it as a parameter, thus the back tick makes things worse.
puppet_server provisioner fails with Puppet Collection 1 with the
following error:
```bash
==> default: Running provisioner: puppet_server...
The `puppet` binary appears not to be in the PATH of the guest. This
could be because the PATH is not properly setup or perhaps Puppet is not
installed on this guest. Puppet provisioning can not continue without
Puppet properly installed.
```
The problem demonstrated in #6065 is that a string has incorrectly been
encoded with US-ASCII even though it contains invalid US-ASCII byte
sequences (any byte with the most significant bit on is invalid in the
US-ASCII encoding).
The thing about doing newline normalization is that it is not actually
sensitive to the presence of US-ASCII byte sequenzes. Additionally, it
is very unlikely that a user will ever be using an encoding where \r\n
is not encoded the same as it would be in ASCII.
This patch first tries the existing method of normalizing the newlines
in the provided script file, if that fails for any reason it force
encodes the string to ASCII-8BIT (which allows the most significant bit
to be on in any individual byte) and then performs the substitution in
that byte space.
This allows Vagrant to calculate if a container's arguments (such as the run
arguments) have changed in the Vagrantfile since the original run.
Fixes GH-3055
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.
Vagrant 1.7.1 creates and injects new ssh keys for each virtual machine.
When it started ansible with the "parallel provisioning trick",
it would only send the ssh key of the targeted virtual machine.
With this change, vagrant now stores the ssh key for each virtual
machines directly in the generated ansible inventory, and thus allow
ansible parallelism.
Note that this change is not sufficient, as it would break vagrant
configuration based on a custom inventory (file or script). This issue
will be addressed in a next commit.
Signed-off-by: Luis Pabón <lpabon@redhat.com>
The Ansible Vagrant provisioner has a race where the inventory file is
updated every time the provisioner runs unless a file is provided.
Therefore if Ansible attempts to provision two nodes in parallel, you
may see the following race:
* System A writes the inventory file and calls Ansible.
* System B starts to provision and truncates the file before
creating a new one.
* Ansible on system A now attempts to read the inventory
file, which is blank. Ansible bombs out with "ERROR: provided
hosts list is empty".
To fix this, we only allow Vagrant to update the inventory file if
it needs to.
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
This allows the display of friendly output to the user when we think an
important action should be labeled a certain way. This makes vagrant
output a lot more useful when many inline shell provisioners are
present.
Luca Invernizzi
Gilles Cornu
Seth Vargo
Thom May
Peter Carrero
Michael Sarahan
Darragh Bailey
Kamjar Gerami
Donald Guy
gavioto20
Daniel Schreiber
Shawn Neal
Gordon Franke
char16t
KaffeJunky123
Mitchell Hashimoto
michaa76
Rickard von Essen
Christian Henz
Jason Casden
Seth Miller
Mitchell Hashimoto
Aneesh Agrawal
Timotei Dolean
Dan Dunckel
Marc Siegfriedt
Johannes Graf
Jurnell Cockhren
Brian Dwyer
Trey Briggs
Sam Phippen
Philip Wigg
konomae
Jason Woods
John Syrinek
Ben Hines
지훈
Pat O'Shea
Mattias Appelgren
William Kral
Luis Pabón
Nicholas Randon
Jeff Quast
arthurvanduynhoven
Tim O'Guin
John Cooper
David Cournapeau
Jeremy Plichta
Taliesin Sisson
Ben Hines
Paul Krohn
James Shubin