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

Module basic.py to create parent dirs of tmpdir if needed #40201

Merged
merged 3 commits into from May 16, 2018

Conversation

jborean93
Copy link
Contributor

SUMMARY

When pipelining is enabled and the path at remote_tmp does not already exists, module.tmpdir will fail to create the tmp directory. This change will ensure the parent dirs exists before creating that module tmp directory.

Fixes #40168

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

module_utils/basic.py

ANSIBLE VERSION
devel

@ansibot ansibot added bug This issue/PR relates to a bug. needs_triage Needs a first human triage before being processed. support:core This issue/PR relates to code supported by the Ansible Engineering Team. test This PR relates to tests. labels May 15, 2018
@ansibot ansibot added the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label May 15, 2018
@abadger
Copy link
Contributor

abadger commented May 16, 2018

Code looks good. I'd assert whether makedirs is called and also add a case where the path already exists (and assert that makedirs is not called in that case).

@@ -51,7 +51,16 @@ class TestAnsibleModuleTmpDir:
def test_tmpdir_property(self, am, monkeypatch, expected):
def mock_mkdtemp(prefix, dir):
return os.path.join(dir, prefix)

Copy link
Contributor

Choose a reason for hiding this comment

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

makedirs_called = 0
def mock_makedirs(path, mode):
    makedirs_called += 1
    expected = os.path.expanduser(os.path.expandvars(am._remote_tmp))
    assert path == expected
    assert mode = 0o700
    return

Copy link
Contributor

Choose a reason for hiding this comment

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

And then after line 71:

assert makedirs_called == 1

@ansibot ansibot removed the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label May 16, 2018
@jborean93
Copy link
Contributor Author

From #40168 (comment)

Tested #40201 and no longer hit the issue noted above.

@jborean93 jborean93 merged commit 5c39c3b into ansible:devel May 16, 2018
@jborean93 jborean93 deleted the module-tmpdir-pipelining branch May 16, 2018 23:52
@mkrizek mkrizek removed the needs_triage Needs a first human triage before being processed. label May 18, 2018
jacum pushed a commit to jacum/ansible that referenced this pull request Jun 26, 2018
)

* Module basic.py to create parent dirs of tmpdir if needed

* Added warning to dir creation

* Assert if make_dirs was called or not in unit tests
ilicmilan pushed a commit to ilicmilan/ansible that referenced this pull request Nov 7, 2018
)

* Module basic.py to create parent dirs of tmpdir if needed

* Added warning to dir creation

* Assert if make_dirs was called or not in unit tests
@ansible ansible locked and limited conversation to collaborators May 16, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug This issue/PR relates to a bug. support:core This issue/PR relates to code supported by the Ansible Engineering Team. test This PR relates to tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MODULE FAILURE: basic.py tmpdir function
4 participants