Skip to content

Conversation

@hardys
Copy link

@hardys hardys commented Jul 18, 2019

This allows us to template in the baseURI from data/data/rhcos.json
when needed for boostrap files (the baremetal platform will soon require
this to download the appropriate image to provision the masters ref
openshift-metal3/kni-installer#100

This allows us to template in the baseURI from data/data/rhcos.json
when needed for boostrap files (the baremetal platform will soon require
this to download the appropriate image to provision the masters ref
openshift-metal3/kni-installer#100
@openshift-ci-robot openshift-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jul 18, 2019
@hardys
Copy link
Author

hardys commented Jul 18, 2019

cc @markmc @stbenjam

@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: hardys
To complete the pull request process, please assign smarterclayton
You can assign the PR to them by writing /assign @smarterclayton in a comment when ready.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

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

@hardys
Copy link
Author

hardys commented Jul 18, 2019

/label platform/baremetal

@openshift-ci-robot
Copy link
Contributor

@hardys: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
ci/prow/unit f2b6c68 link /test unit
ci/prow/e2e-openstack f2b6c68 link /test e2e-openstack
ci/prow/e2e-aws-scaleup-rhel7 f2b6c68 link /test e2e-aws-scaleup-rhel7
ci/prow/e2e-aws-upgrade f2b6c68 link /test e2e-aws-upgrade
ci/prow/e2e-libvirt f2b6c68 link /test e2e-libvirt
ci/prow/e2e-aws f2b6c68 link /test e2e-aws
ci/prow/images f2b6c68 link /test images
ci/prow/govet f2b6c68 link /test govet

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.

Details

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.

}

var rhcosImageBaseURI string
if ri, ok := os.LookupEnv("OPENSHIFT_INSTALL_IMAGE_BASE_URI_OVERRIDE"); ok && ri != "" {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sorry but this new env variable cannot be added. These env variables bypass installer input models and we would like to keep them to moving towards zero number.

Can you provide how you expect this variable to be used?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok I can remove it, I was following the existing conventions, and I was thinking it would be a better alternative to the "hidden" CACHEURL in openshift-metal3/kni-installer@c3e0e8e

The reason this is needed/useful is that for both repeated local testing and CI downloading the large RHCOS image is slow and a waste of bandwidth - also it may be required to avoid hitting installer hard-coded timeouts when using a slow connection that cannot download the images fast enough.

If we remove this variable, I'll need to add a new install-config variable for the baremetal platform that can provide the IP for the CACHEURL (since it's hard-coded in the commit referenced above), would that be a more acceptable approach?

@abhinavdahiya
Copy link
Contributor

  1. what bootimage will you use for bootstrap-host.

  2. why would bootstrap-host download the ANY image from the base uri, compared to the exact image url.

  3. I'm hesitant adding this without seeing how this will be used.

)

// BaseURI fetches the BaseURI where images can be downloaded from
func BaseURI() (string, error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This information is rather kept private to the package... Please use or add other image functions that provide th boot-image directly.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently the downloader container we're using requires the base URI

https://github.com/openshift/ironic-rhcos-downloader/blob/master/get-resource.sh

We can rework that, or pass the full URL to the openstack image perhaps. cc @derekhiggins

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can rework that, or pass the full URL to the openstack image perhaps. cc @derekhiggins

Sounds fine to me, let me know if you want to update the parameters the downloader container expects,

@hardys
Copy link
Author

hardys commented Jul 18, 2019

1. what bootimage will you use for bootstrap-host.

For the baremetal plaform we use the libvirt bootimage for the bootstrap VM on an ephemeral provisioning host (which can later be reprovisioned as a worker, or be external to the rack)

2. why would bootstrap-host download the ANY image from the base uri, compared to the exact image url.

For the baremetal platform we need to use the openstack image for the target baremetal nodes, not the libvirt qemu image used for the bootstrap VM. This is because it provides the necessary config-drive support needed to work with the Ironic provisioning tool.

3. I'm hesitant adding this without seeing how this will be used.

For the full picture check out openshift-metal3/kni-installer#100 - I got feedback from @markmc that this part should/could be generic and not tied to the baremetal platform, and also that we should consider proposing it ahead of the Ironic integration, so that's what I'm doing here, to start the discussion and hopefully get a head-start on the pieces needed to fully land that.

@markmc
Copy link
Contributor

markmc commented Jul 18, 2019

#2037 is related to general problem here - unlike other platforms, we're using a different image for the bootstrap VM than the rest of the machines

@hardys
Copy link
Author

hardys commented Jul 22, 2019

I'll abandon this for now and persue #2061 instead which aims to partially solve #2037 and should mean I can use the existing DeployImage to template the OpenStack URL which we can then use (with some modification to the downloader container, terraform templates and baremetal-operator) to deploy the masters/workers.

@hardys hardys closed this Jul 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

platform/baremetal IPI bare metal hosts platform size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants