-
Notifications
You must be signed in to change notification settings - Fork 199
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
Templating broken when constructing value for ansible_ssh_common_args
#905
Comments
I have the same issue with ansible core 2.11.8, mitogen 0.3.2 |
same issue here, Ansible Core 2.10.17, mitogen 0.3.2 & 0.3.1 |
Occured here too with Ansible 2.11.9 and mitogen 0.3.2 (well, master, more specifically) due to this fella https://github.com/kubernetes-sigs/kubespray/blob/master/roles/kubespray-defaults/defaults/main.yaml#L4 |
with a small test inventory, Ansible 2.10 and git bisect, I found commit c61c063 to be the reason for this bug. Unfortunately this commit is quite big, so I try to manually find the exact reason to maybe find a bugfix / workaround … |
It may be |
You're right, any other change of this commit does not affect the outcome of my tests. But, it may also be that an internal change in Ansible that (also) causes this bug, but I'm not quite sure: While trying to find the smallest partial revert of commit c61c063, I detected a difference in the result of my small ping test depending on the version of Ansible used. Beginning from tag v0.3.2, after applying the diff at the end (which reverts the commit partially), running
So partially reverting this change does work for older Ansible versions (~ 2.10) but not for newer ones (~ 5.4.0 / 2.12.3). This is the diff from the mention above: diff --git a/ansible_mitogen/transport_config.py b/ansible_mitogen/transport_config.py
index 4babbde3..344c3d84 100644
--- a/ansible_mitogen/transport_config.py
+++ b/ansible_mitogen/transport_config.py
@@ -467,9 +467,9 @@ class PlayContextSpec(Spec):
return [
mitogen.core.to_text(term)
for s in (
- C.config.get_config_value("ssh_args", plugin_type="connection", plugin_name="ssh", variables=self._task_vars.get("vars", {})),
- C.config.get_config_value("ssh_common_args", plugin_type="connection", plugin_name="ssh", variables=self._task_vars.get("vars", {})),
- C.config.get_config_value("ssh_extra_args", plugin_type="connection", plugin_name="ssh", variables=self._task_vars.get("vars", {}))
+ getattr(self._play_context, 'ssh_args', ''),
+ getattr(self._play_context, 'ssh_common_args', ''),
+ getattr(self._play_context, 'ssh_extra_args', '')
)
for term in ansible.utils.shlex.shlex_split(s or '')
]
@@ -696,9 +696,22 @@ class MitogenViaSpec(Spec):
return [
mitogen.core.to_text(term)
for s in (
- C.config.get_config_value("ssh_args", plugin_type="connection", plugin_name="ssh", variables=self._task_vars.get("vars", {})),
- C.config.get_config_value("ssh_common_args", plugin_type="connection", plugin_name="ssh", variables=self._task_vars.get("vars", {})),
- C.config.get_config_value("ssh_extra_args", plugin_type="connection", plugin_name="ssh", variables=self._task_vars.get("vars", {}))
+ (
+ self._host_vars.get('ansible_ssh_args') or
+ getattr(C, 'ANSIBLE_SSH_ARGS', None) or
+ os.environ.get('ANSIBLE_SSH_ARGS')
+ # TODO: ini entry. older versions.
+ ),
+ (
+ self._host_vars.get('ansible_ssh_common_args') or
+ os.environ.get('ANSIBLE_SSH_COMMON_ARGS')
+ # TODO: ini entry.
+ ),
+ (
+ self._host_vars.get('ansible_ssh_extra_args') or
+ os.environ.get('ANSIBLE_SSH_EXTRA_ARGS')
+ # TODO: ini entry.
+ ),
)
for term in ansible.utils.shlex.shlex_split(s)
if s |
Encountering the same issue
|
@dnmvisser Have you found a working solution for this issue ? mitogen is a very useful part of our toolset, we'd love to hear if there's a way to make this work. Thank you. |
Nope, I ended up creating files with hard coded IP addresses etc |
We finally got this to work with this combination of versions:
|
I took the time to inspect further and found a difference in the calling of For getting the configuration of mitogen/ansible_mitogen/transport_config.py Line 478 in 89c0cc9
Ansible plugins (here ssh) use a helper Intercepting these calls to Meaning in practice: Given these example host vars: ansible_ssh_common_args: "{{ other_var }}"
other_var: "--my-option" Then the argument
I do not know Ansible's Python code good enough to fix this, probably by resolving the variables properly before passing them to |
Any updates? |
@moreati could you please look into this issue? |
Any workarounds maybe? I use this with kubespray. |
Same issues for while installing kubespray with mitogen 0.3.3. After playing a little with the python script and the responsible file (thx @Zocker1999NET), I find a way to fix it. The fix is to replace Result looks like: def ssh_args(self):
return [
mitogen.core.to_text(term)
for s in (
C.config.get_config_value("ssh_args", plugin_type="connection", plugin_name="ssh", variables=self._task_vars.get("hostvars", {}).get(self._inventory_name, {})),
C.config.get_config_value("ssh_common_args", plugin_type="connection", plugin_name="ssh", variables=self._task_vars.get("hostvars", {}).get(self._inventory_name, {})),
C.config.get_config_value("ssh_extra_args", plugin_type="connection", plugin_name="ssh", variables=self._task_vars.get("hostvars", {}).get(self._inventory_name, {}))
)
for term in ansible.utils.shlex.shlex_split(s or '')
] I won't be able to verify the fix until August, but if someone can play with it, let's share the result! |
@momiji I think this can be a valid fix for this issue. I applied the change to both ssh_args methods in |
I tested @momiji's patch (much appreciated!) with a simple test and a more complex real-world playbook today and everything is worked as expected. We will probably be using this patched mitogen for our playbooks until an official fix comes out, so I'll report back here if we do run into any regressions or issues that may be related. |
Hello, Thanks for the patch @momiji I have playbook tasks:
Playbook run error:
|
I tried this out with ansible 5.10.0 and mitogen-0.3.3 but for me this does not work. I still get the same error ( |
Hi all, after reading your remarks, I've been able to make some tests on my side:
I don't think it's using the bastion feature, so I can't help on the remaining issues. |
Hi, also achieved migration to kubespray 1.19.0 (using ansible 5.7.1) with no issues. |
Hello, PR #956 sent. |
Just to update that
No more error |
Any way to get this merged? I also need to apply the patch to get my setup working... |
I've tried once again today and still no luck with:
If there are people out there that do have a working setup using the above, please post which versions you use:
If anyone is interested, we use a shell wrapper for #
export JUMP_IP=$(aws ec2 describe-instances \
--region ${AWS_DEFAULT_REGION} \
--filters "Name=tag:Name,Values=jumphost" \
--query 'Reservations[0].Instances[]' | \
jq -r 'sort_by(.LaunchTime)|reverse|.[0].PublicIpAddress' )
#
ansible-playbook \
--ssh-common-args="-o ProxyCommand='ssh -o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no -W %h:%p admin@${JUMP_IP}'" \
playbook.yml |
I see something very similar with templates used in
|
I experience the a similar issue with a Jinja2 expression in |
related to or duplicate of #599 |
With the time to upgrade our ansible codebase, it seems we're still blocked by anything above
and
Result:
I tried applying the patch suggested above, to If anyone sub'ed here has found a way forward, please do share. |
Reproducer/proto-test https://gist.github.com/moreati/f6a99ec0249d4ff1082a694f4b6fe07c Duly noted that |
Unfortunately had to stop using |
I still have this issue with
and setting
I got:
It works when I hardcode
|
This expands support to setting them in Play scoped variables. Task scoped variables are also very likely to work, but untested for now. refs mitogen-hq#905
The code in master should |
Mitogen 0.3.15 is out. It supports templated SSH arguments. |
it works! thanks a lot |
Hi, I'm on ansible-core-2.12.2 (thx for all the work in getting that done) and mitogen v0.3.2.
We have some basic jinja inside one of our vars files:
This causes errors:
If I hardcode it like this:
then things work....
The jinja inside the inventory works fine with ansible v3.4.0 (ansible-base 2.10.x)
Any thoughts?
The text was updated successfully, but these errors were encountered: