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

Makefile update #901

Merged
merged 21 commits into from
Aug 9, 2015
Merged

Makefile update #901

merged 21 commits into from
Aug 9, 2015

Conversation

ruflin
Copy link
Owner

@ruflin ruflin commented Jul 28, 2015

Use Docker as default in Makefile to simplify setup further. To not use Docker, the param DOCKER="" must be passed.

@ruflin
Copy link
Owner Author

ruflin commented Jul 28, 2015

@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?

@im-denisenko
Copy link
Contributor

@ruflin I'm ok with it. Should we drop vagrant then to not confuse which one tool is actually used?

@ruflin
Copy link
Owner Author

ruflin commented Jul 29, 2015

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/
Copy link
Owner Author

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?

@ruflin
Copy link
Owner Author

ruflin commented Aug 6, 2015

@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.

ruflin added a commit that referenced this pull request Aug 9, 2015
@ruflin ruflin merged commit 50de12b into master Aug 9, 2015
@ruflin
Copy link
Owner Author

ruflin commented Aug 9, 2015

@im-denisenko I merged it myself as this does not change any of the library functionality itself. I will add more documentation later.

@ruflin ruflin deleted the makefile-update branch August 10, 2015 06:25
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
Copy link
Contributor

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?

Copy link
Owner Author

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 :-)

Copy link
Owner Author

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

@im-denisenko
Copy link
Contributor

@ruflin Sorry I didn't respond in time. I left question in the code.

@im-denisenko
Copy link
Contributor

Our main usage of sudo is here

@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...

@ruflin
Copy link
Owner Author

ruflin commented Aug 16, 2015

@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 :-)

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 this pull request may close these issues.

2 participants