ansistrano / deploy

Ansible role to deploy scripting applications like PHP, Python, Ruby, etc. in a capistrano style
https://ansistrano.com
MIT License
2.37k stars 343 forks source link

Create softlinks for shared paths task ignores ansistrano_shared_path variable #238

Closed SiliconMind closed 5 years ago

SiliconMind commented 7 years ago

The task "Create softlinks for shared paths and files" ignores ansistrano_shared_path and creates shared directory always in the same place. This makes impossible to control where this dir should exist.

Task from tasks/symlink-shared.yml:

- name: ANSISTRANO | Create softlinks for shared paths and files
  file:
    state: link
    path: "{{ ansistrano_release_path.stdout }}/{{ item }}"
    src: "{{ item | regex_replace('[^\\/]*', '..') }}/../shared/{{ item }}"
  with_flattened:
    - "{{ ansistrano_shared_paths }}"
    - "{{ ansistrano_shared_files }}"

In this line:

src: "{{ item | regex_replace('[^\\/]*', '..') }}/../shared/{{ item }}"

ansistrano_shared_path should be used instead of this quirky regex_replace() so that links actually point to the shared dir defined in the playbook.

For this to work also "Ensure shared paths exists" task from tasks/setup.yml needs fixing as it creates shared paths inside {{ ansistrano_deploy_to }}/shared completely ignoring ansistrano_shared_path variable as well:

- name: ANSISTRANO | Ensure shared paths exists
  file:
    state: directory
    path: "{{ ansistrano_deploy_to }}/shared/{{ item }}"
  with_items: "{{ ansistrano_shared_paths }}"
  when: ansistrano_ensure_shared_paths_exist

Here {{ ansistrano_deploy_to }}/shared should be replaced with {{ ansistrano_shared_path }}.

I'll be happy to prepare PR if this makes any sense to you guys. Or maybe there is a purpose in current approach?

ricardclau commented 7 years ago

The regular expression is there to allow multi-folder shared items. It is quirky but there was no better way to achieve it

SiliconMind commented 7 years ago

But it works as well without it. The only difference is that path is not relative but absolute. It could probably be changed to relative with some shell-fu if you really need it.

ricardclau commented 7 years ago

Mmmm I see, it looks like you are right, the tests covers this case and they are passing!

Let me review a bit better but I think your code is much cleaner

Thanks!

ricardclau commented 7 years ago

Ok I remember now

If you check https://github.com/ansistrano/deploy/issues/92 you will see why this became a relative path instead of an absolute one.

I understand this makes ansistrano_shared_path impossible to control but it would break the behaviours in that issue

SiliconMind commented 7 years ago

OK, I get it. Though that's kind kind of an edge case. I'll try figuring out how we can change this to relatives instead but preserving the logic.

ricardclau commented 7 years ago

I think the best option would be to have the folder name configurable (now it is hardcoded to shared) and use that value everywhere.

I believe one could even use something like "../../somewherelese" and then we would also allow shared stuff outside of the deployment main folder as some users requested a while ago

rromanchuk commented 6 years ago

This role is failing out of the box for me for a somewhat vanilla configuration

ansistrano_shared_paths:
      - log
      - tmp/pids
      - tmp/cache
      - tmp/sockets
      - vendor/bundle
      - public/system
    ansistrano_shared_files:
      - config/database.yml
      - config/secrets.yml
      - config/puma/production.rb

Excerpt: failed: [redacted] (item=tmp/pids) => {"changed": false, "failed": true, "item": "tmp/pids", "msg": "src file does not exist, use \"force=yes\" if you really want to create the link:

failed: [] (item=tmp/cache) failed: [] (item=tmp/sockets) failed: [] (item=config/puma/production.rb)

Not exactly why some paths worked though like changed: [] => (item=public/system), sounds related though.

ricardclau commented 6 years ago

The error is self explanatory, it cannot symlink something that does not exist (in your case tmp/pids in /shared) https://github.com/ansistrano/deploy/blob/master/tasks/symlink-shared.yml#L18

rromanchuk commented 6 years ago

They most certainly do exist though, that's why i'm concerned the linked line item is incorrectly expanding the source locations. Let me investigate more for user error

ricardclau commented 6 years ago

Weird, as you said some others do work, maybe some permissions difference in some folders? Cannot thing of anything else :(

rromanchuk commented 6 years ago

If you look at src on the failing folder it looks like the relative path is all of a sudden incorrect. I wonder if there is an easier way to debug ansible's file module. It's hard to see what's actually going on right now.

TASK [carlosbuenosvinos.ansistrano-deploy : ANSISTRANO | Create softlinks for shared paths and files] ******************************************************************************************************

changed: [] => (item=log) => {"changed": true, "dest": "/var/www/myapp/releases/20171107223411Z/log", "failed": false, "gid": 1000, "group": "ubuntu", "item": "log", "mode": "0777", "owner": "ubuntu", "size": 16, "src": "../../shared/log", "state": "link", "uid": 1000}

failed: [52.25.183.215] (item=tmp/pids) => {"changed": false, "failed": true, "item": "tmp/pids", "msg": "src file does not exist, use \"force=yes\" if you really want to create the link: /var/www/myapp/releases/20171107223411Z/tmp/../../../shared/tmp/pids", "path": "/var/www/myapp/releases/20171107223411Z/tmp/pids", "src": "../../../shared/tmp/pids", "state": "absent"}
ubuntu@ip-blah:/var/www/myapp/shared$ ls -la /var/www/myapp/shared/tmp/
total 20
drwxr-xr-x  5 ubuntu ubuntu 4096 Nov  7 20:24 .
drwxr-xr-x 10 ubuntu ubuntu 4096 Nov  7 20:24 ..
drwxrwxr-x  2 ubuntu ubuntu 4096 Nov  7 20:24 cache
drwxrwxr-x  2 ubuntu ubuntu 4096 Nov  7 20:24 pids
drwxrwxr-x  2 ubuntu ubuntu 4096 Nov  7 20:24 sockets
rromanchuk commented 6 years ago

I'm going to do a quick sanity test and hardcode an absolute shared path for source

rromanchuk commented 6 years ago

Oh duh, it's because its trying to create the symlink from shared/tmp/pids to {current_release}/tmp/pids, but the tmp directory doesn't exist in the release folder therefore the confusing error message. I just i have to link the entire tmp directory.

ricardclau commented 6 years ago

Ah yeah, that's really annoying, sorry for not seeing it before, either this or you create tmp in a pre-hook or sth

Some tests reflect this situation, it is far from ideal though as you have seen