Go back to using ~/.ansible/cp as the ControlPath #12236

Merged
merged 1 commit into from Sep 4, 2015

Conversation

Projects
None yet
3 participants
@amenonsen
Contributor

amenonsen commented Sep 3, 2015

This was commented out earlier because of the lack of interprocess
locking and prepare_writeable_dir in v2.

The locking was not needed: it could only protect against other siblings
of this process (since they were all locking a temporary file that was
opened in the parent), and those would be running as the same user and
with the same umask. Also, os.makedirs() tolerates intermediate paths
being created by other processes. For any other kind of error, both
locking and non-locking code paths would fail in the same way.

So all we really need to do is make sure we have write permissions.

@bcoca

View changes

lib/ansible/plugins/connections/ssh.py
+ except (IOError, OSError), e:
+ raise AnsibleError("Could not make dir %s: %s" % (self._cp_dir, e))
+ if not os.access(self._cp_dir, os.W_OK):
+ raise AnsibleError("Cannot write to path %s" % self._cp_dir)

This comment has been minimized.

@bcoca

bcoca Sep 3, 2015

Member

makedirs_safe already does this, just use that function

@bcoca

bcoca Sep 3, 2015

Member

makedirs_safe already does this, just use that function

Go back to using ~/.ansible/cp as the ControlPath
This was commented out earlier because of the lack of interprocess
locking and prepare_writeable_dir in v2.

The locking was not needed: it could only protect against other siblings
of this process (since they were all locking a temporary file that was
opened in the parent), and those would be running as the same user and
with the same umask. Also, os.makedirs() tolerates intermediate paths
being created by other processes. For any other kind of error, both
locking and non-locking code paths would fail in the same way.

So all we really need to do is make sure we have write permissions.

(We also move the cp_dir handling code to where we actually set the
ControlPath ourselves; if the user has set it via ssh_*args already,
we don't need to bother.)

jimi-c added a commit that referenced this pull request Sep 4, 2015

Merge pull request #12236 from amenonsen/ssh-cpdir
Go back to using ~/.ansible/cp as the ControlPath

@jimi-c jimi-c merged commit 1840906 into ansible:devel Sep 4, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@jimi-c

This comment has been minimized.

Show comment
Hide comment
@jimi-c

jimi-c Sep 4, 2015

Member

Merged, thanks!

Member

jimi-c commented Sep 4, 2015

Merged, thanks!

@amenonsen amenonsen deleted the amenonsen:ssh-cpdir branch Dec 11, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment