Skip to content

Commit af045d1

Browse files
bcmendoza820bcmendoza820
authored
feat: Support high performance ingress with no label selector
2 parents 0c600bb + b00cd63 commit af045d1

File tree

2 files changed

+90
-33
lines changed

2 files changed

+90
-33
lines changed

‎cloud/linode/cilium_loadbalancers.go

Lines changed: 49 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ package linode
33
import (
44
"context"
55
"encoding/json"
6-
"errors"
76
"fmt"
87
"net/http"
98
"slices"
@@ -29,6 +28,8 @@ const (
2928
ciliumLBClass = "io.cilium/bgp-control-plane"
3029
ipHolderLabelPrefix = "linode-ccm-ip-holder"
3130
ciliumBGPPeeringPolicyName = "linode-ccm-bgp-peering"
31+
32+
commonControlPlaneLabel = "node-role.kubernetes.io/control-plane"
3233
)
3334

3435
// This mapping is unfortunately necessary since there is no way to get the
@@ -63,7 +64,6 @@ var (
6364
"ap-southeast": 16, // Sydney (Australia)
6465
"ap-northeast": 11, // Tokyo (Japan)
6566
}
66-
errNoBGPSelector = errors.New("no BGP node selector set to configure IP sharing")
6767
)
6868

6969
// getExistingSharedIPsInCluster determines the list of addresses to share on nodes by checking the
@@ -123,6 +123,9 @@ func (l *loadbalancers) shareIPs(ctx context.Context, addrs []string, node *v1.N
123123
if err != nil {
124124
return err
125125
}
126+
if node.Labels == nil {
127+
node.Labels = make(map[string]string)
128+
}
126129
node.Labels[annotations.AnnLinodeNodeIPSharingUpdated] = "true"
127130
retryErr := retry.RetryOnConflict(retry.DefaultRetry, func() error {
128131
_, err := l.kubeClient.CoreV1().Nodes().Update(ctx, node, metav1.UpdateOptions{})
@@ -148,15 +151,17 @@ func (l *loadbalancers) handleIPSharing(ctx context.Context, node *v1.Node) erro
148151
klog.Info("skipping IP while providerID is unset")
149152
return nil
150153
}
151-
if Options.BGPNodeSelector == "" {
152-
return errNoBGPSelector
153-
}
154154
// If performing Service load-balancing via IP sharing + BGP, check for a special annotation
155155
// added by the CCM gets set when load-balancer IPs have been successfully shared on the node
156-
kv := strings.Split(Options.BGPNodeSelector, "=")
157-
// Check if node should be participating in IP sharing via the given selector
158-
if val, ok := node.Labels[kv[0]]; !ok || len(kv) != 2 || val != kv[1] {
159-
// not a selected Node
156+
if Options.BGPNodeSelector != "" {
157+
kv := strings.Split(Options.BGPNodeSelector, "=")
158+
// Check if node should be participating in IP sharing via the given selector
159+
if val, ok := node.Labels[kv[0]]; !ok || len(kv) != 2 || val != kv[1] {
160+
// not a selected Node
161+
return nil
162+
}
163+
} else if _, ok := node.Labels[commonControlPlaneLabel]; ok {
164+
// If there is no node selector specified, default to sharing across worker nodes only
160165
return nil
161166
}
162167
// check if node has been updated with IPs to share
@@ -200,10 +205,6 @@ func (l *loadbalancers) handleIPSharing(ctx context.Context, node *v1.Node) erro
200205
// createSharedIP requests an additional IP that can be shared on Nodes to support
201206
// loadbalancing via Cilium LB IPAM + BGP Control Plane.
202207
func (l *loadbalancers) createSharedIP(ctx context.Context, nodes []*v1.Node) (string, error) {
203-
if Options.BGPNodeSelector == "" {
204-
return "", errNoBGPSelector
205-
}
206-
207208
ipHolder, err := l.ensureIPHolder(ctx)
208209
if err != nil {
209210
return "", err
@@ -237,11 +238,21 @@ func (l *loadbalancers) createSharedIP(ctx context.Context, nodes []*v1.Node) (s
237238
}
238239

239240
// share the IPs with nodes participating in Cilium BGP peering
240-
kv := strings.Split(Options.BGPNodeSelector, "=")
241-
for _, node := range nodes {
242-
if val, ok := node.Labels[kv[0]]; ok && len(kv) == 2 && val == kv[1] {
243-
if err = l.shareIPs(ctx, addrs, node); err != nil {
244-
return "", err
241+
if Options.BGPNodeSelector == "" {
242+
for _, node := range nodes {
243+
if _, ok := node.Labels[commonControlPlaneLabel]; !ok {
244+
if err = l.shareIPs(ctx, addrs, node); err != nil {
245+
return "", err
246+
}
247+
}
248+
}
249+
} else {
250+
kv := strings.Split(Options.BGPNodeSelector, "=")
251+
for _, node := range nodes {
252+
if val, ok := node.Labels[kv[0]]; ok && len(kv) == 2 && val == kv[1] {
253+
if err = l.shareIPs(ctx, addrs, node); err != nil {
254+
return "", err
255+
}
245256
}
246257
}
247258
}
@@ -252,9 +263,6 @@ func (l *loadbalancers) createSharedIP(ctx context.Context, nodes []*v1.Node) (s
252263
// deleteSharedIP cleans up the shared IP for a LoadBalancer Service if it was assigned
253264
// by Cilium LB IPAM, removing it from the ip-holder
254265
func (l *loadbalancers) deleteSharedIP(ctx context.Context, service *v1.Service) error {
255-
if Options.BGPNodeSelector == "" {
256-
return errNoBGPSelector
257-
}
258266
err := l.retrieveKubeClient()
259267
if err != nil {
260268
return err
@@ -421,9 +429,6 @@ func (l *loadbalancers) getCiliumLBIPPool(ctx context.Context, service *v1.Servi
421429

422430
// NOTE: Cilium CRDs must be installed for this to work
423431
func (l *loadbalancers) ensureCiliumBGPPeeringPolicy(ctx context.Context) error {
424-
if Options.BGPNodeSelector == "" {
425-
return errNoBGPSelector
426-
}
427432
regionID, ok := regionIDMap[l.zone]
428433
if !ok {
429434
return fmt.Errorf("unsupported region for BGP: %s", l.zone)
@@ -443,16 +448,32 @@ func (l *loadbalancers) ensureCiliumBGPPeeringPolicy(ctx context.Context) error
443448
}
444449

445450
// otherwise create it
446-
kv := strings.Split(Options.BGPNodeSelector, "=")
447-
if len(kv) != 2 {
448-
return fmt.Errorf("invalid node selector %s", Options.BGPNodeSelector)
451+
var nodeSelector slimv1.LabelSelector
452+
// If no BGPNodeSelector is specified, select all worker nodes.
453+
if Options.BGPNodeSelector == "" {
454+
nodeSelector = slimv1.LabelSelector{
455+
MatchExpressions: []slimv1.LabelSelectorRequirement{
456+
{
457+
Key: commonControlPlaneLabel,
458+
Operator: slimv1.LabelSelectorOpDoesNotExist,
459+
},
460+
},
461+
}
462+
} else {
463+
kv := strings.Split(Options.BGPNodeSelector, "=")
464+
if len(kv) != 2 {
465+
return fmt.Errorf("invalid node selector %s", Options.BGPNodeSelector)
466+
}
467+
468+
nodeSelector = slimv1.LabelSelector{MatchLabels: map[string]string{kv[0]: kv[1]}}
449469
}
470+
450471
ciliumBGPPeeringPolicy := &v2alpha1.CiliumBGPPeeringPolicy{
451472
ObjectMeta: metav1.ObjectMeta{
452473
Name: ciliumBGPPeeringPolicyName,
453474
},
454475
Spec: v2alpha1.CiliumBGPPeeringPolicySpec{
455-
NodeSelector: &slimv1.LabelSelector{MatchLabels: map[string]string{kv[0]: kv[1]}},
476+
NodeSelector: &nodeSelector,
456477
VirtualRouters: []v2alpha1.CiliumBGPVirtualRouter{{
457478
LocalASN: 65001,
458479
ExportPodCIDR: ptr.To(true),

‎cloud/linode/cilium_loadbalancers_test.go

Lines changed: 41 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ package linode
33
import (
44
"context"
55
"encoding/json"
6-
"errors"
76
"fmt"
87
"net"
98
"testing"
@@ -47,6 +46,17 @@ var (
4746
ProviderID: fmt.Sprintf("%s%d", providerIDPrefix, 33333),
4847
},
4948
},
49+
{
50+
ObjectMeta: metav1.ObjectMeta{
51+
Name: "node-control",
52+
Labels: map[string]string{
53+
commonControlPlaneLabel: "",
54+
},
55+
},
56+
Spec: v1.NodeSpec{
57+
ProviderID: fmt.Sprintf("%s%d", providerIDPrefix, 44444),
58+
},
59+
},
5060
}
5161
publicIPv4 = net.ParseIP("45.76.101.25")
5262
ipHolderInstance = linodego.Instance{
@@ -139,19 +149,45 @@ func addNodes(t *testing.T, kubeClient kubernetes.Interface, nodes []*v1.Node) {
139149
}
140150

141151
func testNoBGPNodeLabel(t *testing.T, mc *mocks.MockClient) {
152+
Options.BGPNodeSelector = ""
142153
svc := createTestService()
143154

144155
kubeClient, _ := k8sClient.NewFakeClientset()
145156
ciliumClient := &fakev2alpha1.FakeCiliumV2alpha1{Fake: &kubeClient.CiliumFakeClientset.Fake}
146157
addService(t, kubeClient, svc)
158+
addNodes(t, kubeClient, nodes)
147159
lb := &loadbalancers{mc, zone, kubeClient, ciliumClient, ciliumLBType}
148160

161+
filter := map[string]string{"label": fmt.Sprintf("%s-%s", ipHolderLabelPrefix, zone)}
162+
rawFilter, _ := json.Marshal(filter)
163+
mc.EXPECT().ListInstances(gomock.Any(), linodego.NewListOptions(1, string(rawFilter))).Times(1).Return([]linodego.Instance{}, nil)
164+
dummySharedIP := "45.76.101.26"
165+
mc.EXPECT().CreateInstance(gomock.Any(), gomock.Any()).Times(1).Return(&ipHolderInstance, nil)
166+
mc.EXPECT().GetInstanceIPAddresses(gomock.Any(), ipHolderInstance.ID).Times(1).Return(&linodego.InstanceIPAddressResponse{
167+
IPv4: &linodego.InstanceIPv4Response{
168+
Shared: []*linodego.InstanceIP{{Address: dummySharedIP}},
169+
},
170+
}, nil)
171+
mc.EXPECT().AddInstanceIPAddress(gomock.Any(), ipHolderInstance.ID, true).Times(1).Return(&linodego.InstanceIP{Address: dummySharedIP}, nil)
172+
mc.EXPECT().ShareIPAddresses(gomock.Any(), linodego.IPAddressesShareOptions{
173+
IPs: []string{dummySharedIP},
174+
LinodeID: 11111,
175+
}).Times(1)
176+
mc.EXPECT().ShareIPAddresses(gomock.Any(), linodego.IPAddressesShareOptions{
177+
IPs: []string{dummySharedIP},
178+
LinodeID: 22222,
179+
}).Times(1)
180+
mc.EXPECT().ShareIPAddresses(gomock.Any(), linodego.IPAddressesShareOptions{
181+
IPs: []string{dummySharedIP},
182+
LinodeID: 33333,
183+
}).Times(1)
184+
149185
lbStatus, err := lb.EnsureLoadBalancer(context.TODO(), "linodelb", svc, nodes)
150-
if !errors.Is(err, errNoBGPSelector) {
151-
t.Fatalf("expected %v, got %v... %s", errNoBGPSelector, err, Options.BGPNodeSelector)
186+
if err != nil {
187+
t.Fatalf("expected a nil error, got %v", err)
152188
}
153-
if lbStatus != nil {
154-
t.Fatalf("expected a nil lbStatus, got %v", lbStatus)
189+
if lbStatus == nil {
190+
t.Fatal("expected non-nil lbStatus")
155191
}
156192
}
157193

0 commit comments

Comments
 (0)