-
Notifications
You must be signed in to change notification settings - Fork 729
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
Makefile update #901
Makefile update #901
Conversation
@webdevsHub @im-denisenko I suggest to have the docker environment as default for all make commands. This will simplify the setup even further and engineers will automatically use the docker setup. What do you think? |
@ruflin I'm ok with it. Should we drop vagrant then to not confuse which one tool is actually used? |
I currently still use vagrant to test if the ansible scripts work as expected as these are still used for travis. What I would like to do for travis is to switch to the container based architecture, which means removing sudo. Our main usage of sudo is here: https://github.com/ruflin/Elastica/blob/master/ansible/provision.sh#L5 So the question is if this could be replaced with the way travis installs the dependencies: http://docs.travis-ci.com/user/apt/ If we get this working, we don't the vagrant part anymore. The best case would be if we could directly use our docker-compose setup. I still hope one of the CI/CD platforms is going to support this soon (@travis-ci @codeship ... ;-) ). |
|
||
setup: build | ||
docker-compose scale elasticsearch=3 | ||
docker-compose run elasticsearch chmod -R 777 /mount/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@im-denisenko I'm really unhappy about this line. Not the 777 but that I have to change the access rights after the images are running. What I need is a shared data container where all elasticsearch instances have write access. I tried to do it directly in the data image https://github.com/ruflin/Elastica/pull/901/files#diff-6ddc070b659c249d339814ba724740edR10 or also in the elasticsearch https://github.com/ruflin/Elastica/pull/901/files#diff-66fcadff5e0bc20f3becadf59df62f5bR28 But only the setup part works. I'm quite sure there is just a small command that can be used to get it working. Do you have any experience here?
@im-denisenko I further worked on this. It is finally working also on remote machines with the Snapshots. I tried it on Google Cloud Engine. It still needs some cleanup and documentation needs to be added. If you have some time, have a first look at it to have a second thought on it. |
…eckstyle commands
…al dev-image as it depends on ti.
…rk as default in docker environment
@im-denisenko I merged it myself as this does not change any of the library functionality itself. I will add more documentation later. |
BASEDIR = $(shell pwd) | ||
SOURCE = "${BASEDIR}/lib" | ||
IMAGE = "elastica" | ||
.PHONY: update clean build setup start stop destroy run checkstyle checkstyle-ci code-browser cpd messdetector messdetector-ci dependencies phpunit test tests doc lint syntax-check loc phploc gource |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A lot of targets, some of them just copypasted from ant config file that was deleted far ago :)
Which of them is actually used?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I actually planned to clean up the .PHONY part by putting it on top of every command. Like this no "old" commands should appear here. In general I tried to reduce the number of targets we have in the makefile, but there is still quite a list :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@im-denisenko Here is the pull request for the Makefile. Have a look and merge it if you think that helps: #917
@ruflin Sorry I didn't respond in time. I left question in the code. |
@ruflin Nope, it's actually here. Ansible escalates it's privileges to root when runned, we don't see it because of passwordless sudo in travis. I recently did a lot of ansible-related work on my job, now I know this tool even better, and I'm think I know that must be changed in order to use containers. Give me a couple of days... |
@im-denisenko About the sudo and containers: Let me know when you have an update. I know quite a bit about the containers but I'm not at all an expert with ansible :-) |
Use Docker as default in Makefile to simplify setup further. To not use Docker, the param DOCKER="" must be passed.