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

Describe how to build a Docker image with an example #29047

Merged
merged 7 commits into from
Feb 16, 2024

Conversation

wienans
Copy link
Contributor

@wienans wienans commented Feb 4, 2024

  • Pull Request adds the docker tag command to the docs for container registry.

For Context i was using a own container registry the first time and also didn't pushed any docker images to docker.io so i wasn't aware of the necessity of the docker tag command. i only tried to follow the instructions in the page and got the error:

"An image does not exist locally with the tag: {domain}/{owner}/{image}:{tag}"

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Feb 4, 2024
@pull-request-size pull-request-size bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Feb 4, 2024
@KN4CK3R
Copy link
Member

KN4CK3R commented Feb 4, 2024

That's only related if you want to push an image you did not create in the first place. Maybe reword it to something like

If you want to push an existing image, you must re-tag it.

And add it below the push command section because that should be part of a docker tutorial which the Gitea docs are not.

@wienans
Copy link
Contributor Author

wienans commented Feb 4, 2024

That's only related if you want to push an image you did not create in the first place. Maybe reword it to something like

If you want to push an existing image, you must re-tag it.

And add it below the push command section because that should be part of a docker tutorial which the Gitea docs are not.

Mhm okay. Actually I created that image myself. So not like I wanted to upload the Ubuntu latest or something

@wienans
Copy link
Contributor Author

wienans commented Feb 4, 2024

@lunny moved the part under the full push section and added it as in case of an error.

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Feb 5, 2024
@KN4CK3R
Copy link
Member

KN4CK3R commented Feb 5, 2024

I don't want to block the change but for me this is comparable to:

"We have a command load <file> which loads the file." Now the user executes load /does/not/exist and the command tells them "Can't find the file /does/not/exist". Would you expect that the documentation of load has the part: "If the command tells you the file can't be found, you must rename/move the file to what you passed as parameter to load." I would not expect that in the docs.

We already have a chapter https://docs.gitea.com/usage/packages/container#image-naming-convention which describes how the image name must be structured.

@wienans
Copy link
Contributor Author

wienans commented Feb 5, 2024

@KN4CK3R the confusing thing for me as a relative basic user of docker was that for me the tag was „latest“ and I didn’t get automatically that I need to write a new tag where the i needed tongue the domain image and tag into it.

My idea was to improve the handling for newer users. And I would have found that info in the doc helpful. But I also don’t mind if you don’t merge it.

@KN4CK3R
Copy link
Member

KN4CK3R commented Feb 5, 2024

I think you read the docs and may or may not have noticed the chapter I linked. Could we improve that block, so you don't run into the problem?
For example in that chapter I did not mention the tag (besides the ignored case) only how the image name must be structured. Would have adding :{tag} behind the examples helped you?

@wienans
Copy link
Contributor Author

wienans commented Feb 5, 2024

I saw the chapter but i did not consider it helpful for my issue. Because I wasn’t aware that the „name“ of an image is also called tag? For me the tag was always the :{tag} like latest. For example name of the image ubuntu and tag latest. So I think the double meaning of the tag got me confused.

I thought the naming convention part was only relevant for the server so that there is a structure. Maybe I should have been more sceptical because actually I expected a cli of docker push image:tag remote-url/owner/image:tag or something to say I want to push this image with this tag to that remote. But the cli is obviously not like that. So how would docker push find out which image to push where. Only by naming the image like the remote full path… so at least that is what I deducted out of the info’s I found.

Maybe it would be an idea to write in the naming convention part something about that this is actually meant as the „tag“ to make the connection to docker tag. But in the end that’s a naming problem from docker…

@KN4CK3R
Copy link
Member

KN4CK3R commented Feb 5, 2024

Because I wasn’t aware that the „name“ of an image is also called tag?

No no no 😅 The name of an image is the part infront of the :tag. The tag is not mentioned in the docs because it's not relevant in the first place.

The "image name" is {domain}/{owner}/{image}. Maybe that's the confusing part in the docs because in the "image name" is an additional {image}? If you know Docker, that's not a problem, because you know {image} should be something like apache and the prefix is the url to the docker registry. But for new users that may not be clear. If we can find a better wording for the naming chapter I'm happy to change it.

@wienans
Copy link
Contributor Author

wienans commented Feb 5, 2024

Because I wasn’t aware that the „name“ of an image is also called tag?

No no no 😅 The name of an image is the part infront of the :tag. The tag is not mentioned in the docs because it's not relevant in the first place.

The "image name" is {domain}/{owner}/{image}. Maybe that's the confusing part in the docs because in the "image name" is an additional {image}? If you know Docker, that's not a problem, because you know {image} should be something like apache and the prefix is the url to the docker registry. But for new users that may not be clear. If we can find a better wording for the naming chapter I'm happy to change it.

