Skip to content

Add Aliyun mirror support to hack/install.sh#30907

Merged
vdemeester merged 1 commit intomoby:masterfrom
Hyzhou:master
Apr 3, 2017
Merged

Add Aliyun mirror support to hack/install.sh#30907
vdemeester merged 1 commit intomoby:masterfrom
Hyzhou:master

Conversation

@Hyzhou
Copy link
Copy Markdown
Contributor

@Hyzhou Hyzhou commented Feb 10, 2017

Signed-off-by: hyzhou.zhy hyzhou.zhy@alibaba-inc.com

Add Aliyun mirror support to hack/install.sh
I received an email today, The https transformation for Aliyun mirror is complete. @friism @thaJeztah

we will just run this:

curl -sSL https://get.docker.com/ | sh -s -- --mirror Aliyun

Tested in Aliyun ECS:Ubuntu 16.04、CentOS7.2

- A picture of a cute animal (not mandatory but encouraged)

Signed-off-by: hyzhou.zhy <hyzhou.zhy@alibaba-inc.com>
@Hyzhou
Copy link
Copy Markdown
Contributor Author

Hyzhou commented Feb 27, 2017

@thaJeztah What about this pull request now? :)

@thaJeztah
Copy link
Copy Markdown
Member

Sorry for the delay, @Hyzhou, let me try to ping @dmp42 @andrewhsu again - PTAL

@unclejack
Copy link
Copy Markdown
Contributor

Is there a reason why this has to be modified for every provider? Perhaps there should be a way to specify the mirror as a string which represents the URL where the mirror can be found?

I have some use cases for using a local mirror. It wouldn't always be the same mirror. Even without my particular use case, it'd be nice to not have to add a special case for every single mirror. I can imagine others might want to use their own mirror as well.

@thaJeztah
Copy link
Copy Markdown
Member

@unclejack if you want to use your own mirror, it's probably best to write a custom script; I don't think we should allow setting any arbitrary mirror in the install script

@unclejack
Copy link
Copy Markdown
Contributor

@thaJeztah I understand that. However, I can't say adding mirrors one by one is better either.

@dmp42
Copy link
Copy Markdown
Contributor

dmp42 commented Mar 2, 2017

@unclejack China is special. I don't think we should add more in there.

@thaJeztah I think this is fine and we should merge this, unless there is an issue?

cc @friism

Copy link
Copy Markdown
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, let's go ahead; sorry for the delay 😊

LGTM

Copy link
Copy Markdown
Member

@vdemeester vdemeester left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👼

@vdemeester vdemeester merged commit 9f17b97 into moby:master Apr 3, 2017
@GordonTheTurtle GordonTheTurtle added this to the 17.05.0 milestone Apr 3, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants