Skip to content
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.

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

baremetal: send full ignition to masters #4427

Merged
merged 2 commits into from
Dec 4, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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
hosts = var.hosts
properties = var.properties
root_devices = var.root_devices
driver_infos = var.driver_infos
instance_infos = var.instance_infos
master_ignition_url = var.master_ignition_url
master_ignition_url_ca_cert = var.master_ignition_url_ca_cert
master_ignition_url_headers = var.master_ignition_url_headers
}
6 changes: 4 additions & 2 deletions data/data/baremetal/masters/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,10 @@ 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.master_ignition_url
user_data_url_ca_cert = var.master_ignition_url_ca_cert
user_data_url_headers = var.master_ignition_url_headers
}

data "ironic_introspection" "openshift-master-introspection" {
Expand Down
20 changes: 15 additions & 5 deletions data/data/baremetal/masters/variables.tf
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,6 @@ variable "master_count" {
default = 3
}

variable "ignition" {
type = string
description = "The content of the master ignition file"
}

variable "hosts" {
type = list(map(string))
description = "Hardware details for hosts"
Expand All @@ -33,3 +28,18 @@ variable "instance_infos" {
type = list(map(string))
description = "Instance information for hosts"
}

variable "master_ignition_url" {
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry for the confusion around the naming. These didn't need to change, as you correctly stated that there is already context in the path. You can leave it as is or change it back: Your call.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@staebler since CI passed I'm fine to leave it with the master prefix - it also makes it slightly more explicit when debugging and looking at the tfvars directly I guess

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

variable "master_ignition_url_ca_cert" {
type = string
description = "Root CA cert of the full ignition URL"
}

variable "master_ignition_url_headers" {
type = map(string)
description = "Headers to use when retrieving master_ignition_url"
}
15 changes: 15 additions & 0 deletions data/data/baremetal/variables-baremetal.tf
Original file line number Diff line number Diff line change
Expand Up @@ -52,3 +52,18 @@ variable "instance_infos" {
type = list(map(string))
description = "Instance information for hosts"
}

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

variable "master_ignition_url_ca_cert" {
type = string
description = "Root CA cert of the full ignition URL"
}

variable "master_ignition_url_headers" {
type = map(string)
description = "Headers to pass when retrieving master_ignition_url"
}
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ require (
github.com/metal3-io/baremetal-operator v0.0.0
github.com/metal3-io/cluster-api-provider-baremetal v0.0.0
github.com/mitchellh/cli v1.1.1
github.com/openshift-metal3/terraform-provider-ironic v0.2.3
github.com/openshift-metal3/terraform-provider-ironic v0.2.4
github.com/openshift/api v3.9.1-0.20191111211345-a27ff30ebf09+incompatible
github.com/openshift/client-go v0.0.0-20201020074620-f8fd44879f7c
github.com/openshift/cloud-credential-operator v0.0.0-20200316201045-d10080b52c9e
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -1396,6 +1396,8 @@ github.com/opencontainers/runtime-tools v0.0.0-20181011054405-1d69bd0f9c39/go.mo
github.com/opencontainers/selinux v1.5.2/go.mod h1:yTcKuYAh6R95iDpefGLQaPaRwJFwyzAJufJyiTt7s0g=
github.com/openshift-metal3/terraform-provider-ironic v0.2.3 h1:16pF9y0RN+8jz9h4EUBz5Uv1dhUTPcHrTKz4nzQdd3Q=
github.com/openshift-metal3/terraform-provider-ironic v0.2.3/go.mod h1:ux2W6gsCIYsY/fX5N0V0ZgwFEBNN7P8g6RlH6ACi97k=
github.com/openshift-metal3/terraform-provider-ironic v0.2.4 h1:AThAHxSvN18rdK3PqWJS73gMpOvjPka60LRu3IkIiR8=
github.com/openshift-metal3/terraform-provider-ironic v0.2.4/go.mod h1:ux2W6gsCIYsY/fX5N0V0ZgwFEBNN7P8g6RlH6ACi97k=
github.com/openshift/api v0.0.0-20200601094953-95abe2d2f422 h1:tgKcQVgHscJFBji1uLH5KjA81fGxNQkom5lETA5VURs=
github.com/openshift/api v0.0.0-20200601094953-95abe2d2f422/go.mod h1:l6TGeqJ92DrZBuWMNKcot1iZUHfbYSJyBWHGgg6Dn6s=
github.com/openshift/baremetal-operator v0.0.0-20200715132148-0f91f62a41fe h1:bu99IMkaN6o/JcxpWEb1eT8gDdL9hLcwOmfiVIbXWj8=
Expand Down
1 change: 1 addition & 0 deletions pkg/asset/cluster/tfvars.go
Original file line number Diff line number Diff line change
Expand Up @@ -442,6 +442,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
55 changes: 43 additions & 12 deletions pkg/tfvars/baremetal/baremetal.go
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,10 @@ type config struct {
IronicUsername string `json:"ironic_username"`
IronicPassword string `json:"ironic_password"`

MasterIgnitionURL string `json:"master_ignition_url,omitempty"`
MasterIgnitionURLCACert string `json:"master_ignition_url_ca_cert,omitempty"`
MasterIgnitionURLHeaders map[string]string `json:"master_ignition_url_headers,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 +40,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) {
hardys marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

I was still hoping to get this changed from ignition to masterIgnition, but I won't hold up the PR for it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I can push a follow-up PR to make that consistent

bootstrapOSImage, err := cache.DownloadImageFile(bootstrapOSImage)
if err != nil {
return nil, errors.Wrap(err, "failed to use cached bootstrap libvirt image")
Expand Down Expand Up @@ -156,18 +162,43 @@ func TFVars(libvirtURI, bootstrapProvisioningIP, bootstrapOSImage, externalBridg
})
}

var masterIgn igntypes.Config
if err := json.Unmarshal([]byte(ignition), &masterIgn); err != nil {
return nil, err
}
if len(masterIgn.Ignition.Config.Merge) == 0 {
return nil, errors.Wrap(err, "Empty Merge section in master pointer ignition")
}
ignitionURL := *masterIgn.Ignition.Config.Merge[0].Source
if len(masterIgn.Ignition.Security.TLS.CertificateAuthorities) == 0 {
return nil, errors.Wrap(err, "Empty CertificateAuthorities section in master pointer ignition")
}
ignitionURLCACert := strings.TrimPrefix(
*masterIgn.Ignition.Security.TLS.CertificateAuthorities[0].Source,
"data:text/plain;charset=utf-8;base64,")
// To return the same version as the stub config, the MCS requires a
// header, otherwise we get 2.2.0, e.g:
// "Accept: application/vnd.coreos.ignition+json; version=3.1.0"
ignitionURLHeaders := map[string]string{
"Accept": fmt.Sprintf("application/vnd.coreos.ignition+json;version=%s",
masterIgn.Ignition.Version),
}

cfg := &config{
LibvirtURI: libvirtURI,
BootstrapProvisioningIP: bootstrapProvisioningIP,
BootstrapOSImage: bootstrapOSImage,
IronicUsername: ironicUsername,
IronicPassword: ironicPassword,
Hosts: hosts,
Bridges: bridges,
Properties: properties,
DriverInfos: driverInfos,
RootDevices: rootDevices,
InstanceInfos: instanceInfos,
LibvirtURI: libvirtURI,
BootstrapProvisioningIP: bootstrapProvisioningIP,
BootstrapOSImage: bootstrapOSImage,
IronicUsername: ironicUsername,
IronicPassword: ironicPassword,
Hosts: hosts,
Bridges: bridges,
Properties: properties,
DriverInfos: driverInfos,
RootDevices: rootDevices,
InstanceInfos: instanceInfos,
MasterIgnitionURL: ignitionURL,
MasterIgnitionURLCACert: ignitionURLCACert,
MasterIgnitionURLHeaders: ignitionURLHeaders,
}

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

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion vendor/modules.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1005,7 +1005,7 @@ github.com/opencontainers/go-digest
# github.com/opencontainers/image-spec v1.0.2-0.20190823105129-775207bd45b6
github.com/opencontainers/image-spec/specs-go
github.com/opencontainers/image-spec/specs-go/v1
# github.com/openshift-metal3/terraform-provider-ironic v0.2.3
# github.com/openshift-metal3/terraform-provider-ironic v0.2.4
## explicit
github.com/openshift-metal3/terraform-provider-ironic/ironic
# github.com/openshift/api v3.9.1-0.20191111211345-a27ff30ebf09+incompatible => github.com/openshift/api v0.0.0-20200601094953-95abe2d2f422
Expand Down