Bug 1942608: do not list the image with error locating manifest #4726

QiWang19 · 2021-04-06T22:09:17Z

Fix Bug 1942608, during the call of ListImages, do not list the image if the there's no manifest exist.

Signed-off-by: Qi Wang qiwan@redhat.com

What type of PR is this?

/kind bug

What this PR does / why we need it:

Which issue(s) this PR fixes:

https://bugzilla.redhat.com/show_bug.cgi?id=1942608

Special notes for your reviewer:

Does this PR introduce a user-facing change?

None

codecov · 2021-04-06T22:11:49Z

Codecov Report

Merging #4726 (23096d2) into master (730c035) will increase coverage by 0.04%.
The diff coverage is 22.22%.

❗ Current head 23096d2 differs from pull request most recent head 82f315a. Consider uploading reports for the commit 82f315a to get more accurate results

@@            Coverage Diff             @@
##           master    #4726      +/-   ##
==========================================
+ Coverage   43.02%   43.07%   +0.04%     
==========================================
  Files         107      107              
  Lines        9764     9763       -1     
==========================================
+ Hits         4201     4205       +4     
+ Misses       5110     5104       -6     
- Partials      453      454       +1

QiWang19 · 2021-04-07T00:33:09Z

/retest

mrunalp · 2021-04-07T03:53:31Z

internal/storage/image.go

@@ -315,7 +315,7 @@ func (svc *imageService) ListImages(systemContext *types.SystemContext, filter s
 				return nil, err
 			}
 			results, err = svc.appendCachedResult(systemContext, ref, image, results, newImageCache)
