Optionally use public images

[smuda]

- Description of the problem which is fixed/What is the use case
Current build assumes the developer is a RH employee and uses images not publicly available.

Closes #268

- What I did

• Updated Dockerfile to take images as arguments
• Updated current make target to include (the same) images as arguments when building docker images.
• Added documentation about building docker images from public images.

- How to verify it
Build with existing make targets:

make docker-build
make docker-buildx

Build with public images:

export BUILDER_IMAGE=registry.ci.openshift.org/openshift/release:golang-1.18
export TARGET_IMAGE=registry.ci.openshift.org/origin/4.10:base
make docker-build
make docker-buildx

- Description for the changelog

Added support for overriding images in Makefile.

[openshift-ci]

Hi @smuda. Thanks for your PR.

I'm waiting for a openshift member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

[bpradipt]

@cpmeadors @jensfr ptal

[bpradipt]

/ok-to-test

[bpradipt]

/lgtm
Thanks @smuda

[cpmeadors]

I am concerned with these changes. I understand the desire to have everything build-able for the community. No one thinks we should have private RH images required. The problem I see is that this is working around a problem that cause by internal RH build processes. The proper solution is to use all publicly available resources here and fix downstream.

[smuda]

I agree this is a workaround and it would be much better to have public images that is always used. Could you give me URL's for such images? I'd be happy to adjust the PR accordingly.

[gkurz]

Hi @smuda, thanks for working on this !

My overall concern with the approach you've taken is that it looks over-engineered. Especially, I don't quite understand the goal of the new docker-build-public and docker-buildx-public targets, which are basically duplicating existing targets and applying them to the public images.

It seems that we'd just want something like :

BUILDER_IMAGE  ?= registry.ci.openshift.org/openshift/release:golang-1.18
TARGET_IMAGE   ?= registry.ci.openshift.org/origin/4.10:base

and have docker-build and docker-buildx to use these.

The targets would work as is for upstream and downstream would have to do something like :

make BUILDER_IMAGE=registry.ci.openshift.org/ocp/builder:rhel-8-golang-1.18-openshift-4.11 TARGET_IMAGE=registry.ci.openshift.org/ocp/4.11:base

Am I missing something ?

[gkurz]

/hold

[openshift-ci]

New changes are detected. LGTM label has been removed.

[smuda]

Yes, I might have over-engineered a bit. 😀 I've simplified Makefile changes to simply allow overriding the images and added documentation to DEVELOPMENT.md with the appropriate public images.

The PR still does not address @cpmeadors comment about always having public images, but perhaps it's a step in the right direction?

[bpradipt]

@gkurz ptal

[gkurz]

@gkurz ptal

Ack @bpradipt . Sorry @smuda for the delay...

[gkurz]

Nit: trailing space at end of line.

[smuda]

I've removed trailing whitespace.

[gkurz]

Do we really need to duplicate the default values here ? AFAICT you'll get the default values from the Makefile.

@cpmeadors can you think of a case where this Dockerfile would be used standalone ?

[smuda]

If my memory serves me right, it's required during automatic tests.

[gkurz]

@smuda That rings a bell indeed. Let's add a comment in both places stating that changes should be propagated to the other file as well.

[smuda]

I've added a comment about keeping them in sync.

[openshift-ci]

@smuda: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/sandboxed-containers-operator-e2e	0d423f1	link	false	/test sandboxed-containers-operator-e2e

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.

[gkurz]

Thanks @smuda !