Okay that at least is what I thought before my confusion started. But then I think the problem for starters is that docker cli itself is confusing in this parts. It returns can not find tag while pushing but actually it can not find the image, because the image does not even exist. Sure also the tag does not exist on that image. And then the command to „rename“ it is called docker tag.

Regarding doc improvement as soon as you know about all this the current docs are no problem. Without that for beginners or people like me which never got in touch with that specific part of docker, it might be not clear what exactly is meant with the naming convention and how to apply it. Clearly you could quickly google how to use external registry with docker and stackoverflow will give you the answer, but I think it is worth adding 3 lines to the docs to reduce that need.
So in this case I still would add the specific command to the docs. But after the conversation with you I would put it under naming convention as a command to how to name your image like it is requested from gitea. But I think you are against adding this command at all to the doc, aren’t you?

@techknowlogick
Copy link
Member

Perhaps a disclaimer that the hostname portion is required? And that otherwise docker will default to use docker hub?

@wienans
Copy link
Contributor Author

wienans commented Feb 6, 2024

@KN4CK3R Maybe like this?

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Feb 14, 2024
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
@pull-request-size pull-request-size bot removed the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Feb 16, 2024
@pull-request-size pull-request-size bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Feb 16, 2024
@delvh delvh added the type/docs This PR mainly updates/creates documentation label Feb 16, 2024
@wxiaoguang wxiaoguang merged commit f2d5c6e into go-gitea:main Feb 16, 2024
26 checks passed
@GiteaBot GiteaBot added this to the 1.22.0 milestone Feb 16, 2024
@delvh delvh added the backport/v1.21 This PR should be backported to Gitea 1.21 label Feb 16, 2024
@delvh delvh changed the title Docker Tag Information in Docs Describe how to build a Docker image with an example Feb 16, 2024
@GiteaBot
Copy link
Contributor

I was unable to create a backport for 1.21. @wienans, please send one manually. 🍵

go run ./contrib/backport 29047
...  // fix git conflicts if any
go run ./contrib/backport --continue

@GiteaBot GiteaBot added the backport/manual No power to the bots! Create your backport yourself! label Feb 16, 2024
@KN4CK3R
Copy link
Member

KN4CK3R commented Feb 16, 2024

Do we need a backport for the docs?

@delvh
Copy link
Member

delvh commented Feb 16, 2024

Yes, as they are versioned.

zjjhot added a commit to zjjhot/gitea that referenced this pull request Feb 16, 2024
* giteaofficial/main: (23 commits)
  Remove jQuery from SSH key form parser (go-gitea#29193)
  Refactor request function (go-gitea#29187)
  Docker Tag Information in Docs (go-gitea#29047)
  Fix gitea-action user avatar broken on edited menu (go-gitea#29190)
  Disable parallel Make execution (go-gitea#29186)
  Auto-update the system status in admin dashboard (go-gitea#29163)
  Avoid vue warning in dev mode (go-gitea#29188)
  Update JS and PY dependencies (go-gitea#29184)
  [skip ci] Updated translations via Crowdin
  Implement contributors graph (go-gitea#27882)
  Add support for action artifact serve direct (go-gitea#29120)
  Advertise WebAuthn support (go-gitea#29176)
  Tweak repo header (go-gitea#29134)
  Change webhook-type in create-view (go-gitea#29114)
  Remove jQuery from the comment task list (go-gitea#29170)
  Fix can not select team reviewers when reviewers is empty (go-gitea#29174)
  move sign in labels to be above inputs (go-gitea#28753)
  Refactor locale&string&template related code (go-gitea#29165)
  Extract linguist code to method (go-gitea#29168)
  bump to use go 1.22 (go-gitea#29119)
  ...
@wienans wienans deleted the update-container-doc branch February 16, 2024 14:55
silverwind pushed a commit to silverwind/gitea that referenced this pull request Feb 20, 2024
Add more details for the docker tag when using container registry.

---------

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
Co-authored-by: KN4CK3R <admin@oldschoolhack.me>
lunny pushed a commit to lunny/gitea that referenced this pull request Feb 24, 2024
Add more details for the docker tag when using container registry.

---------

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
Co-authored-by: KN4CK3R <admin@oldschoolhack.me>
@lunny lunny added the backport/done All backports for this PR have been created label Feb 24, 2024
lunny added a commit that referenced this pull request Feb 24, 2024
Backport #29047 

Add more details for the docker tag when using container registry.

Co-authored-by: wienans <40465543+wienans@users.noreply.github.com>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
Co-authored-by: KN4CK3R <admin@oldschoolhack.me>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport/done All backports for this PR have been created backport/manual No power to the bots! Create your backport yourself! backport/v1.21 This PR should be backported to Gitea 1.21 lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. modifies/docs size/S Denotes a PR that changes 10-29 lines, ignoring generated files. type/docs This PR mainly updates/creates documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants