Skip to content
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

fixed issue with using only $env:temp as temp path on windows #25795

Closed

Conversation

mmasztalerczuk
Copy link

SUMMARY
  • The argument tmpdir is not using in mkdtemp function.
    Every time when ansible is creating temporary folder on remote
    machine (which is using Windows as OS) the path has been set
    to value of $env:temp (and ignoring the value of remote_tmp
    from ansible.cfg)
ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

plugins/shell/powershell.py

* The argument `tmpdir` is not using in `mkdtemp` function.
Every time when ansible is creating temporary folder on remote
machine (which is using Windows as OS) the path has been set
to value of $env:temp (and ignoring the value of `remote_tmp`
from ansible.cfg)
@ansibot
Copy link
Contributor

ansibot commented Jun 16, 2017

The test ansible-test compile --python 3.6 failed with the following error:

lib/ansible/plugins/shell/powershell.py:1050:26: SyntaxError: tmpdir = "$env:temp"

The test ansible-test sanity --test ansible-var-precedence-check failed with the following error:

Command "test/sanity/code-smell/ansible-var-precedence-check.py" returned exit status 1.
>>> Standard Output
CHECKING: extra_vars (var passed via the cli)
ERROR !!!
playbook failed (rc=250), stdout: 'b'ansible-playbook 2.4.0 (detached HEAD 06b5ac7e25) last updated 2017/06/16 13:27:39 (GMT +000)\n  config file = /dev/null\n  configured module search path = [\'/root/.ansible/plugins/modules\', \'/usr/share/ansible/plugins/modules\']\n  ansible python module location = /root/src/github.com/ansible/ansible/lib/ansible\n  executable location = /root/src/github.com/ansible/ansible/bin/ansible-playbook\n  python version = 3.5.2 (default, Nov 17 2016, 17:05:23) [GCC 5.4.0 20160609]\nUsing /dev/null as config file\nsetting up inventory plugins\nParsed /tmp/tmpyx385y1j/inventory/hosts inventory source with ini plugin\nstatically imported: /tmp/tmpyx385y1j/included_tasks.yml\nLoading callback plugin default of type stdout, v2.0 from /root/src/github.com/ansible/ansible/lib/ansible/plugins/callback/__init__.py\n\nPLAYBOOK: site.yml *************************************************************\n1 plays in site.yml\n\nPLAY [testhost] ****************************************************************\nthe full traceback was:\n\nTraceback (most recent call last):\n  File "/root/src/github.com/ansible/ansible/bin/ansible-playbook", line 106, in <module>\n    exit_code = cli.run()\n  File "/root/src/github.com/ansible/ansible/lib/ansible/cli/playbook.py", line 130, in run\n    results = pbex.run()\n  File "/root/src/github.com/ansible/ansible/lib/ansible/executor/playbook_executor.py", line 153, in run\n    result = self._tqm.run(play=play)\n  File "/root/src/github.com/ansible/ansible/lib/ansible/executor/task_queue_manager.py", line 285, in run\n    play_return = strategy.run(iterator, play_context)\n  File "/root/src/github.com/ansible/ansible/lib/ansible/plugins/strategy/linear.py", line 217, in run\n    action = action_loader.get(task.action, class_only=True)\n  File "/root/src/github.com/ansible/ansible/lib/ansible/plugins/__init__.py", line 348, in get\n    path = self.find_plugin(name)\n  File "/root/src/github.com/ansible/ansible/lib/ansible/plugins/__init__.py", line 263, in find_plugin\n    for path in (p for p in self._get_paths() if p not in self._searched_paths and os.path.isdir(p)):\n  File "/root/src/github.com/ansible/ansible/lib/ansible/plugins/__init__.py", line 194, in _get_paths\n    ret.extend(self._get_package_paths(subdirs=subdirs))\n  File "/root/src/github.com/ansible/ansible/lib/ansible/plugins/__init__.py", line 157, in _get_package_paths\n    m = __import__(self.package)\n  File "/root/src/github.com/ansible/ansible/lib/ansible/plugins/action/__init__.py", line 34, in <module>\n    from ansible.executor.module_common import modify_module, build_windows_module_payload\n  File "/root/src/github.com/ansible/ansible/lib/ansible/executor/module_common.py", line 40, in <module>\n    from ansible.plugins.shell.powershell import async_watchdog, async_wrapper, become_wrapper, leaf_exec\n  File "/root/src/github.com/ansible/ansible/lib/ansible/plugins/shell/powershell.py", line 1050\n    tmpdir = "$env:temp"\n                       ^\nTabError: inconsistent use of tabs and spaces in indentation\n'' stderr: 'b'ERROR! Unexpected Exception, this is probably a bug: inconsistent use of tabs and spaces in indentation (powershell.py, line 1050)\n''
feature: extra_vars failed

The test ansible-test compile --python 3.5 failed with the following error:

lib/ansible/plugins/shell/powershell.py:1050:26: SyntaxError: tmpdir = "$env:temp"

The test ansible-test sanity --test pep8 failed with the following errors:

lib/ansible/plugins/shell/powershell.py:1050:1: E101 indentation contains mixed spaces and tabs
lib/ansible/plugins/shell/powershell.py:1050:1: W191 indentation contains tabs
lib/ansible/plugins/shell/powershell.py:1052:1: E101 indentation contains mixed spaces and tabs

The test ansible-test sanity --test pylint failed with the following error:

lib/ansible/plugins/shell/powershell.py:1050:0: syntax-error inconsistent use of tabs and spaces in indentation

click here for bot help

@ansibot ansibot added the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Jun 16, 2017
@s-hertel s-hertel removed the needs_triage Needs a first human triage before being processed. label Jun 16, 2017
@gundalow gundalow added the ci_verified Changes made in this PR are causing tests to fail. label Jun 16, 2017
@gundalow gundalow requested a review from nitzmahone June 16, 2017 17:12
@ansibot ansibot removed ci_verified Changes made in this PR are causing tests to fail. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Jun 16, 2017
@ansibot ansibot added stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Jun 29, 2017
if tmpdir is None:
tmpdir = "$env:temp"

return self._encode_script('''(New-Item -Type Directory -Path %s -Name "%s").FullName | Write-Host -Separator '';''' % (tmpdir, basefile))
Copy link
Contributor

@dagwieers dagwieers Jul 13, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if tmpdir consists of a path with spaces in it ?
I expect Powershell to fail on this, an it should be quoted ...Directory -Path "%s" -Name...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, that's correct (the value should be double-quoted).

@ansibot ansibot added the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Jul 13, 2017
@nitzmahone
Copy link
Member

I'm OK with this in general, but IMO this feature needs to be gated on making ansible_remote_tmp settable via inventory (IIRC it currently isn't). Otherwise, anyone who's running with an ansible.cfg that sets a remote_tmp that's not rooted in ~/ could potentially be broken (since we ignored that config for Windows previously). Without that, it also means there's no way to set an absolute Windows path for temp that wouldn't break the Linux module subsystem (eg, local_action, delegate_to: localhost).

