[Baremetal] Support For Deploying with ISOs for Baremetal IPI #5425

imain · 2021-11-27T00:09:48Z

Switch to using an ISO for deployments. Use image-customization-server
to serve up custom ignition settings. Set up custom deploy steps for
RHCOS imaging.

Marked as work in progress because I'm sure there's still more clean up
to do.

Co-Authored-By: Kiran Thyagaraja [email protected]
Co-Authored-By: Caleb Boylan [email protected]

openshift-ci · 2021-11-27T00:10:19Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please assign jstuever after the PR has been reviewed.
You can assign the PR to them by writing /assign @jstuever 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:

OWNERS

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

imain · 2021-11-27T00:17:30Z

EOD Friday here. Putting this up so people can see where we're at. The rebase was pretty messy. I'll fix it up Monday :)

dtantsur · 2021-11-29T07:13:28Z

Looks okay at first glance, will review in depth once ready.

imain · 2021-11-29T20:22:03Z

OK, so there are go mod issues here. Zane and/or I are going to post a patch with just the switch to a newer openshift/cluster-api-provider-baremetal change.

zaneb

There's a few different logical changes here and splitting them into separate commits might help with reviewing, but also with potentially reverting later:

Run image customisation server and pass URLs to Terraform
Use deploy_steps deployment method with Terraform
Add ISO/kernel/initramfs/rootfs preProvisioningOSDownloadURLs to Provisioning CR (we may or may not use this for testing, but if we do we will probably revert later)
Add ISO/kernel/initramfs/rootfs URLs to installer config - this plan has been superseded and we don't want to do this at all

data/data/baremetal/masters/main.tf

data/data/bootstrap/baremetal/files/usr/local/bin/startironic.sh.template

data/data/manifests/openshift/baremetal-provisioning-config.yaml.template

pkg/asset/machines/baremetal/machinesets.go

hardys · 2021-11-30T10:29:07Z

/label platform/baremetal

data/data/bootstrap/baremetal/files/usr/local/bin/startironic.sh.template

pkg/asset/ignition/bootstrap/baremetal/template.go

data/data/bootstrap/baremetal/files/usr/local/bin/startironic.sh.template

imain · 2021-12-07T23:11:38Z

Didn't get as far as I had hoped. This is most of the cleanups. Will split the patch and use secrets for network data tomorrow.

zaneb

Thanks, splitting it up like this helps a lot! So, to summarise what this patch is doing:

We're not adding any fields to the installer config (yay!)
We're changing the default value of the OS URL from the qcow to the comma-separated iso,kernel,ramdisk,rootfs
With the default values, we're using the CoreOS IPA for virtualmedia hosts. (Not yet for PXE, see comment inline.)
If the user overrides the default with a QCOW (as they may have been doing up until now for disconnected installs) the installer would generate the right Provisioning CR manifests for CBO to handle it, but installation will fail (on virtualmedia) because we won't have the URLs for the masters.
This will be resolved when we switch to getting images from machine-os-images - at that point everything will go through the CoreOS IPA.

I don't know that there are any test jobs (i.e. disconnected configuration) that this would break (@hardys @andfasano can you confirm?), so I tend to think this is OK and a lot simpler than supporting both the CoreOS IPA and the RHEL IPA depending on the image input, as we do in CBO.

data/data/bootstrap/baremetal/files/usr/local/bin/startironic.sh.template

pkg/asset/manifests/openshift.go

data/data/bootstrap/baremetal/files/usr/local/bin/startironic.sh.template

pkg/tfvars/baremetal/baremetal.go

data/data/bootstrap/baremetal/files/usr/local/bin/startironic.sh.template

This started as a fork of a metal3-io repo, but has long since diverged, and the actual go.mod file declares the module name to be openshift/ not metal3-io/.

This adds support for deployment via custom deploy_steps.

Co-Authored-By: Kiran Thyagaraja <[email protected]> Co-Authored-By: Caleb Boylan <[email protected]>

…isioning CR

openshift-ci · 2021-12-15T22:09:30Z

@imain: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/e2e-ibmcloud	88cef1c511be11ecb86efe06d69e7264fa327d6b	link	false	`/test e2e-ibmcloud`
ci/prow/e2e-aws-workers-rhel8	88cef1c511be11ecb86efe06d69e7264fa327d6b	link	false	`/test e2e-aws-workers-rhel8`
ci/prow/e2e-crc	88cef1c511be11ecb86efe06d69e7264fa327d6b	link	false	`/test e2e-crc`
ci/prow/e2e-metal-ipi	88cef1c511be11ecb86efe06d69e7264fa327d6b	link	false	`/test e2e-metal-ipi`
ci/prow/e2e-metal-ipi-ovn-dualstack	88cef1c511be11ecb86efe06d69e7264fa327d6b	link	false	`/test e2e-metal-ipi-ovn-dualstack`
ci/prow/e2e-metal-ipi-ovn-ipv6	88cef1c511be11ecb86efe06d69e7264fa327d6b	link	false	`/test e2e-metal-ipi-ovn-ipv6`
ci/prow/e2e-metal-ipi-ovn-ipv6-required	88cef1c511be11ecb86efe06d69e7264fa327d6b	link	true	`/test e2e-metal-ipi-ovn-ipv6-required`
ci/prow/e2e-aws-workers-rhel7	88cef1c511be11ecb86efe06d69e7264fa327d6b	link	false	`/test e2e-aws-workers-rhel7`
ci/prow/okd-e2e-aws-upgrade	88cef1c511be11ecb86efe06d69e7264fa327d6b	link	false	`/test okd-e2e-aws-upgrade`
ci/prow/e2e-metal-ipi-virtualmedia	88cef1c511be11ecb86efe06d69e7264fa327d6b	link	false	`/test e2e-metal-ipi-virtualmedia`
ci/prow/okd-e2e-aws	88cef1c511be11ecb86efe06d69e7264fa327d6b	link	false	`/test okd-e2e-aws`
ci/prow/e2e-metal-ipi-swapped-hosts	88cef1c511be11ecb86efe06d69e7264fa327d6b	link	false	`/test e2e-metal-ipi-swapped-hosts`

Full PR test history. Your PR dashboard.

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.

andfasano · 2021-12-16T13:19:43Z

data/data/bootstrap/baremetal/files/usr/local/bin/startironic.sh.template

+        touch /tmp/nmstate/{{.Name}}.yaml
+    fi
+{{end}}
+


@imain I think that this whole block could be simplified, given that the NetworkConfig field is already available as a template parameter. Ie:

{{range .PlatformData.BareMetal.Hosts}} echo "{{.NetworkConfig}}" > /tmp/nmstate/{{.Name}}.yaml {{end}}

That's actually what I had before. Steve wanted to use the network secrets to get the data. Apparently they could be modified by the user after configuration and before/during deployment.

@hardys just to clarify, do you have more details about this point? In understood that the main purpose of the secret was for day-2 operation.
During the installation the user is expected to provide the configuration in the install-config.yaml, from where the NetworkConfig field is extracted

zaneb · 2021-12-16T17:30:58Z

Moved to #5473
/close

openshift-ci · 2021-12-16T17:31:05Z

@imain: PR needs rebase.

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.

openshift-ci · 2021-12-16T17:34:35Z

@zaneb: Closed this PR.

Details

In response to this:

Moved to #5473
/close

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.

openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 27, 2021

openshift-ci bot requested review from ardaguclu and celebdor November 27, 2021 00:10

imain changed the title ~~[WIP] Support For Using ISOs with Baremetal IPI~~ [WIP] Support For Deploying with ISOs for Baremetal IPI Nov 27, 2021

imain changed the title ~~[WIP] Support For Deploying with ISOs for Baremetal IPI~~ [WIP] [Baremetal] Support For Deploying with ISOs for Baremetal IPI Nov 27, 2021

imain force-pushed the poof_isos branch from 35cf40e to d42a905 Compare November 27, 2021 00:13

imain force-pushed the poof_isos branch from d42a905 to 3540599 Compare November 29, 2021 20:19

zaneb reviewed Nov 29, 2021

View reviewed changes

openshift-ci bot added platform/baremetal IPI bare metal hosts platform needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Nov 30, 2021

hardys reviewed Nov 30, 2021

View reviewed changes

data/data/bootstrap/baremetal/files/usr/local/bin/startironic.sh.template Outdated Show resolved Hide resolved

hardys reviewed Nov 30, 2021

View reviewed changes

data/data/bootstrap/baremetal/files/usr/local/bin/startironic.sh.template Outdated Show resolved Hide resolved

hardys reviewed Nov 30, 2021

View reviewed changes

data/data/bootstrap/baremetal/files/usr/local/bin/startironic.sh.template Outdated Show resolved Hide resolved

