docs: add --node when starting dfclient container in quick_start docs #645

yeya24 · 2019-07-01T09:35:42Z

Signed-off-by: yeya24 [email protected]

Ⅰ. Describe what this PR did

Specify --node to make sure that dfget can access supernode container.

Ⅱ. Does this pull request fix one issue?

fix #643

Ⅲ. Why don't you add test cases (unit test/integration test)? (你真的觉得不需要加测试吗？)

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

codecov-io · 2019-07-01T10:36:11Z

Codecov Report

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

@@            Coverage Diff             @@
##           master     #645      +/-   ##
==========================================
+ Coverage   46.58%   48.24%   +1.66%     
==========================================
  Files         106       99       -7     
  Lines        6187     5889     -298     
==========================================
- Hits         2882     2841      -41     
+ Misses       3072     2824     -248     
+ Partials      233      224       -9

Impacted Files	Coverage Δ
cmd/dfget/app/root.go	`62.09% <0%> (-0.18%)`	⬇️
dfget/config/config.go	`91.66% <0%> (-0.1%)`	⬇️
supernode/server/result_info.go
supernode/server/handler.go
supernode/server/router.go
supernode/server/system_bridge.go
supernode/server/0.3_bridge.go
supernode/server/peer_bridge.go
supernode/server/server.go
supernode/daemon/mgr/scheduler/manager.go	`25.56% <0%> (+0.75%)`	⬆️

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 c9728c1...6f76e4a. Read the comment docs.

lowzj · 2019-07-04T12:14:22Z

docs/quick_start/README.md

@@ -20,10 +20,13 @@ docker run -d --name supernode --restart=always -p 8001:8001 -p 8002:8002 \
 ## Step 2：Deploy Dragonfly Client (dfclient)

 ```bash
-docker run -d --name dfclient -p 65001:65001 dragonflyoss/dfclient:0.4.0 --registry https://index.docker.io
+docker run -d --name dfclient -p 65001:65001 dragonflyoss/dfclient:0.4.0 --registry https://index.docker.io --node `docker inspect supernode -f '{{.NetworkSettings.Networks.bridge.IPAddress}}'`


I don't think this is the best way to make dfclient to access the supernode deployed by docker in quick-start document. It will fail if you deploy them on macOS.
The root cause why dfclient:0.4.0 cannot access supernode:0.4.0 is the wrong command line parameter and wrong Dockerfile.supernode:

In version 0.4.0, we use supernode written by golang, and the correct parameter is --adverties-ip, not -Dsupernode.advertiseIP.

The supernode image builded by Dockerfile.supernode cannot pass parameters to supernode process.

Thanks for pointing this. So how to fix this issue? Do we have to change the Dockerfile first?

Has been fixed by #663.

I think it may be easier to understand for user:

SUPERNODE_IP=`docker inspect supernode -f '{{.NetworkSettings.Networks.bridge.IPAddress}}'` docker run -d --name dfclient -p 65001:65001 dragonflyoss/dfclient:0.4.0 --registry https://index.docker.io --node ${SUPERNODE_IP}

yeya24 · 2019-07-08T08:15:46Z

@lowzj Hi, I have tested this in my mac. And it works well. Do you have time to test it in your local environment?

starnop · 2019-07-09T06:30:45Z

The PR #663 has been merged. PTAL.

starnop · 2019-07-09T06:35:55Z

docs/quick_start/README.md

@@ -45,6 +50,16 @@ We need to modify the Docker Daemon configuration to use the Dragonfly as a pull
 systemctl restart docker
 ```

+For macOS users:


The doc of quick_start should be as concise as possible. We treat Linux as default and maybe you can add an external link for MacOS or something else. WDYT?

Yes, I will remove the part of macos.

lowzj

Could you rebase your code and force push again to fix the CI problem?

lowzj · 2019-07-09T08:50:54Z

docs/quick_start/README.md

@@ -20,10 +20,13 @@ docker run -d --name supernode --restart=always -p 8001:8001 -p 8002:8002 \
 ## Step 2：Deploy Dragonfly Client (dfclient)

 ```bash
-docker run -d --name dfclient -p 65001:65001 dragonflyoss/dfclient:0.4.0 --registry https://index.docker.io
+docker run -d --name dfclient -p 65001:65001 dragonflyoss/dfclient:0.4.0 --registry https://index.docker.io --node `docker inspect supernode -f '{{.NetworkSettings.Networks.bridge.IPAddress}}'`


I think it may be easier to understand for user:

SUPERNODE_IP=`docker inspect supernode -f '{{.NetworkSettings.Networks.bridge.IPAddress}}'` docker run -d --name dfclient -p 65001:65001 dragonflyoss/dfclient:0.4.0 --registry https://index.docker.io --node ${SUPERNODE_IP}

yeya24 · 2019-07-09T09:01:49Z

I have updated this PR. PTAL. @lowzj @starnop

lowzj · 2019-07-09T09:24:51Z

docs/quick_start/README.md


 ## Step 1: Deploy Dragonfly Server (SuperNode)

 ```bash
 docker run -d --name supernode --restart=always -p 8001:8001 -p 8002:8002 \
-    dragonflyoss/supernode:0.4.0 -Dsupernode.advertiseIp=127.0.0.1
+    dragonflyoss/supernode:0.4.1 -D --advertise-ip $(host-ip)


remove -D

$(host-ip) is not clear for use, it cannot be executed in command line directly.

If we use image 0.4.1, we can remove this option --advertise-ip, add --node to dfclient and remind user to use --advertise-ip if the dfclient cannot connect to supernode.

Done. I have remove these two parameters.

starnop · 2019-07-11T02:15:44Z

docs/quick_start/README.md

-> **NOTE**: `supernode.advertiseIp` should be the ip that clients can connect to, `127.0.0.1` here is an example for testing, and it can only be used if the server and client are in the same machine.
+**NOTE**:
+
+- If dfclient cannot connect to supernode directly, you can specify parameter `--advertise-ip` when supernode starts. This ip address must be accessible from dfclient.


Maybe it's better to point out that the IP used by supernode will be the first non-loop address if not specified.

Signed-off-by: yeya24 <[email protected]>

starnop · 2019-07-11T03:18:54Z

LGTM.

docs: add --node when starting dfclient container in quick_start docs

Signed-off-by: Gaius <[email protected]>

pouchrobot added the size/XS label Jul 1, 2019

yeya24 force-pushed the doc/fix-quickstart branch from 9f06e08 to 6f4fa73 Compare July 1, 2019 10:53

allencloud assigned lowzj Jul 1, 2019

yeya24 changed the title ~~add --node when starting dfclient container in quick_start docs~~ chore: add --node when starting dfclient container in quick_start docs Jul 2, 2019

lowzj reviewed Jul 4, 2019

View reviewed changes

yeya24 force-pushed the doc/fix-quickstart branch from 6f4fa73 to 0870085 Compare July 8, 2019 08:17

pouchrobot added size/S and removed size/XS labels Jul 8, 2019

starnop reviewed Jul 9, 2019

View reviewed changes

lowzj reviewed Jul 9, 2019

View reviewed changes

yeya24 force-pushed the doc/fix-quickstart branch from c402aa8 to c5a2032 Compare July 9, 2019 08:56

lowzj changed the title ~~chore: add --node when starting dfclient container in quick_start docs~~ docs: add --node when starting dfclient container in quick_start docs Jul 9, 2019

pouchrobot added the areas/docs label Jul 9, 2019

yeya24 force-pushed the doc/fix-quickstart branch from c5a2032 to c33d257 Compare July 9, 2019 08:58

lowzj reviewed Jul 9, 2019

View reviewed changes

yeya24 force-pushed the doc/fix-quickstart branch from c33d257 to 437cafb Compare July 9, 2019 09:39

starnop reviewed Jul 11, 2019

View reviewed changes

yeya24 force-pushed the doc/fix-quickstart branch from 437cafb to 38ace01 Compare July 11, 2019 02:49

add --node when starting dfclient container

6f76e4a

Signed-off-by: yeya24 <[email protected]>

yeya24 force-pushed the doc/fix-quickstart branch from 38ace01 to 6f76e4a Compare July 11, 2019 02:52

starnop merged commit df23e54 into dragonflyoss:master Jul 11, 2019

pouchrobot mentioned this pull request Jul 12, 2019

WeeklyReport of Dragonfly from 2019-07-05 to 2019-07-12 #672

Closed

yeya24 deleted the doc/fix-quickstart branch September 4, 2019 00:49

starnop added a commit to starnop/Dragonfly that referenced this pull request Nov 27, 2019

Merge pull request dragonflyoss#645 from yeya24/doc/fix-quickstart

1e404d8

docs: add --node when starting dfclient container in quick_start docs

inoc603 pushed a commit to inoc603/Dragonfly that referenced this pull request Dec 23, 2019

Merge pull request dragonflyoss#645 from yeya24/doc/fix-quickstart

0519a79

docs: add --node when starting dfclient container in quick_start docs

sungjunyoung pushed a commit to sungjunyoung/Dragonfly that referenced this pull request May 8, 2022

chore: add lucy-cl maintainer (dragonflyoss#645)

e8f7d69

Signed-off-by: Gaius <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

docs: add --node when starting dfclient container in quick_start docs #645

docs: add --node when starting dfclient container in quick_start docs #645

yeya24 commented Jul 1, 2019

codecov-io commented Jul 1, 2019 •

edited

Loading

lowzj Jul 4, 2019

yeya24 Jul 4, 2019

starnop Jul 9, 2019

lowzj Jul 9, 2019

yeya24 commented Jul 8, 2019

starnop commented Jul 9, 2019

starnop Jul 9, 2019

yeya24 Jul 9, 2019

lowzj left a comment

lowzj Jul 9, 2019

yeya24 commented Jul 9, 2019

lowzj Jul 9, 2019

yeya24 Jul 9, 2019

starnop Jul 11, 2019

starnop commented Jul 11, 2019

docs: add --node when starting dfclient container in quick_start docs #645

docs: add --node when starting dfclient container in quick_start docs #645

Conversation

yeya24 commented Jul 1, 2019

Ⅰ. Describe what this PR did

Ⅱ. Does this pull request fix one issue?

Ⅲ. Why don't you add test cases (unit test/integration test)? (你真的觉得不需要加测试吗？)

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

codecov-io commented Jul 1, 2019 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yeya24 commented Jul 8, 2019

starnop commented Jul 9, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lowzj left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yeya24 commented Jul 9, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

starnop commented Jul 11, 2019

codecov-io commented Jul 1, 2019 •

edited

Loading