Pick image from release payload for crio.conf pause_image

[umohnani8]

We want to pick the release payload image for the infra image.
Passing along the image we get from bootkube during bootstrap to
crio.conf.
Removed infraImage from the ctrcfg CR as well.

Note: we don't know what the infra image is called in the release payload yet, still working on figuring that out. I have seen it being `docker.io/openshift/origin-pod:v4.0.0" in openshift-ansible so using that for now. Will update that once we get a confirmation.
Also opening a PR in installer to add the infra image to bootkube.sh

Fixes #455

Signed-off-by: Urvashi Mohnani umohnani@redhat.com

[umohnani8]

@runcom @mrunalp PTAL

[umohnani8]

bootkube PR openshift/installer#1292

[abhinavdahiya]

missing

machine-config-operator/pkg/operator/operator.go

Line 327 in 5b65a3c

templatectrl.SetupEtcdEnvKey: imgs.SetupEtcdEnv,

[runcom]

Let's chase down what's the pod image, apart from Abhinav comment, code looks already good (I'll defer a full review after we have everything in place)

[umohnani8]

@abhinavdahiya what is missing? I have InfraImageKey there as well https://github.com/openshift/machine-config-operator/pull/471/files#diff-554de5523753fda8c93e7008c9bd947fR328

[abhinavdahiya]

missing

machine-config-operator/pkg/operator/operator.go

Line 327 in 5b65a3c
templatectrl.SetupEtcdEnvKey: imgs.SetupEtcdEnv,

sorry missed the change below all the test_data :P

[ashcrow]

/retest

[cgwalters]

Could also add a test to verify that k8s.gcr.io/pause hasn't been pulled to a node. But looks sane to me.

[umohnani8]

Fixed the infra_image name. Ready for review and merge.

[runcom]

This looks good but I'm wondering about orders of PRs now between here and installer since the note here https://github.com/openshift/machine-config-operator/pull/471/files#diff-c15e2a465008fe4b86940c96d54a17eaR3

@cgwalters ^^^

[cgwalters]

Yep this needs to land first, then the installer PR.

[runcom]

Yep this needs to land first, then the installer PR.

cause this is what actually generates the pod image in the release right?

[runcom]

Just a comment, looks good to me otherwise and we verified with @umohnani8 and @mrunalp that the pod image is indeed in the payload generated from this PR, with #472 @umohnani8 you should be able to test the flow with a dummy value as well.

[runcom]

/approve

[cgwalters]

cause this is what actually generates the pod image in the release right?

No, at least I'm pretty sure that's not true. Is pod an image that's built out of an existing repo? Answer, looks like yes:

# podman inspect quay.io/openshift/origin-pod:v4.0|grep source-location | head -1
                "io.openshift.build.source-location": "https://github.com/openshift/images",

So...if this works, I think it only works because that image is already part of the build system.

(But I am fuzzy on this part. I know for "external" images like etcd and machine-os-content, Clayton added those manually)

[runcom]

So...if this works, I think it only works because that image is already part of the build system.

yes, we went through the doc explaining that yesterday and I forgot to cancel my message 👍

[runcom]

/lgtm

I guess the path now is, merge this, merge installer PR, create another PR here to remove the default

[runcom]

aws limit hit

/retest

[cgwalters]

OK I think I understand the problem. You need to drop the change to image-references temporarily. The value is being substituted in the target cluster, but not at bootstrap time.

[cgwalters]

Although...one thing that changed recently is we started gating installs on machine-config operator being successful. Before that...we could land changes that caused this problem.

I have a feeling we were actually relying on that in order to be able to make a change like this...

[umohnani8]

@cgwalters by temporarily you mean till we get the installer PR in openshift/installer#1292 ?

[cgwalters]

I am thinking the flow needs to be something like this:

diff --git a/pkg/operator/images.go b/pkg/operator/images.go
index 825bbcc..5bcdf5f 100644
--- a/pkg/operator/images.go
+++ b/pkg/operator/images.go
@@ -4,8 +4,12 @@ package operator
 // bootkube.sh provides.  If you want to add a new image, you need
 // to "ratchet" the change as follows:
 //
-// Add the image here and also a CLI option with a default value
-// Change the installer to pass that arg with the image from the CVO
+// 1. Add the image here and also a CLI option to bootstrap.go
+//    with a default value (e.g. something quay.io).  But *ignore* the
+//    value passed on the CLI; always use the default.
+// 1. Change the installer to pass that arg with the image from the CVO
+// 1. Change the code in bootstrap to honor the passed image, and add
+//    it to image-references so it's substituted in the images configmap.
 // (some time later) Change the option to required and drop the default
 type Images struct {
 	MachineConfigController string `json:"machineConfigController"`

[cgwalters]

To rephrase #471 (comment) - we need to add the CLI option to bootstrap.go and land the installer PR to use it, but have it be a no-op. Only then can we enable it in both bootstrap and runtime in this repo.

[cgwalters]

(And the need to do this dance would go away if we changed the MCO bootstrap phase to gain access to the CVO directly)

[cgwalters]

Concretely, I would make a new PR that has this single line change:

bootstrapCmd.PersistentFlags().StringVar(&bootstrapOpts.infraImage, "infra-image", "quay.io/openshift/origin-pod:v4.0", "Image for Infra Containers.")

And keep this PR open - land the installer change, then rebase this PR on top of that.

[umohnani8]

So the tests here are passing now... Did we just have to break it down into 2 steps? So we get this in first then openshift/installer#1292?

[cgwalters]

/lgtm

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cgwalters, runcom, umohnani8

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 [cgwalters,runcom]

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

[umohnani8]

/test e2e-aws-op

[umohnani8]

Now the tests are failing :/ Are the failures expected?

[cgwalters]

Hmm. Something failed early in the bootstrap node there.

[umohnani8]

/retest

[openshift-bot]

/retest

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