ansible-consul icon indicating copy to clipboard operation
ansible-consul copied to clipboard

Improve path handling on Windows

Open nre-ableton opened this issue 2 years ago • 3 comments

This PR fixes a few issues that occur on Windows hosts with regards to path handling.

Primarily, it addresses an issue this role erroneously deleted all the managed service files it creates in the Delete not declared services [Windows] task. The source of this problem was mixing POSIX-style and Windows-style paths, so that the file matching condition would always fail and cause the files to be deleted. This PR introduces some special Windows-only tasks that use win_stat to convert our internal Unix-style paths to Windows-style paths, which match those discovered by the win_find module.

Normally, the Molecule idempotency test would have caught this error, but as Molecule testing on Windows is basically impossible (:crying_cat_face:), it seems to have slipped through the cracks.

This PR also includes similar path conversion tasks when creating the Windows service, and also fixes a bug where a Windows handler was erroneously called from a task with become: true (probably this one), which caused an error on Ansible since become isn't supported in PowerShell, at least not without extra properties.

nre-ableton avatar May 30 '22 09:05 nre-ableton

If https://github.com/ansible-community/ansible-consul/pull/483 is merged, this will likely create a merge conflict with this PR, and it should also be reworked a bit to include loop_control/loop_var (see 6bd934cdcc135b43ce47d70586e206433918a29a, I'll squash this after review).

nre-ableton avatar Jun 14 '22 07:06 nre-ableton

FYI, I have rebased this branch, squashed, and resolved all merge conflicts. Edit: I've also rebased this branch to include the changes in #488 so that it passes CI.

nre-ableton avatar Jul 11 '22 10:07 nre-ableton

Ping @bbaassssiiee

nre-ableton avatar Jul 28 '22 07:07 nre-ableton

Please resolve the conflicts (rebase is not possible now)

bbaassssiiee avatar Aug 29 '22 06:08 bbaassssiiee

@bbaassssiiee Done!

nre-ableton avatar Aug 29 '22 08:08 nre-ableton

@bbaassssiiee Thanks for the merge!

nre-ableton avatar Aug 29 '22 09:08 nre-ableton