Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

create ip holder per cluster + tests, update docs #246

Merged
merged 1 commit into from
Nov 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 45 additions & 40 deletions README.md

Large diffs are not rendered by default.

72 changes: 61 additions & 11 deletions cloud/linode/cilium_loadbalancers.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@
// perform IP sharing (via a specified node selector) have the expected IPs shared
// in the event that a Node joins the cluster after the LoadBalancer Service already
// exists
func (l *loadbalancers) handleIPSharing(ctx context.Context, node *v1.Node) error {
func (l *loadbalancers) handleIPSharing(ctx context.Context, node *v1.Node, ipHolderSuffix string) error {
// ignore cases where the provider ID has been set
if node.Spec.ProviderID == "" {
klog.Info("skipping IP while providerID is unset")
Expand Down Expand Up @@ -182,7 +182,7 @@
// if any of the addrs don't exist on the ip-holder (e.g. someone manually deleted it outside the CCM),
// we need to exclude that from the list
// TODO: also clean up the CiliumLoadBalancerIPPool for that missing IP if that happens
ipHolder, err := l.getIPHolder(ctx)
ipHolder, err := l.getIPHolder(ctx, ipHolderSuffix)
if err != nil {
return err
}
Expand All @@ -207,8 +207,8 @@

// createSharedIP requests an additional IP that can be shared on Nodes to support
// loadbalancing via Cilium LB IPAM + BGP Control Plane.
func (l *loadbalancers) createSharedIP(ctx context.Context, nodes []*v1.Node) (string, error) {
ipHolder, err := l.ensureIPHolder(ctx)
func (l *loadbalancers) createSharedIP(ctx context.Context, nodes []*v1.Node, ipHolderSuffix string) (string, error) {
ipHolder, err := l.ensureIPHolder(ctx, ipHolderSuffix)
if err != nil {
return "", err
}
Expand Down Expand Up @@ -276,7 +276,15 @@
return err
}
bgpNodes := nodeList.Items
ipHolder, err := l.getIPHolder(ctx)

serviceNn := getServiceNn(service)
var ipHolderSuffix string
rahulait marked this conversation as resolved.
Show resolved Hide resolved
if Options.IpHolderSuffix != "" {
ipHolderSuffix = Options.IpHolderSuffix
klog.V(3).Infof("using parameter-based IP Holder suffix %s for Service %s", ipHolderSuffix, serviceNn)
}

ipHolder, err := l.getIPHolder(ctx, ipHolderSuffix)
if err != nil {
// return error or nil if not found since no IP holder means there
// is no IP to reclaim
Expand Down Expand Up @@ -310,48 +318,90 @@

// To hold the IP in lieu of a proper IP reservation system, a special Nanode is
// created but not booted and used to hold all shared IPs.
func (l *loadbalancers) ensureIPHolder(ctx context.Context) (*linodego.Instance, error) {
ipHolder, err := l.getIPHolder(ctx)
func (l *loadbalancers) ensureIPHolder(ctx context.Context, suffix string) (*linodego.Instance, error) {
ipHolder, err := l.getIPHolder(ctx, suffix)
if err != nil {
return nil, err
}
if ipHolder != nil {
return ipHolder, nil
}

label := generateClusterScopedIPHolderLinodeName(l.zone, suffix)
ipHolder, err = l.client.CreateInstance(ctx, linodego.InstanceCreateOptions{
Region: l.zone,
Type: "g6-nanode-1",
Label: fmt.Sprintf("%s-%s", ipHolderLabelPrefix, l.zone),
Label: label,
RootPass: uuid.NewString(),
Image: "linode/ubuntu22.04",
Booted: ptr.To(false),
})
if err != nil {
if linodego.ErrHasStatus(err, http.StatusBadRequest) && strings.Contains(err.Error(), "Label must be unique") {
// TODO (rk): should we handle more status codes on error?
klog.Errorf("failed to create new IP Holder instance %s since it already exists: %s", label, err.Error())
return nil, err
}

Check warning on line 343 in cloud/linode/cilium_loadbalancers.go

View check run for this annotation

Codecov / codecov/patch

cloud/linode/cilium_loadbalancers.go#L339-L343

Added lines #L339 - L343 were not covered by tests
return nil, err
}
klog.Infof("created new IP Holder instance %s", label)

return ipHolder, nil
}

func (l *loadbalancers) getIPHolder(ctx context.Context) (*linodego.Instance, error) {
func (l *loadbalancers) getIPHolder(ctx context.Context, suffix string) (*linodego.Instance, error) {
// even though we have updated the naming convention, leaving this in ensures we have backwards compatibility
filter := map[string]string{"label": fmt.Sprintf("%s-%s", ipHolderLabelPrefix, l.zone)}
rawFilter, err := json.Marshal(filter)
if err != nil {
panic("this should not have failed")
}
var ipHolder *linodego.Instance
// TODO (rk): should we switch to using GET instead of LIST? we would be able to wrap logic around errors
linodes, err := l.client.ListInstances(ctx, linodego.NewListOptions(1, string(rawFilter)))
if err != nil {
return nil, err
}
if len(linodes) == 0 {
// since a list that returns 0 results has a 200/OK status code (no error)

// we assume that either
// a) an ip holder instance does not exist yet
// or
// b) another cluster already holds the linode grant to an ip holder using the old naming convention
filter = map[string]string{"label": generateClusterScopedIPHolderLinodeName(l.zone, suffix)}
rawFilter, err = json.Marshal(filter)
if err != nil {
panic("this should not have failed")

Check warning on line 374 in cloud/linode/cilium_loadbalancers.go

View check run for this annotation

Codecov / codecov/patch

cloud/linode/cilium_loadbalancers.go#L374

Added line #L374 was not covered by tests
}
linodes, err = l.client.ListInstances(ctx, linodego.NewListOptions(1, string(rawFilter)))
if err != nil {
return nil, err
}

Check warning on line 379 in cloud/linode/cilium_loadbalancers.go

View check run for this annotation

Codecov / codecov/patch

cloud/linode/cilium_loadbalancers.go#L378-L379

Added lines #L378 - L379 were not covered by tests
}
if len(linodes) > 0 {
ipHolder = &linodes[0]
}

return ipHolder, nil
}

// generateClusterScopedIPHolderLinodeName attempts to generate a unique name for the IP Holder
// instance used alongside Cilium LoadBalancers and Shared IPs for Kubernetes Services.
// If the `--ip-holder-suffix` arg is passed when running Linode CCM, `suffix` is set to that value.
func generateClusterScopedIPHolderLinodeName(zone, suffix string) (label string) {
// since Linode CCM consumers are varied, we require a method of providing a
// suffix that does not rely on the use of a specific product (ex. LKE) to
// have a specific piece of metadata (ex. annotation(s), label(s) ) present to key off of.

if suffix == "" {
// this avoids a trailing hyphen if suffix is empty (ex. linode-ccm-ip-holder-us-ord-)
label = fmt.Sprintf("%s-%s", ipHolderLabelPrefix, zone)
} else {
label = fmt.Sprintf("%s-%s-%s", ipHolderLabelPrefix, zone, suffix)
}
klog.V(5).Infof("generated IP Holder Linode label: %s", label)
return label
}

func (l *loadbalancers) retrieveCiliumClientset() error {
if l.ciliumClient != nil {
return nil
Expand Down
Loading
Loading