diff --git a/data/data/baremetal/bootstrap/main.tf b/data/data/baremetal/bootstrap/main.tf index a99b9d06cb5..7aa3f51e845 100644 --- a/data/data/baremetal/bootstrap/main.tf +++ b/data/data/baremetal/bootstrap/main.tf @@ -40,8 +40,8 @@ resource "libvirt_domain" "bootstrap" { dynamic "network_interface" { for_each = var.bridges content { - bridge = network_interface.value + bridge = network_interface.value["name"] + mac = network_interface.value["mac"] } } } - diff --git a/data/data/baremetal/bootstrap/variables.tf b/data/data/baremetal/bootstrap/variables.tf index 577a8234346..5d11a4a8a2e 100644 --- a/data/data/baremetal/bootstrap/variables.tf +++ b/data/data/baremetal/bootstrap/variables.tf @@ -14,6 +14,6 @@ variable "ignition" { } variable "bridges" { - type = list(string) - description = "A list of network interfaces to attach to the bootstrap virtual machine" + type = list(map(string)) + description = "A list of network bridge maps, containing the interface name and optionally the MAC address" } diff --git a/data/data/baremetal/main.tf b/data/data/baremetal/main.tf index f3b031f97bb..0a2b02890fe 100644 --- a/data/data/baremetal/main.tf +++ b/data/data/baremetal/main.tf @@ -20,7 +20,7 @@ module "bootstrap" { cluster_id = var.cluster_id image = var.bootstrap_os_image ignition = var.ignition_bootstrap - bridges = compact([var.external_bridge, var.provisioning_bridge]) + bridges = var.bridges } module "masters" { diff --git a/data/data/baremetal/variables-baremetal.tf b/data/data/baremetal/variables-baremetal.tf index 4ea128e3a20..8a810ced260 100644 --- a/data/data/baremetal/variables-baremetal.tf +++ b/data/data/baremetal/variables-baremetal.tf @@ -13,16 +13,6 @@ variable "bootstrap_os_image" { description = "The URL of the bootstrap OS disk image" } -variable "external_bridge" { - type = string - description = "The name of the external bridge" -} - -variable "provisioning_bridge" { - type = string - description = "The name of the provisioning bridge" -} - variable "ironic_username" { type = string description = "Username for authentication to Ironic" @@ -38,6 +28,11 @@ variable "hosts" { description = "Hardware details for hosts" } +variable "bridges" { + type = list(map(string)) + description = "A list of network bridge maps, containing the interface name and optionally the MAC address" +} + variable "properties" { type = list(map(string)) description = "Properties for hosts" diff --git a/data/data/install.openshift.io_installconfigs.yaml b/data/data/install.openshift.io_installconfigs.yaml index c7551d38be2..d51155ff76e 100644 --- a/data/data/install.openshift.io_installconfigs.yaml +++ b/data/data/install.openshift.io_installconfigs.yaml @@ -851,6 +851,9 @@ spec: externalBridge: description: External bridge is used for external communication. type: string + externalMACAddress: + description: ExternalMACAddress is used to allow setting a static MAC address for the bootstrap host on the external network. If left blank, libvirt will generate one for you. + type: string hosts: description: Hosts is the information needed to create the objects in Ironic. items: @@ -949,6 +952,9 @@ spec: provisioningHostIP: description: ClusterProvisioningIP is the IP on the dedicated provisioning network where the baremetal-operator pod runs provisioning services, and an http server to cache some downloaded content e.g RHCOS/IPA images type: string + provisioningMACAddress: + description: ProvisioningMACAddress is used to allow setting a static MAC address for the bootstrap host on the provisioning network. If left blank, libvirt will generate one for you. + type: string provisioningNetwork: default: Managed description: ProvisioningNetwork is used to indicate if we will have a provisioning network, and how it will be managed. diff --git a/docs/user/metal/customization_ipi.md b/docs/user/metal/customization_ipi.md index 954a78db59e..e673f9c2e58 100644 --- a/docs/user/metal/customization_ipi.md +++ b/docs/user/metal/customization_ipi.md @@ -10,7 +10,9 @@ `clusterProvisioningIP` | The third address on the provisioning network. `172.22.0.3` | The IP within the cluster where the provisioning services run. | `bootstrapProvisioningIP` | The second address on the provisioning network. `172.22.0.2` | The IP on the bootstrap VM where the provisioning services run while the control plane is being deployed. | `externalBridge` | `baremetal` | The name of the bridge of the hypervisor attached to the external network. | +`externalMACAddress` | `` | A MAC address to use for the external bridge. This is optional and if blank is generated by libvirt. | `provisioningBridge` | `provisioning` | The name of the bridge on the hypervisor attached to the provisioning network. | +`provisioningMACAddress` | `` | A MAC address to use for the provisioning bridge. This is optional and if blank is generated by libvirt. | `provisioningNetworkCIDR` | `172.22.0.0/24` | The CIDR for the network to use for provisioning. | `provisioningDHCPExternal` | `false` | Flag indicating that DHCP for the provisioning network is managed outside of the cluster by existing infrastructure services. | `provisioningDHCPRange` | The tenth through the second last IP on the provisioning network. `172.22.0.10,172.22.0.254` | The IP range to use for hosts on the provisioning network. | diff --git a/pkg/asset/cluster/tfvars.go b/pkg/asset/cluster/tfvars.go index 7f599ae3098..71db5dda498 100644 --- a/pkg/asset/cluster/tfvars.go +++ b/pkg/asset/cluster/tfvars.go @@ -430,7 +430,9 @@ func (t *TerraformVariables) Generate(parents asset.Parents) error { installConfig.Config.Platform.BareMetal.BootstrapProvisioningIP, string(*rhcosBootstrapImage), installConfig.Config.Platform.BareMetal.ExternalBridge, + installConfig.Config.Platform.BareMetal.ExternalMACAddress, installConfig.Config.Platform.BareMetal.ProvisioningBridge, + installConfig.Config.Platform.BareMetal.ProvisioningMACAddress, installConfig.Config.Platform.BareMetal.Hosts, string(*rhcosImage), ironicCreds.Username, diff --git a/pkg/tfvars/baremetal/baremetal.go b/pkg/tfvars/baremetal/baremetal.go index 00540845834..dcdd707fcd0 100644 --- a/pkg/tfvars/baremetal/baremetal.go +++ b/pkg/tfvars/baremetal/baremetal.go @@ -17,11 +17,10 @@ import ( ) type config struct { - LibvirtURI string `json:"libvirt_uri"` - BootstrapProvisioningIP string `json:"bootstrap_provisioning_ip"` - BootstrapOSImage string `json:"bootstrap_os_image"` - ExternalBridge string `json:"external_bridge"` - ProvisioningBridge string `json:"provisioning_bridge"` + LibvirtURI string `json:"libvirt_uri,omitempty"` + BootstrapProvisioningIP string `json:"bootstrap_provisioning_ip,omitempty"` + BootstrapOSImage string `json:"bootstrap_os_image,omitempty"` + Bridges []map[string]string `json:"bridges"` IronicUsername string `json:"ironic_username"` IronicPassword string `json:"ironic_password"` @@ -35,7 +34,7 @@ type config struct { } // TFVars generates bare metal specific Terraform variables. -func TFVars(libvirtURI, bootstrapProvisioningIP, bootstrapOSImage, externalBridge, provisioningBridge 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 string) ([]byte, error) { bootstrapOSImage, err := cache.DownloadImageFile(bootstrapOSImage) if err != nil { return nil, errors.Wrap(err, "failed to use cached bootstrap libvirt image") @@ -140,15 +139,31 @@ func TFVars(libvirtURI, bootstrapProvisioningIP, bootstrapOSImage, externalBridg instanceInfos = append(instanceInfos, instanceInfo) } + var bridges []map[string]string + + bridges = append(bridges, + map[string]string{ + "name": externalBridge, + "mac": externalMAC, + }) + + if provisioningBridge != "" { + bridges = append( + bridges, + map[string]string{ + "name": provisioningBridge, + "mac": provisioningMAC, + }) + } + cfg := &config{ LibvirtURI: libvirtURI, BootstrapProvisioningIP: bootstrapProvisioningIP, BootstrapOSImage: bootstrapOSImage, - ExternalBridge: externalBridge, - ProvisioningBridge: provisioningBridge, IronicUsername: ironicUsername, IronicPassword: ironicPassword, Hosts: hosts, + Bridges: bridges, Properties: properties, DriverInfos: driverInfos, RootDevices: rootDevices, diff --git a/pkg/types/baremetal/platform.go b/pkg/types/baremetal/platform.go index 0c9f77dc599..c737cc94941 100644 --- a/pkg/types/baremetal/platform.go +++ b/pkg/types/baremetal/platform.go @@ -83,6 +83,12 @@ type Platform struct { // +optional ExternalBridge string `json:"externalBridge,omitempty"` + // ExternalMACAddress is used to allow setting a static MAC address for + // the bootstrap host on the external network. If left blank, libvirt will + // generate one for you. + // +optional + ExternalMACAddress string `json:"externalMACAddress,omitempty"` + // ProvisioningNetwork is used to indicate if we will have a provisioning network, and how it will be managed. // +kubebuilder:default=Managed // +optional @@ -93,6 +99,12 @@ type Platform struct { // +optional ProvisioningBridge string `json:"provisioningBridge,omitempty"` + // ProvisioningMACAddress is used to allow setting a static MAC address for + // the bootstrap host on the provisioning network. If left blank, libvirt will + // generate one for you. + // +optional + ProvisioningMACAddress string `json:"provisioningMACAddress,omitempty"` + // ProvisioningNetworkInterface is the name of the network interface on a control plane // baremetal host that is connected to the provisioning network. ProvisioningNetworkInterface string `json:"provisioningNetworkInterface"` diff --git a/pkg/types/baremetal/validation/libvirt.go b/pkg/types/baremetal/validation/libvirt.go index ac7650ab3b7..c99c73ecdbb 100644 --- a/pkg/types/baremetal/validation/libvirt.go +++ b/pkg/types/baremetal/validation/libvirt.go @@ -6,6 +6,7 @@ import ( "fmt" "github.com/libvirt/libvirt-go" "github.com/openshift/installer/pkg/types/baremetal" + "github.com/openshift/installer/pkg/validate" "github.com/pkg/errors" "k8s.io/apimachinery/pkg/util/validation/field" "strings" @@ -29,15 +30,28 @@ func validateInterfaces(p *baremetal.Platform, fldPath *field.Path) field.ErrorL errorList = append(errorList, field.Invalid(fldPath.Child("externalBridge"), p.ExternalBridge, err.Error())) } + if err := validate.MAC(p.ExternalMACAddress); p.ExternalMACAddress != "" && err != nil { + errorList = append(errorList, field.Invalid(fldPath.Child("externalMACAddress"), p.ExternalMACAddress, err.Error())) + } + if err := findInterface(p.ProvisioningBridge); p.ProvisioningNetwork != baremetal.DisabledProvisioningNetwork && err != nil { errorList = append(errorList, field.Invalid(fldPath.Child("provisioningBridge"), p.ProvisioningBridge, err.Error())) } + if err := validate.MAC(p.ProvisioningMACAddress); p.ProvisioningMACAddress != "" && err != nil { + errorList = append(errorList, field.Invalid(fldPath.Child("provisioningMACAddress"), p.ProvisioningMACAddress, err.Error())) + } + + if p.ProvisioningMACAddress != "" && strings.EqualFold(p.ProvisioningMACAddress, p.ExternalMACAddress) { + errorList = append(errorList, field.Duplicate(fldPath.Child("provisioningMACAddress"), "provisioning and external MAC addresses may not be identical")) + } + return errorList } // interfaceValidator fetches the valid interface names from a particular libvirt instance, and returns a closure // to validate if an interface is found among them + func interfaceValidator(libvirtURI string) (func(string) error, error) { // Connect to libvirt and obtain a list of interface names interfaces := make(map[string]struct{}) diff --git a/pkg/types/baremetal/validation/platform.go b/pkg/types/baremetal/validation/platform.go index dd07ab48ebe..5b83e07210a 100644 --- a/pkg/types/baremetal/validation/platform.go +++ b/pkg/types/baremetal/validation/platform.go @@ -387,6 +387,18 @@ func ValidateProvisioning(p *baremetal.Platform, n *types.Networking, fldPath *f if p.ProvisioningNetworkInterface == "" { allErrs = append(allErrs, field.Invalid(fldPath.Child("provisioningNetworkInterface"), p.ProvisioningNetworkInterface, "no provisioning network interface is configured, please set this value to be the interface on the provisioning network on your cluster's baremetal hosts")) } + + if err := validate.MAC(p.ExternalMACAddress); p.ExternalMACAddress != "" && err != nil { + allErrs = append(allErrs, field.Invalid(fldPath.Child("externalMACAddress"), p.ExternalMACAddress, err.Error())) + } + + if err := validate.MAC(p.ProvisioningMACAddress); p.ProvisioningMACAddress != "" && err != nil { + allErrs = append(allErrs, field.Invalid(fldPath.Child("provisioningMACAddress"), p.ProvisioningMACAddress, err.Error())) + } + + if p.ProvisioningMACAddress != "" && strings.EqualFold(p.ProvisioningMACAddress, p.ExternalMACAddress) { + allErrs = append(allErrs, field.Duplicate(fldPath.Child("provisioningMACAddress"), "provisioning and external MAC addresses may not be identical")) + } } if err := validate.URI(p.LibvirtURI); err != nil { diff --git a/pkg/types/baremetal/validation/platform_test.go b/pkg/types/baremetal/validation/platform_test.go index 6c8dad59e99..024e7602d47 100644 --- a/pkg/types/baremetal/validation/platform_test.go +++ b/pkg/types/baremetal/validation/platform_test.go @@ -280,12 +280,35 @@ func TestValidateProvisioning(t *testing.T) { ExternalBridge("noexist").build(), expected: "Invalid value: \"noexist\": invalid external bridge", }, + { + name: "valid_extbridge_mac", + platform: platform().ExternalMACAddress("CA:FE:CA:FE:CA:FE").build(), + }, { name: "invalid_provbridge", platform: platform(). ProvisioningBridge("noexist").build(), expected: "Invalid value: \"noexist\": invalid provisioning bridge", }, + { + name: "valid_provbridge_mac", + platform: platform().ProvisioningMACAddress("CA:FE:CA:FE:CA:FE").build(), + }, + { + name: "invalid_duplicate_bridge_macs", + platform: platform(). + ProvisioningMACAddress("CA:FE:CA:FE:CA:FE"). + ExternalMACAddress("CA:FE:CA:FE:CA:FE"). + build(), + expected: "Duplicate value: \"provisioning and external MAC addresses may not be identical\"", + }, + { + name: "valid_both_macs_specified", + platform: platform(). + ProvisioningMACAddress("CA:FE:CA:FE:CA:FD"). + ExternalMACAddress("CA:FE:CA:FE:CA:FE"). + build(), + }, { name: "invalid_bootstrapprovip_wrongCIDR", platform: platform(). @@ -568,11 +591,21 @@ func (pb *platformBuilder) ExternalBridge(value string) *platformBuilder { return pb } +func (pb *platformBuilder) ExternalMACAddress(value string) *platformBuilder { + pb.Platform.ExternalMACAddress = value + return pb +} + func (pb *platformBuilder) ProvisioningBridge(value string) *platformBuilder { pb.Platform.ProvisioningBridge = value return pb } +func (pb *platformBuilder) ProvisioningMACAddress(value string) *platformBuilder { + pb.Platform.ProvisioningMACAddress = value + return pb +} + func (pb *platformBuilder) ProvisioningNetworkInterface(value string) *platformBuilder { pb.Platform.ProvisioningNetworkInterface = value return pb