Skip to content
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

pkg/cli/admin/release: include baremetal-installer in release #52

Closed
wants to merge 1 commit into from

Conversation

stbenjam
Copy link
Member

openshift/release#4640 added the baremetal-installer
image.

This adds baremetal-installer to always be included in a release, which
is openshift-installer that includes the baremetal platform.

This adds baremetal-installer to always be included in a release, which
is openshift-installer that includes the baremetal platform.
@markmc
Copy link
Contributor

markmc commented Aug 12, 2019

How about oc adm release extract? Both with --tools and --command?

e.g. looks like 'oc adm release extract --command openshift-installwill extract either the linux or macos binary based on the based on--command-os- how should it work in this case? Addingbaremetal` as an OS seems like the easy answer, but pretty ugly

@wking
Copy link
Member

wking commented Aug 12, 2019

/hold

We're going to want to wait until we have both CI and ART builds of this image. Checking 4.2 CI now:

$ oc image info registry.svc.ci.openshift.org/origin/4.2:baremetal-installer
error: image does not exist

I don't know where ART-build images are pushed. And maybe my CI pullspec is off too? Anyhow, we should get these straightened out and document the expected target locations before moving forward here.

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 12, 2019
@wking
Copy link
Member

wking commented Aug 12, 2019

How about oc adm release extract...

Are there docs for how the baremetal installer is supposed to plug into the user flow that I can look at? I expect extracting just the binary is going to create difficulties because it's dynamically linked (while the stock installer is statically linked), so you might have issues with different/missing dependency libraries on the host system.

@stbenjam
Copy link
Member Author

How about oc adm release extract...

Are there docs for how the baremetal installer is supposed to plug into the user flow that I can look at? I expect extracting just the binary is going to create difficulties because it's dynamically linked (while the stock installer is statically linked), so you might have issues with different/missing dependency libraries on the host system.

Baremetal is a little special, we'll require a RHEL-based deployment host. We've been building these images and consuming them in https://github.com/openshift-metal3/dev-scripts both on EL8 and EL7 fine with no issues related to the dynamic linking. There's a script that extracts the installer and runs it much in the same way other platforms do.

@stbenjam
Copy link
Member Author

stbenjam commented Aug 13, 2019

/hold

We're going to want to wait until we have both CI and ART builds of this image. Checking 4.2 CI now:

$ oc image info registry.svc.ci.openshift.org/origin/4.2:baremetal-installer
error: image does not exist

I don't know where ART-build images are pushed. And maybe my CI pullspec is off too? Anyhow, we should get these straightened out and document the expected target locations before moving forward here.

I think we'll need to sync the master yaml with 4.2, in origin/release. My understanding is that happens automatically at some juncture, but maybe I could just include it in openshift/release#4729? I don't really know the answer to that.

@russellb
Copy link
Member

How about oc adm release extract...

Are there docs for how the baremetal installer is supposed to plug into the user flow that I can look at? I expect extracting just the binary is going to create difficulties because it's dynamically linked (while the stock installer is statically linked), so you might have issues with different/missing dependency libraries on the host system.

Baremetal is a little special, we'll require a RHEL-based deployment host. We've been building these images and consuming them in https://github.com/openshift-metal3/dev-scripts both on EL8 and EL7 fine with no issues related to the dynamic linking. There's a script that extracts the installer and runs it much in the same way other platforms do.

dev-scripts has some additoinal dev environment stuff in it - the product install scripts are being prepared here: https://github.com/openshift-kni/install-scripts - the readme in there provides a bunch of good info.

In terms of host compatibility, the key way we address this is limiting where the installer can be run (on a RHEL8 host only)

@stbenjam
Copy link
Member Author

I have filed #55 to track removing both installer references from oc, with the understanding if this is accepted to add baremetal-installer that it's a temporary thing.

@eparis
Copy link
Member

eparis commented Aug 13, 2019

The 'installer' is currently referenced in the 'samples' operator. I believe this is unreliable and not a pattern we should copy ( @smarterclayton ) I mean, samples operator is something we've said might be the example of the first thing to move from CVO to OLM (and thus the installer would disappear form the release payload).

While I fully support saying that the baremetal-installer image should be referenced under some bare metal code/image I absolutely do not see what is happening with installer as an example of 'doing it right'.

Can we just let this merge and then come up with something coherent for 'installer' before we send baremetall-installer down that path?

@smarterclayton
Copy link
Contributor

smarterclayton commented Aug 13, 2019

If samples operator moves out, we'd move the installer image stream into another core operator. So it's just in samples operator today because that is where we create the openshift namespace and image streams, but it's part of our "cluster API" that the openshift namespace has the "cli" and "installer" images in it (multiple components depend on finding those image streams)

@wking
Copy link
Member

wking commented Aug 13, 2019

We've been building these images and consuming them in https://github.com/openshift-metal3/dev-scripts both on EL8 and EL7 fine with no issues related to the dynamic linking.

Presumably some future baremetal installer will drop RHEL 7 support. How do we manage that transition? OpenShift minor version docs about compatible RHEL? Something more discoverable given just a release image?

@wking
Copy link
Member

wking commented Aug 13, 2019

Also, if we require RHEL, could we just require a container runtime and run the baremetal installer image without extracting the binary to the host filesystem?

@stbenjam
Copy link
Member Author

Also, if we require RHEL, could we just require a container runtime and run the baremetal installer image without extracting the binary to the host filesystem?

No, I don't think so. The reason we require a RHEL (or compatible, like CentOS) host is we launch a bootstrap libvirt VM on it. I don't know we've sorted out the exact requirements for the deployment host - @russellb or @markmc might be able to help there.

@eparis
Copy link
Member

eparis commented Aug 13, 2019

If samples operator moves out, we'd move the installer image stream into another core operator. So it's just in samples operator today because that is where we create the openshift namespace and image streams, but it's part of our "cluster API" that the openshift namespace has the "cli" and "installer" images in it.

So the CLI is referenced under the console and the MCO. The installer under the samples operator. I still don't see a rational pattern here to follow. Like I said, I'm totally fine with installer and cli not being listed here. And with moving the baremetal-installer somewhere else, but what's the sane thing to copy? samples? do we put it under samples? MCO? console?

@eparis
Copy link
Member

eparis commented Aug 13, 2019

/retest

@openshift-ci-robot
Copy link

@stbenjam: The following test failed, say /retest to rerun them all:

Test name Commit Details Rerun command
ci/prow/verify-deps 19ac7e6 link /test verify-deps

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.

@stbenjam
Copy link
Member Author

samples PR is here: openshift/cluster-samples-operator#170

@stbenjam stbenjam closed this Aug 13, 2019
@stbenjam
Copy link
Member Author

How about oc adm release extract? Both with --tools and --command?

e.g. looks like 'oc adm release extract --command openshift-installwill extract either the linux or macos binary based on the based on--command-os- how should it work in this case? Addingbaremetal` as an OS seems like the easy answer, but pretty ugly

@markmc See #57 - what do you think about this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants