Skip to content

Commit d7b456f

Browse files
nikParasyrnikParasyr
committed
Define subnetID on LB member when networks differ
Define subnetID on LB member creation when the user is using different networks for the cluster and the loadbalancer
1 parent b126a7b commit d7b456f

File tree

2 files changed

+254
-0
lines changed

2 files changed

+254
-0
lines changed

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

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

713+
if openStackCluster.Status.Network.ID != openStackCluster.Status.APIServerLoadBalancer.LoadBalancerNetwork.ID {
714+
lbMemberOpts.SubnetID = openStackCluster.Status.Network.Subnets[0].ID
715+
}
716+
713717
if _, err := s.waitForLoadBalancerActive(lbID); err != nil {
714718
return err
715719
}

‎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: "different LB and cluster networks, set SubnetID on member create",
1117+
clusterSpec: makeCluster(nil, 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)