OCPBUGS-4955: Set ImagePullPolicy of bundle unpacker to "IfNotPresent" for image digests #2908

dtfranz · 2022-12-15T22:14:06Z

Description of the change:

Captures the imagePullPolicy selection logic into a func, then utilizes it both in its original location as well as in the bundle unpacker. This allows users to set imagePullPolicy to IfNotPresent by supplying an image digest rather than a tag.

Motivation for the change:

OCPBUGS-4955

openshift-ci-robot · 2022-12-15T22:14:10Z

@dtfranz: This pull request references Jira Issue OCPBUGS-4955, which is invalid:

expected the bug to target only the "4.13.0" version, but multiple target versions were set

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

The bug has been updated to refer to the pull request using the external bug tracker.

In response to this:

Description of the change:

Captures the imagePullPolicy selection logic into a func, then utilizes it both in its original location as well as in the bundle unpacker. This allows users to set imagePullPolicy to IfNotPresent by supplying an image digest rather than a tag.

Motivation for the change:

OCPBUGS-4955

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.

dtfranz · 2022-12-15T22:15:17Z

/jira refresh

openshift-ci-robot · 2022-12-15T22:15:24Z

@dtfranz: This pull request references Jira Issue OCPBUGS-4955, which is valid. The bug has been moved to the POST state.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target version (4.13.0) matches configured target version for branch (4.13.0)
bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @jianzhangbjz

In response to this:

/jira refresh

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.

jianzhangbjz · 2022-12-16T03:03:47Z

Hi @dtfranz , I cannot test this PR since it's for upstream, as follows:

I will test it once it is submitted downstream: https://github.com/openshift/operator-framework-olm

jianzhangbjz · 2022-12-16T03:06:25Z

The CI test fail at:

?   	github.com/operator-framework/operator-lifecycle-manager/pkg/api/wrappers/wrappersfakes	[no test files]
--- FAIL: TestConfigMapUnpacker (0.61s)
    --- FAIL: TestConfigMapUnpacker/CatalogSourcePresent/ConfigMapPresent/JobPresent/DigestImage/Unpacked (0.20s)
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
	panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x28ddf46]

goroutine 115 [running]:
testing.tRunner.func1.2({0x2b025e0, 0x4[32](https://github.com/operator-framework/operator-lifecycle-manager/actions/runs/3708381393/jobs/6285883596#step:5:33)1ba0})
	/opt/hostedtoolcache/go/1.19.4/x64/src/testing/testing.go:1396 +0x[37](https://github.com/operator-framework/operator-lifecycle-manager/actions/runs/3708381393/jobs/6285883596#step:5:38)2
testing.tRunner.func1()

dtfranz · 2022-12-16T18:45:51Z

The CI test fail at:

?   	github.com/operator-framework/operator-lifecycle-manager/pkg/api/wrappers/wrappersfakes	[no test files]
--- FAIL: TestConfigMapUnpacker (0.61s)
    --- FAIL: TestConfigMapUnpacker/CatalogSourcePresent/ConfigMapPresent/JobPresent/DigestImage/Unpacked (0.20s)
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
	panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x28ddf46]

goroutine 115 [running]:
testing.tRunner.func1.2({0x2b025e0, 0x4[32](https://github.com/operator-framework/operator-lifecycle-manager/actions/runs/3708381393/jobs/6285883596#step:5:33)1ba0})
	/opt/hostedtoolcache/go/1.19.4/x64/src/testing/testing.go:1396 +0x[37](https://github.com/operator-framework/operator-lifecycle-manager/actions/runs/3708381393/jobs/6285883596#step:5:38)2
testing.tRunner.func1()

Thanks Jian, yes, this test passed before I pushed but that was a fluke. I've updated the test so it actually passes now.

awgreene

Nice work on this @dtfranz, had one question and one suggestion, but this looks good otherwise.

pkg/controller/bundle/bundle_unpacker_test.go

awgreene · 2022-12-16T21:51:34Z

/approve

openshift-ci · 2022-12-16T21:51:53Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: awgreene, dtfranz

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~~ [awgreene]

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

…r use, implement in bundle unpacker, add tests Signed-off-by: Daniel Franz <dfranz@redhat.com>

Signed-off-by: Daniel Franz <dfranz@redhat.com>

perdasilva · 2022-12-19T16:50:56Z

pkg/controller/bundle/bundle_unpacker_test.go

@@ -398,7 +399,7 @@ func TestConfigMapUnpacker(t *testing.T) {
 			},
 		},
 		{
-			description: "CatalogSourcePresent/ConfigMapPresent/JobPresent/Unpacked",
+			description: "CatalogSourcePresent/ConfigMapPresent/JobPresent/DigestImage/Unpacked",


maybe double-check if tests are needed for non-digest images too?

perdasilva · 2022-12-19T16:52:12Z

pkg/controller/bundle/bundle_unpacker_test.go

@@ -77,7 +77,7 @@ func TestConfigMapUnpacker(t *testing.T) {
 		args        args
 		expected    expected
 	}{
-		{
+		/*{


is this test no longer needed? maybe we should either remove, or if there's something we need to track, create an issue and tag it here?

perdasilva · 2022-12-19T16:52:40Z

pkg/controller/bundle/bundle_unpacker_test.go

@@ -77,7 +77,7 @@ func TestConfigMapUnpacker(t *testing.T) {
 		args        args
 		expected    expected
 	}{
-		/*{
+		{


nevermind XD

perdasilva · 2022-12-19T16:53:43Z

/lgtm

openshift-ci-robot · 2022-12-19T17:02:51Z

@dtfranz: All pull requests linked via external trackers have merged:

operator-framework/operator-lifecycle-manager#2908

Jira Issue OCPBUGS-4955 has been moved to the MODIFIED state.

In response to this:

Description of the change:

Captures the imagePullPolicy selection logic into a func, then utilizes it both in its original location as well as in the bundle unpacker. This allows users to set imagePullPolicy to IfNotPresent by supplying an image digest rather than a tag.

Motivation for the change:

OCPBUGS-4955

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.

openshift-cherrypick-robot · 2023-02-03T19:25:29Z

@dtfranz: cannot checkout release-4.12: error checking out release-4.12: exit status 1. output: error: pathspec 'release-4.12' did not match any file(s) known to git

In response to this:

/cherry-pick release-4.12

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.

openshift-ci-robot added the jira/severity-low Referenced Jira bug's severity is low for the branch this PR is targeting. label Dec 15, 2022

openshift-ci-robot added the jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. label Dec 15, 2022

openshift-ci bot requested review from akihikokuroda and anik120 December 15, 2022 22:14

openshift-ci-robot added jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. labels Dec 15, 2022

openshift-ci-robot removed the jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. label Dec 15, 2022

openshift-ci bot requested a review from jianzhangbjz December 15, 2022 22:15

awgreene reviewed Dec 16, 2022

View reviewed changes

pkg/controller/bundle/bundle_unpacker_test.go Outdated Show resolved Hide resolved

pkg/controller/bundle/bundle_unpacker_test.go Show resolved Hide resolved

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 16, 2022

dtfranz added 5 commits December 16, 2022 14:13

Capture ImagePullPolicy selection from image as func, export for wide…

cae1e0b

…r use, implement in bundle unpacker, add tests Signed-off-by: Daniel Franz <dfranz@redhat.com>

Fixed using wrong image as reference for pull policy inference

23b37eb

Signed-off-by: Daniel Franz <dfranz@redhat.com>

Fixed unit test using wrong vars for path/image/etc.

5b6a543

Signed-off-by: Daniel Franz <dfranz@redhat.com>

Remove comment block

9816763

Signed-off-by: Daniel Franz <dfranz@redhat.com>

Generate hash just once for digestPath

2a5c923

Signed-off-by: Daniel Franz <dfranz@redhat.com>

dtfranz force-pushed the bundle-digest-ifnotpresent branch from c2cb4c8 to 2a5c923 Compare December 16, 2022 22:14

Merge branch 'master' into bundle-digest-ifnotpresent

c336d48

perdasilva reviewed Dec 19, 2022

View reviewed changes

openshift-ci bot assigned perdasilva Dec 19, 2022

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Dec 19, 2022

openshift-merge-robot merged commit cf85bf7 into operator-framework:master Dec 19, 2022

openshift-ci-robot mentioned this pull request Jan 6, 2023

OCPBUGS-4955: Set ImagePullPolicy of bundle unpacker to "IfNotPresent" for image digests openshift/operator-framework-olm#425

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

OCPBUGS-4955: Set ImagePullPolicy of bundle unpacker to "IfNotPresent" for image digests #2908

OCPBUGS-4955: Set ImagePullPolicy of bundle unpacker to "IfNotPresent" for image digests #2908

dtfranz commented Dec 15, 2022

openshift-ci-robot commented Dec 15, 2022

dtfranz commented Dec 15, 2022

openshift-ci-robot commented Dec 15, 2022

jianzhangbjz commented Dec 16, 2022 •

edited

Loading

jianzhangbjz commented Dec 16, 2022

dtfranz commented Dec 16, 2022 •

edited

Loading

awgreene left a comment

awgreene commented Dec 16, 2022

openshift-ci bot commented Dec 16, 2022

perdasilva Dec 19, 2022

perdasilva Dec 19, 2022

perdasilva Dec 19, 2022

perdasilva commented Dec 19, 2022

openshift-ci-robot commented Dec 19, 2022

openshift-cherrypick-robot commented Feb 3, 2023

OCPBUGS-4955: Set ImagePullPolicy of bundle unpacker to "IfNotPresent" for image digests #2908

OCPBUGS-4955: Set ImagePullPolicy of bundle unpacker to "IfNotPresent" for image digests #2908

Conversation

dtfranz commented Dec 15, 2022

openshift-ci-robot commented Dec 15, 2022

dtfranz commented Dec 15, 2022

openshift-ci-robot commented Dec 15, 2022

jianzhangbjz commented Dec 16, 2022 • edited Loading

jianzhangbjz commented Dec 16, 2022

dtfranz commented Dec 16, 2022 • edited Loading

awgreene left a comment

Choose a reason for hiding this comment

awgreene commented Dec 16, 2022

openshift-ci bot commented Dec 16, 2022

perdasilva Dec 19, 2022

Choose a reason for hiding this comment

perdasilva Dec 19, 2022

Choose a reason for hiding this comment

perdasilva Dec 19, 2022

Choose a reason for hiding this comment

perdasilva commented Dec 19, 2022

openshift-ci-robot commented Dec 19, 2022

openshift-cherrypick-robot commented Feb 3, 2023

jianzhangbjz commented Dec 16, 2022 •

edited

Loading

dtfranz commented Dec 16, 2022 •

edited

Loading