-			if err != nil {
+			if err != nil && !strings.Contains(err.Error(), fmt.Sprintf(`error locating item named "%s"`, "manifest")) {


This is too specific. Another option we discussed is keeping a list of images being pulled and then if we get any such error for an image in that list, we skip it.

haircommander · 2021-04-07T22:32:33Z

/override ci/openshift-jenkins/critest_rhel
/override ci/openshift-jenkins/critest_fedora
/approve

LGTM

openshift-ci-robot · 2021-04-07T22:32:36Z

@haircommander: Overrode contexts on behalf of haircommander: ci/openshift-jenkins/critest_fedora, ci/openshift-jenkins/critest_rhel

In response to this:

/override ci/openshift-jenkins/critest_rhel
/override ci/openshift-jenkins/critest_fedora
/approve

LGTM

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

haircommander · 2021-04-07T22:32:40Z

/retest

QiWang19 · 2021-04-08T14:00:28Z

/retest

mrunalp · 2021-04-08T19:54:47Z

server/image_pull.go

@@ -273,7 +273,9 @@ func (s *Server) pullImage(ctx context.Context, pullArgs *pullArguments) (string
 	if len(status.RepoDigests) > 0 {
 		imageRef = status.RepoDigests[0]
 	}
-
+	if _, ok := storage.PulledImages[status.ID]; !ok {


You need to remove it from the map oncer the pull is completed.

mrunalp · 2021-04-08T19:54:57Z

internal/storage/image.go

@@ -93,6 +93,8 @@ type imageService struct {
 	ctx            context.Context
 }

+var PulledImages map[string]int


Need sync on this.

use a sync.Map

QiWang19 · 2021-04-08T21:10:37Z

@mrunalp PTAL

QiWang19 · 2021-04-09T01:03:47Z

/retest

QiWang19 · 2021-04-09T13:11:22Z

/retest

QiWang19 · 2021-04-09T15:26:59Z

@vrothberg @saschagrunert PTAL

internal/storage/image.go

openshift-bot · 2021-04-20T08:52:43Z

/retest

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

TomSweeneyRedHat

LGTM

openshift-bot · 2021-04-20T10:40:43Z

/retest

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

openshift-bot · 2021-04-20T11:52:46Z

/retest

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

saschagrunert · 2021-04-20T13:13:12Z

/override ci/prow/e2e-agnostic

openshift-ci-robot · 2021-04-20T13:13:18Z

@saschagrunert: Overrode contexts on behalf of saschagrunert: ci/prow/e2e-agnostic

In response to this:

/override ci/prow/e2e-agnostic

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

saschagrunert · 2021-04-20T13:13:24Z

/override ci/prow/e2e-gcp

openshift-ci-robot · 2021-04-20T13:13:29Z

@saschagrunert: Overrode contexts on behalf of saschagrunert: ci/prow/e2e-gcp

In response to this:

/override ci/prow/e2e-gcp

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Fix Bug 1942608, during the call of ListImages, do not list the image if the there's no manifest exist. Skip reporting the error locating manifest if the imaage is in the ImageBeingPulled list. Signed-off-by: Qi Wang <qiwan@redhat.com>

saschagrunert

/lgtm

openshift-ci-robot · 2021-04-20T14:24:11Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: haircommander, QiWang19, saschagrunert

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [haircommander,saschagrunert]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

saschagrunert · 2021-04-20T15:15:57Z

/override ci/prow/e2e-gcp
/override ci/prow/e2e-agnostic

openshift-ci-robot · 2021-04-20T15:16:01Z

@saschagrunert: Overrode contexts on behalf of saschagrunert: ci/prow/e2e-agnostic, ci/prow/e2e-gcp

In response to this:

/override ci/prow/e2e-gcp
/override ci/prow/e2e-agnostic

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

QiWang19 · 2021-04-20T16:03:12Z

/test e2e_cgroupv2
/test e2e-agnostic

saschagrunert · 2021-04-20T18:27:57Z

/override ci/prow/e2e-gcp
/override ci/prow/e2e-agnostic

openshift-ci-robot · 2021-04-20T18:28:02Z

@saschagrunert: Overrode contexts on behalf of saschagrunert: ci/prow/e2e-agnostic, ci/prow/e2e-gcp

In response to this:

/override ci/prow/e2e-gcp
/override ci/prow/e2e-agnostic

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

QiWang19 · 2021-04-20T21:50:42Z

/test e2e_rhel
/test e2e_cgroupv2

openshift-bot · 2021-04-20T23:06:31Z

/retest

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

openshift-ci · 2021-04-20T23:12:56Z

@QiWang19: The following test failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
ci/openshift-jenkins/e2e_crun_cgroupv2	`82f315a`	link	`/test e2e_cgroupv2`

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

openshift-bot · 2021-04-20T23:19:31Z

/retest

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

haircommander · 2021-04-21T14:02:01Z

/cherry-pick release-1.21

openshift-cherrypick-robot · 2021-04-21T14:02:49Z

@haircommander: new pull request created: #4785

In response to this:

/cherry-pick release-1.21

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Apply fix cri-o#4726 when ListImage with `filter`. Previous bug https://bugzilla.redhat.com/show_bug.cgi?id=1942608 appeared in the log hyperkube ListImages with filters. `"ListImages with filter from image service failed" err="rpc error: code = Unknown desc = error locating item named \"manifest\" for image with ID \"9e11cc7364c9791ebf68949c852bf121cb4ae5829c925a1da6546be6f63b3536\": file does not exist" filter="nil"` Signed-off-by: Qi Wang <qiwan@redhat.com>

QiWang19 requested review from mrunalp and runcom as code owners April 6, 2021 22:09

openshift-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. labels Apr 6, 2021

openshift-ci-robot requested review from fidencio and rhatdan April 6, 2021 22:09

mrunalp reviewed Apr 7, 2021

View reviewed changes

QiWang19 force-pushed the manifest-locate-error branch from b44fc11 to 7902947 Compare April 7, 2021 20:37

haircommander added this to the 1.21 milestone Apr 7, 2021

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 7, 2021

QiWang19 force-pushed the manifest-locate-error branch from 7902947 to dc07eda Compare April 8, 2021 13:36

QiWang19 force-pushed the manifest-locate-error branch from dc07eda to 1dd33fc Compare April 8, 2021 19:45

haircommander removed this from the 1.21 milestone Apr 8, 2021

mrunalp reviewed Apr 8, 2021

View reviewed changes

QiWang19 force-pushed the manifest-locate-error branch 3 times, most recently from 7f3611a to cc5465b Compare April 8, 2021 21:10

haircommander reviewed Apr 9, 2021

View reviewed changes

internal/storage/image.go Show resolved Hide resolved

QiWang19 force-pushed the manifest-locate-error branch from cc5465b to f7640ca Compare April 9, 2021 19:12

TomSweeneyRedHat reviewed Apr 20, 2021

View reviewed changes

QiWang19 force-pushed the manifest-locate-error branch from 8071f56 to 82f315a Compare April 20, 2021 13:52

openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Apr 20, 2021

saschagrunert approved these changes Apr 20, 2021

View reviewed changes

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Apr 20, 2021

openshift-merge-robot merged commit d1d9cbd into cri-o:master Apr 21, 2021

openshift-cherrypick-robot mentioned this pull request Apr 21, 2021

[release-1.21] Bug 1942608: do not list the image with error locating manifest #4785

Merged

QiWang19 mentioned this pull request Jul 31, 2023

OCPBUGS-11291: skip when ListImage with filter image hasn't done pulling #7184

Closed

Bug 1942608: do not list the image with error locating manifest #4726

Bug 1942608: do not list the image with error locating manifest #4726

Conversation

QiWang19 commented Apr 6, 2021

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?

codecov bot commented Apr 6, 2021 • edited Loading

Codecov Report

QiWang19 commented Apr 7, 2021

mrunalp Apr 7, 2021

Choose a reason for hiding this comment

haircommander commented Apr 7, 2021

openshift-ci-robot commented Apr 7, 2021

haircommander commented Apr 7, 2021

QiWang19 commented Apr 8, 2021

mrunalp Apr 8, 2021

Choose a reason for hiding this comment

mrunalp Apr 8, 2021

Choose a reason for hiding this comment

mrunalp Apr 8, 2021

Choose a reason for hiding this comment

QiWang19 commented Apr 8, 2021

QiWang19 commented Apr 9, 2021

QiWang19 commented Apr 9, 2021

QiWang19 commented Apr 9, 2021

openshift-bot commented Apr 20, 2021

TomSweeneyRedHat left a comment

Choose a reason for hiding this comment

openshift-bot commented Apr 20, 2021

openshift-bot commented Apr 20, 2021

saschagrunert commented Apr 20, 2021

openshift-ci-robot commented Apr 20, 2021

saschagrunert commented Apr 20, 2021

openshift-ci-robot commented Apr 20, 2021

saschagrunert left a comment

Choose a reason for hiding this comment

openshift-ci-robot commented Apr 20, 2021

saschagrunert commented Apr 20, 2021

openshift-ci-robot commented Apr 20, 2021

QiWang19 commented Apr 20, 2021

saschagrunert commented Apr 20, 2021

openshift-ci-robot commented Apr 20, 2021

QiWang19 commented Apr 20, 2021

openshift-bot commented Apr 20, 2021

openshift-ci bot commented Apr 20, 2021

openshift-bot commented Apr 20, 2021

haircommander commented Apr 21, 2021

openshift-cherrypick-robot commented Apr 21, 2021

codecov bot commented Apr 6, 2021 •

edited

Loading