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

Custom download method #219

Closed AnatolyRugalev closed 7 years ago

AnatolyRugalev commented 7 years ago

Hi there!

I'm trying to setup deployment of Jenkins artifacts and I faced issue caused by authentication API of Jenkins. I need to pass force_basic_auth: yes option to get_url module to fix it, but unfortunately, I don't understand how to override download_unarchive update code method.

It would be great to have an ability to set my own update-code method just like it was done for event hooks. For example:

ansistrano_deploy_via: "task"
ansistrano_deploy_task_file: "{{ playbook_dir }}/deploy/jenkins-download.yml"

What do you think?

AnatolyRugalev commented 7 years ago

Just made a quick implementation, you can see it in this commit

Also you can test it right from Galaxy: AnatolyRugalev.ansistrano-deploy-custom,2.3.1

ricardclau commented 7 years ago

You can modify download.yml with filters such as | default(omit) for your parameters or in this case (force_basic_auth) default it to false and add new variables to the role

What you did at #220 is not the way to go IMHO

AnatolyRugalev commented 7 years ago

@ricardclau thank you for clarification.

I see that my solution is not the best. To be honest, I'm new in Ansible, so any suggestions are appreciated.

Do you mean to modify download.yml and make a new PR?

So, what about other options? What if I need to specify custom headers to get_url? Should I add them to download.yml too? There is a huge number of different options and I don't see an easy way to deal with them without forking the whole role. An ability to make custom deploy strategies would be a great feature, I think. Adding force_basic_auth solves my exact problem, but you never know which else options might be needed in future.

May be I don't understand something? Thank you for your time

ricardclau commented 7 years ago

I was checking and these new parameters are only supported in Ansible 2.0. We still support 1.9 so it can be tricky, let´s see if the tests pass

AnatolyRugalev commented 7 years ago

Thank you

ricardclau commented 7 years ago

Hey @AnatolyRugalev can you please test the better_geturl branch with your setup setting the variable accordingly? If this is a hassle we will just create a new tag

AnatolyRugalev commented 7 years ago

@ricardclau tested. Works fine for me

ricardclau commented 7 years ago

Brilliant, will merge and tag today

Regarding a 100% custom strategy, I think the best approach would be to create a noop strategy which basically does nothing so that you would implement the actual strategy in either the before or after update hooks. This would not break anything and may allocate such need

Having said that, if you think about it, there are not that many ways to "deploy" code. We might need a mercurial strategy, support the Google Cloud / Azure equivalent to S3 and similar things but I would prefer adding them as users demand it than opening the door to total custom which adds little benefit to the official deploy_helper module in Ansible

Thoughts?

AnatolyRugalev commented 7 years ago

Huh, I didn't know about deploy_helper module itself because I was searching "ansible capistrano deployment" 😅 and that's why I was asking for customization.

Now I see that your point is absolutely right 👍

ricardclau commented 7 years ago

Yeah, we created Ansistrano some years before deploy_helper existed. We have plans to integrate it once we deprecate Ansible 1.9 and in that scenario your "100% custom" strategy is just using deploy_helper :)

Cool, closing this for now and if more people demand such thing we will reconsider

Thanks for testing! Tag is available for everyone!