Skip to content

Commit ccba74b

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 ccba74b

File tree

2 files changed

+256
-0
lines changed

2 files changed

+256
-0
lines changed

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -710,6 +710,12 @@ 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+
lbMemberOpts.SubnetID = openStackCluster.Status.Network.Subnets[0].ID
717+
}
718+
713719
if _, err := s.waitForLoadBalancerActive(lbID); err != nil {
714720
return err
715721
}

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

Lines changed: 250 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,252 @@ 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+
wantError: nil,
1027+
},
1028+
{
1029+
name: "No LB member, create",
1030+
clusterSpec: makeCluster(nil, clusterNetID),
1031+
expectNetwork: func(*mock.MockNetworkClientMockRecorder) {},
1032+
expectLoadBalancer: func(m *mock.MockLbClientMockRecorder) {
1033+
activeLB := loadbalancers.LoadBalancer{
1034+
ID: lbID,
1035+
Name: clusterResourceName + "-kubeapi",
1036+
ProvisioningStatus: "ACTIVE",
1037+
}
1038+
m.GetLoadBalancer(lbID).Return(&activeLB, nil).AnyTimes()
1039+
1040+
pool := pools.Pool{
1041+
ID: poolID,
1042+
Name: fmt.Sprintf("%s-kubeapi-%d", clusterResourceName, port),
1043+
}
1044+
m.ListPools(pools.ListOpts{Name: pool.Name}).Return([]pools.Pool{pool}, nil)
1045+
1046+
poolMemberName := fmt.Sprintf("%s-kubeapi-%d-%s", clusterResourceName, port, machineName)
1047+
m.ListPoolMember(poolID, pools.ListMembersOpts{Name: poolMemberName}).Return([]pools.Member{}, nil)
1048+
1049+
m.CreatePoolMember(
1050+
poolID,
1051+
gomock.AssignableToTypeOf(pools.CreateMemberOpts{}),
1052+
).DoAndReturn(func(_ string, got pools.CreateMemberOpts) (*pools.Member, error) {
1053+
// SubnetID must be empty here
1054+
g.Expect(got.SubnetID).To(Equal(""))
1055+
return &pools.Member{ID: "member-2"}, nil
1056+
})
1057+
},
1058+
wantError: nil,
1059+
},
1060+
{
1061+
name: "No pool found, return error",
1062+
clusterSpec: makeCluster(nil, clusterNetID),
1063+
expectNetwork: func(*mock.MockNetworkClientMockRecorder) {},
1064+
expectLoadBalancer: func(m *mock.MockLbClientMockRecorder) {
1065+
activeLB := loadbalancers.LoadBalancer{
1066+
ID: lbID,
1067+
Name: clusterResourceName + "-kubeapi",
1068+
ProvisioningStatus: "ACTIVE",
1069+
}
1070+
m.GetLoadBalancer(lbID).Return(&activeLB, nil).AnyTimes()
1071+
1072+
poolName := fmt.Sprintf("%s-kubeapi-%d", clusterResourceName, port)
1073+
m.ListPools(pools.ListOpts{Name: poolName}).Return([]pools.Pool{}, nil)
1074+
},
1075+
wantError: errors.New("load balancer pool does not exist yet"),
1076+
},
1077+
{
1078+
name: "LB member with wrong address, re-create",
1079+
clusterSpec: makeCluster(nil, clusterNetID),
1080+
expectNetwork: func(*mock.MockNetworkClientMockRecorder) {},
1081+
expectLoadBalancer: func(m *mock.MockLbClientMockRecorder) {
1082+
activeLB := loadbalancers.LoadBalancer{
1083+
ID: lbID,
1084+
Name: clusterResourceName + "-kubeapi",
1085+
ProvisioningStatus: "ACTIVE",
1086+
}
1087+
m.GetLoadBalancer(lbID).Return(&activeLB, nil).AnyTimes()
1088+
1089+
pool := pools.Pool{
1090+
ID: poolID,
1091+
Name: fmt.Sprintf("%s-kubeapi-%d", clusterResourceName, port),
1092+
}
1093+
m.ListPools(pools.ListOpts{Name: pool.Name}).Return([]pools.Pool{pool}, nil)
1094+
1095+
member := pools.Member{
1096+
Name: fmt.Sprintf("%s-kubeapi-%d-%s", clusterResourceName, port, machineName),
1097+
Address: wrongMemberIP,
1098+
ID: memberID,
1099+
}
1100+
m.ListPoolMember(poolID, pools.ListMembersOpts{Name: member.Name}).Return([]pools.Member{member}, nil)
1101+
1102+
m.DeletePoolMember(poolID, memberID).Return(nil)
1103+
1104+
m.CreatePoolMember(
1105+
poolID,
1106+
gomock.AssignableToTypeOf(pools.CreateMemberOpts{}),
1107+
).DoAndReturn(func(_ string, got pools.CreateMemberOpts) (*pools.Member, error) {
1108+
// SubnetID must be empty here
1109+
g.Expect(got.SubnetID).To(Equal(""))
1110+
return &pools.Member{ID: "member-2"}, nil
1111+
})
1112+
},
1113+
wantError: nil,
1114+
},
1115+
{
1116+
name: "ovn provider and different networks, set SubnetID on member create",
1117+
clusterSpec: makeCluster(ptr.To("ovn"), lbNetOtherID),
1118+
expectNetwork: func(*mock.MockNetworkClientMockRecorder) {
1119+
// not used by this path
1120+
},
1121+
expectLoadBalancer: func(m *mock.MockLbClientMockRecorder) {
1122+
// LB initially ACTIVE whenever we wait
1123+
activeLB := loadbalancers.LoadBalancer{
1124+
ID: lbID,
1125+
Name: clusterResourceName + "-kubeapi",
1126+
ProvisioningStatus: "ACTIVE",
1127+
}
1128+
m.GetLoadBalancer(lbID).Return(&activeLB, nil).AnyTimes()
1129+
1130+
pool := pools.Pool{
1131+
ID: poolID,
1132+
Name: fmt.Sprintf("%s-kubeapi-%d", clusterResourceName, port),
1133+
}
1134+
m.ListPools(pools.ListOpts{Name: pool.Name}).Return([]pools.Pool{pool}, nil)
1135+
1136+
memberName := fmt.Sprintf("%s-kubeapi-%d-%s", clusterResourceName, port, machineName)
1137+
m.ListPoolMember(poolID, pools.ListMembersOpts{Name: memberName}).Return([]pools.Member{}, nil)
1138+
1139+
// Expect CreatePoolMember; capture opts to assert SubnetID is set
1140+
m.CreatePoolMember(
1141+
poolID,
1142+
gomock.AssignableToTypeOf(pools.CreateMemberOpts{}),
1143+
).DoAndReturn(func(_ string, got pools.CreateMemberOpts) (*pools.Member, error) {
1144+
g.Expect(got.Address).To(Equal(memberIP))
1145+
g.Expect(got.ProtocolPort).To(Equal(port))
1146+
expName := fmt.Sprintf("%s-kubeapi-%d-%s", clusterResourceName, port, machineName)
1147+
g.Expect(got.Name).To(Equal(expName))
1148+
g.Expect(got.SubnetID).To(Equal(subnetID))
1149+
// Tags should pass through
1150+
g.Expect(got.Tags).To(ConsistOf("k8s", "clusterapi"))
1151+
return &pools.Member{ID: "member-1", Address: memberIP, ProtocolPort: port}, nil
1152+
})
1153+
},
1154+
wantError: nil,
1155+
},
1156+
}
1157+
1158+
for _, tt := range lbtests {
1159+
t.Run(tt.name, func(t *testing.T) {
1160+
g := NewWithT(t)
1161+
log := testr.New(t)
1162+
1163+
mockScopeFactory := scope.NewMockScopeFactory(mockCtrl, "")
1164+
lbs, err := NewService(scope.NewWithLogger(mockScopeFactory, log))
1165+
g.Expect(err).NotTo(HaveOccurred())
1166+
1167+
tt.expectNetwork(mockScopeFactory.NetworkClient.EXPECT())
1168+
tt.expectLoadBalancer(mockScopeFactory.LbClient.EXPECT())
1169+
1170+
err = lbs.ReconcileLoadBalancerMember(tt.clusterSpec, openStackMachine, clusterName, memberIP)
1171+
if tt.wantError != nil {
1172+
g.Expect(err).To(MatchError(tt.wantError))
1173+
} else {
1174+
g.Expect(err).NotTo(HaveOccurred())
1175+
}
1176+
})
1177+
}
1178+
}

0 commit comments

Comments
 (0)