Skip to content

Commit 2d6c13c

Browse files
committed
create ip holder per cluster + tests, update docs
Signed-off-by: Ross Kirkpatrick <[email protected]>
1 parent dc407dd commit 2d6c13c

9 files changed

+368
-131
lines changed

README.md

+41-40
Large diffs are not rendered by default.

cloud/linode/cilium_loadbalancers.go

+63-11
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ func (l *loadbalancers) shareIPs(ctx context.Context, addrs []string, node *v1.N
145145
// perform IP sharing (via a specified node selector) have the expected IPs shared
146146
// in the event that a Node joins the cluster after the LoadBalancer Service already
147147
// exists
148-
func (l *loadbalancers) handleIPSharing(ctx context.Context, node *v1.Node) error {
148+
func (l *loadbalancers) handleIPSharing(ctx context.Context, node *v1.Node, ipHolderSuffix string) error {
149149
// ignore cases where the provider ID has been set
150150
if node.Spec.ProviderID == "" {
151151
klog.Info("skipping IP while providerID is unset")
@@ -179,7 +179,7 @@ func (l *loadbalancers) handleIPSharing(ctx context.Context, node *v1.Node) erro
179179
// if any of the addrs don't exist on the ip-holder (e.g. someone manually deleted it outside the CCM),
180180
// we need to exclude that from the list
181181
// TODO: also clean up the CiliumLoadBalancerIPPool for that missing IP if that happens
182-
ipHolder, err := l.getIPHolder(ctx)
182+
ipHolder, err := l.getIPHolder(ctx, ipHolderSuffix)
183183
if err != nil {
184184
return err
185185
}
@@ -204,8 +204,8 @@ func (l *loadbalancers) handleIPSharing(ctx context.Context, node *v1.Node) erro
204204

205205
// createSharedIP requests an additional IP that can be shared on Nodes to support
206206
// loadbalancing via Cilium LB IPAM + BGP Control Plane.
207-
func (l *loadbalancers) createSharedIP(ctx context.Context, nodes []*v1.Node) (string, error) {
208-
ipHolder, err := l.ensureIPHolder(ctx)
207+
func (l *loadbalancers) createSharedIP(ctx context.Context, nodes []*v1.Node, ipHolderSuffix string) (string, error) {
208+
ipHolder, err := l.ensureIPHolder(ctx, ipHolderSuffix)
209209
if err != nil {
210210
return "", err
211211
}
@@ -273,7 +273,20 @@ func (l *loadbalancers) deleteSharedIP(ctx context.Context, service *v1.Service)
273273
return err
274274
}
275275
bgpNodes := nodeList.Items
276-
ipHolder, err := l.getIPHolder(ctx)
276+
277+
serviceNn := getServiceNn(service)
278+
var ipHolderSuffix string
279+
if Options.IpHolderSuffix != "" {
280+
ipHolderSuffix = Options.IpHolderSuffix
281+
klog.V(3).Infof("using parameter-based IP Holder suffix %s for Service %s", ipHolderSuffix, serviceNn)
282+
} else {
283+
if service.Namespace != "" {
284+
ipHolderSuffix = service.Namespace
285+
klog.V(3).Infof("using service-based IP Holder suffix %s for Service %s", ipHolderSuffix, serviceNn)
286+
}
287+
}
288+
289+
ipHolder, err := l.getIPHolder(ctx, ipHolderSuffix)
277290
if err != nil {
278291
// return error or nil if not found since no IP holder means there
279292
// is no IP to reclaim
@@ -307,48 +320,87 @@ func (l *loadbalancers) deleteSharedIP(ctx context.Context, service *v1.Service)
307320

308321
// To hold the IP in lieu of a proper IP reservation system, a special Nanode is
309322
// created but not booted and used to hold all shared IPs.
310-
func (l *loadbalancers) ensureIPHolder(ctx context.Context) (*linodego.Instance, error) {
311-
ipHolder, err := l.getIPHolder(ctx)
323+
func (l *loadbalancers) ensureIPHolder(ctx context.Context, suffix string) (*linodego.Instance, error) {
324+
ipHolder, err := l.getIPHolder(ctx, suffix)
312325
if err != nil {
313326
return nil, err
314327
}
315328
if ipHolder != nil {
316329
return ipHolder, nil
317330
}
318-
331+
label := generateClusterScopedIPHolderLinodeName(l.zone, suffix)
319332
ipHolder, err = l.client.CreateInstance(ctx, linodego.InstanceCreateOptions{
320333
Region: l.zone,
321334
Type: "g6-nanode-1",
322-
Label: fmt.Sprintf("%s-%s", ipHolderLabelPrefix, l.zone),
335+
Label: label,
323336
RootPass: uuid.NewString(),
324337
Image: "linode/ubuntu22.04",
325338
Booted: ptr.To(false),
326339
})
327340
if err != nil {
341+
lerr := linodego.NewError(err)
342+
if lerr.Code == http.StatusConflict {
343+
// TODO (rk): should we handle more status codes on error?
344+
klog.Errorf("failed to create new IP Holder instance %s since it already exists: %s", label, err.Error())
345+
return nil, err
346+
}
328347
return nil, err
329348
}
349+
klog.Infof("created new IP Holder instance %s", label)
330350

331351
return ipHolder, nil
332352
}
333353

334-
func (l *loadbalancers) getIPHolder(ctx context.Context) (*linodego.Instance, error) {
354+
func (l *loadbalancers) getIPHolder(ctx context.Context, suffix string) (*linodego.Instance, error) {
355+
// even though we have updated the naming convention, leaving this in ensures we have backwards compatibility
335356
filter := map[string]string{"label": fmt.Sprintf("%s-%s", ipHolderLabelPrefix, l.zone)}
336357
rawFilter, err := json.Marshal(filter)
337358
if err != nil {
338359
panic("this should not have failed")
339360
}
340361
var ipHolder *linodego.Instance
362+
// TODO (rk): should we switch to using GET instead of LIST? we would be able to wrap logic around errors
341363
linodes, err := l.client.ListInstances(ctx, linodego.NewListOptions(1, string(rawFilter)))
342364
if err != nil {
343365
return nil, err
344366
}
367+
if len(linodes) == 0 {
368+
// since a list that returns 0 results has a 200/OK status code (no error)
369+
370+
// we assume that either
371+
// a) an ip holder instance does not exist yet
372+
// or
373+
// b) another cluster already holds the linode grant to an ip holder using the old naming convention
374+
filter = map[string]string{"label": generateClusterScopedIPHolderLinodeName(l.zone, suffix)}
375+
rawFilter, err = json.Marshal(filter)
376+
if err != nil {
377+
panic("this should not have failed")
378+
}
379+
linodes, err = l.client.ListInstances(ctx, linodego.NewListOptions(1, string(rawFilter)))
380+
if err != nil {
381+
return nil, err
382+
}
383+
}
345384
if len(linodes) > 0 {
346385
ipHolder = &linodes[0]
347386
}
348-
349387
return ipHolder, nil
350388
}
351389

390+
// generateClusterScopedIPHolderLinodeName attempts to generate a unique name for the IP Holder
391+
// instance used alongside Cilium LoadBalancers and Shared IPs for Kubernetes Services.
392+
// The `suffix` is set to either value of the `--ip-holder-suffix` parameter, if it is set.
393+
// The backup method involves keying off the service namespace.
394+
// While it _is_ possible to have an empty suffix passed in, it would mean that kubernetes
395+
// allowed the creation of a service without a namespace, which is highly improbable.
396+
func generateClusterScopedIPHolderLinodeName(zone, suffix string) string {
397+
// since Linode CCM consumers are varied, we require a method of providing a
398+
// suffix that does not rely on the use of a specific product (ex. LKE) to
399+
// have a specific piece of metadata (ex. annotation(s), label(s) ) present to key off of.
400+
//
401+
return fmt.Sprintf("%s-%s-%s", ipHolderLabelPrefix, zone, suffix)
402+
}
403+
352404
func (l *loadbalancers) retrieveCiliumClientset() error {
353405
if l.ciliumClient != nil {
354406
return nil

0 commit comments

Comments
 (0)