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

docs: add registry (quay.io/) for pre-loading images for kind #18017

Merged
merged 1 commit into from Nov 29, 2021

Conversation

adamzhoul
Copy link
Contributor

@adamzhoul adamzhoul commented Nov 26, 2021

Please ensure your pull request adheres to the following guidelines:

  • For first time contributors, read Submitting a pull request
  • All code is covered by unit and/or runtime tests where feasible.
  • All commits contain a well written commit description including a title,
    description and a Fixes: #XXX line if the commit addresses a particular
    GitHub issue.
  • All commits are signed off. See the section Developer’s Certificate of Origin
  • Provide a title or release-note blurb suitable for the release notes.
  • Thanks for contributing!

in doc, it recommends docker pull image, but the command is :

docker pull cilium/cilium:|IMAGE_TAG|

this will download from docker.io
However, in operator, it loads images from quay.io
we should keep them the same, otherwise, we download for nothing.

Signed-off-by: adamzhoul adamzhoul186@gmail.com

@adamzhoul adamzhoul requested a review from a team as a code owner November 26, 2021 03:34
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Nov 26, 2021
@qmonnet
Copy link
Member

qmonnet commented Nov 26, 2021

Hi and thanks!
Can you please explain why this is needed? I would expect Docker to be able to fetch the image even without the explicit domain (although defaulting to Docker hub)?

@qmonnet qmonnet added the release-note/misc This PR makes changes that have no direct user impact. label Nov 26, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot removed the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Nov 26, 2021
@qmonnet
Copy link
Member

qmonnet commented Nov 26, 2021

Apologies, I had missed your explanation in the second part of your PR description. Makes sense, in that case.
Would you mind repeating the motivation for the change in the commit description, please?

@qmonnet qmonnet changed the title add domain to img in kind doc docs: add registry (quay.io/) for pre-loading images for kind Nov 26, 2021
@adamzhoul
Copy link
Contributor Author

done

@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.11.0 Nov 26, 2021
@qmonnet qmonnet added the area/documentation Impacts the documentation, including textual changes, sphinx, or other doc generation code. label Nov 26, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.10.6 Nov 26, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot added this to Needs backport from master in 1.9.12 Nov 26, 2021
@qmonnet
Copy link
Member

qmonnet commented Nov 26, 2021

One last thing, could you fix your Signed-off-by: tag in the commit description please? Checkpatch does not recognise it because of the missing angle brackets around the email address (name <email>).

in doc, it recommends docker pull image, but the command is :
  docker pull cilium/cilium:|IMAGE_TAG|

this will download from docker.io
However, in operator, it loads images from quay.io
we should keep them the same, otherwise, we download for nothing.

Signed-off-by: adamzhoul <adamzhoul186@gmail.com>
@adamzhoul
Copy link
Contributor Author

sorry for the trouble.
added now.

@qmonnet
Copy link
Member

qmonnet commented Nov 29, 2021

No need to apologies, thanks again for your contribution!

@qmonnet qmonnet merged commit 4758bef into cilium:master Nov 29, 2021
@qmonnet qmonnet mentioned this pull request Nov 30, 2021
5 tasks
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from master to Backport pending to v1.10 in 1.10.6 Nov 30, 2021
@qmonnet qmonnet mentioned this pull request Nov 30, 2021
3 tasks
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from master to Backport pending to v1.9 in 1.9.12 Nov 30, 2021
@qmonnet qmonnet mentioned this pull request Nov 30, 2021
18 tasks
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Needs backport from master to Backport pending to v1.10 in 1.11.0 Nov 30, 2021
@nathanjsweet nathanjsweet added backport-done/1.11 The backport for Cilium 1.11.x for this PR is done. and removed backport-pending/1.11 labels Dec 2, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.10 to Backport done to v1.11 in 1.11.0 Dec 2, 2021
@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.9 to Backport done to v1.9 in 1.9.12 Dec 2, 2021
@joestringer
Copy link
Member

@adamzhoul thanks for the fix, for the authors file is it OK to refer to you as Adam Zhoul? Or do you go by another name?

@maintainer-s-little-helper maintainer-s-little-helper bot moved this from Backport pending to v1.10 to Backport done to v1.10 in 1.10.6 Dec 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/documentation Impacts the documentation, including textual changes, sphinx, or other doc generation code. backport-done/1.11 The backport for Cilium 1.11.x for this PR is done. release-note/misc This PR makes changes that have no direct user impact.
Projects
No open projects
1.10.6
Backport done to v1.10
1.11.0
Backport done to v1.11
1.9.12
Backport done to v1.9
Development

Successfully merging this pull request may close these issues.

None yet

4 participants