Skip to content

Commit 0bf4234

Browse files
nikParasyrnikParasyr
committed
Define subnetID when creating ovn LB member
Define subnetID on ovn LB member creation when the user is using different networks for the cluster and the loadbalancer
1 parent cb572a9 commit 0bf4234

File tree

2 files changed

+259
-0
lines changed

2 files changed

+259
-0
lines changed

‎pkg/cloud/services/loadbalancer/loadbalancer.go‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -710,6 +710,13 @@ func (s *Service) ReconcileLoadBalancerMember(openStackCluster *infrav1.OpenStac
710710
Tags: openStackCluster.Spec.Tags,
711711
}
712712

713+
lbProvider := openStackCluster.Spec.APIServerLoadBalancer.Provider
714+
if lbProvider != nil && *lbProvider == "ovn" &&
715+
openStackCluster.Status.Network.ID != openStackCluster.Status.APIServerLoadBalancer.LoadBalancerNetwork.ID {
716+
717+
lbMemberOpts.SubnetID = openStackCluster.Status.Network.Subnets[0].ID
718+
}
719+
713720
if _, err := s.waitForLoadBalancerActive(lbID); err != nil {
714721
return err
715722
}

‎pkg/cloud/services/loadbalancer/loadbalancer_test.go‎

Lines changed: 252 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ import (
3232
"github.com/gophercloud/gophercloud/v2/openstack/loadbalancer/v2/providers"
3333
. "github.com/onsi/gomega" //nolint:revive
3434
"go.uber.org/mock/gomock"
35+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3536
"k8s.io/utils/ptr"
3637
clusterv1beta1 "sigs.k8s.io/cluster-api/api/core/v1beta1"
3738

@@ -926,3 +927,254 @@ func Test_getOrCreateAPILoadBalancer(t *testing.T) {
926927
})
927928
}
928929
}
930+
931+
func Test_ReconcileLoadBalancerMember(t *testing.T) {
932+
g := NewWithT(t)
933+
mockCtrl := gomock.NewController(t)
934+
defer mockCtrl.Finish()
935+
936+
const (
937+
clusterName = "AAAAA"
938+
clusterResourceName = "k8s-clusterapi-cluster-AAAAA"
939+
memberIP = "10.0.0.1"
940+
wrongMemberIP = "10.0.0.20"
941+
port = 6443
942+
machineName = "machine-1"
943+
944+
clusterNetID = "aaaaaaaa-bbbb-cccc-dddd-111111111111"
945+
subnetID = "aaaaaaaa-bbbb-cccc-dddd-222222222222"
946+
lbID = "aaaaaaaa-bbbb-cccc-dddd-333333333333"
947+
listenerID = "aaaaaaaa-bbbb-cccc-dddd-444444444444"
948+
poolID = "aaaaaaaa-bbbb-cccc-dddd-555555555555"
949+
memberID = "aaaaaaaa-bbbb-cccc-dddd-666666666666"
950+
lbNetOtherID = "aaaaaaaa-bbbb-cccc-dddd-999999999999"
951+
)
952+
953+
makeCluster := func(provider *string, lbNetworkID string) *infrav1.OpenStackCluster {
954+
return &infrav1.OpenStackCluster{
955+
Spec: infrav1.OpenStackClusterSpec{
956+
APIServerLoadBalancer: &infrav1.APIServerLoadBalancer{
957+
Enabled: ptr.To(true),
958+
Provider: provider,
959+
Network: &infrav1.NetworkParam{
960+
ID: &lbNetworkID,
961+
},
962+
},
963+
DisableAPIServerFloatingIP: ptr.To(true),
964+
ControlPlaneEndpoint: &clusterv1beta1.APIEndpoint{
965+
Host: apiHostname,
966+
Port: port,
967+
},
968+
Tags: []string{"k8s", "clusterapi"},
969+
},
970+
Status: infrav1.OpenStackClusterStatus{
971+
APIServerLoadBalancer: &infrav1.LoadBalancer{
972+
ID: lbID,
973+
LoadBalancerNetwork: &infrav1.NetworkStatusWithSubnets{
974+
NetworkStatus: infrav1.NetworkStatus{
975+
ID: lbNetworkID,
976+
},
977+
},
978+
},
979+
Network: &infrav1.NetworkStatusWithSubnets{
980+
NetworkStatus: infrav1.NetworkStatus{
981+
ID: clusterNetID,
982+
},
983+
Subnets: []infrav1.Subnet{
984+
{ID: subnetID},
985+
},
986+
},
987+
},
988+
}
989+
}
990+
991+
openStackMachine := &infrav1.OpenStackMachine{
992+
ObjectMeta: metav1.ObjectMeta{Name: machineName},
993+
}
994+
995+
lbtests := []struct {
996+
name string
997+
clusterSpec *infrav1.OpenStackCluster
998+
expectNetwork func(m *mock.MockNetworkClientMockRecorder)
999+
expectLoadBalancer func(m *mock.MockLbClientMockRecorder)
1000+
wantError error
1001+
}{
1002+
{
1003+
name: "LB member exists, dont create",
1004+
clusterSpec: makeCluster(nil, clusterNetID),
1005+
expectNetwork: func(*mock.MockNetworkClientMockRecorder) {},
1006+
expectLoadBalancer: func(m *mock.MockLbClientMockRecorder) {
1007+
activeLB := loadbalancers.LoadBalancer{
1008+
ID: lbID,
1009+
Name: clusterResourceName + "-kubeapi",
1010+
ProvisioningStatus: "ACTIVE",
1011+
}
1012+
m.GetLoadBalancer(lbID).Return(&activeLB, nil).AnyTimes()
1013+
1014+
pool := pools.Pool{
1015+
ID: poolID,
1016+
Name: fmt.Sprintf("%s-kubeapi-%d", clusterResourceName, port),
1017+
}
1018+
m.ListPools(pools.ListOpts{Name: pool.Name}).Return([]pools.Pool{pool}, nil)
1019+
1020+
member := pools.Member{
1021+
Name: fmt.Sprintf("%s-kubeapi-%d-%s", clusterResourceName, port, machineName),
1022+
Address: memberIP,
1023+
}
1024+
m.ListPoolMember(poolID, pools.ListMembersOpts{Name: member.Name}).Return([]pools.Member{member}, nil)
1025+
1026+
},
1027+
wantError: nil,
1028+
},
1029+
{
1030+
name: "No LB member, create",
1031+
clusterSpec: makeCluster(nil, clusterNetID),
1032+
expectNetwork: func(*mock.MockNetworkClientMockRecorder) {},
1033+
expectLoadBalancer: func(m *mock.MockLbClientMockRecorder) {
1034+
activeLB := loadbalancers.LoadBalancer{
1035+
ID: lbID,
1036+
Name: clusterResourceName + "-kubeapi",
1037+
ProvisioningStatus: "ACTIVE",
1038+
}
1039+
m.GetLoadBalancer(lbID).Return(&activeLB, nil).AnyTimes()
1040+
1041+
pool := pools.Pool{
1042+
ID: poolID,
1043+
Name: fmt.Sprintf("%s-kubeapi-%d", clusterResourceName, port),
1044+
}
1045+
m.ListPools(pools.ListOpts{Name: pool.Name}).Return([]pools.Pool{pool}, nil)
1046+
1047+
poolMemberName := fmt.Sprintf("%s-kubeapi-%d-%s", clusterResourceName, port, machineName)
1048+
m.ListPoolMember(poolID, pools.ListMembersOpts{Name: poolMemberName}).Return([]pools.Member{}, nil)
1049+
1050+
m.CreatePoolMember(
1051+
poolID,
1052+
gomock.AssignableToTypeOf(pools.CreateMemberOpts{}),
1053+
).DoAndReturn(func(_ string, got pools.CreateMemberOpts) (*pools.Member, error) {
1054+
// SubnetID must be empty here
1055+
g.Expect(got.SubnetID).To(Equal(""))
1056+
return &pools.Member{ID: "member-2"}, nil
1057+
})
1058+
},
1059+
wantError: nil,
1060+
},
1061+
{
1062+
name: "No pool found, return error",
1063+
clusterSpec: makeCluster(nil, clusterNetID),
1064+
expectNetwork: func(*mock.MockNetworkClientMockRecorder) {},
1065+
expectLoadBalancer: func(m *mock.MockLbClientMockRecorder) {
1066+
activeLB := loadbalancers.LoadBalancer{
1067+
ID: lbID,
1068+
Name: clusterResourceName + "-kubeapi",
1069+
ProvisioningStatus: "ACTIVE",
1070+
}
1071+
m.GetLoadBalancer(lbID).Return(&activeLB, nil).AnyTimes()
1072+
1073+
poolName := fmt.Sprintf("%s-kubeapi-%d", clusterResourceName, port)
1074+
m.ListPools(pools.ListOpts{Name: poolName}).Return([]pools.Pool{}, nil)
1075+
1076+
},
1077+
wantError: errors.New("load balancer pool does not exist yet"),
1078+
},
1079+
{
1080+
name: "LB member with wrong address, re-create",
1081+
clusterSpec: makeCluster(nil, clusterNetID),
1082+
expectNetwork: func(*mock.MockNetworkClientMockRecorder) {},
1083+
expectLoadBalancer: func(m *mock.MockLbClientMockRecorder) {
1084+
activeLB := loadbalancers.LoadBalancer{
1085+
ID: lbID,
1086+
Name: clusterResourceName + "-kubeapi",
1087+
ProvisioningStatus: "ACTIVE",
1088+
}
1089+
m.GetLoadBalancer(lbID).Return(&activeLB, nil).AnyTimes()
1090+
1091+
pool := pools.Pool{
1092+
ID: poolID,
1093+
Name: fmt.Sprintf("%s-kubeapi-%d", clusterResourceName, port),
1094+
}
1095+
m.ListPools(pools.ListOpts{Name: pool.Name}).Return([]pools.Pool{pool}, nil)
1096+
1097+
member := pools.Member{
1098+
Name: fmt.Sprintf("%s-kubeapi-%d-%s", clusterResourceName, port, machineName),
1099+
Address: wrongMemberIP,
1100+
ID: memberID,
1101+
}
1102+
m.ListPoolMember(poolID, pools.ListMembersOpts{Name: member.Name}).Return([]pools.Member{member}, nil)
1103+
1104+
m.DeletePoolMember(poolID, memberID).Return(nil)
1105+
1106+
m.CreatePoolMember(
1107+
poolID,
1108+
gomock.AssignableToTypeOf(pools.CreateMemberOpts{}),
1109+
).DoAndReturn(func(_ string, got pools.CreateMemberOpts) (*pools.Member, error) {
1110+
// SubnetID must be empty here
1111+
g.Expect(got.SubnetID).To(Equal(""))
1112+
return &pools.Member{ID: "member-2"}, nil
1113+
})
1114+
},
1115+
wantError: nil,
1116+
},
1117+
{
1118+
name: "ovn provider and different networks, set SubnetID on member create",
1119+
clusterSpec: makeCluster(ptr.To("ovn"), lbNetOtherID),
1120+
expectNetwork: func(*mock.MockNetworkClientMockRecorder) {
1121+
// not used by this path
1122+
},
1123+
expectLoadBalancer: func(m *mock.MockLbClientMockRecorder) {
1124+
// LB initially ACTIVE whenever we wait
1125+
activeLB := loadbalancers.LoadBalancer{
1126+
ID: lbID,
1127+
Name: clusterResourceName + "-kubeapi",
1128+
ProvisioningStatus: "ACTIVE",
1129+
}
1130+
m.GetLoadBalancer(lbID).Return(&activeLB, nil).AnyTimes()
1131+
1132+
pool := pools.Pool{
1133+
ID: poolID,
1134+
Name: fmt.Sprintf("%s-kubeapi-%d", clusterResourceName, port),
1135+
}
1136+
m.ListPools(pools.ListOpts{Name: pool.Name}).Return([]pools.Pool{pool}, nil)
1137+
1138+
memberName := fmt.Sprintf("%s-kubeapi-%d-%s", clusterResourceName, port, machineName)
1139+
m.ListPoolMember(poolID, pools.ListMembersOpts{Name: memberName}).Return([]pools.Member{}, nil)
1140+
1141+
// Expect CreatePoolMember; capture opts to assert SubnetID is set
1142+
m.CreatePoolMember(
1143+
poolID,
1144+
gomock.AssignableToTypeOf(pools.CreateMemberOpts{}),
1145+
).DoAndReturn(func(_ string, got pools.CreateMemberOpts) (*pools.Member, error) {
1146+
g.Expect(got.Address).To(Equal(memberIP))
1147+
g.Expect(got.ProtocolPort).To(Equal(port))
1148+
expName := fmt.Sprintf("%s-kubeapi-%d-%s", clusterResourceName, port, machineName)
1149+
g.Expect(got.Name).To(Equal(expName))
1150+
g.Expect(got.SubnetID).To(Equal(subnetID))
1151+
// Tags should pass through
1152+
g.Expect(got.Tags).To(ConsistOf("k8s", "clusterapi"))
1153+
return &pools.Member{ID: "member-1", Address: memberIP, ProtocolPort: port}, nil
1154+
})
1155+
},
1156+
wantError: nil,
1157+
},
1158+
}
1159+
1160+
for _, tt := range lbtests {
1161+
t.Run(tt.name, func(t *testing.T) {
1162+
g := NewWithT(t)
1163+
log := testr.New(t)
1164+
1165+
mockScopeFactory := scope.NewMockScopeFactory(mockCtrl, "")
1166+
lbs, err := NewService(scope.NewWithLogger(mockScopeFactory, log))
1167+
g.Expect(err).NotTo(HaveOccurred())
1168+
1169+
tt.expectNetwork(mockScopeFactory.NetworkClient.EXPECT())
1170+
tt.expectLoadBalancer(mockScopeFactory.LbClient.EXPECT())
1171+
1172+
err = lbs.ReconcileLoadBalancerMember(tt.clusterSpec, openStackMachine, clusterName, memberIP)
1173+
if tt.wantError != nil {
1174+
g.Expect(err).To(MatchError(tt.wantError))
1175+
} else {
1176+
g.Expect(err).NotTo(HaveOccurred())
1177+
}
1178+
})
1179+
}
1180+
}

0 commit comments

Comments
 (0)