Decouple bootstrap and cluster bootimages #2061
Conversation
openshift-ci-robot
added
the
size/L
hardys
force-pushed
the
issue_2037
branch
2 times, most recently
from
7ab054f to
ecd3cf0
Compare
|
cc @stbenjam - this part is working for me now, further PRs to enable bootstrap hosted Ironic (based on openshift-metal3/kni-installer#100 work) coming soon, but feedback welcome. |
|
Otherwise I think this looks great, will be happy to start seeing the image stuff disappear from the platform config. |
The goal of the image in the machines is the Image type native to the platform, example AMI on AWS, Azure Image on Azure, GCP Image on GCP. the terraform is given a form of the boot-image that can be converted to this platform dependent image. This allows terraform to do the part of creating these image objects that later on in the future can be done by the cluster using the release-image when creating these machines. |
|
Build SUCCESS, see build http://10.8.144.11:8080/job/dev-tools/915/ |
Ack, understood - it's just that this single-image abstraction won't work with the current state of baremetal IPI support - perhaps this is something of a special case, but I think the only alternative would be to get a new RHCOS build which supports both Ironic config drive partitions, and qemu fw_cfg driver for Ignition, I'm not sure if that is viable based on previous discussions on this topic. I'll address the other comments and hopefully we can make progress with this approach for now, thanks for the prompt review! :) |
|
Review comments addressed, thanks for the feedback! |
|
Build SUCCESS, see build http://10.8.144.11:8080/job/dev-tools/918/ |
|
CI failure looks unrelated AFAICS It seems we've got some DNS issues impacting the AWS jobs atm, will retest when those are confirmed resolved. |
The URL provided internally by the installer is the full path to the openstack qcow file, and the baseURI is supposed to be an internal detail, so we need to handle this case for the planned switch to use installer generated image references: openshift/installer#2037 openshift/installer#2061 The previous baseURI method is maintained for backwards compatibilty and can be removed later when the installer changes are complete, and at that point we probably want to remove the latest symlinks as I think we need to deal with explicit image references in the providerSpec, then have the BMO (and terraform) look at the mirror location for the locally downloaded/compressed version.
pkg/asset/rhcos/bootstrap_image.go
Outdated
There was a problem hiding this comment.
qq: the openstack image cannot be used to boot a VM using QEMU in libvirt backend? Is there a technical limitation?
There was a problem hiding this comment.
@abhinavdahiya The reason is that the libvirt backend uses the qemu_fw_cfg driver to pass the Ignition config, and the only image which has this enabled is the QEMU one:
It might be that in future we can rework that to enable passing the config via a config-drive so that we can use the openstack image in both places, but I've not investigated that yet, or had any discussion with the terraform-provider-libvirt community
|
/retest |
|
/lgtm |
|
/test e2e-openstack |
|
/assign @staebler |
|
@abhinavdahiya Thanks for the previous review, comments addressed so would appreciate another review when you get a moment, anything else which needs addressing before this can merge? |
|
/unassign @staebler |
|
Can you update the dep graph... https://github.com/openshift/installer/blob/master/docs/design/assetgeneration.md#dependency-graph |
For the baremetal platform we require two different bootimages, the QEMU one for the libvirt based bootstrap VM, and the OpenStack one that contains the necessary Ironic config drive support to pass data to ignition. So we rework the Image abstraction by adding a new BootstrapImage type/asset, which returns the same as Image in all cases except for the baremetal platform. This also aligns with the tfvars renamed in openshift#2044 and allows us to pass the rhcos.QEMU image via BootstrapImage to terraform but leaves the OpenStack image URL available for future use to deploy masters via follow-up PRs that implement issue openshift#2060 and also correctly set the worker machineset providerSpec for the baremetal-operator. Related: openshift#2037
openshift-ci-robot
added
size/XXL
|
@abhinavdahiya I've updated the graph (@hardys is on PTO, so watching these PR's for him) |
|
/lgtm |
|
Build FAILURE, see build http://10.8.144.11:8080/job/dev-tools/958/ |
|
/test e2e-aws |
|
@abhinavdahiya This is ready for another look, I've updated the dep graph. |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: abhinavdahiya, hardys, russellb, stbenjam The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci-robot
added
the
approved
|
/retest Please review the full test history for this PR and help us cut down flakes. |
1 similar comment
|
/retest Please review the full test history for this PR and help us cut down flakes. |
|
@hardys: 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. DetailsInstructions 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. |
9 participants
Some of the underlying assumptions of the rhcos.Image abstraction make it difficult when dealing with a platform that requires different images for bootstrap and cluster machines:
In the case of the baremetal IPI platform, we use the QEMU image to launch a VM bootstrap node, then the OpenStack image is used to deploy masters via terraform and later workers via the baremetal-operator.
This PR doesn't modify the data consumed by terraform or the BMO for masters/workers yet, that is planned via follow-up PRs that complete #2060
This is the first step towards fixing #2037