Skip to content

Commit e83311b

Browse files
openshift-merge-bot[bot]openshift-merge-bot[bot]
authored
Merge pull request #9571 from shiftstack/OCPBUGS-53140
OCPBUGS-53140: Validation for API and Ingress VIPs when using user-managed load balancer
2 parents 6807ce3 + 6317f88 commit e83311b

File tree

4 files changed

+166
-25
lines changed

4 files changed

+166
-25
lines changed

‎pkg/types/openstack/defaults/platform.go

Lines changed: 35 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66

77
"github.com/apparentlymart/go-cidr/cidr"
88

9+
configv1 "github.com/openshift/api/config/v1"
910
"github.com/openshift/installer/pkg/types"
1011
"github.com/openshift/installer/pkg/types/openstack"
1112
)
@@ -25,32 +26,43 @@ func SetPlatformDefaults(p *openstack.Platform, n *types.Networking) {
2526
p.Cloud = DefaultCloudName
2627
}
2728
}
28-
// APIVIP returns the internal virtual IP address (VIP) put in front
29-
// of the Kubernetes API server for use by components inside the
30-
// cluster. The DNS static pods running on the nodes resolve the
31-
// api-int record to APIVIP.
32-
if len(p.APIVIPs) == 0 && p.DeprecatedAPIVIP == "" {
33-
vip, err := cidr.Host(&n.MachineNetwork[0].CIDR.IPNet, 5)
34-
if err != nil {
35-
// This will fail validation and abort the install
36-
p.APIVIPs = []string{fmt.Sprintf("could not derive API VIP from machine networks: %s", err.Error())}
37-
} else {
38-
p.APIVIPs = []string{vip.String()}
29+
// When there is no loadbalancer present create a default Openshift managed loadbalancer
30+
if p.LoadBalancer == nil {
31+
p.LoadBalancer = &configv1.OpenStackPlatformLoadBalancer{
32+
Type: configv1.LoadBalancerTypeOpenShiftManagedDefault,
3933
}
4034
}
4135

42-
// IngressVIP returns the internal virtual IP address (VIP) put in
43-
// front of the OpenShift router pods. This provides the internal
44-
// accessibility to the internal pods running on the worker nodes,
45-
// e.g. `console`. The DNS static pods running on the nodes resolve
46-
// the wildcard apps record to IngressVIP.
47-
if len(p.IngressVIPs) == 0 && p.DeprecatedIngressVIP == "" {
48-
vip, err := cidr.Host(&n.MachineNetwork[0].CIDR.IPNet, 7)
49-
if err != nil {
50-
// This will fail validation and abort the install
51-
p.IngressVIPs = []string{fmt.Sprintf("could not derive Ingress VIP from machine networks: %s", err.Error())}
52-
} else {
53-
p.IngressVIPs = []string{vip.String()}
36+
// When using user-managed loadbalancer do not generate default API and Ingress VIPs
37+
if p.LoadBalancer.Type != configv1.LoadBalancerTypeUserManaged {
38+
// APIVIP returns the internal virtual IP address (VIP) put in front
39+
// of the Kubernetes API server for use by components inside the
40+
// cluster. The DNS static pods running on the nodes resolve the
41+
// api-int record to APIVIP.
42+
if len(p.APIVIPs) == 0 && p.DeprecatedAPIVIP == "" {
43+
vip, err := cidr.Host(&n.MachineNetwork[0].CIDR.IPNet, 5)
44+
if err != nil {
45+
// This will fail validation and abort the install
46+
p.APIVIPs = []string{fmt.Sprintf("could not derive API VIP from machine networks: %s", err.Error())}
47+
} else {
48+
p.APIVIPs = []string{vip.String()}
49+
}
50+
}
51+
52+
// IngressVIP returns the internal virtual IP address (VIP) put in
53+
// front of the OpenShift router pods. This provides the internal
54+
// accessibility to the internal pods running on the worker nodes,
55+
// e.g. `console`. The DNS static pods running on the nodes resolve
56+
// the wildcard apps record to IngressVIP.
57+
if len(p.IngressVIPs) == 0 && p.DeprecatedIngressVIP == "" {
58+
vip, err := cidr.Host(&n.MachineNetwork[0].CIDR.IPNet, 7)
59+
if err != nil {
60+
// This will fail validation and abort the install
61+
p.IngressVIPs = []string{fmt.Sprintf("could not derive Ingress VIP from machine networks: %s", err.Error())}
62+
} else {
63+
p.IngressVIPs = []string{vip.String()}
64+
}
5465
}
5566
}
67+
5668
}

‎pkg/types/openstack/defaults/platform_test.go

Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
package defaults
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/assert"
7+
8+
configv1 "github.com/openshift/api/config/v1"
9+
"github.com/openshift/installer/pkg/ipnet"
10+
"github.com/openshift/installer/pkg/types"
11+
"github.com/openshift/installer/pkg/types/openstack"
12+
)
13+
14+
func validPlatform() *openstack.Platform {
15+
return &openstack.Platform{
16+
Cloud: "test-cloud",
17+
ExternalNetwork: "test-network",
18+
}
19+
}
20+
21+
func validNetworking() *types.Networking {
22+
return &types.Networking{
23+
NetworkType: "OVNKubernetes",
24+
MachineNetwork: []types.MachineNetworkEntry{{
25+
CIDR: *ipnet.MustParseCIDR("10.0.0.0/16"),
26+
}},
27+
}
28+
}
29+
30+
func TestSetPlatformDefaults(t *testing.T) {
31+
cases := []struct {
32+
name string
33+
platform *openstack.Platform
34+
config *types.InstallConfig
35+
networking *types.Networking
36+
expectedLB configv1.PlatformLoadBalancerType
37+
expectedAPIVIPs []string
38+
expectedIngressVIPs []string
39+
}{
40+
{
41+
name: "No load balancer provided",
42+
platform: func() *openstack.Platform {
43+
p := validPlatform()
44+
return p
45+
}(),
46+
networking: validNetworking(),
47+
expectedAPIVIPs: []string{"10.0.0.5"},
48+
expectedIngressVIPs: []string{"10.0.0.7"},
49+
expectedLB: "OpenShiftManagedDefault",
50+
},
51+
{
52+
name: "Default Openshift Managed load balancer VIPs provided",
53+
platform: func() *openstack.Platform {
54+
p := validPlatform()
55+
p.LoadBalancer = &configv1.OpenStackPlatformLoadBalancer{
56+
Type: configv1.LoadBalancerTypeOpenShiftManagedDefault,
57+
}
58+
p.APIVIPs = []string{"10.0.0.2"}
59+
p.IngressVIPs = []string{"10.0.0.3"}
60+
return p
61+
}(),
62+
networking: validNetworking(),
63+
expectedAPIVIPs: []string{"10.0.0.2"},
64+
expectedIngressVIPs: []string{"10.0.0.3"},
65+
expectedLB: "OpenShiftManagedDefault",
66+
},
67+
{
68+
name: "Default Openshift Managed load balancer no VIPs provided",
69+
platform: func() *openstack.Platform {
70+
p := validPlatform()
71+
p.LoadBalancer = &configv1.OpenStackPlatformLoadBalancer{
72+
Type: configv1.LoadBalancerTypeOpenShiftManagedDefault,
73+
}
74+
return p
75+
}(),
76+
networking: validNetworking(),
77+
expectedAPIVIPs: []string{"10.0.0.5"},
78+
expectedIngressVIPs: []string{"10.0.0.7"},
79+
expectedLB: "OpenShiftManagedDefault",
80+
},
81+
{
82+
name: "User managed load balancer VIPs provided",
83+
platform: func() *openstack.Platform {
84+
p := validPlatform()
85+
p.LoadBalancer = &configv1.OpenStackPlatformLoadBalancer{
86+
Type: configv1.LoadBalancerTypeUserManaged,
87+
}
88+
p.APIVIPs = []string{"192.168.100.10"}
89+
p.IngressVIPs = []string{"192.168.100.11"}
90+
return p
91+
}(),
92+
networking: validNetworking(),
93+
expectedAPIVIPs: []string{"192.168.100.10"},
94+
expectedIngressVIPs: []string{"192.168.100.11"},
95+
expectedLB: "UserManaged",
96+
},
97+
{
98+
name: "User managed load balancer no VIPs provided",
99+
platform: func() *openstack.Platform {
100+
p := validPlatform()
101+
p.LoadBalancer = &configv1.OpenStackPlatformLoadBalancer{
102+
Type: configv1.LoadBalancerTypeUserManaged,
103+
}
104+
return p
105+
}(),
106+
networking: validNetworking(),
107+
expectedAPIVIPs: []string(nil),
108+
expectedIngressVIPs: []string(nil),
109+
expectedLB: "UserManaged",
110+
},
111+
}
112+
for _, tc := range cases {
113+
t.Run(tc.name, func(t *testing.T) {
114+
SetPlatformDefaults(tc.platform, tc.networking)
115+
assert.Equal(t, tc.expectedLB, tc.platform.LoadBalancer.Type, "unexpected loadbalancer")
116+
assert.Equal(t, tc.expectedAPIVIPs, tc.platform.APIVIPs, "unexpected APIVIPs")
117+
assert.Equal(t, tc.expectedIngressVIPs, tc.platform.IngressVIPs, "unexpected IngressVIPs")
118+
})
119+
}
120+
}

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

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,13 @@ def setUp(self):
2020
self.cluster_infra = yaml.load(f, Loader=yaml.FullLoader)
2121

2222
def test_load_balancer(self):
23-
"""Assert that the Cluster infrastructure object does not contain the LoadBalancer configuration."""
24-
self.assertNotIn("loadBalancer", self.cluster_infra["status"]["platformStatus"]["openstack"])
23+
"""Assert that the Cluster infrastructure object contains the default LoadBalancer configuration."""
24+
self.assertIn("loadBalancer", self.cluster_infra["status"]["platformStatus"]["openstack"])
25+
26+
loadBalancer = self.cluster_infra["status"]["platformStatus"]["openstack"]["loadBalancer"]
27+
28+
self.assertIn("type", loadBalancer)
29+
self.assertEqual("OpenShiftManagedDefault", loadBalancer["type"])
2530

2631

2732
if __name__ == '__main__':

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,4 +30,8 @@ platform:
3030
cloud: ${OS_CLOUD}
3131
loadBalancer:
3232
type: UserManaged
33+
apiVIPs:
34+
- 10.0.128.10
35+
ingressVIPs:
36+
- 10.0.128.20
3337
pullSecret: ${PULL_SECRET}

0 commit comments

Comments
 (0)