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

fix: use fully qualified image names #148

Merged
merged 1 commit into from
Jul 12, 2023

Conversation

ctron
Copy link
Contributor

@ctron ctron commented Jul 11, 2023

What type of PR is this?

  • bug
  • feature
  • enhancement

What problem(s) does this PR solve?

Issue(s) number:

  • Deployment might pull unintended images
  • Deployment gets stuck on systems asking for confirmation of non-qualified images

Description:

Using an unqualified image name might yield different results on different systems. The deployment actually relies on docker's default, which is 'docker.io'. Other system might not have this default, and some systems might even pause and interactively ask the user for a decision.

How do you solve it?

Providing a fully qualified image name solves this issues and ensures that the intended image is pulled.

Special notes for your reviewer, ex. impact of this fix, design document, etc:

Using an unqualified image name might yield different results on
different systems. The deployment actually relies on docker's default,
which is 'docker.io'. Other system might not have this default, and
some systems might even pause and interactively ask the user for a
decision.

Providing a fully qualified image name solves this issues and ensures
that the intended image is pulled.
@CLAassistant
Copy link

CLAassistant commented Jul 11, 2023

CLA assistant check
All committers have signed the CLA.

@wey-gu
Copy link
Contributor

wey-gu commented Jul 11, 2023

Dear @ctron ,

Thanks a lot for the contribution, indeed this(no fqdn approach) is implicit.

In case we are in env w/o proper internet access, thus using docker registry mirrors, will this fqdn approach bypass the mirror due to such explicit image names, please?

Thanks.

BR//Wey

@wey-gu wey-gu mentioned this pull request Jul 11, 2023
3 tasks
@ctron
Copy link
Contributor Author

ctron commented Jul 11, 2023

I think in a case without internet access, using a proxy is the right approach: https://docs.docker.com/config/daemon/systemd/#httphttps-proxy

I tried to some reasonable explanation on what registry-mirrors actually does. But couldn't find any, other some some vague statements.

I did find some documentation from Google on this: https://cloud.google.com/container-registry/docs/pulling-cached-images … it reads like the mirror can just mirror anything. So expectation would be, that it also can pull docker.io images.

@wey-gu
Copy link
Contributor

wey-gu commented Jul 11, 2023

Thanks a lot, @ctron !

I'll also double-check/reproduce that mirror will go through these fqdn images, too, as in China, the traffic to the docker hub is impacted, thus many of us will use a docker hub mirror to enable the image pulling, thus it's not the same case as w/o internet(to use a proxy). In case that is the case, we could consider putting the default compose file as you changed and create another one in non-fqdn fashion.

Thanks!

Copy link

@Nicole00 Nicole00 left a comment

Choose a reason for hiding this comment

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

LGTM

@Nicole00 Nicole00 merged commit 7b916e6 into vesoft-inc:master Jul 12, 2023
@ctron ctron deleted the feature/fqn_1 branch July 12, 2023 06:36
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.

4 participants