Skip to content

baremetal: send full ignition to masters #4359

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.

Sign up for GitHub

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

Closed
kirankt wants to merge 1 commit into from
Closed

baremetal: send full ignition to masters #4359

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 9 additions & 7 deletions data/data/baremetal/main.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,13 @@ module "bootstrap" {
module "masters" {
source = "./masters"

master_count = var.master_count
ignition = var.ignition_master
hosts = var.hosts
properties = var.properties
root_devices = var.root_devices
driver_infos = var.driver_infos
instance_infos = var.instance_infos
master_count = var.master_count
ignition = var.ignition_master
hosts = var.hosts
properties = var.properties
root_devices = var.root_devices
driver_infos = var.driver_infos
instance_infos = var.instance_infos
ignition_url = var.ignition_url
ignition_url_ca_cert = var.ignition_url_ca_cert
}
5 changes: 3 additions & 2 deletions data/data/baremetal/masters/main.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,9 @@ resource "ironic_deployment" "openshift-master-deployment" {
count.index,
)

instance_info = var.instance_infos[count.index]
user_data = var.ignition
instance_info = var.instance_infos[count.index]
user_data_url = var.ignition_url
user_data_url_ca_cert = var.ignition_url_ca_cert
}

data "ironic_introspection" "openshift-master-introspection" {
Expand Down
10 changes: 10 additions & 0 deletions data/data/baremetal/masters/variables.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,3 +33,13 @@ variable "instance_infos" {
type = list(map(string))
description = "Instance information for hosts"
}

variable "ignition_url" {
type = string
description = "The URL of the full ignition"
}

variable "ignition_url_ca_cert" {
type = string
description = "Root CA cert of the full ignition URL"
}
10 changes: 10 additions & 0 deletions data/data/baremetal/variables-baremetal.tf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,3 +52,13 @@ variable "instance_infos" {
type = list(map(string))
description = "Instance information for hosts"
}

variable "ignition_url" {
type = string
description = "The URL of the full ignition"
}

variable "ignition_url_ca_cert" {
type = string
description = "Root CA cert of the full ignition URL"
}
3 changes: 3 additions & 0 deletions pkg/asset/cluster/tfvars.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import (
"github.com/openshift/installer/pkg/asset/machines"
"github.com/openshift/installer/pkg/asset/openshiftinstall"
"github.com/openshift/installer/pkg/asset/rhcos"
"github.com/openshift/installer/pkg/asset/tls"
rhcospkg "github.com/openshift/installer/pkg/rhcos"
"github.com/openshift/installer/pkg/tfvars"
awstfvars "github.com/openshift/installer/pkg/tfvars/aws"
Expand Down Expand Up @@ -92,6 +93,7 @@ func (t *TerraformVariables) Dependencies() []asset.Asset {
&machines.Master{},
&machines.Worker{},
&baremetalbootstrap.IronicCreds{},
&tls.RootCA{},
}
}

Expand Down Expand Up @@ -442,6 +444,7 @@ func (t *TerraformVariables) Generate(parents asset.Parents) error {
string(*rhcosImage),
ironicCreds.Username,
ironicCreds.Password,
masterIgn,
)
if err != nil {
return errors.Wrapf(err, "failed to get %s Terraform variables", platform)
Expand Down
16 changes: 15 additions & 1 deletion pkg/tfvars/baremetal/baremetal.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import (
"path"
"strings"

igntypes "github.com/coreos/ignition/v2/config/v3_1/types"

"github.com/metal3-io/baremetal-operator/pkg/bmc"
"github.com/metal3-io/baremetal-operator/pkg/hardware"
"github.com/openshift/installer/pkg/tfvars/internal/cache"
Expand All @@ -25,6 +27,9 @@ type config struct {
IronicUsername string `json:"ironic_username"`
IronicPassword string `json:"ironic_password"`

IgnitionURL string `json:"ignition_url,omitempty"`
IgnitionURLCACert string `json:"ignition_url_ca_cert,omitempty"`

// Data required for control plane deployment - several maps per host, because of terraform's limitations
Hosts []map[string]interface{} `json:"hosts"`
RootDevices []map[string]interface{} `json:"root_devices"`
Expand All @@ -34,7 +39,7 @@ type config struct {
}

// TFVars generates bare metal specific Terraform variables.
func TFVars(libvirtURI, bootstrapProvisioningIP, bootstrapOSImage, externalBridge, externalMAC, provisioningBridge, provisioningMAC string, platformHosts []*baremetal.Host, image, ironicUsername, ironicPassword string) ([]byte, error) {
func TFVars(libvirtURI, bootstrapProvisioningIP, bootstrapOSImage, externalBridge, externalMAC, provisioningBridge, provisioningMAC string, platformHosts []*baremetal.Host, image, ironicUsername, ironicPassword, ignition string) ([]byte, error) {
bootstrapOSImage, err := cache.DownloadImageFile(bootstrapOSImage)
if err != nil {
return nil, errors.Wrap(err, "failed to use cached bootstrap libvirt image")
Expand Down Expand Up @@ -155,6 +160,13 @@ func TFVars(libvirtURI, bootstrapProvisioningIP, bootstrapOSImage, externalBridg
})
}

var masterIgn igntypes.Config
if err := json.Unmarshal([]byte(ignition), &masterIgn); err != nil {
return nil, err
}
ignitionURL := masterIgn.Ignition.Config.Merge[0].Source
Copy link
Copy Markdown
Member

@stbenjam stbenjam Nov 13, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

So we're extracting the merge out of the stub ignition: what happens to user customizations? Does the ignition from MCS contain them? I know there were some discussions about that but I haven't followed the progress.

Copy link
Copy Markdown
Contributor Author

@kirankt kirankt Nov 13, 2020

Choose a reason for hiding this comment

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

Yes, this is just a minor rework of extracting the URL and CA cert. Regarding user customizations, the current idea is to compare the original stub with any user customizations and, if they are different, the installer will create the appropriate machineconfig manifest for the change. This way when MCO becomes active, the final rendered ignition will contain the user's modification. This PR will go to work as usual and pass the URL and cert to the TF provider, which will extract the rendered ignition.

Copy link
Copy Markdown
Contributor Author

@kirankt kirankt Nov 13, 2020

Choose a reason for hiding this comment

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

Forgot to mention that there will be another PR that will be responsible for creating the machineconfig to reconcile any changes to the stub ignition.

Copy link
Copy Markdown

@hardys hardys Nov 24, 2020

Choose a reason for hiding this comment

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

Any customizations should be accessible via the MCS rendered config if we rebase this on #4413

ignitionURLCACert := masterIgn.Ignition.Security.TLS.CertificateAuthorities[0].Source
Copy link
Copy Markdown

@hardys hardys Nov 27, 2020

Choose a reason for hiding this comment

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

terraform-provider-ignition currently expects just the base64 encoded cert, but this passes the entire dataurl - we need to strip the dataurl prefix here until/unless we rework the provider to accept the full dataurl


cfg := &config{
LibvirtURI: libvirtURI,
BootstrapProvisioningIP: bootstrapProvisioningIP,
Expand All @@ -167,6 +179,8 @@ func TFVars(libvirtURI, bootstrapProvisioningIP, bootstrapOSImage, externalBridg
DriverInfos: driverInfos,
RootDevices: rootDevices,
InstanceInfos: instanceInfos,
IgnitionURL: *ignitionURL,
IgnitionURLCACert: *ignitionURLCACert,
}

return json.MarshalIndent(cfg, "", " ")
Expand Down