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

image: set error when we receive unknown schema for the image #16319

Merged
merged 2 commits into from
Sep 13, 2017

Conversation

mfojtik
Copy link
Contributor

@mfojtik mfojtik commented Sep 13, 2017

@dmage @soltysh

The error we get is:

I0913 06:32:59.350152   24974 importer.go:551] unsupported manifest type: *manifestlist.DeserializedManifestList

This will result into the .Image field to be nil which we then later panic on. I'm not sure how this was not broken before or if something changed in DockerHub. The mongo and redis images in DockerHub both have manifest lists, but we should be converting them to schema2 automatically?

This PR just propagate that error to image stream status instead of panicking the importer. We should investigate further.

@openshift-ci-robot openshift-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Sep 13, 2017
@openshift-merge-robot openshift-merge-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 13, 2017
@openshift-ci-robot openshift-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Sep 13, 2017
@soltysh
Copy link
Contributor

soltysh commented Sep 13, 2017

We've agreed with Michal to have it still, since it prevents nasty panics and provides more friendly error (wrt to the first commit). The other one actually fixes the problem.

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Sep 13, 2017
@mfojtik
Copy link
Contributor Author

mfojtik commented Sep 13, 2017

@smarterclayton we might need to backport this down to 3.3 as z-stream :-(((

@deads2k
Copy link
Contributor

deads2k commented Sep 13, 2017

Nothing merged since this started and can't until this goes in. Marking retest-not-required just to hurry it along.

@dmage
Copy link
Contributor

dmage commented Sep 13, 2017

/approve

@openshift-merge-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dmage, mfojtik, soltysh

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these OWNERS Files:

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@mfojtik
Copy link
Contributor Author

mfojtik commented Sep 13, 2017

/test cmd

etcd timeout

@bparees
Copy link
Contributor

bparees commented Sep 13, 2017

still failing a dockerhub integration test:

--- FAIL: TestImageStreamImportDockerHub (1.98s)
    			imageimporter_test.go:837: unexpected repository: &image.RepositoryImportStatus{Status:v1.Status{TypeMeta:v1.TypeMeta{Kind:"", APIVersion:""}, ListMeta:v1.ListMeta{SelfLink:"", ResourceVersion:""}, Status:"Failure", Message:"one of the images from this repository failed to import", Reason:"ImportFailed", Details:(*v1.StatusDetails)(nil), Code:0}, Images:[]image.ImageImportStatus{image.ImageImportStatus{Tag:"latest", Status:v1.Status{TypeMeta:v1.TypeMeta{Kind:"", APIVersion:""}, ListMeta:v1.ListMeta{SelfLink:"", ResourceVersion:""}, Status:"Success", Message:"", Reason:"", Details:(*v1.StatusDetails)(nil), Code:0}, Image:(*image.Image)(0xc4207e4c00)}, image.ImageImportStatus{Tag:"3.5", Status:v1.Status{TypeMeta:v1.TypeMeta{Kind:"", APIVersion:""}, ListMeta:v1.ListMeta{SelfLink:"", ResourceVersion:""}, Status:"Success", Message:"", Reason:"", Details:(*v1.StatusDetails)(nil), Code:0}, Image:(*image.Image)(0xc4207e5c00)}, image.ImageImportStatus{Tag:"3.5-windowsservercore", Status:v1.Status{TypeMeta:v1.TypeMeta{Kind:"", APIVersion:""}, ListMeta:v1.ListMeta{SelfLink:"", ResourceVersion:""}, Status:"Failure", Message:"dockerimage.image.openshift.io \"mongo:3.5-windowsservercore\" not found", Reason:"NotFound", Details:(*v1.StatusDetails)(0xc42053e060), Code:404}, Image:(*image.Image)(nil)}}, AdditionalTags:[]string{"3.4", "3.4-windowsservercore", "3.4-rc", "3.3", "3.3-windowsservercore", "3.2", "3.2-windowsservercore", "3.2-rc", "3.1", "3.0", "3.0-windowsservercore", "2.8", "2.7", "2.6", "2.4", "2.2", "3.5.12", "3.5.12-windowsservercore", "3.5.11", "3.5.11-windowsservercore", "3.5.10", "3.5.10-windowsservercore", "3.5.9", "3.5.9-windowsservercore", "3.5.8", "3.5.8-windowsservercore", "3.5.7", "3.5.7-windowsservercore", "3.5.6", "3.5.6-windowsservercore", "3.5.5", "3.5.5-windowsservercore", "3.4.9", "3.4.8", "3.4.7", "3.4.7-windowsservercore", "3.4.6", "3.4.6-windowsservercore", "3.4.5", "3.4.5-windowsservercore", "3.4.4", "3.4.4-windowsservercore", "3.4.3", "3.4.3-windowsservercore", "3.4.2", "3.4.1", "3.4.1-windowsservercore", "3.4.0", "3.4.0-windowsservercore", "3.4.0-rc5-windowsservercore", "3.4.0-rc5", "3.4.0-rc4-windowsservercore", "3.4.0-rc4", "3.4.0-rc3-windowsservercore", "3.4.0-rc3", "3.4.0-rc2-windowsservercore", "3.4.0-rc2", "3.3.15", "3.3.15-windowsservercore", "3.3.14", "3.3.14-windowsservercore", "3.3.12", "3.3.12-windowsservercore", "3.3.11", "3.3.11-windowsservercore", "3.3.10", "3.3.9", "3.3.8", "3.3.6", "3.3.5", "3.2.16", "3.2.16-windowsservercore", "3.2.15", "3.2.15-windowsservercore", "3.2.14", "3.2.14-windowsservercore", "3.2.13", "3.2.13-windowsservercore", "3.2.12", "3.2.12-windowsservercore", "3.2.11", "3.2.11-windowsservercore", "3.2.10", "3.2.10-windowsservercore", "3.2.9", "3.2.9-windowsservercore", "3.2.8", "3.2.7", "3.2.6", "3.2.5", "3.2.4", "3.2.3", "3.2.1", "3.2.0", "3.2.0-rc6", "3.2.0-rc4", "3.2.0-rc3", "3.2.0-rc2", "3.2.0-rc1", "3.1.9", "3.1.8", "3.1.7", "3.1.6", "3.1.5", "3.1.4", "3.0.15", "3.0.15-windowsservercore", "3.0.14", "3.0.14-windowsservercore", "3.0.13", "3.0.13-windowsservercore", "3.0.12", "3.0.12-windowsservercore", "3.0.11", "3.0.10", "3.0.9", "3.0.8", "3.0.7", "3.0.6", "3.0.5", "3.0.4", "3.0.3", "3.0.2", "3.0.1", "3.0.0", "3.0.0-rc8", "3.0.0-rc7", "3.0.0-rc6", "2.8.0", "2.8.0-rc5", "2.8.0-rc4", "2.8.0-rc3", "2.8.0-rc0", "2.7.8", "2.7.7", "2.7.6", "2.7.5", "2.6.12", "2.6.11", "2.6.10", "2.6.9", "2.6.8", "2.6.7", "2.6.6", "2.6.5", "2.6.4", "2.6.1", "2.4.14", "2.4.13", "2.4.12", "2.4.11", "2.4.10", "2.2.7", "2", "3", "3-windowsservercore", "3.4-rc-windowsservercore", "rc", "rc-windowsservercore", "unstable", "unstable-windowsservercore", "windowsservercore"}}
    		FAIL

@mfojtik
Copy link
Contributor Author

mfojtik commented Sep 13, 2017

@bparees yeah, @dmage is working on alternative solution... continuing this path I will have to ignore the NotFound tags (we failed to fetch them because they are pointing to windows manifests...)

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-merge-robot
Copy link
Contributor

Automatic merge from submit-queue

@openshift-merge-robot openshift-merge-robot merged commit 2ffdc48 into openshift:master Sep 13, 2017
openshift-merge-robot added a commit that referenced this pull request Sep 21, 2017
Automatic merge from submit-queue (batch tested with PRs 13840, 16033, 16322, 16441)

Import manifest lists with linux/amd64 platform

This is an alternative for #16319
@mfojtik mfojtik deleted the fix-import branch September 5, 2018 21:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. queue/blocks-others queue/critical-fix retest-not-required size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants