Remove beget/sprutio-nginx, add official nginx with custom config and certs #38

f-andrey · 2016-08-31T12:22:09Z

Так как beget/sprutio-nginx не содержит никаких изменений, но усложняет изменение конфига, при изменении конфига, сейчас надо перегенерировать контейнер, что не удобно.

Предлагаю вообще отказаться от него и вынести все конфиги на отдельный волум. Так можно легко изменять любые опции не изменяя контейнер.

TODO:

так как сейчас используется устаревший docker-compose не позволяющий делать явные зависимости, то после первого создания проекта, контейнер nginx надо перезапустить (так как он стартует ещё без сертификатов и крашится)
для того что бы данные изменения стали работоспособны надо обновить beget/sprutio-app (так как он содержит скрипт генерации ключей, который изменён)

ghost · 2016-08-31T12:25:34Z

docker-compose.yml

@@ -25,14 +25,14 @@ rpc:
    - "./rpc.env"

 nginx:
-  image: beget/sprutio-nginx
+  image: nginx:1.9


предлагаю использовать nginx:alpine, он меньше в размерах

ghost · 2016-08-31T12:27:58Z

Предлагаю вообще отказаться от него и вынести все конфиги на отдельный волум. Так можно легко изменять любые опции не изменяя контейнер.

в целом идея разумная, LGTM

ghost · 2016-08-31T12:35:17Z

docker build -t beget/sprutio-nginx -f Dockerfile.nginx ./"

также нужно убрать данную строчку (и строчку с push) из .travis.yml

f-andrey · 2016-08-31T12:49:12Z

Да, про тесты то я забыл, но он тут на автомате всё одно похоже успешно не пройдёт, так как в нём есть процедура авторизации.

ghost · 2016-08-31T12:51:47Z

Да, про тесты то я забыл, но он тут на автомате всё одно похоже успешно не пройдёт, так как в нём есть процедура авторизации.

этот момент я поправлю после принятия MR чтобы не ребейзить
да и это не совсем тесты, так как работают только для master ветки

что думаете по поводу использования nginx:alpine вместо nginx:latest?

f-andrey · 2016-08-31T12:59:01Z

Пока не настолько разбираюсь в теме, что бы сделать осознанный выбор, но пожалуй версию посвежее было бы не плохо. А так как остальные части, всё одно тянут базовые контейнеры не :alpine то думаю, использование :alpine тут, врядли даст какие то преимущества

ghost · 2016-08-31T13:15:41Z

Пока не настолько разбираюсь в теме, что бы сделать осознанный выбор, но пожалуй версию посвежее было бы не плохо. А так как остальные части, всё одно тянут базовые контейнеры не :alpine то думаю, использование :alpine тут, врядли даст какие то преимущества

ну значительной разницы конечно не будет, но чем меньше образ тем лучше)
из базовых образов только python:3.4, остальное можно заменить на :alpine версии

f-andrey · 2016-08-31T13:47:09Z

Ну в общем то я наверное скорее за :alpine

ghost · 2016-08-31T13:49:11Z

ну тогда осталось поменять тег в docker-compose, протестировать и слить коммиты

ghost · 2016-09-01T08:44:29Z

app/init-ssl.sh


 if [ -s sprutio.key -a -s sprutio.crt ]; then
    exit 0
 fi

 openssl req -nodes -newkey rsa:4096 -keyout sprutio.key -out sprutio.csr -subj "/C=RU/O=Beget/CN=sprut.io"
 openssl x509 -req -days 365 -in sprutio.csr -signkey sprutio.key -out sprutio.crt
+
+chmod 600 ../certs


здесь наверное должно быть 700 (иначе получается что права rw на директорию)

Да, что то по запарке не то натворил, пока пытался всё же сделать, что бы нгинкс стартовал с первого раза, пока понял, что он скорее всего просто запускается раньше пока сертификаты просто готовы.

ghost · 2016-09-01T08:50:13Z

так как сейчас используется устаревший docker-compose не позволяющий делать явные зависимости, то после первого создания проекта, контейнер nginx надо перезапустить (так как он стартует ещё без сертификатов и крашится)

на самом деле можно поступить по другому
sprutio-nginx оставить, но вместо добавления конфига в контейнер - генерировать там ssl сертификат при старте (если файлов нет)
конфиг также монтировать снаружи

и второй момент - в скрипте ./run.sh также нужно скачивать конфиг nginx из репозитория

ghost · 2016-09-01T08:51:52Z

кстати генерацию сертификатов можно вообще вызывать внутри ./dev.sh и ./run.sh

ghost · 2016-09-01T10:56:19Z

также я исправил билды для travis в #40
нужно сделать rebase относительно master
и лучше все коммиты из данного PR перетащить в другую ветку (не master), чтобы было удобнее стягивать обновления из master в этом репозитории в свой master

f-andrey · 2016-09-01T12:48:41Z

Не знаю, стоит ли из за такого мелкого функционала делать изменённый контейнер, мне понравился подход https://github.com/vmware/harbor/tree/master/Deploy они просто хранят все настройки проекта в отдельном конфиге, при инслалляции выполняют отдельный скрипт который готовит окружение, а потом уже запускают контейнеры в готовой среде.

Насчёт бранча, да пожалуй стоит, но текущее может стоит вмержить как есть, я вроде разрешил как то ручками, а дальше, да завести бранч, для изменений, если что то оптимизировать. Я просто не так много и часто работаю с публичными гитами, поэтому постоянно забываю о таких вещах.

ghost · 2016-09-01T13:17:13Z

Не знаю, стоит ли из за такого мелкого функционала делать изменённый контейнер, мне понравился подход https://github.com/vmware/harbor/tree/master/Deploy они просто хранят все настройки проекта в отдельном конфиге, при инслалляции выполняют отдельный скрипт который готовит окружение, а потом уже запускают контейнеры в готовой среде.

да, можно и не делать
в этом случае нужно вынести вызов init-ssl.sh в скрипты запуска dev.sh(run.sh) и в скрипт run.sh добавить скачивание конфигов nginx перед запуском контейнера

Насчёт бранча, да пожалуй стоит, но текущее может стоит вмержить как есть,

нет, в таком виде уже сложно будет смержить, так как тут мешанина из коммитов
нужно сделать следующее

добавить текущий репозиторий как remote
git remote add upstream https://github.com/LTD-Beget/sprutio
git remote update
переименовать текущий мастер
git branch -m master old-master
создать новый master из upstream
git checkout -b master upstream/master
создать отдельную ветку
git checkout -b nginx
перетащить коммиты в эту ветку
git log --author=andrey@bsdnir.info --format=%H old-master | while read line; do git cherry-pick $line; done
запушить ветку nginx в свой репозиторий и создать новый PR со ссылкой на текущий (а текущий закроем)

ну и потом обновить свой master на гитхабе на консистентное состояние
git checkout master
git push -fu origin master

… cers

f-andrey · 2016-09-01T14:39:05Z

Спасибо за подсказку, буду теперь знать как оно вручную это всё выковыривать, вроде довёл до сливабельного состояния.

ghost · 2016-09-01T15:17:02Z

так, отлично
осталось еще пара моментов

вынести init-ssl.sh из app в корень и добавить его вызов в скрипты dev.sh и run.sh (с необходимой адаптацией)
это решит проблему с первым запуском nginx и отсутствием сертификатов
добавить в run.sh скачивание конфигов nginx по аналогии с docker-compose.yml

ну и убрать прокидывание ssl сертификатов "./nginx/certs:/app/nginx/certs:rw" в сервис app

f-andrey mentioned this pull request Aug 31, 2016

Избавится от захардкоженного IPv4-only #39

Open

ghost reviewed Aug 31, 2016
View reviewed changes

ghost self-assigned this Aug 31, 2016

ghost reviewed Sep 1, 2016
View reviewed changes

f-andrey and others added 7 commits September 1, 2016 17:13

Remove beget/sprutio-nginx, add official nginx with custom config and…

cb7e08e

… cers

fix chmod sprutio.key

486534f

fix dir chmod

772bffb

Delete Dockerfile.nginx not need

b784a08

Fix travis.yml after remove sprutio-nginx

4402d77

Chenge nginx to nginx:mainline-alpine

a2a1152

Fix certs dir rights

6ba27ee

f-andrey force-pushed the master branch from 8882bbc to 6ba27ee Compare September 1, 2016 14:20

Remove beget/sprutio-nginx not need

d64cf96

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove beget/sprutio-nginx, add official nginx with custom config and certs #38

Remove beget/sprutio-nginx, add official nginx with custom config and certs #38

f-andrey commented Aug 31, 2016

ghost Aug 31, 2016

ghost commented Aug 31, 2016

ghost commented Aug 31, 2016

f-andrey commented Aug 31, 2016 •

edited

Loading

ghost commented Aug 31, 2016

f-andrey commented Aug 31, 2016

ghost commented Aug 31, 2016

f-andrey commented Aug 31, 2016

ghost commented Aug 31, 2016

ghost Sep 1, 2016

f-andrey Sep 1, 2016

ghost commented Sep 1, 2016

ghost commented Sep 1, 2016

ghost commented Sep 1, 2016

f-andrey commented Sep 1, 2016

ghost commented Sep 1, 2016

f-andrey commented Sep 1, 2016

ghost commented Sep 1, 2016

Remove beget/sprutio-nginx, add official nginx with custom config and certs #38

Are you sure you want to change the base?

Remove beget/sprutio-nginx, add official nginx with custom config and certs #38

Conversation

f-andrey commented Aug 31, 2016

ghost Aug 31, 2016

Choose a reason for hiding this comment

ghost commented Aug 31, 2016

ghost commented Aug 31, 2016

f-andrey commented Aug 31, 2016 • edited Loading

ghost commented Aug 31, 2016

f-andrey commented Aug 31, 2016

ghost commented Aug 31, 2016

f-andrey commented Aug 31, 2016

ghost commented Aug 31, 2016

ghost Sep 1, 2016

Choose a reason for hiding this comment

f-andrey Sep 1, 2016

Choose a reason for hiding this comment

ghost commented Sep 1, 2016

ghost commented Sep 1, 2016

ghost commented Sep 1, 2016

f-andrey commented Sep 1, 2016

ghost commented Sep 1, 2016

f-andrey commented Sep 1, 2016

ghost commented Sep 1, 2016

f-andrey commented Aug 31, 2016 •

edited

Loading