Skip to content

Comments

Bootstrap Ignition Config: provisional state#72

Closed
iamemilio wants to merge 2 commits intoopenshift:masterfrom
iamemilio:installerIgnitionShim
Closed

Bootstrap Ignition Config: provisional state#72
iamemilio wants to merge 2 commits intoopenshift:masterfrom
iamemilio:installerIgnitionShim

Conversation

@iamemilio
Copy link

@openshift-ci-robot openshift-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Oct 17, 2019
@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

asset similar to the [node asset](https://github.com/openshift/installer/blob/master/pkg/asset/ignition/machine/node.go)
for bootstrap ignition shims that other platforms can **opt in** to.

## Motivation
Copy link
Contributor

Choose a reason for hiding this comment

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

So I see 3 points in this motivation:

  • terraform-provider-ignition is stale and they are not taking on improvements. Which is the reason the Openstack team which is the heaviest user of the provider in terraform templates in the installer does not think they have the capacity to maintain a fork.
  • CA bundle use requires ign >2.3 schema and because of reason above updating the provider to support this is probalematic
  • Also the team wants to add bootstrap-shim.ign file to output of ignition-configs for all platforms as it would help Openstack UPI primarily but also other platforms.

correct?

Lets break this up into concise sections, one big flowing is difficult to read

Copy link
Author

Choose a reason for hiding this comment

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

Yeah that seems right. I'll just break this into bullets

### Goals

Our goal is to create an installer asset, written in Go, that creates an Ignition
shim for the bootstrap node. This shim should contain only the data
Copy link
Contributor

Choose a reason for hiding this comment

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

This shim should contain only the data
required to succesfully fetch the bootstrap Ignition config from its endpoint.

I would like to see examples of this endpoint in the user-stories.


We are proposing to create a new installer asset in `installer/pkg/asset/ignition/bootstrap/shim.go`.
This assest would have similar design to the [node asset] (https://github.com/openshift/installer/blob/master/pkg/asset/ignition/machine/node.go),
and will serve as a pointer to the main bootstrap ignition config. Platforms can further customize this
Copy link
Contributor

Choose a reason for hiding this comment

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

so the plan is to create a bootstrap-shim.ign file that has empty endpoint in Go, that the terraform code can edit to fill the actual endpoint?? like for AWS an s3 bucket's file URL created in terraform, Azure/GCP the signed URL to the storage account blob file again created in terraform.

For platforms like vSphere/baremetal, what is the expectation when the user sees this file, modify the ignition to add the expected endpoint?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think i want to put a file in front of the user (bootstrap-shim.ign) that requires modification from user and they will either most definitely fail to edit correctly.

Copy link
Author

Choose a reason for hiding this comment

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

take a look at this proof of concept: openshift/installer#2132

Copy link
Contributor

Choose a reason for hiding this comment

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

@iamemilio If you can point me to the changeset that would answer my question, that would be really nice

Copy link
Contributor

Choose a reason for hiding this comment

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

Also include the example of the shim.ign file here, that definitely useful here.


## Proposal

We are proposing to create a new installer asset in `installer/pkg/asset/ignition/bootstrap/shim.go`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Have you guys considered using string manipulation to create a 2.3.0 schema ign json file that has the CAbundle attached??

That would solve the IPI Openstack problem. and as for other platforms there isn't a need for this on cloud and i think for UPI we are more suited to helping our users creating a ign using bootstrap.ign using existing tools or making sure our UPI templates can safely generate those.

Copy link
Author

Choose a reason for hiding this comment

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

I was not aware of this solution, we havn't considered it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Would this also allow for Spec3 support? Possibly related: @vrutkovs had to remove the terraform plugin for now (and with it all of Azure support) in the Spec3 PR on Installer: openshift/installer#2548

Choose a reason for hiding this comment

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

That doesn't affect spec3 support much. Terraform provider still needs to be forked so that it could set ign version 3.0.0 in pointer ignition set in machine's user-data

Copy link
Author

Choose a reason for hiding this comment

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

Maybe in 4.4, the OpenStack and Azure teams could team up on making an OpenShift fork of the terraform-ignition-provider

Copy link

Choose a reason for hiding this comment

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

Maybe in 4.4, the OpenStack and Azure teams could team up on making an OpenShift fork of the terraform-ignition-provider

that would be brilliant because folks who started with 4.0 might have already got some investment in other wrappers around and moving away from it would cause disruption for sure

@iamemilio iamemilio closed this Nov 1, 2019
@iamemilio iamemilio reopened this Nov 12, 2019
@russellb
Copy link
Contributor

@iamemilio It looks like the underlying reason you needed this was resolved in openshift/installer#2587, is that right? I think we can just close this enhancement proposal now?

@iamemilio iamemilio closed this Oct 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants