From e6735018849be1eba64cfd2a42ce32fe45922f6d Mon Sep 17 00:00:00 2001 From: Angus Salkeld Date: Tue, 17 Nov 2020 17:52:41 +1000 Subject: [PATCH 1/3] baremetal: don't setup the provisioning interface if the ProvisioningIP is not set --- .../baremetal/files/usr/local/bin/startironic.sh.template | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/data/data/bootstrap/baremetal/files/usr/local/bin/startironic.sh.template b/data/data/bootstrap/baremetal/files/usr/local/bin/startironic.sh.template index cd86b47577f..434def6fff4 100755 --- a/data/data/bootstrap/baremetal/files/usr/local/bin/startironic.sh.template +++ b/data/data/bootstrap/baremetal/files/usr/local/bin/startironic.sh.template @@ -25,6 +25,8 @@ done # Start the provisioning nic if not already started PROVISIONING_NIC={{.PlatformData.BareMetal.ProvisioningInterface}} +{{ if .PlatformData.BareMetal.ProvisioningIP }} + if ! nmcli -t device | grep "$PROVISIONING_NIC:ethernet:connected"; then 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 @@ -42,6 +44,8 @@ while [ -z "$(ip -o addr show dev $PROVISIONING_NIC | grep -v link)" ]; do sleep 1 done +{{ end }} + # set password for ironic basic auth # The ironic container contains httpd (and thus httpd-tools), so rely on it to # supply the htpasswd command From 8543b6208c2553ab596188c05485de2c6464d38a Mon Sep 17 00:00:00 2001 From: Angus Salkeld Date: Wed, 18 Nov 2020 10:07:08 +1000 Subject: [PATCH 2/3] baremetal: make the BootstrapProvioningIP optional when the ProvisoningNetwork=Disabled --- .../ignition/bootstrap/baremetal/template.go | 15 +++++++------- pkg/types/baremetal/defaults/platform_test.go | 20 ++++++++++++++++++- pkg/types/baremetal/validation/platform.go | 8 +++++--- .../baremetal/validation/platform_test.go | 8 ++++++++ 4 files changed, 40 insertions(+), 11 deletions(-) diff --git a/pkg/asset/ignition/bootstrap/baremetal/template.go b/pkg/asset/ignition/bootstrap/baremetal/template.go index 9749b0d8f50..580a55ca798 100644 --- a/pkg/asset/ignition/bootstrap/baremetal/template.go +++ b/pkg/asset/ignition/bootstrap/baremetal/template.go @@ -71,14 +71,15 @@ func GetTemplateData(config *baremetal.Platform, networks []types.MachineNetwork templateData.ProvisioningInterface = "ens3" templateData.ProvisioningDNSMasq = false - for _, network := range networks { - if network.CIDR.Contains(net.ParseIP(templateData.ProvisioningIP)) { - templateData.ProvisioningIPv6 = network.CIDR.IP.To4() == nil - - cidr, _ := network.CIDR.Mask.Size() - templateData.ProvisioningCIDR = cidr + if templateData.ProvisioningIP != "" { + for _, network := range networks { + if network.CIDR.Contains(net.ParseIP(templateData.ProvisioningIP)) { + templateData.ProvisioningIPv6 = network.CIDR.IP.To4() == nil + + cidr, _ := network.CIDR.Mask.Size() + templateData.ProvisioningCIDR = cidr + } } - } } diff --git a/pkg/types/baremetal/defaults/platform_test.go b/pkg/types/baremetal/defaults/platform_test.go index 0ebe4ead970..08b38bb785a 100644 --- a/pkg/types/baremetal/defaults/platform_test.go +++ b/pkg/types/baremetal/defaults/platform_test.go @@ -2,12 +2,12 @@ package defaults import ( "errors" - "github.com/openshift/installer/pkg/ipnet" "testing" "github.com/stretchr/testify/assert" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "github.com/openshift/installer/pkg/ipnet" "github.com/openshift/installer/pkg/types" "github.com/openshift/installer/pkg/types/baremetal" ) @@ -124,6 +124,24 @@ func TestSetPlatformDefaults(t *testing.T) { IngressVIP: "192.168.111.3", }, }, + { + name: "disabled_provisioning_network_no_bootstrap_ip", + platform: &baremetal.Platform{ + ProvisioningNetwork: baremetal.DisabledProvisioningNetwork, + ClusterProvisioningIP: "192.168.111.8", + }, + expected: &baremetal.Platform{ + BootstrapProvisioningIP: "", + ClusterProvisioningIP: "192.168.111.8", + LibvirtURI: "qemu:///system", + ExternalBridge: "baremetal", + ProvisioningBridge: "", + ProvisioningNetwork: baremetal.DisabledProvisioningNetwork, + ProvisioningNetworkCIDR: machineNetwork, + APIVIP: "192.168.111.2", + IngressVIP: "192.168.111.3", + }, + }, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { diff --git a/pkg/types/baremetal/validation/platform.go b/pkg/types/baremetal/validation/platform.go index 5b83e07210a..b2c9149eb82 100644 --- a/pkg/types/baremetal/validation/platform.go +++ b/pkg/types/baremetal/validation/platform.go @@ -351,9 +351,11 @@ func ValidateProvisioning(p *baremetal.Platform, n *types.Networking, fldPath *f // will be run on the external network. Users must provide IP's on the // machine networks to host those services. case baremetal.DisabledProvisioningNetwork: - // Ensure bootstrapProvisioningIP is in one of the machine networks - if err := validateIPinMachineCIDR(p.BootstrapProvisioningIP, n); err != nil { - allErrs = append(allErrs, field.Invalid(fldPath.Child("bootstrapProvisioningIP"), p.BootstrapProvisioningIP, fmt.Sprintf("provisioning network is disabled, %s", err.Error()))) + // If set, ensure bootstrapProvisioningIP is in one of the machine networks + if p.BootstrapProvisioningIP != "" { + if err := validateIPinMachineCIDR(p.BootstrapProvisioningIP, n); err != nil { + allErrs = append(allErrs, field.Invalid(fldPath.Child("bootstrapProvisioningIP"), p.BootstrapProvisioningIP, fmt.Sprintf("provisioning network is disabled, %s", err.Error()))) + } } // Ensure clusterProvisioningIP is in one of the machine networks diff --git a/pkg/types/baremetal/validation/platform_test.go b/pkg/types/baremetal/validation/platform_test.go index e2470a42111..5a61495f937 100644 --- a/pkg/types/baremetal/validation/platform_test.go +++ b/pkg/types/baremetal/validation/platform_test.go @@ -412,6 +412,14 @@ func TestValidateProvisioning(t *testing.T) { ClusterProvisioningIP("192.168.111.2"). BootstrapProvisioningIP("192.168.111.3").build(), }, + { + name: "valid_provisioningDisabled_no_boostrap_ip", + config: installConfig().Network(networking().Network("192.168.111.0/24")).build(), + platform: platform(). + ProvisioningNetwork(baremetal.DisabledProvisioningNetwork). + ClusterProvisioningIP("192.168.111.2"). + BootstrapProvisioningIP("").build(), + }, { name: "invalid_provisioningDisabled_IPs_not_in_machineCIDR", config: installConfig().Network(networking().Network("192.168.111.0/24")).build(), From be2d969672d3716c5af42c601ea79c23d1a19438 Mon Sep 17 00:00:00 2001 From: Angus Salkeld Date: Wed, 18 Nov 2020 12:31:33 +1000 Subject: [PATCH 3/3] baremetal: use the APIVIP when provisionNetwork is disabled and BootstrapProvisioningIP is empty --- pkg/asset/cluster/tfvars.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/pkg/asset/cluster/tfvars.go b/pkg/asset/cluster/tfvars.go index 71db5dda498..b87dd938f6a 100644 --- a/pkg/asset/cluster/tfvars.go +++ b/pkg/asset/cluster/tfvars.go @@ -425,9 +425,14 @@ func (t *TerraformVariables) Generate(parents asset.Parents) error { Data: data, }) case baremetal.Name: + provisioningIP := installConfig.Config.Platform.BareMetal.BootstrapProvisioningIP + if installConfig.Config.Platform.BareMetal.ProvisioningNetwork == baremetal.DisabledProvisioningNetwork && provisioningIP == "" { + provisioningIP = installConfig.Config.Platform.BareMetal.APIVIP + } + data, err = baremetaltfvars.TFVars( installConfig.Config.Platform.BareMetal.LibvirtURI, - installConfig.Config.Platform.BareMetal.BootstrapProvisioningIP, + provisioningIP, string(*rhcosBootstrapImage), installConfig.Config.Platform.BareMetal.ExternalBridge, installConfig.Config.Platform.BareMetal.ExternalMACAddress,