OCPBUGS-53140: Validation for API and Ingress VIPs when using user-managed load balancer

pkg/types/openstack/defaults/platform.go

@@ -6,6 +6,7 @@ import (
 
 	"github.com/apparentlymart/go-cidr/cidr"
 
+	configv1 "github.com/openshift/api/config/v1"
 	"github.com/openshift/installer/pkg/types"
 	"github.com/openshift/installer/pkg/types/openstack"
 )
@@ -25,32 +26,43 @@ func SetPlatformDefaults(p *openstack.Platform, n *types.Networking) {
 			p.Cloud = DefaultCloudName
 		}
 	}
-	// APIVIP returns the internal virtual IP address (VIP) put in front
-	// of the Kubernetes API server for use by components inside the
-	// cluster. The DNS static pods running on the nodes resolve the
-	// api-int record to APIVIP.
-	if len(p.APIVIPs) == 0 && p.DeprecatedAPIVIP == "" {
-		vip, err := cidr.Host(&n.MachineNetwork[0].CIDR.IPNet, 5)
-		if err != nil {
-			// This will fail validation and abort the install
-			p.APIVIPs = []string{fmt.Sprintf("could not derive API VIP from machine networks: %s", err.Error())}
-		} else {
-			p.APIVIPs = []string{vip.String()}
+	// When there is no loadbalancer present create a default Openshift managed loadbalancer
+	if p.LoadBalancer == nil {
+		p.LoadBalancer = &configv1.OpenStackPlatformLoadBalancer{
+			Type: configv1.LoadBalancerTypeOpenShiftManagedDefault,
 		}
 	}
 
-	// IngressVIP returns the internal virtual IP address (VIP) put in
-	// front of the OpenShift router pods. This provides the internal
-	// accessibility to the internal pods running on the worker nodes,
-	// e.g. `console`. The DNS static pods running on the nodes resolve
-	// the wildcard apps record to IngressVIP.
-	if len(p.IngressVIPs) == 0 && p.DeprecatedIngressVIP == "" {
-		vip, err := cidr.Host(&n.MachineNetwork[0].CIDR.IPNet, 7)
-		if err != nil {
-			// This will fail validation and abort the install
-			p.IngressVIPs = []string{fmt.Sprintf("could not derive Ingress VIP from machine networks: %s", err.Error())}
-		} else {
-			p.IngressVIPs = []string{vip.String()}
+	// When using user-managed loadbalancer do not generate default API and Ingress VIPs
+	if p.LoadBalancer.Type != configv1.LoadBalancerTypeUserManaged {
+		// APIVIP returns the internal virtual IP address (VIP) put in front
+		// of the Kubernetes API server for use by components inside the
+		// cluster. The DNS static pods running on the nodes resolve the
+		// api-int record to APIVIP.
+		if len(p.APIVIPs) == 0 && p.DeprecatedAPIVIP == "" {
+			vip, err := cidr.Host(&n.MachineNetwork[0].CIDR.IPNet, 5)
+			if err != nil {
+				// This will fail validation and abort the install
+				p.APIVIPs = []string{fmt.Sprintf("could not derive API VIP from machine networks: %s", err.Error())}
+			} else {
+				p.APIVIPs = []string{vip.String()}
+			}
+		}
+
+		// IngressVIP returns the internal virtual IP address (VIP) put in
+		// front of the OpenShift router pods. This provides the internal
+		// accessibility to the internal pods running on the worker nodes,
+		// e.g. `console`. The DNS static pods running on the nodes resolve
+		// the wildcard apps record to IngressVIP.
+		if len(p.IngressVIPs) == 0 && p.DeprecatedIngressVIP == "" {
+			vip, err := cidr.Host(&n.MachineNetwork[0].CIDR.IPNet, 7)
+			if err != nil {
+				// This will fail validation and abort the install
+				p.IngressVIPs = []string{fmt.Sprintf("could not derive Ingress VIP from machine networks: %s", err.Error())}
+			} else {
+				p.IngressVIPs = []string{vip.String()}
+			}
 		}
 	}
+
 }

pkg/types/openstack/defaults/platform_test.go

@@ -0,0 +1,120 @@
+package defaults
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+
+	configv1 "github.com/openshift/api/config/v1"
+	"github.com/openshift/installer/pkg/ipnet"
+	"github.com/openshift/installer/pkg/types"
+	"github.com/openshift/installer/pkg/types/openstack"
+)
+
+func validPlatform() *openstack.Platform {
+	return &openstack.Platform{
+		Cloud:           "test-cloud",
+		ExternalNetwork: "test-network",
+	}
+}
+
+func validNetworking() *types.Networking {
+	return &types.Networking{
+		NetworkType: "OVNKubernetes",
+		MachineNetwork: []types.MachineNetworkEntry{{
+			CIDR: *ipnet.MustParseCIDR("10.0.0.0/16"),
+		}},
+	}
+}
+
+func TestSetPlatformDefaults(t *testing.T) {
+	cases := []struct {
+		name                string
+		platform            *openstack.Platform
+		config              *types.InstallConfig
+		networking          *types.Networking
+		expectedLB          configv1.PlatformLoadBalancerType
+		expectedAPIVIPs     []string
+		expectedIngressVIPs []string
+	}{
+		{
+			name: "No load balancer provided",
+			platform: func() *openstack.Platform {
+				p := validPlatform()
+				return p
+			}(),
+			networking:          validNetworking(),
+			expectedAPIVIPs:     []string{"10.0.0.5"},
+			expectedIngressVIPs: []string{"10.0.0.7"},
+			expectedLB:          "OpenShiftManagedDefault",
+		},
+		{
+			name: "Default Openshift Managed load balancer VIPs provided",
+			platform: func() *openstack.Platform {
+				p := validPlatform()
+				p.LoadBalancer = &configv1.OpenStackPlatformLoadBalancer{
+					Type: configv1.LoadBalancerTypeOpenShiftManagedDefault,
+				}
+				p.APIVIPs = []string{"10.0.0.2"}
+				p.IngressVIPs = []string{"10.0.0.3"}
+				return p
+			}(),
+			networking:          validNetworking(),
+			expectedAPIVIPs:     []string{"10.0.0.2"},
+			expectedIngressVIPs: []string{"10.0.0.3"},
+			expectedLB:          "OpenShiftManagedDefault",
+		},
+		{
+			name: "Default Openshift Managed load balancer no VIPs provided",
+			platform: func() *openstack.Platform {
+				p := validPlatform()
+				p.LoadBalancer = &configv1.OpenStackPlatformLoadBalancer{
+					Type: configv1.LoadBalancerTypeOpenShiftManagedDefault,
+				}
+				return p
+			}(),
+			networking:          validNetworking(),
+			expectedAPIVIPs:     []string{"10.0.0.5"},
+			expectedIngressVIPs: []string{"10.0.0.7"},
+			expectedLB:          "OpenShiftManagedDefault",
+		},
+		{
+			name: "User managed load balancer VIPs provided",
+			platform: func() *openstack.Platform {
+				p := validPlatform()
+				p.LoadBalancer = &configv1.OpenStackPlatformLoadBalancer{
+					Type: configv1.LoadBalancerTypeUserManaged,
+				}
+				p.APIVIPs = []string{"192.168.100.10"}
+				p.IngressVIPs = []string{"192.168.100.11"}
+				return p
+			}(),
+			networking:          validNetworking(),
+			expectedAPIVIPs:     []string{"192.168.100.10"},
+			expectedIngressVIPs: []string{"192.168.100.11"},
+			expectedLB:          "UserManaged",
+		},
+		{
+			name: "User managed load balancer no VIPs provided",
+			platform: func() *openstack.Platform {
+				p := validPlatform()
+				p.LoadBalancer = &configv1.OpenStackPlatformLoadBalancer{
+					Type: configv1.LoadBalancerTypeUserManaged,
+				}
+				return p
+			}(),
+			networking:          validNetworking(),
+			expectedAPIVIPs:     []string(nil),
+			expectedIngressVIPs: []string(nil),
+			expectedLB:          "UserManaged",
+		},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			SetPlatformDefaults(tc.platform, tc.networking)
+			assert.Equal(t, tc.expectedLB, tc.platform.LoadBalancer.Type, "unexpected loadbalancer")
+			assert.Equal(t, tc.expectedAPIVIPs, tc.platform.APIVIPs, "unexpected APIVIPs")
+			assert.Equal(t, tc.expectedIngressVIPs, tc.platform.IngressVIPs, "unexpected IngressVIPs")
+		})
+	}
+}

scripts/openstack/manifest-tests/lb-default-stable/test_cluster-infra.py

@@ -20,8 +20,13 @@ def setUp(self):
             self.cluster_infra = yaml.load(f, Loader=yaml.FullLoader)
 
     def test_load_balancer(self):
-        """Assert that the Cluster infrastructure object does not contain the LoadBalancer configuration."""
-        self.assertNotIn("loadBalancer", self.cluster_infra["status"]["platformStatus"]["openstack"])
+        """Assert that the Cluster infrastructure object contains the default LoadBalancer configuration."""
+        self.assertIn("loadBalancer", self.cluster_infra["status"]["platformStatus"]["openstack"])
+
+        loadBalancer = self.cluster_infra["status"]["platformStatus"]["openstack"]["loadBalancer"]
+
+        self.assertIn("type", loadBalancer)
+        self.assertEqual("OpenShiftManagedDefault", loadBalancer["type"])
 
 
 if __name__ == '__main__':

scripts/openstack/manifest-tests/lb-unmanaged/install-config.yaml

@@ -30,4 +30,8 @@ platform:
     cloud: ${OS_CLOUD}
     loadBalancer:
       type: UserManaged
+    apiVIPs:
+      - 10.0.128.10
+    ingressVIPs:
+      - 10.0.128.20
 pullSecret: ${PULL_SECRET}