Add ForcePull to BuildOverrides

[coreydaley]

Use the ForcePull pointer from BuildOverridesConfig

https://issues.redhat.com/browse/BUILD-96

[coreydaley]

/assign @adambkaplan @gabemontero

[gabemontero]

in case it is masked by github's PR review ... I think the comment from 13 days ago around line 45 of overrides.go was resolved incorrectly @coreydaley @adambkaplan

the default should be IfNotPresent, which is what k8s go doc says, and hence I don't think applyForcePullToPod should need modification

[coreydaley]

If the system admin has set ForcePull to false, shouldn't we propagate that down also?
So if a user has set it to corev1.PullAlways, and the sys admin set ForcePull to false, shouldn't that change the users config to corev1.IfNotPresent?

[gabemontero]

If the system admin has set ForcePull to false, shouldn't we propagate that down also?

duh ... correct ... also updated my comment above ... feel free to resolve

So if a user has set it to corev1.PullAlways, and the sys admin set ForcePull to false, shouldn't that change the users config to corev1.IfNotPresent?

ok for this one, the user does not have access to set pull always, etc. ... they just control force pull, and force pull should either translate to always or ifnotpresent

Your code still has pullPolicy := corev1.PullNever

[coreydaley]

Your code still has pullPolicy := corev1.PullNever

Yep, I will fix that, I think it got reset when I made a recent change :(

[coreydaley]

@gabemontero changed to IfNotPresent instead of Never

[gabemontero]

default pull policy looks good @coreydaley

one more suggestion came to me ... add a unit test that confirms false (and not set at all for that matter perhaps) results in pull if not present being set

[gabemontero]

LGTM. Let’s wait and see if @adambkaplan wants to chime in. If he doesn’t by scrum tomorrow we’ll ask him then.

…

On Wed, Jul 8, 2020 at 2:12 PM Corey Daley ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In pkg/build/controller/build/overrides/overrides_test.go <#123 (comment)> : > @@ -34,7 +34,8 @@ func TestBuildOverrideForcePull(t *testing.T) { ops := []admission.Operation{admission.Create, admission.Update} for _, test := range tests { for _, op := range ops { - overrides := BuildOverrides{Config: &openshiftcontrolplanev1.BuildOverridesConfig{ForcePull: true}} + truePtr := true updated and pushed — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#123 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AA3NU5A6OD7IXPPLTQHN3HLR2SZINANCNFSM4OIXBG4Q> .

[coreydaley]

@adambkaplan Do you have any comments on this?

[adambkaplan]

@coreydaley nit in the code and PR title. These are consumed by the public in our changelogs.

Suggested change: "Allow ForcePull to be set to False in BuildOverrides".

[adambkaplan]

/approve

[coreydaley]

@adambkaplan @gabemontero all comments have been addressed and all requested updates completed. ptal

[gabemontero]

/lgtm

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: adambkaplan, coreydaley, gabemontero

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 [adambkaplan]

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

[openshift-bot]

/retest

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

[openshift-bot]

/retest

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

[adambkaplan]

/retitle Add ForcePull to BuildOverrides