@nitzmahone
Copy link
Member

Sounds like @bcoca may already have done some work on the inventory-settable remote_tmp- more to come...

@alikins
Copy link
Contributor

alikins commented Oct 4, 2017

Looks related to #31022

@ansibot ansibot added the new_contributor This PR is the first contribution by a new community member. label Oct 18, 2017
@ansibot ansibot removed the new_contributor This PR is the first contribution by a new community member. label Nov 3, 2017
@dagwieers
Copy link
Contributor

Beware that the deadline for getting new features/modules accepted in Ansible v2.5 is nearing, it is set to either 2018-01-15 or 2018-01-31. If you are blocked, or you need feedback, please discuss on IRC channel #ansible-windows or add a comment to the Windows Working Group meeting agenda to get it resolved.

@ansibot
Copy link
Contributor

ansibot commented Jan 4, 2018

@jborean93
Copy link
Contributor

@mmasztalerczuk thanks for the PR, this fix was eventually added in #34967 once the shell config options were added in another PR.

@jborean93 jborean93 closed this Jan 17, 2018
@ansibot ansibot added bug This issue/PR relates to a bug. and removed bugfix_pull_request labels Mar 6, 2018
@ansible ansible locked and limited conversation to collaborators Apr 26, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.4 This issue/PR affects Ansible v2.4 bug This issue/PR relates to a bug. c:plugins/shell needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. plugins/shell stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. support:core This issue/PR relates to code supported by the Ansible Engineering Team. windows Windows community
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants