Bug 1796929: baremetal: make provisioning networks more configurable

data/data/bootstrap/baremetal/files/usr/local/bin/startironic.sh.template

@@ -11,6 +11,10 @@ COREOS_DOWNLOADER_IMAGE=$(image_for ironic-machine-os-downloader || image_for ir
 # This image is templated in via the installer pkg/asset/ignition/bootstrap/bootstrap.go
 RHCOS_BOOT_IMAGE_URL="{{.BootImage}}"
 
+# This DHCP range is used by dnsmasq to serve DHCP to the cluster. If empty
+# dnsmasq will only serve TFTP, and DHCP will be disabled.
+DHCP_RANGE="{{.PlatformData.BareMetal.ProvisioningDHCPRange}}"
+
 # First we stop any previously started containers, because ExecStop only runs when the ExecStart process
 # e.g this script is still running, but we exit if *any* of the containers exits unexpectedly
 for name in ironic-api ironic-conductor ironic-inspector dnsmasq httpd mariadb ipa-downloader coreos-downloader; do
@@ -19,10 +23,9 @@ for name in ironic-api ironic-conductor ironic-inspector dnsmasq httpd mariadb i
 done
 
 # Start the provisioning nic if not already started
-# Note removal of the hard-coded subnet tracked via https://github.com/openshift/installer/issues/2091
 PROVISIONING_NIC=ens4
 if ! nmcli -t device | grep "$PROVISIONING_NIC:ethernet:connected:provisioning"; then
-    nmcli c add type ethernet ifname $PROVISIONING_NIC con-name provisioning ip4 172.22.0.2/24 gw4 172.22.0.1
+    nmcli c add type ethernet ifname $PROVISIONING_NIC con-name provisioning {{ if .PlatformData.BareMetal.ProvisioningIPv6 }} ip6 {{ else }} ip4 {{ end }} {{.PlatformData.BareMetal.ProvisioningIP}}/{{.PlatformData.BareMetal.ProvisioningCIDR}}
     nmcli c up provisioning
 fi
 
@@ -63,6 +66,7 @@ podman run -d --net host --privileged --name mariadb \
 
 podman run -d --net host --privileged --name dnsmasq \
      --env PROVISIONING_INTERFACE=$PROVISIONING_NIC \
+     --env DHCP_RANGE=$DHCP_RANGE \
      -v $IRONIC_SHARED_VOLUME:/shared:z --entrypoint /bin/rundnsmasq ${IRONIC_IMAGE}
 
 podman run -d --net host --privileged --name httpd \

docs/user/metal/install_ipi.md

@@ -46,10 +46,18 @@ purposes:
     * `*.apps.<cluster-name>.<base-domain>` - pointing to the Ingress VIP
 
 * **NIC #2 - Provisioning Network**
-  * A private, non-routed network, used for PXE based provisioning.
-  * DHCP is automated for this network.
-  * Addressing for this network is currently hard coded as `172.22.0.0/24`, but
-    will be made configurable in the future.
+  * A private network used for PXE based provisioning.
+  * DHCP is automated for this network by default, to rely on external
+    DHCP, set the platform's `provisioningDHCPExternal` option to `true`
+  * Addressing for this network defaults to `172.22.0.0/24`, but is
+    configurable by setting the `provisioningNetworkCIDR` option.
+  * Two IP's are required to be available for use, one for the bootstrap
+    host, and one as a provisioning IP in the running cluster. By
+    default, these are the 2nd and 3rd addresses in the
+    `provisioningNetworkCIDR` (e.g. 172.22.0.2, and 172.22.0.3).
+  * To specify the name of the provisioning network interface,
+    set the `provisioningNetworkInterface` option. This is the network interface
+    on a master that is connected to the provisioning network.
 
 * **Out-of-band Management Network**
   * Servers will typically have an additional NIC used by the onboard

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

@@ -0,0 +1,39 @@
+package baremetal
+
+import (
+	"github.com/openshift/installer/pkg/types/baremetal"
+)
+
+// TemplateData holds data specific to templates used for the baremetal platform.
+type TemplateData struct {
+	// ProvisioningIP holds the IP the bootstrap node will use to service Ironic, TFTP, etc.
+	ProvisioningIP string
+
+	// ProvisioningIPv6 determines if we are using IPv6 or not.
+	ProvisioningIPv6 bool
+
+	// ProvisioningCIDR has the integer CIDR notation, e.g. 255.255.255.0 should be "24"
+	ProvisioningCIDR int
+
+	// ProvisioningDHCPRange has the DHCP range, if DHCP is not external. Otherwise it
+	// should be blank.
+	ProvisioningDHCPRange string
+}
+
+// GetTemplateData returns platform-specific data for bootstrap templates.
+func GetTemplateData(config *baremetal.Platform) *TemplateData {
+	var templateData TemplateData
+
+	templateData.ProvisioningIP = config.BootstrapProvisioningIP
+
+	cidr, _ := config.ProvisioningNetworkCIDR.Mask.Size()
+	templateData.ProvisioningCIDR = cidr
+
+	templateData.ProvisioningIPv6 = config.ProvisioningNetworkCIDR.IP.To4() == nil
+
+	if !config.ProvisioningDHCPExternal {
+		templateData.ProvisioningDHCPRange = config.ProvisioningDHCPRange
+	}
+
+	return &templateData
+}

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

@@ -0,0 +1,39 @@
+package baremetal
+
+import (
+	"github.com/openshift/installer/pkg/ipnet"
+	"github.com/openshift/installer/pkg/types/baremetal"
+	"github.com/stretchr/testify/assert"
+	"testing"
+)
+
+func TestTemplatingIPv4(t *testing.T) {
+	bareMetalConfig := baremetal.Platform{
+		ProvisioningNetworkCIDR:  ipnet.MustParseCIDR("172.22.0.0/24"),
+		BootstrapProvisioningIP:  "172.22.0.2",
+		ProvisioningDHCPExternal: false,
+		ProvisioningDHCPRange:    "172.22.0.10,172.22.0.100",
+	}
+
+	result := GetTemplateData(&bareMetalConfig)
+
+	assert.Equal(t, result.ProvisioningDHCPRange, "172.22.0.10,172.22.0.100")
+	assert.Equal(t, result.ProvisioningCIDR, 24)
+	assert.Equal(t, result.ProvisioningIPv6, false)
+	assert.Equal(t, result.ProvisioningIP, "172.22.0.2")
+}
+
+func TestTemplatingIPv6(t *testing.T) {
+	bareMetalConfig := baremetal.Platform{
+		ProvisioningNetworkCIDR:  ipnet.MustParseCIDR("fd2e:6f44:5dd8:b856::0/64"),
+		BootstrapProvisioningIP:  "fd2e:6f44:5dd8:b856::2",
+		ProvisioningDHCPExternal: true,
+	}
+
+	result := GetTemplateData(&bareMetalConfig)
+
+	assert.Equal(t, result.ProvisioningDHCPRange, "")
+	assert.Equal(t, result.ProvisioningCIDR, 64)
+	assert.Equal(t, result.ProvisioningIPv6, true)
+	assert.Equal(t, result.ProvisioningIP, "fd2e:6f44:5dd8:b856::2")
+}

pkg/asset/ignition/bootstrap/bootstrap.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"encoding/json"
 	"fmt"
+	"github.com/openshift/installer/pkg/asset/ignition/bootstrap/baremetal"
 	"io"
 	"io/ioutil"
 	"os"
@@ -30,6 +31,7 @@ import (
 	"github.com/openshift/installer/pkg/asset/rhcos"
 	"github.com/openshift/installer/pkg/asset/tls"
 	"github.com/openshift/installer/pkg/types"
+	baremetaltypes "github.com/openshift/installer/pkg/types/baremetal"
 )
 
 const (
@@ -50,6 +52,13 @@ type bootstrapTemplateData struct {
 	Registries            []sysregistriesv2.Registry
 	BootImage             string
 	ClusterDomain         string
+	PlatformData          platformTemplateData
+}
+
+// platformTemplateData is the data to use to replace values in bootstrap
+// template files that are specific to one platform.
+type platformTemplateData struct {
+	BareMetal *baremetal.TemplateData
 }
 
 // Bootstrap is an asset that generates the ignition config for bootstrap nodes.
@@ -223,6 +232,14 @@ func (a *Bootstrap) getTemplateData(installConfig *types.InstallConfig, releaseI
 		registries = append(registries, registry)
 	}
 
+	// Generate platform-specific baremetal data
+	var platformData platformTemplateData
+
+	switch installConfig.Platform.Name() {
+	case baremetaltypes.Name:
+		platformData.BareMetal = baremetal.GetTemplateData(installConfig.Platform.BareMetal)
+	}
+
 	return &bootstrapTemplateData{
 		AdditionalTrustBundle: installConfig.AdditionalTrustBundle,
 		FIPS:                  installConfig.FIPS,
@@ -233,6 +250,7 @@ func (a *Bootstrap) getTemplateData(installConfig *types.InstallConfig, releaseI
 		Registries:            registries,
 		BootImage:             string(*rhcosImage),
 		ClusterDomain:         installConfig.ClusterDomain(),
+		PlatformData:          platformData,
 	}, nil
 }
 

pkg/types/baremetal/defaults/platform.go

@@ -2,36 +2,64 @@ package defaults
 
 import (
 	"fmt"
+	"github.com/openshift/installer/pkg/ipnet"
 	"net"
 
+	"github.com/apparentlymart/go-cidr/cidr"
 	"github.com/openshift/installer/pkg/types"
 	"github.com/openshift/installer/pkg/types/baremetal"
 )
 
 // Defaults for the baremetal platform.
 const (
-	LibvirtURI              = "qemu:///system"
-	BootstrapProvisioningIP = "172.22.0.2"
-	ClusterProvisioningIP   = "172.22.0.3"
-	ExternalBridge          = "baremetal"
-	ProvisioningBridge      = "provisioning"
-	HardwareProfile         = "default"
-	APIVIP                  = ""
-	IngressVIP              = ""
+	LibvirtURI                   = "qemu:///system"
+	ProvisioningNetworkCIDR      = "172.22.0.0/24"
+	ExternalBridge               = "baremetal"
+	ProvisioningBridge           = "provisioning"
+	ProvisioningNetworkInterface = "ens3"
+	HardwareProfile              = "default"
+	APIVIP                       = ""
+	IngressVIP                   = ""
 )
 
+// Wrapper for net.LookupHost so we can override in the test
+var lookupHost = func(host string) (addrs []string, err error) {
+	return net.LookupHost(host)
+}
+
 // SetPlatformDefaults sets the defaults for the platform.
 func SetPlatformDefaults(p *baremetal.Platform, c *types.InstallConfig) {
 	if p.LibvirtURI == "" {
 		p.LibvirtURI = LibvirtURI
 	}
 
+	if p.ProvisioningNetworkCIDR == nil {
+		p.ProvisioningNetworkCIDR = ipnet.MustParseCIDR(ProvisioningNetworkCIDR)
+	}
+
+	// If the user doesn't provide an explicit DHCP range, and DHCP is not
+	// disabled, then we set a default value from the 10th to 100th
+	// address in the network.
+	if !p.ProvisioningDHCPExternal && p.ProvisioningDHCPRange == "" {
+		startIP, _ := cidr.Host(&p.ProvisioningNetworkCIDR.IPNet, 10)
+		endIP, _ := cidr.Host(&p.ProvisioningNetworkCIDR.IPNet, 100)
+		p.ProvisioningDHCPRange = fmt.Sprintf("%s,%s", startIP, endIP)
+	}
+
 	if p.BootstrapProvisioningIP == "" {
-		p.BootstrapProvisioningIP = BootstrapProvisioningIP
+		// Default to the second address in provisioning network, e.g 172.22.0.2
+		ip, err := cidr.Host(&p.ProvisioningNetworkCIDR.IPNet, 2)
+		if err == nil {
+			p.BootstrapProvisioningIP = ip.String()
+		}
 	}
 
 	if p.ClusterProvisioningIP == "" {
-		p.ClusterProvisioningIP = ClusterProvisioningIP
+		// Default to the third address in provisioning network, e.g 172.22.0.3
+		ip, err := cidr.Host(&p.ProvisioningNetworkCIDR.IPNet, 3)
+		if err == nil {
+			p.ClusterProvisioningIP = ip.String()
+		}
 	}
 
 	if p.ExternalBridge == "" {
@@ -42,6 +70,10 @@ func SetPlatformDefaults(p *baremetal.Platform, c *types.InstallConfig) {
 		p.ProvisioningBridge = ProvisioningBridge
 	}
 
+	if p.ProvisioningNetworkInterface == "" {
+		p.ProvisioningNetworkInterface = ProvisioningNetworkInterface
+	}
+
 	for _, host := range p.Hosts {
 		if host.HardwareProfile == "" {
 			host.HardwareProfile = HardwareProfile
@@ -50,7 +82,7 @@ func SetPlatformDefaults(p *baremetal.Platform, c *types.InstallConfig) {
 
 	if p.APIVIP == APIVIP {
 		// This name should resolve to exactly one address
-		vip, err := net.LookupHost("api." + c.ClusterDomain())
+		vip, err := lookupHost("api." + c.ClusterDomain())
 		if err != nil {
 			// This will fail validation and abort the install
 			p.APIVIP = fmt.Sprintf("DNS lookup failure: %s", err.Error())
@@ -61,7 +93,7 @@ func SetPlatformDefaults(p *baremetal.Platform, c *types.InstallConfig) {
 
 	if p.IngressVIP == IngressVIP {
 		// This name should resolve to exactly one address
-		vip, err := net.LookupHost("test.apps." + c.ClusterDomain())
+		vip, err := lookupHost("test.apps." + c.ClusterDomain())
 		if err != nil {
 			// This will fail validation and abort the install
 			p.IngressVIP = fmt.Sprintf("DNS lookup failure: %s", err.Error())