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

bugfix: download images from imageListWithAuth.yaml #1835

Merged

Conversation

kakaZhou719
Copy link
Member

Describe what this PR does / why we need it

Does this pull request fix one issue?

Describe how you did it

add build flag --image-list-with-auth to support download container images from imageListWithAuth.yaml
example:
sealer build -t abc:v1 --image-list-with-auth imageListWithAuth.yaml

if it set, sealer will read its content and download extra container images to rootfs.

Describe how to verify it

Special notes for reviews

@github-actions github-actions bot added the ImageBuilding related to all staff with image building label Nov 3, 2022
@kakaZhou719 kakaZhou719 force-pushed the fix-add-imageListWithAuth-to-build-flag branch from 66cca80 to 757fc8a Compare November 3, 2022 10:59
@codecov-commenter
Copy link

Codecov Report

Base: 21.66% // Head: 21.65% // Decreases project coverage by -0.01% ⚠️

Coverage data is based on head (66cca80) compared to base (f936ae5).
Patch coverage: 0.00% of modified lines in pull request are covered.

❗ Current head 66cca80 differs from pull request most recent head 757fc8a. Consider uploading reports for the commit 757fc8a to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1835      +/-   ##
==========================================
- Coverage   21.66%   21.65%   -0.02%     
==========================================
  Files          72       72              
  Lines        6581     6586       +5     
==========================================
  Hits         1426     1426              
- Misses       4969     4974       +5     
  Partials      186      186              
Impacted Files Coverage Δ
build/buildimage/middleware.go 0.00% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Member

@jsparter jsparter left a comment

Choose a reason for hiding this comment

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

I have a question, The user images is scattered in the file imageList, imageListWithAuth, application. If the same image appears in all three places, is it not remove duplications yet?

@kakaZhou719
Copy link
Member Author

if target image is already exist in building container, will skip to download it.

@jsparter
Copy link
Member

jsparter commented Nov 7, 2022

if target image is already exist in building container, will skip to download it.

I saw that build/buildimage/utils.go FormatImages is doing so, but there is no other func do this. So, what component skips duplications? DefaultImageSaver?

@@ -78,6 +78,9 @@ func NewBuildCmd() *cobra.Command {
},
}
buildCmd.Flags().StringVarP(&buildFlags.Kubefile, "file", "f", "Kubefile", "Kubefile filepath")
//todo we can support imageList Flag to download extra container image rather than copy it to rootfs
buildCmd.Flags().StringVar(&buildFlags.ImageList, "image-list", "filepath", "`pathname` of imageList filepath, if set, sealer will read its content and download extra container")
Copy link
Collaborator

Choose a reason for hiding this comment

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

what's the difference for image-list and image-list-with-auth?

And how about combine them?

Copy link
Member Author

Choose a reason for hiding this comment

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

  1. image-list: extra container images list which is no auth required need to be saved to clusterimage.
  2. image-list-with-auth: extra container images list which is auth required need to be saved to clusterimage, see example: http://sealer.cool/docs/getting-started/build-cloudimage.html#more-build-examples

Copy link
Collaborator

Choose a reason for hiding this comment

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

what's the purpose for store the image list at present?

@starnop
Copy link
Collaborator

starnop commented Nov 7, 2022

TODO: combine imageList and imageWithAuth @kakaZhou719

@starnop starnop merged commit d744765 into sealerio:main Nov 7, 2022
@kakaZhou719 kakaZhou719 deleted the fix-add-imageListWithAuth-to-build-flag branch March 8, 2023 05:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ImageBuilding related to all staff with image building
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants