Refactor nodejs static building

duker33 commented 6 years ago

Обязательна для #269

Задачу создал из этого коммента https://github.com/fidals/shopelectro/pull/246#discussion_r158480687

Сейчас у нас проблема в том, что make static зависит от make build. Это создаёт сложности. Да и к билду статики через ноду давненько есть вопросы (у Дюка точно). Пересмотри билд статики и сделай так, чтобы make build среди прочего билдил и статику.

Итого критерий завершения задачи: make build билдит статику и код архитектурно хорош

duker33 commented 6 years ago

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

duker33 commented 6 years ago

@dveselov , @artemiy312 , отвечаю на коммент Артемия:

контейнеру se-nodejs хорошо бы делать свой npm install при билде образа, а не при запуске контейнера. pip install, например, происходит у нас именно при билде образа
а вот gulp build хорошо бы оставить на старт контейнера. Получается, долгий npm install спрятали в образ, который пушится/пуллится. А быстрый gulp build делаем каждый раз, когда меняем js-код или стили

Если мы реализуем предложение выше, получится такая картина:

Билд образа для se-nodejs будет зависеть только от packages.json. Т.е. только от кода SE, больше ни от чего
dc run nodejs gulp build будет только и делать, что билдить статику гулпом. Разрабатывать станет удобнее

Получатся примерно такие рецепты:

make build:
  dc build --no-cache nodejs  # just if packages.json changed
  dc build --no-cache se-py  # fetch <DEPS>/refarm-site/front/ dir
  dc nodejs gulp build  # build front static to folder `<SRC>/front/build`. Dir `<DEPS>/refarm-site/front/build` is used as source for build too.
  dcp build se-src  # copy all site sources inside se-src image. `<SRC>/front/build` directory is copied too
  dcp build se-py  # rebase prod se-py on it's dev version
  dc build  # build other services

make push:
  dcp push se-src  # contains front/build static
  dcp build se-py  # contain pip installed deps. refarm-site too

ArtemijRodionov commented 6 years ago

Да, все верно, но есть пару замечаний по комментам:

dcp build se-src # copy <SRC>/front/build inside se-src image dcp push se-src # contains front/build static

se-src содержит в себе не только статику, но и исходный код SE, чтобы наши образы были ни от чего независимы.

duker33 commented 6 years ago

@artemiy312 , @dveselov , пофиксил комменты. Но сами формулировки нужно будет ещё доработать. Это уж при реализации задачи

ivnglkv commented 6 years ago

@duker33 Во время изучения задачи и предполагаемого решения возник следующий вопрос: dc build --no-cache se-py # fetch <DEPS>/refarm-site/front/ dir Не могу понять, как на этапе билда скачать исходники refarm-site в том [volume] se-python DEPS_DIR. Если в python/dev/Dockerfile прописать wget refarm-site.zip, то при запуске gulp build каталог /usr/app/deps из se-python будет недоступен в se-nodejs из-за того, что том DEPS_DIR ляжет сверху этого каталога, если я все верно затестил. Можно при билде se-nodejs их скачивать, но пока не могу оценить, насколько это решение будет верным.

Извиняюсь за путаное объяснение, скорее всего оно такое потому, что в голове ещё всё не до конца разложилось по полочкам.

duker33 commented 6 years ago

@ivnglkv , да, это у нас ошибка в коде. Добавь плз аналогичную строчку в коде SE https://github.com/fidals/stroyprombeton/blob/master/docker/docker-compose.yml#L10 Разумеется, в файле на локальном компе .env переменная REFARM_SITE корректно заполнена.

Потом перезапусти. При перезапуске снеси se-source, а то новый volume не смонтируется:

dc rm se-source

И при следующем старте se-python в /usr/app будут правильные зависимости

ivnglkv commented 6 years ago

@duker33 ок. Я просто подумал, что идея все-таки в том, чтобы не требовать наличия кода refarm-site на компе разработчика/сервере

ArtemijRodionov commented 6 years ago

@ivnglkv наличие кода refarm-site на компе необязательно, т.к. он должен подтягиваться с помощью pip. Единсвенная причина, для чего может понадобиться монтировать refarm-site в контейнер, чтобы протестить его изменения

ivnglkv commented 6 years ago

@artemiy312, по-моему, немного не так

наличие кода refarm-site на компе необязательно, т.к. он должен подтягиваться с помощью pip

Он устанавливается с помощью pip в /usr/local/..., или что-то вроде того. А gulp build ищет файлы в /usr/app/deps/[ecommerce, general_admin]/. А этот /usr/app/deps в текущем состоянии репозитория ничем не заполняется, если я все правильно прочитал. Поэтому и спросил.

А так — да, мне кажется, что так и должно быть, чтобы не нужно было скачивать самому refarm-site и монтировать его. Да и задумывалось так, наверное.

ArtemijRodionov commented 6 years ago

@ivnglkv

se-nodejs:
    build:
      context: ..
      dockerfile: docker/node/Dockerfile
    container_name: se-nodejs
    command: bash -c 'gulp watch || (npm install && npm install -g gulp-cli && gulp build && gulp watch)'
    volumes_from:
      - se-source
      - se-python
    environment:
- DEPS_DIR=$DEPS_DIR

se-python с установленными зависимостями (в том числе refarm-site) монтируется (volume_from) в se-nodejs. Чтобы se-nodejs искал в нужном месте нужно переопределить $DEPS_DIR в .env . Это недочет нашего .env.dist

ivnglkv commented 6 years ago

Мм, ты имеешь в виду, что директива volumes_from монтирует не просто тома, указанные в volumes, а вообще весь контейнер?

Я просто понял после чтения доков, что если написано

se-source:
  volumes:
    - ./../:$SRC_DIR # code volume
    - /opt/media/shopelectro/:$SRC_DIR/media/

se-python:
  volumes:
    - $DEPS_DIR
  volumes-from:
    - se-source

se-nodejs:
  volumes-from:
    - se-source
    - se-python

то в se-nodejs окажутся только

./../:$SRC_DIR                                 # from se-source
/opt/media/shopelectro/:$SRC_DIR/media/        # from se-source
$DEPS_DIR                                      # from se-python

Неправильно прочитал?

duker33 commented 6 years ago

@ivnglkv , ты всё правильно понял из доков. Во всяком случае, я их понимаю также =)

Можно попробовать в DEPS_DIR указать /usr/local/site-packages/python3.6/ (или как-то так), мб так прокатит. В образе fidals/se:prod (секция se-python из docker-compose-production.yml) в DEPS_DIR=/usr/app/deps зависимости переносит pip из CI (линк на код CI)

Всё равно рекомендую юзать через REFARM_SRC на локали, т.к. может настать момент, когда тебе нужно будет поработать с кодом refarm_site и посмотреть на примере SE что получилось. Тогда и понадобится REFARM_SRC.

Вообще система сборки сейчас мне видится не самой лучшей на свете, да. Всё путано, вязано-перевязано с CI, прод слишком сильно отличается от дева и всё такое

ivnglkv commented 6 years ago

К сожалению, во время выполнения команды docker-compose build тома не подключаются. А когда запустишь контейнер, тома подключатся. Таким образом, если что-то записывал во время билда в директорию, куда подключается том, то этого не увидишь в запущенном контейнере.

За линк на команду из CI спасибо, буду иметь в виду, что такое есть.

Ну да, такой юзкейс вполне понятен :)

ArtemijRodionov commented 6 years ago

@ivnglkv Ты все верно понял. Я хотел сказать, что тебе нужно переопределить переменную DEPS_DIR на путь к зависимостям отrefarm-site внутри se-python

duker33 commented 6 years ago

@ivnglkv , кажется, я погорячился с этой задачей. Го мы её у тебя заберём, выдам другие. Или ты готов за неё взяться?

Решай

duker33 commented 6 years ago

План решения:

запускаем ~npm install && gulp build при билде образа. Сейчас запускаем при старте контейнера
код refarm-site забираем по path из переменной REFARM_SRC

Основная проблема с refarm-site в предположении где лежит код на хост-машине. Это предположение - источник страданий. Сейчас drone ~бессовестно~ копирует его в /usr/app/deps из python site-packages.

Думаю, мы будем просто скачивать последнюю версию refarm-site/packages.json прямо с гит-репы в node/Dockerfile

ivnglkv commented 6 years ago

@duker33, не, я возьму. Частично уже сделал, просто не коммитил пока ничего

fidals / shopelectro

Refactor nodejs static building #248