Merge pull request #384 from openshift-cherrypick-robot/cherry-pick-3…

…80-to-release-4.14

[release-4.14] OCPBUGS-23393,OCPBUGS-23392: fix IRONIC_EXTERNAL_URL_V6

provisioning/baremetal_config_test.go

@@ -115,6 +115,7 @@ type provisioningBuilder struct {
 }
 
 const testProvisioningIP = "172.30.20.3"
+const testProvisioningIPv6 = "fd2e:6f44:5dd8:b856::2"
 
 func managedProvisioning() *provisioningBuilder {
 	return &provisioningBuilder{
@@ -134,7 +135,7 @@ func managedIPv6Provisioning() *provisioningBuilder {
 	return &provisioningBuilder{
 		metal3iov1alpha1.ProvisioningSpec{
 			ProvisioningInterface:     "eth0",
-			ProvisioningIP:            "fd2e:6f44:5dd8:b856::2",
+			ProvisioningIP:            testProvisioningIPv6,
 			ProvisioningNetworkCIDR:   "fd2e:6f44:5dd8:b856::0/80",
 			ProvisioningMacAddresses:  []string{"34:b3:2d:81:f8:fb", "34:b3:2d:81:f8:fc", "34:b3:2d:81:f8:fd"},
 			ProvisioningDHCPRange:     "fd2e:6f44:5dd8:b856::10,fd2e:6f44:5dd8:b856::ff",

provisioning/baremetal_pod.go

@@ -268,8 +268,7 @@ func setIronicExternalIp(name string, config *metal3iov1alpha1.ProvisioningSpec)
 }
 
 func setIronicExternalUrl(info *ProvisioningInfo) (corev1.EnvVar, error) {
-	ironicIPs, err := getServerInternalIPs(info.OSClient)
-
+	ironicIPs, err := GetRealIronicIPs(info)
 	if err != nil {
 		return corev1.EnvVar{}, fmt.Errorf("Failed to get Ironic IP when setting external url: %w", err)
 	}

provisioning/bmo_pod_test.go

@@ -83,12 +83,22 @@ func TestNewBMOContainers(t *testing.T) {
 		expectedContainers []corev1.Container
 	}{
 		{
-			name:   "ManagedSpec",
+			name:   "ManagedSpec with IPv4",
 			config: managedProvisioning().build(),
 			expectedContainers: []corev1.Container{
 				withEnv(
 					containers["metal3-baremetal-operator"],
-					envWithValue("IRONIC_EXTERNAL_URL_V6", "https://[fd2e:6f44:5dd8:c956::16]:6183"),
+				),
+			},
+			sshkey: "sshkey",
+		},
+		{
+			name:   "ManagedSpec with IPv6",
+			config: managedIPv6Provisioning().build(),
+			expectedContainers: []corev1.Container{
+				withEnv(
+					containers["metal3-baremetal-operator"],
+					envWithValue("IRONIC_EXTERNAL_URL_V6", fmt.Sprintf("https://[%s]:6183", testProvisioningIPv6)),
 				),
 			},
 			sshkey: "sshkey",

provisioning/image_customization.go

@@ -240,7 +240,7 @@ func newImageCustomizationDeployment(info *ProvisioningInfo, ironicIPs []string,
 }
 
 func EnsureImageCustomizationDeployment(info *ProvisioningInfo) (updated bool, err error) {
-	ironicIPs, inspectorIPs, err := GetIronicIPs(*info)
+	ironicIPs, inspectorIPs, err := GetIronicIPs(info)
 	if err != nil {
 		return false, fmt.Errorf("unable to determine Ironic's IP to pass to the machine-image-customization-controller: %w", err)
 	}

provisioning/ironic_proxy.go

@@ -107,13 +107,14 @@ func createContainerIronicProxy(ironicIP string, images *Images) corev1.Containe
 }
 
 func newIronicProxyPodTemplateSpec(info *ProvisioningInfo) (*corev1.PodTemplateSpec, error) {
-	ironicIP, err := getPodHostIP(info.Client.CoreV1(), info.Namespace)
+	ironicIPs, err := getPodIPs(info.Client.CoreV1(), info.Namespace)
 	if err != nil {
 		return nil, errors.Wrap(err, "cannot figure out the upstream IP for ironic proxy")
 	}
 
 	containers := []corev1.Container{
-		createContainerIronicProxy(ironicIP, info.Images),
+		// Even in a dual-stack environment, we don't really care which IP address to use since both are accessible internally.
+		createContainerIronicProxy(ironicIPs[0], info.Images),
 	}
 
 	tolerations := []corev1.Toleration{

provisioning/utils.go

@@ -2,6 +2,7 @@ package provisioning
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"net"
 
@@ -55,14 +56,27 @@ func getPod(podClient coreclientv1.PodsGetter, targetNamespace string) (corev1.P
 	return pods[0], nil
 }
 
-func getPodHostIP(podClient coreclientv1.PodsGetter, targetNamespace string) (string, error) {
+// getPodIPs returns pod IPs for the Metal3 pod (and thus Ironic and its httpd).
+func getPodIPs(podClient coreclientv1.PodsGetter, targetNamespace string) (ips []string, err error) {
 	pod, err := getPod(podClient, targetNamespace)
 	if err != nil {
-		return "", err
+		return nil, err
+	}
+	// NOTE(dtantsur): we can use PodIPs here because with host networking they're identical to HostIP
+	for _, ip := range pod.Status.PodIPs {
+		if ip.IP != "" {
+			ips = append(ips, ip.IP)
+		}
+	}
+	if len(ips) == 0 {
+		// This is basically a safeguard to be able to assume that the returned slice is not empty later on
+		err = errors.New("the metal3 pod does not have any podIP's yet")
 	}
-	return pod.Status.HostIP, nil
+	return
 }
 
+// getServerInternalIPs returns virtual IPs on which Kubernetes is accessible.
+// These are the IPs on which the proxied services (currently Ironic and Inspector) should be accessed by external consumers.
 func getServerInternalIPs(osclient osclientset.Interface) ([]string, error) {
 	infra, err := osclient.ConfigV1().Infrastructures().Get(context.Background(), "cluster", metav1.GetOptions{})
 	if err != nil {
@@ -99,59 +113,43 @@ func getServerInternalIPs(osclient osclientset.Interface) ([]string, error) {
 	}
 }
 
-func GetIronicIPs(info ProvisioningInfo) (ironicIPs []string, inspectorIPs []string, err error) {
-	var podIP string
+// GetRealIronicIPs returns the actual IPs on which Ironic is accessible without a proxy.
+// The provisioning IP is used when present and not disallowed for virtual media via configuration.
+func GetRealIronicIPs(info *ProvisioningInfo) ([]string, error) {
 	config := info.ProvConfig.Spec
-
 	if config.ProvisioningNetwork != metal3iov1alpha1.ProvisioningNetworkDisabled && !config.VirtualMediaViaExternalNetwork {
-		podIP = config.ProvisioningIP
-	} else {
-		podIP, err = getPodHostIP(info.Client.CoreV1(), info.Namespace)
-		if err != nil {
-			return
-		}
+		return []string{config.ProvisioningIP}, nil
 	}
 
-	if UseIronicProxy(&config) {
+	return getPodIPs(info.Client.CoreV1(), info.Namespace)
+}
+
+// GetIronicIPs returns Ironic IPs for external consumption, potentially behind an HA proxy.
+// Without a proxy, the provisioning IP is used when present and not disallowed for virtual media via configuration.
+func GetIronicIPs(info *ProvisioningInfo) (ironicIPs []string, inspectorIPs []string, err error) {
+	podIPs, err := GetRealIronicIPs(info)
+	if err != nil {
+		return
+	}
+
+	if UseIronicProxy(&info.ProvConfig.Spec) {
 		ironicIPs, err = getServerInternalIPs(info.OSClient)
 		if err != nil {
 			err = fmt.Errorf("error fetching internalIPs: %w", err)
 			return
 		}
 
 		if ironicIPs == nil {
-			ironicIPs = []string{podIP}
+			ironicIPs = podIPs
 		}
 	} else {
-		ironicIPs = []string{podIP}
+		ironicIPs = podIPs
 	}
 
 	inspectorIPs = ironicIPs // keep returning separate variables for future enhancements
 	return ironicIPs, inspectorIPs, err
 }
 
-func GetPodIP(podClient coreclientv1.PodsGetter, targetNamespace string, networkType NetworkStackType) (string, error) {
-	pod, err := getPod(podClient, targetNamespace)
-	if err != nil {
-		return "", err
-	}
-
-	for _, podIP := range pod.Status.PodIPs {
-		if networkType == NetworkStackDual {
-			return podIP.IP, nil
-		}
-		ip := net.ParseIP(podIP.IP)
-		if networkType == NetworkStackV6 && ip.To4() == nil {
-			return podIP.IP, nil
-		}
-		if networkType == NetworkStackV4 && ip.To4() != nil {
-			return podIP.IP, nil
-		}
-	}
-
-	return "", fmt.Errorf("Pod doesn't have an IP address of the requested type")
-}
-
 func IpOptionForProvisioning(config *metal3iov1alpha1.ProvisioningSpec, networkStack NetworkStackType) string {
 	var optionValue string
 	ip := net.ParseIP(config.ProvisioningIP)