chroot works for me. Here is what I have in my inventory file:

mock-fedora20 ansible_ssh_host=/var/lib/mock/fedora-20-x86_64/root ansible_connection=chroot

and how I invoke it:

sudo ansible mock-fedora20 -i /etc/ansible/hosts.chroot -m command -a 'whoami'

I think it was a conscious decision to limit the inventory hostname and make use of ansible_host for things that couldn't obey the limit. @bcoca and I both thought that this should work as a way to specify the host without having to use an inventory file though:

sudo ansible localhost -e 'ansible_host=/var/lib/mock/fedora-20-x86_64/root' -m command -a 'cat /etc/motd' -c chroot

and it doesn't at the moment....

So your solution work fine, but

ansible -vvv -c chroot all -i '/var/lib/mock/fedora-22-x86_64/root/,' -m ping

result into:

Using /etc/ansible/ansible.cfg as config file
Unexpected Exception: 'NoneType' object has no attribute 'split'
the full traceback was:

Traceback (most recent call last):
  File "/home/misc/checkout/git/ansible/bin/ansible", line 85, in <module>
    sys.exit(cli.run())
  File "/home/misc/checkout/git/ansible/lib/ansible/cli/adhoc.py", line 187, in run
    result = self._tqm.run(play)
  File "/home/misc/checkout/git/ansible/lib/ansible/executor/task_queue_manager.py", line 237, in run
    play_return = strategy.run(iterator, play_context)
  File "/home/misc/checkout/git/ansible/lib/ansible/plugins/strategy/linear.py", line 217, in run
    task_vars = self._variable_manager.get_vars(loader=self._loader, play=iterator._play, host=host, task=task)
  File "/home/misc/checkout/git/ansible/lib/ansible/vars/__init__.py", line 245, in get_vars
    all_vars = combine_vars(all_vars, host.get_vars())
  File "/home/misc/checkout/git/ansible/lib/ansible/inventory/host.py", line 129, in get_vars
    results['inventory_hostname_short'] = self.name.split('.')[0]
AttributeError: 'NoneType' object has no attribute 'split'

I would say that it seems more logical to use this method as it seems the most natural. But maybe another solution would be to have the parsing and verification of the line be tied to the connexion plugin used, since the semantic depend on what you are using.

So, just for reference, the plugin I wrote (and who work, i just need now to write maybe some documentation and do more tests) can be found on https://github.com/mscherer/ansible/blob/guestfish/lib/ansible/plugins/connection/guestfs.py

I just tested with a Fedora 22 (not working on Fedora 23 due to lack of python 2, and writing code that can be run on python 2 and python 3 is left as a exercise for me), and I can remove and add file with the shell command, among others.

At 2015-12-19 08:41:56 -0800, notifications@github.com wrote:

I would say that it seems more logical to use this method as it seems
the most natural.

Your patch says "if it's not invalid, add it; otherwise, add it anyway".
I'm sorry, but that doesn't seem either logical or natural to me.

It's unfortunate that Ansible, in the past, accepted any garbage one
threw at it in the form of inventory hostnames. I regret that fixing
this causes some backwards-incompatibility (it never crossed my mind
that anyone was using paths as hostnames, though), but I still think
that fixing it is the right thing to do.

Inventory hostnames should be hostnames. If your connection plugin wants
something other than a hostname to connect to, set it in ansible_host.

-- Abhijit

Sure, it doesn't seems logical because you see ansible as a tool that can only do ssh, but it had connexion plugins since a long time ( like right from the start ).

That's too bad that the original code did use the word 'ssh' in various variable ( such as 'ansible_ssh_host', which was renamed for 2.0 to 'ansible_host' maybe for clarity), but that doesn't mean others people didn't use it for more than ssh.

The current code do force the conversion, because it assume that only ssh is used, but do not verify the conversion so do not show a error message to the user, so people are just left with a cryptic backtrace later.

The workaround is also not very optimal. Instructions like "use a random name, then set a variable ansible_host for the target", when the target is not a host are a bit counter intuitive, and clearly more complicated than the current way. It also prevent usage with the '-i target,' notation for ad-hoc command (cf Toshioa last comment). Using -e for that also make things more complicated than before.

So from the user point of view, things are more complicated, and the current code do not bring a improvement to them either in the current form)

And since ansible can't assume that the connexion will be using hostname:port, there isn't reason to hardcode that specific syntax as the default one. The hostname verification should be done by the connexion plugin to be used, not hardcoded.

I do not claim that my patch is perfect and I am more than ok to revisit. First time I did a patch without proposing a PR, I was asked to make a PR first then discuss, so yes, I just pushed what fixed for me to be able to finish my work as a basis to start the discussion.

So we have 2 issues to deal with for now:

First is the lack of return code checking who result in a backtrace. This is easy to fix, but we can't fix this one without knowing what kind of fix we want. It would be trivial to verify that hostname is None and show a error message, this would avoid the backtrace, but as I said, I do not see that satisfying.

Your position is that only hostname:port should be there, and my position is that we should tolerate some freedom for the other plugins who deal with different target than hostname:port. So I think we agree that for the ssh/paramiko connexion, that's the right thing to do, no discussion here.

We just disagree on the solution on how to let the non ssh plugins work and what to do with them when we encounter them using the pre-20 way of doing things.

Which bring to the 2nd issue, that the syntax got harder and that the new way to do things is not working for the non default case with ad hoc commands, which is a problem.

The solution you propose of using a per connexion variable is good on the extensibility point of view, and has the advantage of being easier to code (since no change in the engine). I would also permit to not be limited by the format of 1 single string (for example, guestfish can connect to a ton of things to edit a image, and I have no idea on how to support that in the plugin in a clean way)

But (IMHO), this also has the problem of being less practical for the user, since this requires a specific learning curve per connexion plugin, requires us to document all of them (which wouldn't be a problem, but still more work to do), and bring risks of having the syntax drift between plugins (one being ansible_chroot_path, the other being ansible_jail_directory, for example), which would also make things harder to understand for users, and things are less coherent between plugins (ie, local, paramiko, ssh, winrm are using a different way than all the others)

It also force people to give a valid hostname (ie, not just a arbitary label, despites the fact that's what it just became) which is not used by the plugin, thus causing confusion, and force people to have a more complex mental model of how ansible work. It also break compatibility with existing practices.

We also disagree on needing or not hostname, as if that's unused, then this shouldn't be needed to specify it.

And the current status, ie using ansible_host to carry information for the non standard connexion plugin has some problem too, since it just mean that we do have different code paths between the implicit declaration of the hostname and the explicit one ( ie, ansible_ssh_hostname=foo, or ansible_host ), and so either we check both of them (but then, we block the workaround proposed by Toshio), or we have a different behavior depending on how that's specified, and I think we can also agree we want to avoid that as this will likely creates hard to find bugs.

Your proposal of chaining connexion plugins is interesting, but I think it got refused last time it was discussed some years ago. While it would solve the jump host case nicely, and would permit new stuff (like ssh + docker connexion plugin to connect inside a remote docker container for orchestration, which would be great), but no good design was found yet. We can't easily compose the plugins unfortunately.

I am sure there is a better way that do not make things harder for non default case (and without sacrificing the goal of simplicity that Ansible is striving).

What about the suggestion of checking and enforcing the hostname verification if the connexion is ssh/paramiko/winrm, or something similar ?

This way, we keep the verification for the default, which is what we both agree to be the right thing to do, and it still provides people with the compatibility and the same syntax for non default plugins.

The problem is that we need to accept the name given by inventory to be able to add it to the inventory to later be able to decide what connexion plugin to use, so we can't just check that at the start of parse_inventory, it has to be done at the end since it has to be done once all variables are set to decide what is the right course of action.

I would propose this verification should also be done by the Host class, since it is the one will all informations required.

(while on it, it would be good to get ride of the duplicate Host class in ./lib/ansible/new_inventory/host.py to avoid confusion, not sure if it used somewhere)

@jimi-c, @bcoca, and I talked about this this morning and came to the conclusion that whether something is valid or not depends on which connection plugin it's associated with. We tossed around several ideas (for instance using a uri-like string for inventory_hostnames that weren't ssh compatible) Currently we're thinking that we should probably continue to do parsing here, in the inventory parser but not do validation until we're in the connection plugin.

So the code in inventory would still use parse_address but, similar to mscherer's PR, it would construct a Host() object whether it was parsable as an address or not. Later, when a connection plugin is given a host to operate on, we'd run parse_address on the name and decide whether it validated.

Our first thoughts were to implement that as a check in inventory that called a helper method in the associated connection plugin in order to validate the address. But bcoca pointed out that one inventory_hostname entry could be serviced by multiple connection_plugins so we can't determine this from inventory alone. A playbook or extra_vars can set ansible_connection to choose between using docker or ssh connection depending on whether the user is running the playbook from the host running the docker containers or a remote host, for instance.

If we get validation into the plugins for 2.1 then 2.0 would be the only release which doesn't accept the inventory_hostnames that users are using today. That wouldn't make much sense. So we'd be okay implementing this in a way that skips validation until it's implemented in the conenction plugin.

@crab that would be fine but we'd need it ASAP. We're trying to cut rc3 today (We wanted it last week but we got caught up in fixing both integration tests and the integration test frramework).

@crab could be added to both ssh and paramiko.