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

Update containers/storage and containers/image #187

Closed
wants to merge 2 commits into from

Conversation

nalind
Copy link
Member

@nalind nalind commented Jul 13, 2017

This bumps the versions of containers/storage and containers/image that we use, and updates the shortcuts that we take when committing to local storage to compensate for assumptions that it made which are no longer true (now it makes different ones).

@nalind
Copy link
Member Author

nalind commented Jul 13, 2017

commit.go Outdated
@@ -116,10 +130,10 @@ func (b *Builder) shallowCopy(dest types.ImageReference, src types.ImageReferenc
Size: int64(len(config)),
}
_, err = destImage.PutBlob(bytes.NewReader(config), configBlobInfo)
if err != nil && len(config) > 0 {
if err != nil {
Copy link
Member

Choose a reason for hiding this comment

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

Merge into a single line.

if _, err := destImage.PutBlob(bytes.NewReader(config), configBlobInfo); err != nil {

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixing.

commit.go Outdated
}
defer layerDiff.Close()
// Write a copy of the layer as a blob, for the new image to reference.
_, err = destImage.PutBlob(layerDiff, types.BlobInfo{Digest: "", Size: -1})
Copy link
Member

Choose a reason for hiding this comment

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

Single line if _, err := ...

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixing.

commit.go Outdated
case archive.Bzip2:
// Until the image specs define a media type for bzip2-compressed layers, even if we know
// how to decompress them, we can't try to compress layers with bzip2.
return errors.New("media type for bzip2-compressed layers is not defined")
Copy link
Member

Choose a reason for hiding this comment

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

Should we return ENOSUP?

Copy link
Member Author

Choose a reason for hiding this comment

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

We'd still have to wrap it to indicate what it is that isn't supported... sure, why not.

@nalind
Copy link
Member Author

nalind commented Jul 13, 2017

Updated.

@rhatdan
Copy link
Member

rhatdan commented Jul 13, 2017

I hit the easy to understand stuff, have to rely on the Test suites for the rest. LGTM

@nalind nalind force-pushed the storage-update branch 10 times, most recently from 8e6861c to 7446c9a Compare July 20, 2017 19:11
@rh-atomic-bot
Copy link
Collaborator

☔ The latest upstream changes (presumably b9b2a8a) made this pull request unmergeable. Please resolve the merge conflicts.

@nalind nalind force-pushed the storage-update branch 4 times, most recently from 3414a75 to 199d77b Compare July 26, 2017 14:37
@nalind nalind force-pushed the storage-update branch 2 times, most recently from b3d18b1 to b872b7d Compare July 27, 2017 22:25
@rh-atomic-bot
Copy link
Collaborator

☔ The latest upstream changes (presumably be5bcd5) made this pull request unmergeable. Please resolve the merge conflicts.

@rhatdan
Copy link
Member

rhatdan commented Jul 28, 2017

@nalind needs rebase.

@nalind nalind force-pushed the storage-update branch 2 times, most recently from 3aa0c22 to 84a7705 Compare November 30, 2017 23:33
@rh-atomic-bot
Copy link
Collaborator

☔ The latest upstream changes (presumably 15792b2) made this pull request unmergeable. Please resolve the merge conflicts.

@nalind nalind force-pushed the storage-update branch 2 times, most recently from 3b19abf to c3b4855 Compare December 4, 2017 18:26
@nalind nalind force-pushed the storage-update branch 5 times, most recently from 36ff9a0 to 8a80d89 Compare December 11, 2017 21:14
Update containers/image and containers/storage to current master
(17449738f2bb4c6375c20dcdcfe2a6cccf03f312 and
0d32dfce498e06c132c60dac945081bf44c22464, respectively).

Also updates github.com/docker/docker, golang.org/x/sys, and
golang.org/x/text.

Signed-off-by: Nalin Dahyabhai <[email protected]>
Update shallowCopy() to work with the newer version of image.
Remove things from Push() that we don't need to do any more.
Preserve digests in image names, make sure we update creation times, and
add a test to ensure that we can pull, commit, and push using such names
as sources.

Signed-off-by: Nalin Dahyabhai <[email protected]>
@nalind nalind changed the title Do not merge: update containers/storage and containers/image Update containers/storage and containers/image Dec 14, 2017
@nalind
Copy link
Member Author

nalind commented Dec 14, 2017

@rhatdan, @TomSweeneyRedHat, is there anything you want changed here?

@TomSweeneyRedHat
Copy link
Member

LGTM and I know it fixes up a number of older issues. I'm also trusting in the tests to shake stuff out. I'd say push the big red button.

@rhatdan
Copy link
Member

rhatdan commented Dec 14, 2017

LGTM

@rhatdan
Copy link
Member

rhatdan commented Dec 14, 2017

@rh-atomic-bot r+

@rh-atomic-bot
Copy link
Collaborator

📌 Commit b5f18b0 has been approved by rhatdan

@rh-atomic-bot
Copy link
Collaborator

⚡ Test exempted: pull fully rebased and already tested.

rh-atomic-bot pushed a commit that referenced this pull request Dec 14, 2017
Update shallowCopy() to work with the newer version of image.
Remove things from Push() that we don't need to do any more.
Preserve digests in image names, make sure we update creation times, and
add a test to ensure that we can pull, commit, and push using such names
as sources.

Signed-off-by: Nalin Dahyabhai <[email protected]>

Closes: #187
Approved by: rhatdan
@nalind nalind deleted the storage-update branch December 14, 2017 20:59
nalind referenced this pull request in nalind/buildah Dec 14, 2017
Update shallowCopy() to work with the newer version of image.
Remove things from Push() that we don't need to do any more.
Preserve digests in image names, make sure we update creation times, and
add a test to ensure that we can pull, commit, and push using such names
as sources.

Signed-off-by: Nalin Dahyabhai <[email protected]>

Closes: #187
Approved by: rhatdan

Signed-off-by: Nalin Dahyabhai <[email protected]>
nalind referenced this pull request in nalind/buildah Dec 14, 2017
Since we merged #187, push-by-ID should work for images with schema 1
manifests.

Signed-off-by: Nalin Dahyabhai <[email protected]>
nalind pushed a commit that referenced this pull request Apr 2, 2018
Also add buildah as a requirement so that kpod build tests will run

Signed-off-by: Daniel J Walsh <[email protected]>

Closes: #187
Approved by: mheon
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 30, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants