Skip to content

Commit

Permalink
fix pc misuse (ansible#82157)
Browse files Browse the repository at this point in the history
use the more up to date 'task' as play_context only has the 'initial' values.
  • Loading branch information
bcoca authored Nov 16, 2023
1 parent e2d108d commit 3d9e5c8
Show file tree
Hide file tree
Showing 10 changed files with 33 additions and 32 deletions.
2 changes: 2 additions & 0 deletions changelogs/fragments/pc_fixes.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
bugfixes:
- moved assemble, raw, copy, fetch, reboot, script and wait_for_connection to query task instead of play_context ensuring they get the lastest and most correct data.
2 changes: 1 addition & 1 deletion lib/ansible/plugins/action/assemble.py
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ def run(self, tmp=None, task_vars=None):

if path_checksum != dest_stat['checksum']:

if self._play_context.diff:
if self._task.diff:
diff = self._get_diff_data(dest, path, task_vars)

remote_path = self._connection._shell.join_path(self._connection._shell.tmpdir, 'src')
Expand Down
6 changes: 3 additions & 3 deletions lib/ansible/plugins/action/copy.py
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ def _ensure_invocation(self, result):
# NOTE: do not add to this. This should be made a generic function for action plugins.
# This should also use the same argspec as the module instead of keeping it in sync.
if 'invocation' not in result:
if self._play_context.no_log:
if self._task.no_log:
result['invocation'] = "CENSORED: no_log is set"
else:
# NOTE: Should be removed in the future. For now keep this broken
Expand Down Expand Up @@ -283,10 +283,10 @@ def _copy_file(self, source_full, source_rel, content, content_tempfile,
if local_checksum != dest_status['checksum']:
# The checksums don't match and we will change or error out.

if self._play_context.diff and not raw:
if self._task.diff and not raw:
result['diff'].append(self._get_diff_data(dest_file, source_full, task_vars, content))

if self._play_context.check_mode:
if self._task.check_mode:
self._remove_tempfile_if_content_defined(content, content_tempfile)
result['changed'] = True
return result
Expand Down
2 changes: 1 addition & 1 deletion lib/ansible/plugins/action/fetch.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ def run(self, tmp=None, task_vars=None):
del tmp # tmp no longer has any effect

try:
if self._play_context.check_mode:
if self._task.check_mode:
raise AnsibleActionSkip('check mode not (yet) supported for this module')

source = self._task.args.get('src', None)
Expand Down
2 changes: 1 addition & 1 deletion lib/ansible/plugins/action/raw.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ def run(self, tmp=None, task_vars=None):
result = super(ActionModule, self).run(tmp, task_vars)
del tmp # tmp no longer has any effect

if self._play_context.check_mode:
if self._task.check_mode:
# in --check mode, always skip this module execution
result['skipped'] = True
return result
Expand Down
2 changes: 1 addition & 1 deletion lib/ansible/plugins/action/reboot.py
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,7 @@ def run(self, tmp=None, task_vars=None):
msg = 'Running {0} with local connection would reboot the control node.'.format(self._task.action)
return {'changed': False, 'elapsed': 0, 'rebooted': False, 'failed': True, 'msg': msg}

if self._play_context.check_mode:
if self._task.check_mode:
return {'changed': True, 'elapsed': 0, 'rebooted': True}

if task_vars is None:
Expand Down
4 changes: 2 additions & 2 deletions lib/ansible/plugins/action/script.py
Original file line number Diff line number Diff line change
Expand Up @@ -150,11 +150,11 @@ def run(self, tmp=None, task_vars=None):
# like become and environment args
if getattr(self._connection._shell, "_IS_WINDOWS", False):
# FUTURE: use a more public method to get the exec payload
pc = self._play_context
pc = self._task
exec_data = ps_manifest._create_powershell_wrapper(
to_bytes(script_cmd), source, {}, env_dict, self._task.async_val,
pc.become, pc.become_method, pc.become_user,
pc.become_pass, pc.become_flags, "script", task_vars, None
self._play_context.become_pass, pc.become_flags, "script", task_vars, None
)
# build the necessary exec wrapper command
# FUTURE: this still doesn't let script work on Windows with non-pipelined connections or
Expand Down
2 changes: 1 addition & 1 deletion lib/ansible/plugins/action/wait_for_connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ def run(self, tmp=None, task_vars=None):
sleep = int(self._task.args.get('sleep', self.DEFAULT_SLEEP))
timeout = int(self._task.args.get('timeout', self.DEFAULT_TIMEOUT))

if self._play_context.check_mode:
if self._task.check_mode:
display.vvv("wait_for_connection: skipping for check_mode")
return dict(skipped=True)

Expand Down
41 changes: 19 additions & 22 deletions test/units/plugins/action/test_raw.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,20 @@ def setUp(self):
def tearDown(self):
pass

def _build_task(self, params=None):

task = MagicMock(Task)
task.async_val = False
task.diff = False
task.check_mode = False
task.environment = None

if params is None:
task.args = {'_raw_params': 'Args1'}
else:
task.args = params

return task
# The current behavior of the raw aciton in regards to executable is currently in question;
# the test_raw_executable_is_not_empty_string verifies the current behavior (whether it is desireed or not.
# Please refer to the following for context:
Expand All @@ -44,11 +58,7 @@ def tearDown(self):

def test_raw_executable_is_not_empty_string(self):

task = MagicMock(Task)
task.async_val = False

task.args = {'_raw_params': 'Args1'}
self.play_context.check_mode = False
task = self._build_task()

self.mock_am = ActionModule(task, self.connection, self.play_context, loader=None, templar=None, shared_loader_obj=None)
self.mock_am._low_level_execute_command = Mock(return_value={})
Expand All @@ -60,22 +70,14 @@ def test_raw_executable_is_not_empty_string(self):

def test_raw_check_mode_is_True(self):

task = MagicMock(Task)
task.async_val = False

task.args = {'_raw_params': 'Args1'}
self.play_context.check_mode = True
task = self._build_task()
task.check_mode = True

self.mock_am = ActionModule(task, self.connection, self.play_context, loader=None, templar=None, shared_loader_obj=None)

def test_raw_test_environment_is_None(self):

task = MagicMock(Task)
task.async_val = False

task.args = {'_raw_params': 'Args1'}
task.environment = None
self.play_context.check_mode = False
task = self._build_task()

self.mock_am = ActionModule(task, self.connection, self.play_context, loader=None, templar=None, shared_loader_obj=None)
self.mock_am._low_level_execute_command = Mock(return_value={})
Expand All @@ -85,12 +87,7 @@ def test_raw_test_environment_is_None(self):

def test_raw_task_vars_is_not_None(self):

task = MagicMock(Task)
task.async_val = False

task.args = {'_raw_params': 'Args1'}
task.environment = None
self.play_context.check_mode = False
task = self._build_task()

self.mock_am = ActionModule(task, self.connection, self.play_context, loader=None, templar=None, shared_loader_obj=None)
self.mock_am._low_level_execute_command = Mock(return_value={})
Expand Down
2 changes: 2 additions & 0 deletions test/units/plugins/action/test_reboot.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ def module_task(mocker, task_args):
task.action = 'reboot'
task.args = task_args
task.async_val = False
task.check_mode = False
task.diff = False
return task


Expand Down

0 comments on commit 3d9e5c8

Please sign in to comment.