synchronize module ignores destination host

Hey guys,

I was looking forward for proper rsync wrapper, so I’m very happy to finally see synchronize module in tree. Currently ansible-doc synchronize sheds some light on its proper usage, but apparently not enough to solve the problem I’m experiencing. My problematic task is the following:

A quick check confirms this:

$ ansible -i antares.ginsys.net, all -m synchronize -a “src=~/src/ansible/ dest=~/tmp/” -C -vvv
<127.0.0.1> ESTABLISH CONNECTION FOR USER: serge
<127.0.0.1> EXEC [‘ssh’, ‘-tt’, ‘-q’, ‘-o’, ‘ControlMaster=auto’, ‘-o’, ‘ControlPersist=60s’, ‘-o’, ‘ControlPath=/tmp/ansible-ssh-%h-%p-%r’, ‘-o’, ‘Port=22’, ‘-o’, ‘KbdInteractiveAuthentication=no’, ‘-o’, ‘PasswordAuthentication=no’, ‘-o’, ‘ConnectTimeout=10’, ‘127.0.0.1’, “/bin/sh -c ‘mkdir -p $HOME/.ansible/tmp/ansible-1376508120.59-146532073015922 && chmod a+rx $HOME/.ansible/tmp/ansible-1376508120.59-146532073015922 && echo $HOME/.ansible/tmp/ansible-1376508120.59-146532073015922’”]
antares.ginsys.net | FAILED => Authentication or permission failure. In some cases, you may have been able to authenticate and did not have permissions on the remote directory. Consider changing the remote temp path in ansible.cfg to a path rooted in “/tmp”. Failed command was: mkdir -p $HOME/.ansible/tmp/ansible-1376508120.59-146532073015922 && chmod a+rx $HOME/.ansible/tmp/ansible-1376508120.59-146532073015922 && echo $HOME/.ansible/tmp/ansible-1376508120.59-146532073015922, exited with result 255

It connects to localhost instead of the remote server.

Correction:

IIRC, the synchronize action plugin will deploy the module locally, then issue an rsync to the remote server. So that localhost connection is in fact correct. (though I don’t understand why this should happen that way, instead of running that directly from the action plugin?)

But still, something still issues a problem. -m ping works flawlessly.

Hmm, this didn’t happen for me.

Tim, seeing this was your addition, do you have time to investigate?

It’s not happening to me either. I’ve been overloaded this week because I’m on vacation next week that I’m not going to have a chance to investigate.

From the limited info I can see the synchronize module isn’t even running leading me to believe its something in the action module.

My guess is that some path is getting generated and perhaps created before the new setup (init) method can be called to override the delegate to the localhost. To check I’d turn off temp file clean up and look at the path it says is failing on for clues.

To Serge’s question – remote to remote mirroring. Reread the pull request thread you and I had over this if you need more detail.

Ok so I need to remove this from source control and the docs until it’s operational then, as we’re getting close to ship time.

I’m happy to take this when complete.

I'm not opposed to someone else working on this.

Besides it sounds like the bug is in the core. The synchronize module is just exposing it.

<tim/>

If the bug is in core and nothing else exposes it, is it a bug? :slight_smile:

I don’t find that answer amusing Michael.

I cleaned out my branch since my work was in core. I’ve spent too much time on this already and I’m in no mood to dig my work out to resolve whatever issue the core has.

What I’m saying here is that you were trying to get a way to control the local host forcing in the synchronize module, and there isn’t as far as I understand it another usage of this in core.

Thus it’s more of a supporting feature that isn’t there needed to develop the module rather than a problem affecting other modules in Ansible.

That is, the issue is made manifest by the attempt at trying to make it do something it doesn’t do, but it’s at a code level that doesn’t affect any other module.

Ergo, it’s something you need to do more easily and don’t have a way maybe, but not a bug in core.

If that is the case it is as if Tim had to work around an incomplete API to implement this module. Will the core be improved to make this easier and more predictable for other developers?

Also, what’s the future plan of this module? As it is now, there’s a considerable amount of work nobody can benefit from just laying there.

Tim could have submitted a code change to the API, but we’re not really talking about an API, we’re talking about internals.

Easy project for someone to take up I’d think.

I think I have this working a bit better now, the cause of this problem, is rather simple as it turns out. I have a few other minor changes i would like for this module to make it function how one would expect.

Now that this is outside of the devel branch, How is it best to create a pull request - one squashed commit with the original work + my changes, or a pull request containing multiple commits?

Great!

Multiple commits is ok.

It would be nice to preserve TIm’s authorship for his commits, which would be lost in the squash merge.

Haven’t seen a patch or pull request on this on github for this. Regardless I met Michael last night in NYC at the meetup at Gawker and mentioned what I found looking at the code. Unfortunately I don’t have the bandwidth to figure out how to reproduce the error or test these theories right now, so I’m putting them out there in the eventual someone is so motivated to pick up where I’m leaving off for now.

Looking again this morning I’m not so sure I had pinpointed things going wrong. I was thinking that by the time the setup hook in the synchronize module fires to override the delegate with local host a lot of things about the original host have been put in the inject data. My original thinking was that the place where the override happens needed to preceed setting up inject.

Note sure about that though because the delegate_to logic in runner/init.py does try to revise all the inject data from the original to the delegate. Where it might be failing in the the try block starting around line 578. If something goes wrong there that is specific to Ansible (I’m looking at the template.template() call a few lines later) it quitely handles that exception by just setting the host and post. The user, pass, key file, connection and interpreter are not. So after the setting the inventory host’s ssh variables, if setting up the delegate throws the right type of exception some values belonging to original could be sticking around to confuse what follows.

Just thinking outloud.

Sorry for the delay on this, but I have now created a pull request with my fixes and improvements.

https://github.com/ansible/ansible/pull/4019

Please do all test it out and let me know how it works for you.

I went over this for anyone that is interested in providing feedback.

https://github.com/ansible/ansible/pull/4019
https://github.com/smoothify/ansible/commit/b28297a3d4f1c76e5c08f574f32d6cd938986723

A bit off-topic that reviewing this code reminded me of something I’ve been meaning to ask.

A lot of modules are dependent on a command line. sychronize is an example of that. Shouldn’t we have a standard means of specifying or overriding the path to a bin/exe? Ansible implements such functionality for any interpreter such as ansible_python_interpreter and it also supports one for sudo_exe in the configs. But’s missing is a mechanism for me to specify that rsync is /found/in/some/weird/path/our/sysadmin/forces/us/to/use/rsync.

Know that was an issue for me I implemented an rsync_path option. I 'm thinking this sort of thing would be beneficial to any module that depends on a system tool.

Thoughts?

This makes a lot of sense to me.