Skip to content

Commit ee82a04

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #2799 from nikParasyr/issue-2790
🐛 Define subnetID on LB member when networks differ
2 parents 1709f7c + d7b456f commit ee82a04

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)