diff --git a/data/data/baremetal/main.tf b/data/data/baremetal/main.tf index 1c5c634e331..bdfe4127911 100644 --- a/data/data/baremetal/main.tf +++ b/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 diff --git a/data/data/baremetal/masters/main.tf b/data/data/baremetal/masters/main.tf index f30d93a565e..26439f7ca7c 100644 --- a/data/data/baremetal/masters/main.tf +++ b/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,7 +9,7 @@ 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" }, ] @@ -17,14 +17,14 @@ resource "ironic_node_v1" "openshift-master-host" { 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" { diff --git a/data/data/baremetal/masters/variables.tf b/data/data/baremetal/masters/variables.tf index b6f39fec3d5..7475593911e 100644 --- a/data/data/baremetal/masters/variables.tf +++ b/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" } diff --git a/data/data/baremetal/variables-baremetal.tf b/data/data/baremetal/variables-baremetal.tf index 8203223ba13..b62ae3bc655 100644 --- a/data/data/baremetal/variables-baremetal.tf +++ b/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" } diff --git a/pkg/asset/cluster/tfvars.go b/pkg/asset/cluster/tfvars.go index d8c93232216..0076fd94842 100644 --- a/pkg/asset/cluster/tfvars.go +++ b/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, diff --git a/pkg/asset/ignition/bootstrap/baremetal/template.go b/pkg/asset/ignition/bootstrap/baremetal/template.go index 69cef1c333b..fe8a07fbb66 100644 --- a/pkg/asset/ignition/bootstrap/baremetal/template.go +++ b/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) } } diff --git a/pkg/asset/machines/baremetal/hosts.go b/pkg/asset/machines/baremetal/hosts.go index d4aa46f3b95..7ba7fa0f7b6 100644 --- a/pkg/asset/machines/baremetal/hosts.go +++ b/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,68 +93,27 @@ 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, @@ -103,7 +121,9 @@ func Hosts(config *types.InstallConfig, machines []machineapi.Machine) (*HostSet Name: machine.ObjectMeta.Name, } newHost.Spec.Online = true + numMasters++ } + settings.Hosts = append(settings.Hosts, newHost) } diff --git a/pkg/asset/machines/baremetal/hosts_test.go b/pkg/asset/machines/baremetal/hosts_test.go index 4e7aac0f481..724be20e6d3 100644 --- a/pkg/asset/machines/baremetal/hosts_test.go +++ b/pkg/asset/machines/baremetal/hosts_test.go @@ -5,6 +5,7 @@ import ( "testing" machineapi "github.com/openshift/machine-api-operator/pkg/apis/machine/v1beta1" + "github.com/stretchr/testify/assert" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -14,240 +15,457 @@ import ( baremetalhost "github.com/metal3-io/baremetal-operator/apis/metal3.io/v1alpha1" ) -func makeConfig(nHosts int) *types.InstallConfig { - config := &types.InstallConfig{ - Platform: types.Platform{ - BareMetal: &baremetaltypes.Platform{}, +func TestHosts(t *testing.T) { + testCases := []struct { + Scenario string + Machines []machineapi.Machine + Config *types.InstallConfig + ExpectedSecrets []corev1.Secret + ExpectedHosts []baremetalhost.BareMetalHost + ExpectedError string + ExpectedSetting *HostSettings + }{ + { + Scenario: "no-platform", + Config: &types.InstallConfig{ + Platform: types.Platform{ + BareMetal: nil, + }, + }, + + ExpectedError: "no baremetal platform in configuration", + }, + { + Scenario: "no-hosts", + Config: config().build(), + + ExpectedSetting: settings().build(), + }, + { + Scenario: "default", + Machines: machines(machine("machine-0")), + Config: configHosts(hostType("master-0").bmc("usr0", "pwd0").role("master")), + + ExpectedSetting: settings(). + secrets(secret("master-0-bmc-secret").data("usr0", "pwd0")). + hosts(host("master-0").consumerRef("machine-0").annotation("baremetalhost.metal3.io/paused", "").externallyProvisioned()).build(), + }, + { + Scenario: "default-norole", + Machines: machines(machine("machine-0")), + Config: configHosts(hostType("master-0").bmc("usr0", "pwd0")), + + ExpectedSetting: settings(). + secrets(secret("master-0-bmc-secret").data("usr0", "pwd0")). + hosts(host("master-0").consumerRef("machine-0").annotation("baremetalhost.metal3.io/paused", "").externallyProvisioned()).build(), + }, + { + Scenario: "3-hosts-3-machines-norole-all", + Machines: machines( + machine("machine-0"), + machine("machine-1"), + machine("machine-2")), + Config: configHosts( + hostType("master-0").bmc("usr0", "pwd0"), + hostType("master-1").bmc("usr1", "pwd1"), + hostType("master-2").bmc("usr2", "pwd2")), + + ExpectedSetting: settings(). + secrets( + secret("master-0-bmc-secret").data("usr0", "pwd0"), + secret("master-1-bmc-secret").data("usr1", "pwd1"), + secret("master-2-bmc-secret").data("usr2", "pwd2")). + hosts( + host("master-0").consumerRef("machine-0").annotation("baremetalhost.metal3.io/paused", "").externallyProvisioned(), + host("master-1").consumerRef("machine-1").annotation("baremetalhost.metal3.io/paused", "").externallyProvisioned(), + host("master-2").consumerRef("machine-2").annotation("baremetalhost.metal3.io/paused", "").externallyProvisioned()).build(), + }, + { + Scenario: "4-hosts-3-machines", + Machines: machines( + machine("machine-0"), + machine("machine-1"), + machine("machine-2")), + Config: configHosts( + hostType("master-0").bmc("usr0", "pwd0").role("master"), + hostType("master-1").bmc("usr1", "pwd1").role("master"), + hostType("master-2").bmc("usr2", "pwd2").role("master"), + hostType("master-3").bmc("usr3", "pwd3").role("worker")), + + ExpectedSetting: settings(). + secrets( + secret("master-0-bmc-secret").data("usr0", "pwd0"), + secret("master-1-bmc-secret").data("usr1", "pwd1"), + secret("master-2-bmc-secret").data("usr2", "pwd2"), + secret("master-3-bmc-secret").data("usr3", "pwd3")). + hosts( + host("master-0").consumerRef("machine-0").annotation("baremetalhost.metal3.io/paused", "").externallyProvisioned(), + host("master-1").consumerRef("machine-1").annotation("baremetalhost.metal3.io/paused", "").externallyProvisioned(), + host("master-2").consumerRef("machine-2").annotation("baremetalhost.metal3.io/paused", "").externallyProvisioned(), + host("master-3")).build(), + }, + { + Scenario: "4-hosts-3-machines-norole", + Machines: machines( + machine("machine-0"), + machine("machine-1"), + machine("machine-2")), + Config: configHosts( + hostType("master-0").bmc("usr0", "pwd0"), + hostType("master-1").bmc("usr1", "pwd1"), + hostType("master-2").bmc("usr2", "pwd2"), + hostType("worker-0").bmc("wrk0", "pwd0")), + + ExpectedSetting: settings(). + secrets( + secret("master-0-bmc-secret").data("usr0", "pwd0"), + secret("master-1-bmc-secret").data("usr1", "pwd1"), + secret("master-2-bmc-secret").data("usr2", "pwd2"), + secret("worker-0-bmc-secret").data("wrk0", "pwd0")). + hosts( + host("master-0").consumerRef("machine-0").annotation("baremetalhost.metal3.io/paused", "").externallyProvisioned(), + host("master-1").consumerRef("machine-1").annotation("baremetalhost.metal3.io/paused", "").externallyProvisioned(), + host("master-2").consumerRef("machine-2").annotation("baremetalhost.metal3.io/paused", "").externallyProvisioned(), + host("worker-0")).build(), + }, + { + Scenario: "5-hosts-3-machines", + Machines: machines( + machine("machine-0"), + machine("machine-1"), + machine("machine-2")), + Config: configHosts( + hostType("master-0").bmc("usr0", "pwd0").role("master"), + hostType("master-1").bmc("usr1", "pwd1").role("master"), + hostType("master-2").bmc("usr2", "pwd2").role("master"), + hostType("worker-0").bmc("wrk0", "pwd0").role("worker"), + hostType("worker-1").bmc("wrk1", "pwd1").role("worker")), + + ExpectedSetting: settings(). + secrets( + secret("master-0-bmc-secret").data("usr0", "pwd0"), + secret("master-1-bmc-secret").data("usr1", "pwd1"), + secret("master-2-bmc-secret").data("usr2", "pwd2"), + secret("worker-0-bmc-secret").data("wrk0", "pwd0"), + secret("worker-1-bmc-secret").data("wrk1", "pwd1")). + hosts( + host("master-0").consumerRef("machine-0").annotation("baremetalhost.metal3.io/paused", "").externallyProvisioned(), + host("master-1").consumerRef("machine-1").annotation("baremetalhost.metal3.io/paused", "").externallyProvisioned(), + host("master-2").consumerRef("machine-2").annotation("baremetalhost.metal3.io/paused", "").externallyProvisioned(), + host("worker-0"), + host("worker-1")).build(), + }, + { + Scenario: "5-hosts-3-machines-mixed", + Machines: machines( + machine("machine-0"), + machine("machine-1"), + machine("machine-2")), + Config: configHosts( + hostType("master-1").bmc("usr1", "pwd1").role("master"), + hostType("worker-0").bmc("wrk0", "pwd0").role("worker"), + hostType("worker-1").bmc("wrk1", "pwd1").role("worker"), + hostType("master-0").bmc("usr0", "pwd0"), + hostType("master-2").bmc("usr2", "pwd2")), + + ExpectedSetting: settings(). + secrets( + secret("master-1-bmc-secret").data("usr1", "pwd1"), + secret("worker-0-bmc-secret").data("wrk0", "pwd0"), + secret("worker-1-bmc-secret").data("wrk1", "pwd1"), + secret("master-0-bmc-secret").data("usr0", "pwd0"), + secret("master-2-bmc-secret").data("usr2", "pwd2")). + hosts( + host("master-1").consumerRef("machine-0").annotation("baremetalhost.metal3.io/paused", "").externallyProvisioned(), + host("worker-0"), + host("worker-1"), + host("master-0").consumerRef("machine-1").annotation("baremetalhost.metal3.io/paused", "").externallyProvisioned(), + host("master-2").consumerRef("machine-2").annotation("baremetalhost.metal3.io/paused", "").externallyProvisioned()).build(), + }, + { + Scenario: "4-hosts-3-machines-norole-master", + Machines: machines( + machine("machine-0"), + machine("machine-1"), + machine("machine-2")), + Config: configHosts( + hostType("worker-0").bmc("wrk0", "pwd0").role("worker"), + hostType("master-0").bmc("usr0", "pwd0"), + hostType("master-1").bmc("usr1", "pwd1"), + hostType("master-2").bmc("usr2", "pwd2")), + + ExpectedSetting: settings(). + secrets( + secret("worker-0-bmc-secret").data("wrk0", "pwd0"), + secret("master-0-bmc-secret").data("usr0", "pwd0"), + secret("master-1-bmc-secret").data("usr1", "pwd1"), + secret("master-2-bmc-secret").data("usr2", "pwd2")). + hosts( + host("worker-0"), + host("master-0").consumerRef("machine-0").annotation("baremetalhost.metal3.io/paused", "").externallyProvisioned(), + host("master-1").consumerRef("machine-1").annotation("baremetalhost.metal3.io/paused", "").externallyProvisioned(), + host("master-2").consumerRef("machine-2").annotation("baremetalhost.metal3.io/paused", "").externallyProvisioned()).build(), + }, + { + Scenario: "4-hosts-3-machines-norole-worker", + Machines: machines( + machine("machine-0"), + machine("machine-1"), + machine("machine-2")), + Config: configHosts( + hostType("master-0").bmc("usr0", "pwd0").role("master"), + hostType("master-1").bmc("usr1", "pwd1").role("master"), + hostType("master-2").bmc("usr2", "pwd2").role("master"), + hostType("worker-0").bmc("wrk0", "pwd0")), + + ExpectedSetting: settings(). + secrets( + secret("master-0-bmc-secret").data("usr0", "pwd0"), + secret("master-1-bmc-secret").data("usr1", "pwd1"), + secret("master-2-bmc-secret").data("usr2", "pwd2"), + secret("worker-0-bmc-secret").data("wrk0", "pwd0")). + hosts( + host("master-0").consumerRef("machine-0").annotation("baremetalhost.metal3.io/paused", "").externallyProvisioned(), + host("master-1").consumerRef("machine-1").annotation("baremetalhost.metal3.io/paused", "").externallyProvisioned(), + host("master-2").consumerRef("machine-2").annotation("baremetalhost.metal3.io/paused", "").externallyProvisioned(), + host("worker-0")).build(), }, } - config.Platform.BareMetal.Hosts = make([]*baremetaltypes.Host, nHosts) - for i := 0; i < nHosts; i++ { - config.Platform.BareMetal.Hosts[i] = &baremetaltypes.Host{ - Name: fmt.Sprintf("host%d", i), - BMC: baremetaltypes.BMC{ - Username: fmt.Sprintf("user%d", i), - Password: fmt.Sprintf("password%d", i), - }, - } + for _, tc := range testCases { + t.Run(tc.Scenario, func(t *testing.T) { + settings, err := Hosts(tc.Config, tc.Machines) + + if tc.ExpectedError != "" { + assert.EqualError(t, err, tc.ExpectedError) + } + + if tc.ExpectedSetting != nil { + for i, h := range tc.ExpectedSetting.Hosts { + assert.Equal(t, h, settings.Hosts[i], fmt.Sprintf("%s and %s are not equal", h.Name, settings.Hosts[i].Name)) + } + + for i, s := range tc.ExpectedSetting.Secrets { + assert.Equal(t, s, settings.Secrets[i], s.Name, fmt.Sprintf("%s and %s are not equal", s.Name, settings.Secrets[i].Name)) + } + } + }) } +} - return config +func configHosts(builders ...*hostTypeBuilder) *types.InstallConfig { + return config().hosts(builders...).build() } -func makeMachines(n int) []machineapi.Machine { - results := []machineapi.Machine{} +type installConfigBuilder struct { + types.InstallConfig +} - for i := 0; i < n; i++ { - results = append(results, machineapi.Machine{ - ObjectMeta: metav1.ObjectMeta{ - Name: fmt.Sprintf("machine%d", i), +func (ib *installConfigBuilder) build() *types.InstallConfig { + return &ib.InstallConfig +} + +func config() *installConfigBuilder { + return &installConfigBuilder{ + types.InstallConfig{ + Platform: types.Platform{ + BareMetal: &baremetaltypes.Platform{}, }, - }) + }, } +} - return results +func (ib *installConfigBuilder) hosts(builders ...*hostTypeBuilder) *installConfigBuilder { + ib.Platform.BareMetal.Hosts = []*baremetaltypes.Host{} + + for _, hb := range builders { + ib.Platform.BareMetal.Hosts = append(ib.Platform.BareMetal.Hosts, hb.build()) + } + return ib } -func TestHosts(t *testing.T) { - testCases := []struct { - Scenario string - Machines []machineapi.Machine - Config *types.InstallConfig - Expected HostSettings - ExpectError bool - }{ - { - Scenario: "no platform", - Config: &types.InstallConfig{}, - ExpectError: true, - }, +type hostTypeBuilder struct { + baremetaltypes.Host +} - { - Scenario: "no hosts", - Config: makeConfig(0), - Expected: HostSettings{}, +func (htb *hostTypeBuilder) build() *baremetaltypes.Host { + return &htb.Host +} + +func hostType(name string) *hostTypeBuilder { + return &hostTypeBuilder{ + Host: baremetaltypes.Host{ + Name: name, + BootMACAddress: "c0:ff:ee:ca:fe:00", + BootMode: baremetaltypes.UEFI, + RootDeviceHints: &baremetaltypes.RootDeviceHints{ + DeviceName: "userd_devicename", + }, }, + } +} - { - Scenario: "3 hosts and machines", - Config: makeConfig(3), - Machines: makeMachines(3), - Expected: HostSettings{ - Hosts: []baremetalhost.BareMetalHost{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "host0", - }, - Spec: baremetalhost.BareMetalHostSpec{ - Online: true, - BMC: baremetalhost.BMCDetails{ - CredentialsName: "host0-bmc-secret", - }, - ConsumerRef: &corev1.ObjectReference{ - Name: "machine0", - }, - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "host1", - }, - Spec: baremetalhost.BareMetalHostSpec{ - Online: true, - BMC: baremetalhost.BMCDetails{ - CredentialsName: "host1-bmc-secret", - }, - ConsumerRef: &corev1.ObjectReference{ - Name: "machine1", - }, - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "host2", - }, - Spec: baremetalhost.BareMetalHostSpec{ - Online: true, - BMC: baremetalhost.BMCDetails{ - CredentialsName: "host2-bmc-secret", - }, - ConsumerRef: &corev1.ObjectReference{ - Name: "machine2", - }, - }, - }, - }, +func (htb *hostTypeBuilder) role(role string) *hostTypeBuilder { + htb.Role = role + return htb +} + +func (htb *hostTypeBuilder) bmc(user, password string) *hostTypeBuilder { + htb.BMC = baremetaltypes.BMC{ + Username: user, + Password: password, + } + return htb +} + +type machineBuilder struct { + machineapi.Machine +} + +func (mb *machineBuilder) build() *machineapi.Machine { + return &mb.Machine +} + +func machine(name string) *machineBuilder { + return &machineBuilder{ + machineapi.Machine{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "v1", + Kind: "Machine", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: "namespace", }, }, + } +} - { - Scenario: "4 hosts and 3 machines", - Config: makeConfig(4), - Machines: makeMachines(3), - Expected: HostSettings{ - Hosts: []baremetalhost.BareMetalHost{ - { - ObjectMeta: metav1.ObjectMeta{ - Name: "host0", - }, - Spec: baremetalhost.BareMetalHostSpec{ - Online: true, - BMC: baremetalhost.BMCDetails{ - CredentialsName: "host0-bmc-secret", - }, - ConsumerRef: &corev1.ObjectReference{ - Name: "machine0", - }, - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "host1", - }, - Spec: baremetalhost.BareMetalHostSpec{ - Online: true, - BMC: baremetalhost.BMCDetails{ - CredentialsName: "host1-bmc-secret", - }, - ConsumerRef: &corev1.ObjectReference{ - Name: "machine1", - }, - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "host2", - }, - Spec: baremetalhost.BareMetalHostSpec{ - Online: true, - BMC: baremetalhost.BMCDetails{ - CredentialsName: "host2-bmc-secret", - }, - ConsumerRef: &corev1.ObjectReference{ - Name: "machine2", - }, - }, - }, - { - ObjectMeta: metav1.ObjectMeta{ - Name: "host3", - }, - Spec: baremetalhost.BareMetalHostSpec{ - Online: true, - BMC: baremetalhost.BMCDetails{ - CredentialsName: "host3-bmc-secret", - }, - }, - }, +func machines(builders ...*machineBuilder) []machineapi.Machine { + m := []machineapi.Machine{} + + for _, mb := range builders { + m = append(m, *mb.build()) + } + return m +} + +type hostBuilder struct { + baremetalhost.BareMetalHost +} + +func host(name string) *hostBuilder { + return &hostBuilder{ + baremetalhost.BareMetalHost{ + + TypeMeta: metav1.TypeMeta{ + APIVersion: "metal3.io/v1alpha1", + Kind: "BareMetalHost", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: "openshift-machine-api", + }, + Spec: baremetalhost.BareMetalHostSpec{ + BMC: baremetalhost.BMCDetails{ + CredentialsName: name + "-bmc-secret", }, + RootDeviceHints: &baremetalhost.RootDeviceHints{ + DeviceName: "userd_devicename", + }, + BootMode: "UEFI", + BootMACAddress: "c0:ff:ee:ca:fe:00", + Online: true, }, }, } +} - for _, tc := range testCases { - t.Run(tc.Scenario, func(t *testing.T) { - actual, err := Hosts(tc.Config, tc.Machines) +func (hb *hostBuilder) build() *baremetalhost.BareMetalHost { + return &hb.BareMetalHost +} - if tc.ExpectError { - if err == nil { - t.Error("expected error but did not get one") - } - } else { +func (hb *hostBuilder) externallyProvisioned() *hostBuilder { + hb.Spec.ExternallyProvisioned = true + return hb +} - if err != nil { - t.Error("did not expect error but got one") - return - } +func (hb *hostBuilder) annotation(key, value string) *hostBuilder { + if hb.Annotations == nil { + hb.Annotations = map[string]string{} + } - if len(tc.Expected.Hosts) != len(actual.Hosts) { - t.Errorf("Expected %d hosts but received %d (%v)", - len(tc.Expected.Hosts), - len(actual.Hosts), - actual.Hosts, - ) - return - } + hb.Annotations[key] = value + return hb +} - if len(tc.Expected.Hosts) != len(actual.Secrets) { - t.Errorf("Expected %d secrets to go with hosts, got %d", - len(tc.Expected.Hosts), len(actual.Secrets)) - return - } +func (hb *hostBuilder) consumerRef(name string) *hostBuilder { + hb.Spec.ConsumerRef = &corev1.ObjectReference{ + APIVersion: "v1", + Kind: "Machine", + Namespace: "namespace", + Name: name, + } + return hb +} - // Make sure the first few hosts that correspond to - // machines have their consumer ref set correctly - for i := 0; i < len(tc.Machines); i++ { - if actual.Hosts[i].Spec.ConsumerRef.Name != tc.Expected.Hosts[i].Spec.ConsumerRef.Name { - t.Errorf("Expected host %d to link to %q but got %q", - i, - tc.Expected.Hosts[i].Spec.ConsumerRef.Name, - actual.Hosts[i].Spec.ConsumerRef.Name, - ) - } - } +type secretBuilder struct { + corev1.Secret +} - // Make sure any extra hosts do not have a consumer set. - for i := len(actual.Hosts) - 1; i > len(tc.Machines); i-- { - if actual.Hosts[i].Spec.ConsumerRef != nil { - t.Errorf("Expected host %d to have no consumer but has %v", - i, actual.Hosts[i].Spec.ConsumerRef, - ) - } - } +func secret(name string) *secretBuilder { + return &secretBuilder{ + corev1.Secret{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "v1", + Kind: "Secret", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: "openshift-machine-api", + }, + }, + } +} - for i, sec := range actual.Secrets { - expectedName := fmt.Sprintf("host%d-bmc-secret", i) - if sec.Name != expectedName { - t.Errorf("Expected secret name %d to be %q but got %q", - i, - expectedName, - sec.Name, - ) - } - } +func (sb *secretBuilder) data(user, password string) *secretBuilder { + sb.Data = map[string][]byte{ + "username": []byte(user), + "password": []byte(password), + } + return sb +} - } - }) +func (sb *secretBuilder) build() *corev1.Secret { + return &sb.Secret +} + +type hostSettingsBuilder struct { + HostSettings +} + +func (hsb *hostSettingsBuilder) secrets(builders ...*secretBuilder) *hostSettingsBuilder { + hsb.Secrets = []corev1.Secret{} + for _, sb := range builders { + hsb.Secrets = append(hsb.Secrets, *sb.build()) + } + return hsb +} + +func (hsb *hostSettingsBuilder) hosts(builders ...*hostBuilder) *hostSettingsBuilder { + hsb.Hosts = []baremetalhost.BareMetalHost{} + for _, hb := range builders { + hsb.Hosts = append(hsb.Hosts, *hb.build()) + } + return hsb +} + +func (hsb *hostSettingsBuilder) build() *HostSettings { + return &hsb.HostSettings +} + +func settings() *hostSettingsBuilder { + return &hostSettingsBuilder{ + HostSettings: HostSettings{}, } } diff --git a/pkg/tfvars/baremetal/baremetal.go b/pkg/tfvars/baremetal/baremetal.go index 65ebb41824b..4ae7042f3fd 100644 --- a/pkg/tfvars/baremetal/baremetal.go +++ b/pkg/tfvars/baremetal/baremetal.go @@ -27,23 +27,43 @@ type config struct { IronicPassword string `json:"ironic_password"` // Data required for control plane deployment - several maps per host, because of terraform's limitations - Hosts []map[string]interface{} `json:"hosts"` + Masters []map[string]interface{} `json:"masters"` RootDevices []map[string]interface{} `json:"root_devices"` Properties []map[string]interface{} `json:"properties"` DriverInfos []map[string]interface{} `json:"driver_infos"` InstanceInfos []map[string]interface{} `json:"instance_infos"` } +type imageDownloadFunc func(baseURL string) (string, error) + +var ( + imageDownloader imageDownloadFunc +) + +func init() { + imageDownloader = cache.DownloadImageFile +} + // TFVars generates bare metal specific Terraform variables. -func TFVars(libvirtURI, apiVIP, imageCacheIP, bootstrapOSImage, externalBridge, externalMAC, provisioningBridge, provisioningMAC string, platformHosts []*baremetal.Host, image, ironicUsername, ironicPassword, ignition string) ([]byte, error) { - bootstrapOSImage, err := cache.DownloadImageFile(bootstrapOSImage) +func TFVars(numControlPlaneReplicas int64, libvirtURI, apiVIP, imageCacheIP, bootstrapOSImage, externalBridge, externalMAC, provisioningBridge, provisioningMAC string, platformHosts []*baremetal.Host, image, ironicUsername, ironicPassword, ignition string) ([]byte, error) { + bootstrapOSImage, err := imageDownloader(bootstrapOSImage) if err != nil { return nil, errors.Wrap(err, "failed to use cached bootstrap libvirt image") } - var hosts, rootDevices, properties, driverInfos, instanceInfos []map[string]interface{} + var masters, rootDevices, properties, driverInfos, instanceInfos []map[string]interface{} + // Select the first N hosts as masters, excluding the workers for _, host := range platformHosts { + if len(masters) >= int(numControlPlaneReplicas) { + break + } + + if host.IsWorker() { + //Skipping workers + continue + } + // Get hardware profile if host.HardwareProfile == "default" { host.HardwareProfile = hardware.DefaultProfileName @@ -145,7 +165,7 @@ func TFVars(libvirtURI, apiVIP, imageCacheIP, bootstrapOSImage, externalBridge, instanceInfo["capabilities"] = "secure_boot:true" } - hosts = append(hosts, hostMap) + masters = append(masters, hostMap) properties = append(properties, propertiesMap) driverInfos = append(driverInfos, driverInfo) rootDevices = append(rootDevices, rootDevice) @@ -176,7 +196,7 @@ func TFVars(libvirtURI, apiVIP, imageCacheIP, bootstrapOSImage, externalBridge, BootstrapOSImage: bootstrapOSImage, IronicUsername: ironicUsername, IronicPassword: ironicPassword, - Hosts: hosts, + Masters: masters, Bridges: bridges, Properties: properties, DriverInfos: driverInfos, diff --git a/pkg/tfvars/baremetal/baremetal_test.go b/pkg/tfvars/baremetal/baremetal_test.go new file mode 100644 index 00000000000..b33c0ed7eeb --- /dev/null +++ b/pkg/tfvars/baremetal/baremetal_test.go @@ -0,0 +1,174 @@ +// Package baremetal contains bare metal specific Terraform-variable logic. +package baremetal + +import ( + "encoding/json" + "testing" + + "github.com/openshift/installer/pkg/types/baremetal" + "github.com/stretchr/testify/assert" +) + +func TestMastersSelectionByRole(t *testing.T) { + + cases := []struct { + scenario string + + numControlPlaneReplicas int64 + libvirtURI string + apiVIP string + imageCacheIP string + bootstrapOSImage string + externalBridge string + externalMAC string + provisioningBridge string + provisioningMAC string + platformHosts []*baremetal.Host + image string + ironicUsername string + ironicPassword string + ignition string + + expectedError string + expectedHosts []string + }{ + { + scenario: "filter-workers", + numControlPlaneReplicas: 2, + platformHosts: platformHosts( + host("master-0", "master"), + host("master-1", "master"), + host("worker-0", "worker"), + ), + expectedHosts: []string{"master-0", "master-1"}, + }, + { + scenario: "filter-all-workers", + numControlPlaneReplicas: 1, + platformHosts: platformHosts( + host("master-0", "master"), + host("worker-0", "worker"), + host("worker-1", "worker"), + ), + expectedHosts: []string{"master-0"}, + }, + { + scenario: "hosts-norole", + numControlPlaneReplicas: 2, + platformHosts: platformHosts( + host("worker-0", "worker"), + host("master-0", ""), + host("master-1", ""), + ), + expectedHosts: []string{"master-0", "master-1"}, + }, + { + scenario: "no-role", + numControlPlaneReplicas: 3, + platformHosts: platformHosts( + host("master-0", ""), + host("master-1", ""), + host("master-2", ""), + ), + expectedHosts: []string{"master-0", "master-1", "master-2"}, + }, + { + scenario: "more-masters-than-required", + numControlPlaneReplicas: 2, + platformHosts: platformHosts( + host("master-0", "master"), + host("master-1", "master"), + host("master-2", "master"), + ), + expectedHosts: []string{"master-0", "master-1"}, + }, + { + scenario: "more-hosts-than-required-mixed", + numControlPlaneReplicas: 2, + platformHosts: platformHosts( + host("master-0", "master"), + host("master-1", "master"), + host("master-2", ""), + ), + expectedHosts: []string{"master-0", "master-1"}, + }, + { + scenario: "more-hosts-than-required-norole", + numControlPlaneReplicas: 2, + platformHosts: platformHosts( + host("master-0", ""), + host("master-1", ""), + host("master-2", ""), + ), + expectedHosts: []string{"master-0", "master-1"}, + }, + { + scenario: "more-hosts-than-required-mixed-again", + numControlPlaneReplicas: 2, + platformHosts: platformHosts( + host("worker-0", "worker"), + host("worker-1", "worker"), + host("worker-2", "worker"), + host("master-0", ""), + host("master-1", ""), + host("master-2", ""), + ), + expectedHosts: []string{"master-0", "master-1"}, + }, + } + + for _, tc := range cases { + t.Run(tc.scenario, func(t *testing.T) { + + imageDownloader = func(baseURL string) (string, error) { + return "", nil + } + + data, err := TFVars( + tc.numControlPlaneReplicas, + tc.libvirtURI, + tc.apiVIP, + tc.imageCacheIP, + tc.bootstrapOSImage, + tc.externalBridge, + tc.externalMAC, + tc.provisioningBridge, + tc.provisioningMAC, + tc.platformHosts, + tc.image, + tc.ironicUsername, + tc.ironicPassword, + tc.ignition) + + if tc.expectedError == "" { + assert.NoError(t, err) + } else { + assert.Regexp(t, tc.expectedError, err) + } + + var cfg config + err = json.Unmarshal(data, &cfg) + assert.Nil(t, err) + + assert.Equal(t, len(tc.expectedHosts), len(cfg.Masters)) + for i, hostName := range tc.expectedHosts { + assert.Equal(t, hostName, cfg.Masters[i]["name"]) + } + }) + } +} + +func host(name, tag string) *baremetal.Host { + return &baremetal.Host{ + Name: name, + Role: tag, + HardwareProfile: "default", + BMC: baremetal.BMC{ + Address: "redfish+http://192.168.111.1:8000/redfish/v1/Systems/e4427260-6250-4df9-9e8a-120f78a46aa6", + }, + } +} + +func platformHosts(hosts ...*baremetal.Host) []*baremetal.Host { + return hosts +} diff --git a/pkg/types/baremetal/defaults/platform.go b/pkg/types/baremetal/defaults/platform.go index 7c096a5bb19..fb6c55345ed 100644 --- a/pkg/types/baremetal/defaults/platform.go +++ b/pkg/types/baremetal/defaults/platform.go @@ -2,8 +2,10 @@ package defaults import ( "fmt" - "github.com/openshift/installer/pkg/ipnet" "net" + "sort" + + "github.com/openshift/installer/pkg/ipnet" "github.com/apparentlymart/go-cidr/cidr" "github.com/openshift/installer/pkg/types" @@ -116,4 +118,10 @@ func SetPlatformDefaults(p *baremetal.Platform, c *types.InstallConfig) { p.IngressVIP = vip[0] } } + + if p.Hosts != nil { + sort.SliceStable(p.Hosts, func(i, j int) bool { + return p.Hosts[i].CompareByRole(p.Hosts[j]) + }) + } } diff --git a/pkg/types/baremetal/defaults/platform_test.go b/pkg/types/baremetal/defaults/platform_test.go index d19c8d1b806..79fc0a8368d 100644 --- a/pkg/types/baremetal/defaults/platform_test.go +++ b/pkg/types/baremetal/defaults/platform_test.go @@ -189,3 +189,86 @@ func TestSetPlatformDefaults(t *testing.T) { }) } } + +func TestBaremetalHostsSortByRole(t *testing.T) { + cases := []struct { + name string + hosts []*baremetal.Host + expectedHosts []string + }{ + { + name: "default", + hosts: []*baremetal.Host{ + {Name: "master-0", Role: "master"}, + {Name: "master-1", Role: "master"}, + {Name: "master-2", Role: "master"}, + {Name: "worker-0", Role: "worker"}, + {Name: "worker-1", Role: "worker"}, + }, + expectedHosts: []string{ + "master-0", "master-1", "master-2", "worker-0", "worker-1", + }, + }, + { + name: "norole", + hosts: []*baremetal.Host{ + {Name: "master-0"}, + {Name: "master-1"}, + {Name: "master-2"}, + {Name: "worker-0"}, + {Name: "worker-1"}, + }, + expectedHosts: []string{ + "master-0", "master-1", "master-2", "worker-0", "worker-1", + }, + }, + { + name: "mixed", + hosts: []*baremetal.Host{ + {Name: "worker-0", Role: "worker"}, + {Name: "master-0", Role: "master"}, + {Name: "worker-1", Role: "worker"}, + {Name: "master-1", Role: "master"}, + {Name: "master-2", Role: "master"}, + }, + expectedHosts: []string{ + "master-0", "master-1", "master-2", "worker-0", "worker-1", + }, + }, + { + name: "mixed-norole", + hosts: []*baremetal.Host{ + {Name: "worker-0", Role: "worker"}, + {Name: "master-0", Role: "master"}, + {Name: "worker-1", Role: ""}, + {Name: "master-1", Role: "master"}, + {Name: "master-2", Role: "master"}, + }, + expectedHosts: []string{ + "master-0", "master-1", "master-2", "worker-0", "worker-1", + }, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + + ic := &types.InstallConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: testClusterName, + }, + Platform: types.Platform{ + BareMetal: &baremetal.Platform{ + Hosts: tc.hosts, + }, + }, + BaseDomain: "test", + } + SetPlatformDefaults(ic.Platform.BareMetal, ic) + + for i, h := range ic.Platform.BareMetal.Hosts { + assert.Equal(t, h.Name, tc.expectedHosts[i]) + } + }) + } +} diff --git a/pkg/types/baremetal/platform.go b/pkg/types/baremetal/platform.go index 56d1396043f..ab2a2696988 100644 --- a/pkg/types/baremetal/platform.go +++ b/pkg/types/baremetal/platform.go @@ -25,6 +25,11 @@ const ( Legacy BootMode = "legacy" ) +const ( + masterRole string = "master" + workerRole string = "worker" +) + // Host stores all the configuration data for a baremetal host. type Host struct { Name string `json:"name,omitempty" validate:"required,uniqueField"` @@ -36,6 +41,23 @@ type Host struct { BootMode BootMode `json:"bootMode,omitempty"` } +// IsMaster checks if the current host is a master +func (h *Host) IsMaster() bool { + return h.Role == masterRole +} + +// IsWorker checks if the current host is a worker +func (h *Host) IsWorker() bool { + return h.Role == workerRole +} + +var sortIndex = map[string]int{masterRole: -1, workerRole: 0, "": 1} + +// CompareByRole allows to compare two hosts by the Role +func (h *Host) CompareByRole(k *Host) bool { + return sortIndex[h.Role] < sortIndex[k.Role] +} + // ProvisioningNetwork determines how we will use the provisioning network. // +kubebuilder:validation:Enum="";Managed;Unmanaged;Disabled type ProvisioningNetwork string diff --git a/pkg/types/baremetal/validation/platform.go b/pkg/types/baremetal/validation/platform.go index 8fc342f0b27..f6192f53867 100644 --- a/pkg/types/baremetal/validation/platform.go +++ b/pkg/types/baremetal/validation/platform.go @@ -12,6 +12,7 @@ import ( "github.com/apparentlymart/go-cidr/cidr" "github.com/go-playground/validator/v10" "github.com/pkg/errors" + "github.com/sirupsen/logrus" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/validation/field" @@ -270,21 +271,47 @@ func validateOSImages(p *baremetal.Platform, fldPath *field.Path) field.ErrorLis return platformErrs } +// ensure that the number of hosts is enough to cover the ControlPlane +// and Compute replicas. Hosts without role will be considered eligible +// for the ControlPlane or Compute requirements. func validateHostsCount(hosts []*baremetal.Host, installConfig *types.InstallConfig) error { - hostsNum := int64(len(hosts)) - counter := int64(0) + numRequiredMasters := int64(0) + if installConfig.ControlPlane != nil && installConfig.ControlPlane.Replicas != nil { + numRequiredMasters += *installConfig.ControlPlane.Replicas + } + numRequiredWorkers := int64(0) for _, worker := range installConfig.Compute { if worker.Replicas != nil { - counter += *worker.Replicas + numRequiredWorkers += *worker.Replicas } } - if installConfig.ControlPlane != nil && installConfig.ControlPlane.Replicas != nil { - counter += *installConfig.ControlPlane.Replicas + + numMasters := int64(0) + numWorkers := int64(0) + + for _, h := range hosts { + if h.IsMaster() { + numMasters++ + } else if h.IsWorker() { + numWorkers++ + } else { + logrus.Warn(fmt.Sprintf("Host %s hasn't any role configured", h.Name)) + if numMasters < numRequiredMasters { + numMasters++ + } else if numWorkers < numRequiredWorkers { + numWorkers++ + } + } } - if hostsNum < counter { - return fmt.Errorf("not enough hosts found (%v) to support all the configured ControlPlane and Compute replicas (%v)", hostsNum, counter) + + if numMasters < numRequiredMasters { + return fmt.Errorf("not enough hosts found (%v) to support all the configured ControlPlane replicas (%v)", numMasters, numRequiredMasters) + } + + if numWorkers < numRequiredWorkers { + return fmt.Errorf("not enough hosts found (%v) to support all the configured Compute replicas (%v)", numWorkers, numRequiredWorkers) } return nil diff --git a/pkg/types/baremetal/validation/platform_test.go b/pkg/types/baremetal/validation/platform_test.go index 7144638e07d..91637fa05fd 100644 --- a/pkg/types/baremetal/validation/platform_test.go +++ b/pkg/types/baremetal/validation/platform_test.go @@ -60,32 +60,134 @@ func TestValidatePlatform(t *testing.T) { expected: "bare metal hosts are missing", }, { - name: "toofew_hosts", + name: "toofew_masters_norole", config: installConfig(). BareMetalPlatform( platform().Hosts( - host1())). + host1().Role("worker"), + host2())). + ControlPlane( + machinePool().Replicas(3)). + Compute( + machinePool().Replicas(1)).build(), + expected: "baremetal.Hosts: Required value: not enough hosts found \\(1\\) to support all the configured ControlPlane replicas \\(3\\)", + }, + { + name: "toofew_masters", + config: installConfig(). + BareMetalPlatform( + platform().Hosts( + host1().Role("master"), + host2().Role("worker"))). ControlPlane( machinePool().Replicas(3)). Compute( - machinePool().Replicas(2), + machinePool().Replicas(1)).build(), + expected: "baremetal.Hosts: Required value: not enough hosts found \\(1\\) to support all the configured ControlPlane replicas \\(3\\)", + }, + { + name: "toofew_workers", + config: installConfig(). + BareMetalPlatform( + platform().Hosts( + host1().Role("master"), + host2().Role("worker"))). + ControlPlane( + machinePool().Replicas(1)). + Compute( machinePool().Replicas(3)).build(), - expected: "baremetal.Hosts: Required value: not enough hosts found \\(1\\) to support all the configured ControlPlane and Compute replicas \\(8\\)", + expected: "baremetal.Hosts: Required value: not enough hosts found \\(1\\) to support all the configured Compute replicas \\(3\\)", }, { name: "enough_hosts", + config: installConfig(). + BareMetalPlatform( + platform().Hosts( + host1().Role("master"), + host2().Role("worker"))). + ControlPlane( + machinePool().Replicas(1)). + Compute( + machinePool().Replicas(1)).build(), + }, + { + name: "enough_hosts_norole", config: installConfig(). BareMetalPlatform( platform().Hosts( host1(), - host2())). + host2(), + host3())). ControlPlane( + machinePool().Replicas(1)). + Compute( + machinePool().Replicas(2)).build(), + }, + { + name: "enough_hosts_mixed", + config: installConfig(). + BareMetalPlatform( + platform().Hosts( + host1().Role("master"), + host2().Role("worker"), + host3(), + host4())). + ControlPlane( + machinePool().Replicas(2)). + Compute( + machinePool().Replicas(2)).build(), + }, + { + name: "not_enough_hosts_norole", + config: installConfig(). + BareMetalPlatform( + platform().Hosts( + host1(), + host2(), + host3())). + ControlPlane( + machinePool().Replicas(2)). + Compute( + machinePool().Replicas(2)).build(), + expected: "baremetal.Hosts: Required value: not enough hosts found \\(1\\) to support all the configured Compute replicas \\(2\\)", + }, + { + name: "more_than_enough_hosts", + config: installConfig(). + BareMetalPlatform( + platform().Hosts( + host1().Role("master"), + host2().Role("master"), + host3().Role("worker"), + host4().Role("worker"), + host5())). + ControlPlane( + machinePool().Replicas(1)). + Compute( + machinePool().Replicas(1)).build(), + }, + { + name: "norole_for_workers", + config: installConfig(). + BareMetalPlatform( + platform().Hosts( + host1().Role("master"), + host2().Role("master"), + host3().Role("master"), + host4(), + host5())). + ControlPlane( + machinePool().Replicas(3)). + Compute( machinePool().Replicas(2)).build(), }, { name: "missing_name", - platform: platform(). - Hosts(host1().Name("")).build(), + config: installConfig(). + BareMetalPlatform( + platform().Hosts( + host1().Name(""))). + ControlPlane(machinePool().Replicas(1)).build(), expected: "baremetal.hosts\\[0\\].Name: Required value: missing Name", }, { @@ -492,6 +594,48 @@ func host2() *hostBuilder { } } +func host3() *hostBuilder { + return &hostBuilder{ + baremetal.Host{ + Name: "host3", + BootMACAddress: "CA:FE:CA:FE:00:02", + BMC: baremetal.BMC{ + Username: "root", + Password: "password", + Address: "ipmi://192.168.111.3", + }, + }, + } +} + +func host4() *hostBuilder { + return &hostBuilder{ + baremetal.Host{ + Name: "host4", + BootMACAddress: "CA:FE:CA:FE:00:03", + BMC: baremetal.BMC{ + Username: "root", + Password: "password", + Address: "ipmi://192.168.111.4", + }, + }, + } +} + +func host5() *hostBuilder { + return &hostBuilder{ + baremetal.Host{ + Name: "host5", + BootMACAddress: "CA:FE:CA:FE:00:04", + BMC: baremetal.BMC{ + Username: "root", + Password: "password", + Address: "ipmi://192.168.111.5", + }, + }, + } +} + func (hb *hostBuilder) build() *baremetal.Host { return &hb.Host } @@ -526,6 +670,11 @@ func (hb *hostBuilder) BMCPassword(value string) *hostBuilder { return hb } +func (hb *hostBuilder) Role(value string) *hostBuilder { + hb.Host.Role = value + return hb +} + type platformBuilder struct { baremetal.Platform } diff --git a/pkg/types/validation/installconfig_test.go b/pkg/types/validation/installconfig_test.go index c2cfcc90254..3baa35ce223 100644 --- a/pkg/types/validation/installconfig_test.go +++ b/pkg/types/validation/installconfig_test.go @@ -109,6 +109,7 @@ func validBareMetalPlatform() *baremetal.Platform { Hosts: []*baremetal.Host{ { Name: "host1", + Role: "master", BootMACAddress: "CA:FE:CA:FE:00:00", BMC: baremetal.BMC{ Username: "root", @@ -118,6 +119,7 @@ func validBareMetalPlatform() *baremetal.Platform { }, { Name: "host2", + Role: "worker", BootMACAddress: "CA:FE:CA:FE:00:01", BMC: baremetal.BMC{ Username: "root",