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.

Sign up for GitHub

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

Jump to bottom

Add --include-non-distributable flag to copy command #62

Merged
merged 20 commits into from
Feb 3, 2021
Merged

Add --include-non-distributable flag to copy command #62

merged 20 commits into from
Feb 3, 2021

Conversation

DennisDenuto
Copy link
Contributor

@DennisDenuto DennisDenuto commented Jan 28, 2021 •
edited
Loading

Why

To enable an operator wishing to collect a bundle that includes software that includes non-distributable bits (e.g. Windows-based products) for use in an air-gapped scenario

Implements #8

@DennisDenuto DennisDenuto marked this pull request as draft January 28, 2021 01:26
@gcheadle-vmware gcheadle-vmware mentioned this pull request Jan 28, 2021
Closed
5 tasks
@DennisDenuto @cari-lynn
Backfill a test for copying a bundle to a tarball
0a47bfb 
- Add test helper to mock registry

Co-authored-by: Dennis Leon <leonde@vmware.com>
Co-authored-by: Cari Dean <cdean@pivotal.io>
cari-lynn and others added 9 commits February 3, 2021 08:57
@cari-lynn @DennisDenuto
Add test for non-distributable flag
5cf80b8 
Co-authored-by: Dennis Leon <leonde@vmware.com>
@DennisDenuto
Test refactor
030f7b6 
- Use go-container registry to create a 'random' layer (instead of creating a layer in our test code)
- read the copied tarball using imgpks tar reader (instead of having untar logic in our test code)

Authored-by: Dennis Leon <leonde@vmware.com>
@DennisDenuto @gcheadle-vmware
Allow writing non-distributable layers when writting to a tarball
e0e7780 
Co-authored-by: Dennis Leon <leonde@vmware.com>
Co-authored-by: Garrett Cheadle <gcheadle@vmware.com>
@gcheadle-vmware @DennisDenuto
Introduce the --include-non-distributable flag
e84477b 
- wire in flag using cobra
- add a warning message when a user has provided the non-distributable
flag but none of the layers copied were 'non distributable'

Co-Authored-By: Dennis Leon <leonde@vmware.com>
@DennisDenuto @gcheadle-vmware
Refactor
ebd6128 
- delegate checking whether an image layer should be written to tar to a
new type 'ImageLayerWriteCheck'
- move printing of warning message to top level (tar image writer set)

Co-authored-by: Dennis Leon <leonde@vmware.com>
Co-authored-by: Garrett Cheadle <gcheadle@vmware.com>
@gcheadle-vmware @DennisDenuto
refactor
de661f5 
- enabled bundles to add their images to the fake registry
- images can specify if they have a non-distributable layer

Co-Authored-By: Dennis Leon <leonde@vmware.com>
@DennisDenuto @danielhelfand
Allow writing non-distributable layers when copying to a repo
979c439 
Co-authored-by: Dennis Leon <leonde@vmware.com>
Co-authored-by: Daniel Helfand <helfand.4@gmail.com>
@DennisDenuto
Refactor
cfa33a4 
- copy_repo_src_test has a nested test structure
- 'foreign_described_layer' that would panic due to not being implemented is deleted and instead a 'described_layer' is used

Authored-by: Dennis Leon <leonde@vmware.com>
@DennisDenuto
Test refactor
243ea40 
- Make it clear what the 'change' from the default registry config is.
In this case, only the manifest url validation regex needs to be changed
from the default registry configuration

Authored-by: Dennis Leon <leonde@vmware.com>
@DennisDenuto DennisDenuto marked this pull request as ready for review February 3, 2021 17:03
gcheadle-vmware
gcheadle-vmware approved these changes Feb 3, 2021
View reviewed changes
Copy link
Contributor

@gcheadle-vmware gcheadle-vmware left a comment

Choose a reason for hiding this comment

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

Looks great 👍 Let's be sure to squash some of these commits before the merge.

DennisDenuto and others added 10 commits February 3, 2021 15:06
@DennisDenuto
Add warning message when copying layers to a tar and repo
242ce4c 
- warning message indicates when a layer has been skipped
@DennisDenuto
Refactor
5de6cd3 
- Remove unused field from copy command

Authored-by: Dennis Leon <leonde@vmware.com>
@DennisDenuto
Performance improvement
048e129 
- when copying to a repo, only push the non-distributable layers
individually as blobs. Let the go-container-registry library push the
other layers, since it does so in an efficient manner.

Authored-by: Dennis Leon <leonde@vmware.com>
@DennisDenuto
Refactor
baef0c7 
- Move ImagesReaderWriter to a different package. It now lives closer to where it is used
- reorganize imports
- rename objects of type ImagesMetadata
- ImageLayerWriterChecker -> ImageLayerWriterFilter to communicate that
this is being used to 'filter' layers when writing
@DennisDenuto
Refactor
8c246e1 
- move the printing of the non-distributable flag warning message closer
to the ui layer and out of the lower layers of the code (registry and
tarball writer)

Authored-by: Dennis Leon <leonde@vmware.com>
@DennisDenuto @cari-lynn
Improve error handling around copying a tarfile that is missing a layer
8f1375a 
- when copying a tarfile that doesn't contain every layer inside of it,
stop retrying 5 times. Instead only try once and print a warning message
giving a hint to the user about what the issue might be.
- Made the image (that contained a non-distributable layer) used in the e2e test, use a correct layer.URL
- refactored warning the user so that it can be re-used when the source is
a tarfile or a repo.
- refactored the Retry function, to allowing writing tests around it to be easier.

Co-authored-by: Dennis Leon <leonde@vmware.com>
Co-authored-by: Cari Dean <cdean@pivotal.io>
@DennisDenuto @cari-lynn
Refactor
221f1eb 
- Move the Retry function into the util package.
- Make tar_writer use the common util.Retry function

Co-authored-by: Dennis Leon <leonde@vmware.com>
Co-authored-by: Cari Dean <cdean@pivotal.io>
@DennisDenuto @cari-lynn
Backfill tests using an imageindex during copy
cc9df71 
Co-authored-by: Dennis Leon <leonde@vmware.com>
Co-authored-by: Cari Dean <cdean@pivotal.io>
@DennisDenuto @cari-lynn
Refactor Export to return ids used to print a warning message to the …
ea5dfc6 
…user

Co-authored-by: Dennis Leon <leonde@vmware.com>
Co-authored-by: Cari Dean <cdean@pivotal.io>
@DennisDenuto @cari-lynn
Fix failing tests and linting errors
a5e697a 
- FakeRegistry should keep state of images that are written to it, so
that it may serve them later
- Registry no longer needs a logger. logging is done in the 'ui' layer

Co-authored-by: Dennis Leon <leonde@vmware.com>
Co-authored-by: Cari Dean <cdean@pivotal.io>
@StevenLocke StevenLocke merged commit 3aff9d1 into develop Feb 3, 2021
@StevenLocke StevenLocke deleted the foreign-layers-176099730 branch February 3, 2021 23:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gcheadle-vmware gcheadle-vmware gcheadle-vmware approved these changes

Assignees
No one assigned
Labels
cla-not-required
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@DennisDenuto @gcheadle-vmware @StevenLocke @vmwclabot @cari-lynn