Merge pull request #8161 from zaneb/baremetal-in-all-builds

CORS-3419,OCPBUGS-30941: Include baremetal IPI in openshift-installer

Author: openshift-merge-bot[bot]

pkg/destroy/baremetal/baremetal.go

@@ -1,6 +1,3 @@
-//go:build baremetal
-// +build baremetal
-
 package baremetal
 
 import (

pkg/destroy/baremetal/register.go

@@ -1,6 +1,3 @@
-//go:build baremetal
-// +build baremetal
-
 // Package baremetal provides a cluster-destroyer for bare metal clusters.
 package baremetal
 

pkg/types/baremetal/validation/libvirt.go

@@ -1,6 +1,3 @@
-//go:build baremetal
-// +build baremetal
-
 package validation
 
 import (
@@ -9,53 +6,12 @@ import (
 	"strings"
 
 	libvirt "github.com/digitalocean/go-libvirt"
-	"k8s.io/apimachinery/pkg/util/validation/field"
-
-	"github.com/openshift/installer/pkg/types/baremetal"
-	"github.com/openshift/installer/pkg/validate"
+	"github.com/sirupsen/logrus"
 )
 
-func init() {
-	dynamicProvisioningValidators = append(dynamicProvisioningValidators, validateInterfaces)
-}
-
-// validateInterfaces ensures that any interfaces required by the platform exist on the libvirt host.
-func validateInterfaces(p *baremetal.Platform, fldPath *field.Path) field.ErrorList {
-	errorList := field.ErrorList{}
-
-	findInterface, err := interfaceValidator(p.LibvirtURI)
-	if err != nil {
-		errorList = append(errorList, field.InternalError(fldPath.Child("libvirtURI"), err))
-		return errorList
-	}
-
-	if err := findInterface(p.ExternalBridge); err != nil {
-		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
+// libvirtInterfaceValidator 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) {
+func libvirtInterfaceValidator(libvirtURI string) (func(string) error, error) {
 	// Connect to libvirt and obtain a list of interface names
 	interfaces := make(map[string]struct{})
 	var exists = struct{}{}
@@ -69,7 +25,11 @@ func interfaceValidator(libvirtURI string) (func(string) error, error) {
 	if err != nil {
 		return nil, err
 	}
-	defer virt.Disconnect()
+	defer func() {
+		if err := virt.Disconnect(); err != nil {
+			logrus.Errorln("error disconnecting from libvirt:", err)
+		}
+	}()
 
 	networks, _, err := virt.ConnectListAllNetworks(1, libvirt.ConnectListNetworksActive)
 	if err != nil {

pkg/types/baremetal/validation/platform.go

@@ -26,13 +26,9 @@ import (
 	"github.com/openshift/installer/pkg/validate"
 )
 
-// dynamicProvisioningValidator is a function that validates certain fields in the platform.
-type dynamicProvisioningValidator func(*baremetal.Platform, *field.Path) field.ErrorList
+type interfaceValidatorFactory func(string) (func(string) error, error)
 
-// dynamicProvisioningValidators is an array of dynamicProvisioningValidator functions. This array can be added to by an init function, and
-// is intended to be used for validations that require dependencies not built with the default tags, e.g. libvirt
-// libraries.
-var dynamicProvisioningValidators []dynamicProvisioningValidator
+var interfaceValidator interfaceValidatorFactory = libvirtInterfaceValidator
 
 func validateIPinMachineCIDR(vip string, n *types.Networking) error {
 	var networks []string
@@ -484,6 +480,8 @@ func ValidateProvisioning(p *baremetal.Platform, n *types.Networking, fldPath *f
 
 	allErrs = append(allErrs, ValidateProvisioningNetworking(p, n, fldPath)...)
 
+	allErrs = append(allErrs, validateProvisioningBootstrapNetworking(p, fldPath)...)
+
 	return allErrs
 }
 
@@ -579,9 +577,41 @@ func ValidateProvisioningNetworking(p *baremetal.Platform, n *types.Networking,
 
 	allErrs = append(allErrs, validateHostsBMCOnly(p.Hosts, fldPath)...)
 
-	for _, validator := range dynamicProvisioningValidators {
-		allErrs = append(allErrs, validator(p, fldPath)...)
+	return allErrs
+}
+
+// validateProvisioningBootstrapNetworking checks that provisioning network requirements specified is valid for the bootstrap VM.
+func validateProvisioningBootstrapNetworking(p *baremetal.Platform, fldPath *field.Path) field.ErrorList {
+	errorList := field.ErrorList{}
+
+	if interfaceValidator != nil {
+		findInterface, err := interfaceValidator(p.LibvirtURI)
+		if err != nil {
+			errorList = append(errorList, field.InternalError(fldPath.Child("libvirtURI"), err))
+			return errorList
+		}
+
+		if err := findInterface(p.ExternalBridge); err != nil {
+			errorList = append(errorList, field.Invalid(fldPath.Child("externalBridge"), p.ExternalBridge, 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()))
+		}
+
 	}
 
-	return allErrs
+	if err := validate.MAC(p.ExternalMACAddress); p.ExternalMACAddress != "" && err != nil {
+		errorList = append(errorList, field.Invalid(fldPath.Child("externalMACAddress"), p.ExternalMACAddress, 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
 }

pkg/types/baremetal/validation/platform_test.go

@@ -1,6 +1,7 @@
 package validation
 
 import (
+	"fmt"
 	"net/http"
 	"net/http/httptest"
 	"strings"
@@ -418,21 +419,22 @@ func TestValidateProvisioning(t *testing.T) {
 	}))
 	defer imagesServer.Close()
 
-	interfaceValidator := func(p *baremetal.Platform, fldPath *field.Path) field.ErrorList {
-		errorList := field.ErrorList{}
-
-		if p.ExternalBridge != "br0" {
-			errorList = append(errorList, field.Invalid(fldPath.Child("externalBridge"), p.ExternalBridge,
-				"invalid external bridge"))
-		}
-		if p.ProvisioningBridge != "br1" {
-			errorList = append(errorList, field.Invalid(fldPath.Child("provisioningBridge"), p.ProvisioningBridge,
-				"invalid provisioning bridge"))
-		}
+	origInterfaceValidator := interfaceValidator
+	t.Cleanup(func() {
+		interfaceValidator = origInterfaceValidator
+	})
+	interfaceValidator = func(libvirtURI string) (func(string) error, error) {
+		return func(interfaceName string) error {
+			interfaceNames := []string{"br0", "br1"}
+			for _, foundInterface := range interfaceNames {
+				if foundInterface == interfaceName {
+					return nil
+				}
+			}
 
-		return errorList
+			return fmt.Errorf("could not find interface %q, valid interfaces are %s", interfaceName, strings.Join(interfaceNames, ", "))
+		}, nil
 	}
-	dynamicProvisioningValidators = append(dynamicProvisioningValidators, interfaceValidator)
 
 	cases := []struct {
 		name     string
@@ -524,7 +526,7 @@ func TestValidateProvisioning(t *testing.T) {
 			name: "invalid_extbridge",
 			platform: platform().
 				ExternalBridge("noexist").build(),
-			expected: "Invalid value: \"noexist\": invalid external bridge",
+			expected: "Invalid value: \"noexist\": could not find interface \"noexist\", valid interfaces are br0, br1",
 		},
 		{
 			name:     "valid_extbridge_mac",
@@ -534,7 +536,7 @@ func TestValidateProvisioning(t *testing.T) {
 			name: "invalid_provbridge",
 			platform: platform().
 				ProvisioningBridge("noexist").build(),
-			expected: "Invalid value: \"noexist\": invalid provisioning bridge",
+			expected: "Invalid value: \"noexist\": could not find interface \"noexist\", valid interfaces are br0, br1",
 		},
 		{
 			name:     "valid_provbridge_mac",

pkg/types/installconfig.go

@@ -39,6 +39,7 @@ var (
 	PlatformNames = []string{
 		aws.Name,
 		azure.Name,
+		baremetal.Name,
 		gcp.Name,
 		ibmcloud.Name,
 		nutanix.Name,
@@ -50,7 +51,6 @@ var (
 	// hidden-but-supported platform names. This list isn't presented
 	// to the user in the interactive wizard.
 	HiddenPlatformNames = []string{
-		baremetal.Name,
 		external.Name,
 		none.Name,
 	}