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

Synchronize cannot find directories from sub roles like copy does #381

Open
Zocker1999NET opened this issue Jul 18, 2022 · 3 comments · May be fixed by #433
Open

Synchronize cannot find directories from sub roles like copy does #381

Zocker1999NET opened this issue Jul 18, 2022 · 3 comments · May be fixed by #433

Comments

@Zocker1999NET
Copy link

Zocker1999NET commented Jul 18, 2022

SUMMARY

When role A includes role B as dependency, role B can access files & templates from role A's directories using a relative path if this path does not exists in its own directories. This works for nearly all builtin modules from Ansible (EDIT: found the documentation, see here). This also works for the copy module but not for synchronize.

I marked it as a feature request, because this feature is nowhere stated for synchronize itself.

ISSUE TYPE
  • Feature Idea
COMPONENT NAME

ansible.posix.synchronize

ADDITIONAL INFORMATION

This feature helps to e.g. create a role managing a docker-compose project in general (upload repo with docker-compose.yml and Dockerfiles, build & launch compose, configure other stuff required, …), further called "role_b", which can be reused by multiple roles holding the repo with docker-compose.yml and Dockerfiles for certain services, further called "role_a".

The multi file example I describe below shows what works with copy and what I want to do with synchronize:

Example files with contents

site.yml

# run example locally
- hosts: localhost
  connection: local
  gather_facts: no
  roles:
    - role: role_a

roles/role_a/meta/main.yml

# this role holds a repo in its files/ directory

dependencies:
  # include role with copy/synchronize
  - name: role_b

roles/role_a/files/repo/docker-compose.yml

# Imagine a docker-compose.yml using multiple Dockerfiles …

roles/role_a/files/repo/Dockerfile

# Imagine multiple Dockerfiles used by docker-compose.yml

roles/role_b/tasks/main.yml

# check_mode so nothing will be applied

# this works
- name: Copy temp
  check_mode: yes
  ansible.builtin.copy:
    src: repo/
    dest: /tmp

# this not
- name: Sync temp
  check_mode: yes
  ansible.posix.synchronize:
    mode: push
    src: repo/
    dest: /tmp
  ignore_errors: yes
@Zocker1999NET
Copy link
Author

Looked into the module and how Ansible solved that. The relatively simple patch below fixed it in my case. When I have the time, I can run the tests of this repository and submit a PR. If someone has more time than me to test it, please submit it yourself:

diff --git a/plugins/action/synchronize.py b/plugins/action/synchronize.py
index a5752b9..d8eacab 100644
--- a/plugins/action/synchronize.py
+++ b/plugins/action/synchronize.py
@@ -48,10 +48,7 @@ class ActionModule(ActionBase):
         if ':' in path or path.startswith('/'):
             return path
 
-        if self._task._role is not None:
-            path = self._loader.path_dwim_relative(self._task._role._role_path, 'files', path)
-        else:
-            path = self._loader.path_dwim_relative(self._loader.get_basedir(), 'files', path)
+        path = self._find_needle('files', path)
 
         if original_path and original_path[-1] == '/' and path[-1] != '/':
             # make sure the dwim'd path ends in a trailing "/"

I discovered that the native Ansible modules like copy do not call DataLoader.path_dwim_relative or *_stack itself, but the call the method ActionBase.find_needle, which then calls the stack method. The stack method does check for parent roles as well, like the documentation describes it.

@mnaser mnaser linked a pull request Mar 21, 2023 that will close this issue
@mnaser
Copy link

mnaser commented Mar 21, 2023

@Zocker1999NET I pushed up your change into a PR since it's been quite a few months since you brought it up :)

@Zocker1999NET
Copy link
Author

@mnaser Thanks, I already forgot about that 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants