-
Notifications
You must be signed in to change notification settings - Fork 255
WIP: *: Extract installer from the target update payload #210
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Take advantage of openshift/origin@59dab63d (Temporarily force the installer to be part of the payload, 2018-12-08, openshift/origin#21637) to avoid installer/update-payload mismatches. This does not address install-config.yaml/installer mismatches, but it's a step in the right direction. Also: * Replace 'shell' syntax highlighting with 'console', because these blocks have prompts. And switch from the traditionally-PS2 '>' to the traditionally-PS1 '$' for those prompts. * Stop exporting variables that are not needed by subprocesses. * Drop OPENSHIFT_INSTALL_*. This information is provided via install-config.yaml, and the installer ignores the environment variables since openshift/installer@6be4c253 (*: remove support for environment variables, 2018-12-10, openshift/installer#861). There's an outstanding FIXME while I wait for guidance about who's job it is to pull the installer image out of the update payload. If we do that in generate.go, we need to vendor some not-really-designed-as-a-library origin code. If we do it in the launched container, we need both oc and Podman (or some other way to actually run the discovered installer image).
Env: env, | ||
Command: []string{"/bin/sh", "-c"}, | ||
Args: []string{"cp -v /bin/openshift-install /output && ls -la /output"}, | ||
// FIXME: should this code or the remote container find and execute the installer container? | ||
// $ oc adm release info --image-for installer registry.svc.ci.openshift.org/openshift/origin-release:v4.0 | ||
// registry.svc.ci.openshift.org/openshift/origin-v4.0-2019-02-05-150550@sha256:33a38e5e93d29e283ca93da8dde370b792b128181deac4b7e0d3c78e473deccd |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is going to be a bit tricky, the extracting from a release image is a bit time consuming. That feels like it should not be in generate.go, which will soon be getting run more frequently on sync to make sure the install job looks like it should (if the cd changes it will be updated and restarted). We wouldn't want to repeatedly do this.
Can a container execute a container?
CC @csrwng
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We wouldn't want to repeatedly do this
On this front, if wherever you're pulling it has a layer cache, this should just be "check the tag, and, good, I already have that manifest and an unpacked image filesystem" (like pull-always) unless the tag had shifted, and you'd want the re-pull then anyway. If we're willing to commit to not clobbering tags (e.g. not using the floating registry.svc.ci.openshift.org/openshift/origin-release:v4.0), then yeah, we could cache the resolved installer payload in Go.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can a container execute a container?
@dgoodwin not sure how it would be needed in this case
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Referring to the comment above re a remote container finding and executing the installer container.
If we do it in controllers it could be slow, if we do it in a pod we're just doing a lookup for the install container to run another pod with.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe a controller maintaining a configmap of release image to extracted installer image, for every release image that shows up in a clusterdeployment. Install job can't start until our item shows up there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would a release image be required now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would a release image be required now?
No, I set the same OKD default master installer builds use. But those are garbage-collected after 72 hours, so you probably want to set an update payload (or change the default).
@wking: PR needs rebase. 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. |
@wking: The following test failed, say
Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. 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. |
Obsolete via #241 /close |
@dgoodwin: Closed this PR. In response to this:
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. |
Take advantage of openshift/origin@59dab63d (openshift/origin#21637) to avoid installer/update-payload mismatches. This does not address
install-config.yaml
/installer mismatches, but it's a step in the right direction.Also:
shell
syntax highlighting withconsole
, because these blocks have prompts. And switch from the traditionally-PS2
>
to the traditionally-PS1
$
for those prompts.OPENSHIFT_INSTALL_*
. This information is provided viainstall-config.yaml
, and the installer ignores the environment variables since openshift/installer@6be4c253 (*: remove support for environment variables installer#861).There's an outstanding FIXME while I wait for guidance about who's job it is to pull the installer image out of the update payload. If we do that in
generate.go
, we need to vendor some not-really-designed-as-a-library origin code. If we do it in the launched container, we need bothoc
andpodman
(or some other way to actually run the discovered installer image).