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

When pulling image or bundle inform the caller if the content is cacheable or not #390

Closed
Tracked by #664
joaopapereira opened this issue May 13, 2022 · 3 comments · Fixed by #424
Closed
Tracked by #664
Assignees
Labels
carvel accepted This issue should be considered for future work and that the triage process has been completed enhancement This issue is a feature request

Comments

@joaopapereira
Copy link
Member

Describe the problem/challenge you have
When another tool like vendir tries to pull a bundle or image using imgpkg, it should be informed if the contents are cacheable or not. This solution will allow tools to not rely as much on the registry and can store some of the image information on the disk.

Describe the solution you'd like
I am proposing 2 possible UX implementations for this feature.

Option 1: Create a machine-readable output for the pull command
In this option add --output-type that can have the value yaml and would look something like the following example:

image: localhost:5000/test@sha256:a9a409d96a51b16e3cbb214ad192004e57cf5c7c18ea66b9860c83395048e5eb
imageslock:
    path: /tmp/.imgpkg/images.yml
    updated: true
cacheable: true

or for recursive pulls

image: localhost:5000/test@sha256:a9a409d96a51b16e3cbb214ad192004e57cf5c7c18ea66b9860c83395048e5eb
imageslock:
  path: /tmp/.imgpkg/images.yml
  updated: true
cacheable: true
inner-bundles:
- image: localhost:5000/test@sha256:a9a409d96a51b16e3cbb214ad192004e57cf5c7c18ea66b9860c83395048e555
  path: /tmp/.imgpkg/bundles/sha256-a9a409d96a51b16e3cbb214ad192004e57cf5c7c18ea66b9860c83395048e555
  imageslock:
    path: /tmp/.imgpkg/bundles/sha256-a9a409d96a51b16e3cbb214ad192004e57cf5c7c18ea66b9860c83395048e555imgpkg/images.yml
    updated: true

Pros:

  • Provide a machine-readable output by calling the CLI
  • YAML output is standard and it is easy to parse
    Cons:
  • Force other tools that call imgpkg as a CLI to know the version used (cannot use the flag on an older version of imgpkg)
  • Other tools would have to call the CLI and parse the output

Option 2: Dedicated API to provide this information
In this option, the tools can include imgpkg as a library and that would allow them to have more control over processing the outputs.

One possible API could be:

type BundleInfo struct {
  ImageRef string
  ImagesLockPath string
  Path string
  NestedBundles []BundleInfo
}

type Status struct {
  Cacheable bool
  ImageRef string
  IsBundle bool
  NestedBundles []BundleInfo
}

// Pull Download the contents of the image referenced by imageRef to the folder outputPath
func Pull(ctx context.Context, imageRef string, outputPath string, registryOpts registry.Opts) (Status, error)

// PullRecursive Downloads the contents of the Bundle referenced by imageRef to the folder outputPath.
// This functions should error out when imageRef does not point to a Bundle
func PullRecursive(ctx context.Context, imageRef string, outputPath string, registryOpts registry.Opts) (Status, error)

Pros:

  • No need to do output processing
  • More control over action to take when the API call ends
    Cons:
  • Other tools would have to include imgpkg as a library

Note: Adding some text to the current output was also considered. Parsing text is a process that can be error-prone and changing versions of the text can cause other tools that expect a particular text to fail. Due to this we advise against this implementation and didn't provide it as an option.


Looking at the pros and cons I believe the best solution would be to create an API that can be used by other tools. This will solve compatibility issues between these tools and imgpkg and would allow these tools to have better control over the errors that might surface from the pull.

Anything else you would like to add:
From the following scenarios, let's check which ones imgpkg knows are not going to change and can be cacheable

  • Pull image using a tag
    In this scenario, the contents are not cacheable because the tags can be moved to a different image
  • Pull image using the SHA
    In this scenario, the contents are cacheable
  • Pull bundle using a tag
    In this scenario, the contents are not cacheable because the tags can be moved to a different bundle
  • Pull bundle using the SHA and the bundle is fully relocated
    In this scenario, the contents are cacheable because nothing will change on the contents of the bundle
  • Pull bundle using the SHA and the bundle is NOT fully relocated
    In this scenario, the contents are not cacheable because is in the future the bundle becomes fully copied the ImagesLock file can be updated with the new location of the images
  • Pull bundle using the SHA and the bundle is partially copied
    In this scenario, the contents are not cacheable because is in the future the remaining images might be copied and that will update the ImagesLock file

TLDR; currently imgpkg cannot partially copy a bundle.


Vote on this request

This is an invitation to the community to vote on issues, to help us prioritize our backlog. Use the "smiley face" up to the right of this comment to vote.

👍 "I would like to see this addressed as soon as possible"
👎 "There are other more important things to focus on right now"

We are also happy to receive and review Pull Requests if you want to help work on this issue.

@github-actions
Copy link

This issue is being marked as stale due to a long period of inactivity and will be closed in 5 days if there is no response.

@github-actions github-actions bot added the stale This issue has had no activity for a while and will be closed soon label Jun 23, 2022
@joaopapereira joaopapereira added discussion This issue is not a bug or feature and a conversation is needed to find an appropriate resolution and removed carvel triage This issue has not yet been reviewed for validity labels Jun 23, 2022
@joaopapereira
Copy link
Member Author

we are still in the process of discussing this implementation

@joaopapereira joaopapereira removed the stale This issue has had no activity for a while and will be closed soon label Jun 27, 2022
@joaopapereira joaopapereira self-assigned this Aug 1, 2022
@joaopapereira joaopapereira added carvel accepted This issue should be considered for future work and that the triage process has been completed and removed discussion This issue is not a bug or feature and a conversation is needed to find an appropriate resolution labels Aug 1, 2022
@joaopapereira
Copy link
Member Author

We decided to go with Option 2. This means that we did not implement the yaml output for the command. We can add that in the future if the need raises

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
carvel accepted This issue should be considered for future work and that the triage process has been completed enhancement This issue is a feature request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant