-
Notifications
You must be signed in to change notification settings - Fork 782
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
Fixed the actual download address of the packages. #223
base: master
Are you sure you want to change the base?
Fixed the actual download address of the packages. #223
Conversation
Signed-off-by: Jintao Zhang <[email protected]>
@thaJeztah PATL, thanks! Before the modification, only the mirror was used to download the yum repo file, but the download of the rpm package was still through download.docker.com. |
if [ -n "$mirror" ]; then | ||
case "$mirror" in | ||
Aliyun) | ||
sed -i "s#download.docker.com#mirrors.aliyun.com/docker-ce#g" /etc/yum.repos.d/docker-ce.repo | ||
;; | ||
AzureChinaCloud) | ||
sed -i "s#download.docker.com#mirror.azure.cn/docker-ce#g" /etc/yum.repos.d/docker-ce.repo | ||
;; | ||
esac | ||
fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like the indentation is using spaces instead of tabs here.
Also wondering if we should repeat the case
here, or if we should (e.g.) add a RPM_REPO_URL
variable to the existing code higher up;
Lines 63 to 70 in 6580690
case "$mirror" in | |
Aliyun) | |
DOWNLOAD_URL="https://mirrors.aliyun.com/docker-ce" | |
;; | |
AzureChinaCloud) | |
DOWNLOAD_URL="https://mirror.azure.cn/docker-ce" | |
;; | |
esac |
DOWNLOAD_URL
to always update the docker-ce.repo
URL?
And if that variable is set to do the replace.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
More in general though, I'm a bit unsure if we should keep these mirror options; are they still useful? I recall that support for our CDN's has improved behind the GFW, so perhaps it's no longer needed for "most" situations.
Looking at the mirrors, is the aliyun mirror still operational?
- https://mirrors.aliyun.com/docker-ce redirects to http://pub.mirrors.aliyun.com/docker-ce/ (does not resolve for me)
- https://mirror.azure.cn/docker-ce (looks to be working still)
There's also the rootless-install script; which has the URLs hard-coded; https://github.com/docker/docker-install/blob/6580690119f66d3e31b8bc84e7ac9f9e3d68559b/rootless-install.sh
Which adds a bit to my concern if adding extra complexity is worth it, or if we should remove it altogether.
We already have the DOWNLOAD_URL
variable (and/or RPM_REPO_URL
); if we added the replace for docker-ce.repo
, we could remove the switch for these specific mirrors
Will have to check with others what's the desired approach
Thank you very much. At present, CDN can work normally in some unrestricted areas, http://pub.mirrors.aliyun.com/docker-ce/ Services are no longer available |
fix #213