Bug 2025862: Improve host role management during assets creation

data/data/baremetal/main.tf

@@ -28,7 +28,7 @@ module "masters" {
 
   master_count   = var.master_count
   ignition       = var.ignition_master
-  hosts          = var.hosts
+  masters        = var.masters
   properties     = var.properties
   root_devices   = var.root_devices
   driver_infos   = var.driver_infos

data/data/baremetal/masters/main.tf

@@ -1,6 +1,6 @@
 resource "ironic_node_v1" "openshift-master-host" {
   count          = var.master_count
-  name           = var.hosts[count.index]["name"]
+  name           = var.masters[count.index]["name"]
   resource_class = "baremetal"
 
   inspect   = true
@@ -9,22 +9,22 @@ resource "ironic_node_v1" "openshift-master-host" {
 
   ports = [
     {
-      address     = var.hosts[count.index]["port_address"]
+      address     = var.masters[count.index]["port_address"]
       pxe_enabled = "true"
     },
   ]
 
   properties  = var.properties[count.index]
   root_device = var.root_devices[count.index]
 
-  driver      = var.hosts[count.index]["driver"]
+  driver      = var.masters[count.index]["driver"]
   driver_info = var.driver_infos[count.index]
 
-  boot_interface       = var.hosts[count.index]["boot_interface"]
-  management_interface = var.hosts[count.index]["management_interface"]
-  power_interface      = var.hosts[count.index]["power_interface"]
-  raid_interface       = var.hosts[count.index]["raid_interface"]
-  vendor_interface     = var.hosts[count.index]["vendor_interface"]
+  boot_interface       = var.masters[count.index]["boot_interface"]
+  management_interface = var.masters[count.index]["management_interface"]
+  power_interface      = var.masters[count.index]["power_interface"]
+  raid_interface       = var.masters[count.index]["raid_interface"]
+  vendor_interface     = var.masters[count.index]["vendor_interface"]
 }
 
 resource "ironic_deployment" "openshift-master-deployment" {

data/data/baremetal/masters/variables.tf

@@ -9,27 +9,27 @@ variable "ignition" {
   description = "The content of the master ignition file"
 }
 
-variable "hosts" {
+variable "masters" {
   type        = list(map(string))
-  description = "Hardware details for hosts"
+  description = "Hardware details for masters"
 }
 
 variable "properties" {
   type        = list(map(string))
-  description = "Properties for hosts"
+  description = "Properties for masters"
 }
 
 variable "root_devices" {
   type        = list(map(string))
-  description = "Root devices for hosts"
+  description = "Root devices for masters"
 }
 
 variable "driver_infos" {
   type        = list(map(string))
-  description = "BMC information for hosts"
+  description = "BMC information for masters"
 }
 
 variable "instance_infos" {
   type        = list(map(string))
-  description = "Instance information for hosts"
+  description = "Instance information for masters"
 }

data/data/baremetal/variables-baremetal.tf

@@ -28,9 +28,9 @@ variable "ironic_password" {
   description = "Password for authentication to Ironic"
 }
 
-variable "hosts" {
+variable "masters" {
   type        = list(map(string))
-  description = "Hardware details for hosts"
+  description = "Hardware details for masters"
 }
 
 variable "bridges" {
@@ -40,20 +40,20 @@ variable "bridges" {
 
 variable "properties" {
   type        = list(map(string))
-  description = "Properties for hosts"
+  description = "Properties for masters"
 }
 
 variable "root_devices" {
   type        = list(map(string))
-  description = "Root devices for hosts"
+  description = "Root devices for masters"
 }
 
 variable "driver_infos" {
   type        = list(map(string))
-  description = "BMC information for hosts"
+  description = "BMC information for masters"
 }
 
 variable "instance_infos" {
   type        = list(map(string))
-  description = "Instance information for hosts"
+  description = "Instance information for masters"
 }

pkg/asset/cluster/tfvars.go

@@ -574,6 +574,7 @@ func (t *TerraformVariables) Generate(parents asset.Parents) error {
 		}
 
 		data, err = baremetaltfvars.TFVars(
+			*installConfig.Config.ControlPlane.Replicas,
 			installConfig.Config.Platform.BareMetal.LibvirtURI,
 			installConfig.Config.Platform.BareMetal.APIVIP,
 			imageCacheIP,

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

@@ -74,7 +74,7 @@ func GetTemplateData(config *baremetal.Platform, networks []types.MachineNetwork
 
 		var dhcpAllowList []string
 		for _, host := range config.Hosts {
-			if host.Role == "master" {
+			if host.IsMaster() {
 				dhcpAllowList = append(dhcpAllowList, host.BootMACAddress)
 			}
 		}

pkg/asset/machines/baremetal/hosts.go

@@ -12,6 +12,7 @@ import (
 	baremetalhost "github.com/metal3-io/baremetal-operator/apis/metal3.io/v1alpha1"
 
 	"github.com/openshift/installer/pkg/types"
+	"github.com/openshift/installer/pkg/types/baremetal"
 )
 
 // HostSettings hold the information needed to build the manifests to
@@ -25,6 +26,64 @@ type HostSettings struct {
 	Secrets []corev1.Secret
 }
 
+func createSecret(host *baremetal.Host) (*corev1.Secret, baremetalhost.BMCDetails) {
+	bmc := baremetalhost.BMCDetails{}
+	if host.BMC.Username != "" && host.BMC.Password != "" {
+		// Each host needs a secret to hold the credentials for
+		// communicating with the management controller that drives
+		// that host.
+		secret := &corev1.Secret{
+			TypeMeta: metav1.TypeMeta{
+				APIVersion: "v1",
+				Kind:       "Secret",
+			},
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      fmt.Sprintf("%s-bmc-secret", host.Name),
+				Namespace: "openshift-machine-api",
+			},
+			Data: map[string][]byte{
+				"username": []byte(host.BMC.Username),
+				"password": []byte(host.BMC.Password),
+			},
+		}
+		bmc.Address = host.BMC.Address
+		bmc.CredentialsName = secret.Name
+		bmc.DisableCertificateVerification = host.BMC.DisableCertificateVerification
+
+		return secret, bmc
+	}
+	return nil, bmc
+}
+
+func createBaremetalHost(host *baremetal.Host, bmc baremetalhost.BMCDetails) baremetalhost.BareMetalHost {
+
+	// Map string 'default' to hardware.DefaultProfileName
+	if host.HardwareProfile == "default" {
+		host.HardwareProfile = hardware.DefaultProfileName
+	}
+
+	newHost := baremetalhost.BareMetalHost{
+		TypeMeta: metav1.TypeMeta{
+			APIVersion: baremetalhost.GroupVersion.String(),
+			Kind:       "BareMetalHost",
+		},
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      host.Name,
+			Namespace: "openshift-machine-api",
+		},
+		Spec: baremetalhost.BareMetalHostSpec{
+			Online:          true,
+			BMC:             bmc,
+			BootMACAddress:  host.BootMACAddress,
+			HardwareProfile: host.HardwareProfile,
+			BootMode:        baremetalhost.BootMode(host.BootMode),
+			RootDeviceHints: host.RootDeviceHints.MakeCRDHints(),
+		},
+	}
+
+	return newHost
+}
+
 // Hosts returns the HostSettings with details of the hardware being
 // used to construct the cluster.
 func Hosts(config *types.InstallConfig, machines []machineapi.Machine) (*HostSettings, error) {
@@ -34,76 +93,37 @@ func Hosts(config *types.InstallConfig, machines []machineapi.Machine) (*HostSet
 		return nil, fmt.Errorf("no baremetal platform in configuration")
 	}
 
-	for i, host := range config.Platform.BareMetal.Hosts {
-		bmc := baremetalhost.BMCDetails{}
-		if host.BMC.Username != "" && host.BMC.Password != "" {
-			// Each host needs a secret to hold the credentials for
-			// communicating with the management controller that drives
-			// that host.
-			secret := corev1.Secret{
-				TypeMeta: metav1.TypeMeta{
-					APIVersion: "v1",
-					Kind:       "Secret",
-				},
-				ObjectMeta: metav1.ObjectMeta{
-					Name:      fmt.Sprintf("%s-bmc-secret", host.Name),
-					Namespace: "openshift-machine-api",
-				},
-				Data: map[string][]byte{
-					"username": []byte(host.BMC.Username),
-					"password": []byte(host.BMC.Password),
-				},
-			}
-			bmc.Address = host.BMC.Address
-			bmc.CredentialsName = secret.Name
-			bmc.DisableCertificateVerification = host.BMC.DisableCertificateVerification
-			settings.Secrets = append(settings.Secrets, secret)
-		}
+	numRequiredMasters := len(machines)
+	numMasters := 0
+	for _, host := range config.Platform.BareMetal.Hosts {
 
-		// Map string 'default' to hardware.DefaultProfileName
-		if host.HardwareProfile == "default" {
-			host.HardwareProfile = hardware.DefaultProfileName
+		secret, bmc := createSecret(host)
+		if secret != nil {
+			settings.Secrets = append(settings.Secrets, *secret)
 		}
+		newHost := createBaremetalHost(host, bmc)
 
-		newHost := baremetalhost.BareMetalHost{
-			TypeMeta: metav1.TypeMeta{
-				APIVersion: baremetalhost.GroupVersion.String(),
-				Kind:       "BareMetalHost",
-			},
-			ObjectMeta: metav1.ObjectMeta{
-				Name:      host.Name,
-				Namespace: "openshift-machine-api",
-			},
-			Spec: baremetalhost.BareMetalHostSpec{
-				Online:          true,
-				BMC:             bmc,
-				BootMACAddress:  host.BootMACAddress,
-				HardwareProfile: host.HardwareProfile,
-				BootMode:        baremetalhost.BootMode(host.BootMode),
-				RootDeviceHints: host.RootDeviceHints.MakeCRDHints(),
-			},
-		}
-		if i < len(machines) {
+		if !host.IsWorker() && numMasters < numRequiredMasters {
 			// Setting ExternallyProvisioned to true and adding a
 			// ConsumerRef without setting Image associates the host
 			// with a machine without triggering provisioning. We only
-			// want to do that for control plane hosts. We assume the
-			// first known hosts are the control plane and that the
-			// hosts are in the same order as the control plane
-			// machines.
+			// want to do that for control plane hosts.
 			newHost.Spec.ExternallyProvisioned = true
 			// Pause reconciliation until we can annotate with the initial
 			// status containing the HardwareDetails
 			newHost.ObjectMeta.Annotations = map[string]string{"baremetalhost.metal3.io/paused": ""}
-			machine := machines[i]
+			// Link the new host to the currently available machine
+			machine := machines[numMasters]
 			newHost.Spec.ConsumerRef = &corev1.ObjectReference{
 				APIVersion: machine.TypeMeta.APIVersion,
 				Kind:       machine.TypeMeta.Kind,
 				Namespace:  machine.ObjectMeta.Namespace,
 				Name:       machine.ObjectMeta.Name,
 			}
 			newHost.Spec.Online = true
+			numMasters++
 		}
+
 		settings.Hosts = append(settings.Hosts, newHost)
 	}