hardys reviewed Nov 30, 2021

View reviewed changes

pkg/asset/ignition/bootstrap/baremetal/template.go Outdated Show resolved Hide resolved

hardys reviewed Nov 30, 2021

View reviewed changes

data/data/bootstrap/baremetal/files/usr/local/bin/startironic.sh.template Outdated Show resolved Hide resolved

hardys reviewed Nov 30, 2021

View reviewed changes

data/data/bootstrap/baremetal/files/usr/local/bin/startironic.sh.template Outdated Show resolved Hide resolved

zaneb reviewed Dec 1, 2021

View reviewed changes

data/data/bootstrap/baremetal/files/usr/local/bin/startironic.sh.template Outdated Show resolved Hide resolved

zaneb mentioned this pull request Dec 6, 2021

Start image customization controller openshift/cluster-baremetal-operator#208

Merged

imain force-pushed the poof_isos branch from 3540599 to b8bd7f0 Compare December 7, 2021 23:10

openshift-ci bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 7, 2021

imain force-pushed the poof_isos branch 2 times, most recently from 2ef5d09 to 4b44fdd Compare December 8, 2021 20:10

zaneb reviewed Dec 8, 2021

View reviewed changes

zaneb mentioned this pull request Dec 11, 2021

Add the image-customization controller to the images configmap openshift/cluster-baremetal-operator#221

Merged

zaneb reviewed Dec 11, 2021

View reviewed changes

data/data/bootstrap/baremetal/files/usr/local/bin/startironic.sh.template Outdated Show resolved Hide resolved

imain force-pushed the poof_isos branch 5 times, most recently from 594819d to 7dde9ae Compare December 13, 2021 17:46

imain changed the title ~~[WIP] [Baremetal] Support For Deploying with ISOs for Baremetal IPI~~ [Baremetal] Support For Deploying with ISOs for Baremetal IPI Dec 13, 2021

openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 13, 2021

imain force-pushed the poof_isos branch 4 times, most recently from 01207ef to 88cef1c Compare December 15, 2021 19:26

zaneb and others added 7 commits December 15, 2021 14:27

Reference openshift/cluster-api-provider-baremetal directly

0e83811

This started as a fork of a metal3-io repo, but has long since diverged, and the actual go.mod file declares the module name to be openshift/ not metal3-io/.

Update cluster-api-provider-baremetal to latest

c72688c

Update terraform-provider-ironic to v0.2.7

72c6f19

This adds support for deployment via custom deploy_steps.

Use image customization controller to serve customized images.

3cb6090

Co-Authored-By: Kiran Thyagaraja <[email protected]> Co-Authored-By: Caleb Boylan <[email protected]>

Use Deploy Steps to deploy RHCOS on hosts

db0b0a5

Add ISO/kernel/initramfs/rootfs preProvisioningOSDownloadURLs to Prov…

a88a377

…isioning CR

Add ISO/kernel/initramfs/rootfs URLs to installer config

c35a7a4

imain force-pushed the poof_isos branch from 88cef1c to c35a7a4 Compare December 15, 2021 19:27

andfasano reviewed Dec 16, 2021

View reviewed changes

openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 16, 2021

openshift-ci bot closed this Dec 16, 2021

[Baremetal] Support For Deploying with ISOs for Baremetal IPI #5425

[Baremetal] Support For Deploying with ISOs for Baremetal IPI #5425

Uh oh!

Conversation

imain commented Nov 27, 2021

Uh oh!

openshift-ci bot commented Nov 27, 2021

Uh oh!

imain commented Nov 27, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dtantsur commented Nov 29, 2021

Uh oh!

imain commented Nov 29, 2021

Uh oh!

zaneb left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hardys commented Nov 30, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

imain commented Dec 7, 2021

Uh oh!

zaneb left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

openshift-ci bot commented Dec 15, 2021

Uh oh!

andfasano Dec 16, 2021

Choose a reason for hiding this comment

Uh oh!

imain Dec 16, 2021

Choose a reason for hiding this comment

Uh oh!

andfasano Dec 17, 2021

Choose a reason for hiding this comment

Uh oh!

zaneb commented Dec 16, 2021

Uh oh!

openshift-ci bot commented Dec 16, 2021

Uh oh!

openshift-ci bot commented Dec 16, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

imain commented Nov 27, 2021 •

edited

Loading