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

考虑支持一下docker #113

Merged
merged 2 commits into from
Jun 12, 2018
Merged

考虑支持一下docker #113

merged 2 commits into from
Jun 12, 2018

Conversation

fduxiao
Copy link

@fduxiao fduxiao commented Jun 9, 2018

可以更方便地部署(权限管理,配置管理,scale up,daemonize等)

@huangyoukun
Copy link
Collaborator

提交注释不符合规范,可以参考如下:
chore(docker): add dockerfile

@codecov-io
Copy link

codecov-io commented Jun 11, 2018

Codecov Report

Merging #113 into master will increase coverage by 0.03%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #113      +/-   ##
==========================================
+ Coverage   32.33%   32.36%   +0.03%     
==========================================
  Files          39       39              
  Lines        2069     2070       +1     
==========================================
+ Hits          669      670       +1     
  Misses       1400     1400
Impacted Files Coverage Δ
bin/tsw/plug.js 94.11% <0%> (ø) ⬆️
bin/lib/api/libdcapi/dcapi.js
bin/lib/util/http.more.js
bin/lib/default/config.default.js
bin/lib/api/keyman/index.js
bin/lib/api/L5/L5.api.js
bin/lib/data/cmem.tsw.js
bin/lib/util/isTST.js
bin/lib/api/tnm2/index.js
bin/lib/util/gzipHttp.js
... and 9 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e0bde17...e7ef9cc. Read the comment docs.

@huangyoukun
Copy link
Collaborator

huangyoukun commented Jun 11, 2018

@fduxiao 启动方式有一点需要讨论下

docker应该算是linux+生产|测试 ,使用node --inspect index.js启动不太合适。

生产环境启动方式,可以更新为 bin/proxy/startup.sh

如果只是开发环境,如上启动是没问题的,可以忽略,回复确认下就可以合并了

@fduxiao
Copy link
Author

fduxiao commented Jun 11, 2018

@huangyoukun 另外我现在docker里代码是直接从git仓里来的,做生产环境的话,确实我见到过有的仓是这样做的(保证稳定),但是有时候把当前目录加进去也是常有的(ADD . /TSW),你建议使用哪种方法?
(新提交里我还是改成里ADD . /TSW,因为你那个bin/proxy/startup.sh没有+x权限,所以干脆用当前代码来build,并且给startup.sh也加了权限,以及我发现alpine有很多依赖项要加进去,现在还在手动解依赖,稍晚点再提交上来)

@fduxiao
Copy link
Author

fduxiao commented Jun 11, 2018

以及,现在我在markdown里面把tag写成了tsw,但其实如果Tencent在docker hub上有账号的话,可以用Tencent/tsw作为tag,并且设置自动编译,这样会更加方便一些?

@huangyoukun
Copy link
Collaborator

@fduxiao master会保持稳定的,master其实是从Tecnent生产环境验证后合并进去的,脚本未有x权限我加一下。

@huangyoukun
Copy link
Collaborator

huangyoukun commented Jun 11, 2018

+x 提交了 #114

huangyoukun added a commit that referenced this pull request Jun 11, 2018
chore(chmod): chmod executable for shell #113
@fduxiao
Copy link
Author

fduxiao commented Jun 11, 2018

docker的启动是把entrypoint做可执行文件,把CMD当作参数(包括docker run直接给定的参数)创建进程的,一个docker container到entrypoint退出即停止。

因此,现在的start.sh脚本造成的daemonize会使start.sh退出后,entrypoint退出,因此container也退出了,

(当然,可以在保持entrypoint运行的时候fork多个进程,gitlab的docker镜像就是这样的形式)

所以建议给出一个不daemonize的启动脚本
(一般也不建议使用&作为daemonize方式,按现在这种写法,在ssh里zsh的退出会自动杀掉&产生的进程,除非是用nohup之类的做过完整daemonize流程的功能)

我现在加了一个简单的环境变量来控制是否添加(请即刻merge到master上,否则影响docker里的代码),眼下这样对整体代码的影响最小,后续如何处理可能需要大规模讨论一下?

@huangyoukun huangyoukun merged commit 06fa5b4 into Tencent:master Jun 12, 2018
@fduxiao fduxiao deleted the dockerfile branch June 13, 2018 07:31
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.

3 participants