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.

Sign up for GitHub

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

Jump to bottom

[improvement] stop using ip-holder nanode in favor of new reserved IP support #234

Closed
wants to merge 2 commits into from
Closed

[improvement] stop using ip-holder nanode in favor of new reserved IP support #234

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
265dab7
stop using ip-holder nanode in favor of new reserved IP support
AshleyDumaine Sep 23, 2024
ef21409
ignore deprecation warning for now like the others
AshleyDumaine Sep 24, 2024
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
2 changes: 1 addition & 1 deletion .golangci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,4 @@ linters:
issues:
exclude-rules:
- path: cloud/linode/fake_linode_test.go
text: 'SA1019: (.+).(NodeBalancersPagedResponse|NodeBalancerConfigsPagedResponse|NodeBalancerNodesPagedResponse|FirewallDevicesPagedResponse) is deprecated: (NodeBalancersPagedResponse|NodeBalancerConfigsPagedResponse|NodeBalancerNodesPagedResponse|FirewallDevicesPagedResponse) exists for historical compatibility and should not be used.'
text: 'SA1019: (.+).(NodeBalancersPagedResponse|NodeBalancerConfigsPagedResponse|NodeBalancerNodesPagedResponse|FirewallDevicesPagedResponse|NodeBalancerFirewallsPagedResponse) is deprecated: (NodeBalancersPagedResponse|NodeBalancerConfigsPagedResponse|NodeBalancerNodesPagedResponse|FirewallDevicesPagedResponse|NodeBalancerFirewallsPagedResponse) exists for historical compatibility and should not be used.'
2 changes: 1 addition & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ For general feature and usage notes, refer to the [Getting Started with Linode N
#### Using IP Sharing instead of NodeBalancers
Alternatively, the Linode CCM can integrate with [Cilium's BGP Control Plane](https://docs.cilium.io/en/stable/network/bgp-control-plane/)
to perform load-balancing via IP sharing on labeled Nodes. This option does not create a backing NodeBalancer and instead
provisions a new IP on an ip-holder Nanode to share for the desired region. See [Shared IP LoadBalancing](#shared-ip-load-balancing).
provisions a new reserved IP to share for the desired region. See [Shared IP LoadBalancing](#shared-ip-load-balancing).

#### Annotations
The Linode CCM accepts several annotations which affect the properties of the underlying NodeBalancer deployment.
Expand Down
125 changes: 12 additions & 113 deletions cloud/linode/cilium_loadbalancers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,13 @@

import (
"context"
"encoding/json"
"fmt"
"net/http"
"slices"
"strings"

"github.com/cilium/cilium/pkg/k8s/apis/cilium.io/v2alpha1"
ciliumclient "github.com/cilium/cilium/pkg/k8s/client/clientset/versioned/typed/cilium.io/v2alpha1"
slimv1 "github.com/cilium/cilium/pkg/k8s/slim/k8s/apis/meta/v1"
"github.com/google/uuid"
"github.com/linode/linode-cloud-controller-manager/cloud/annotations"
"github.com/linode/linodego"
v1 "k8s.io/api/core/v1"
Expand All @@ -26,10 +23,8 @@

const (
ciliumLBClass = "io.cilium/bgp-control-plane"
ipHolderLabelPrefix = "linode-ccm-ip-holder"
ciliumBGPPeeringPolicyName = "linode-ccm-bgp-peering"

commonControlPlaneLabel = "node-role.kubernetes.io/control-plane"
commonControlPlaneLabel = "node-role.kubernetes.io/control-plane"
)

// This mapping is unfortunately necessary since there is no way to get the
Expand Down Expand Up @@ -88,21 +83,6 @@
return addrs, nil
}

func (l *loadbalancers) getExistingSharedIPs(ctx context.Context, ipHolder *linodego.Instance) ([]string, error) {
if ipHolder == nil {
return nil, nil
}
ipHolderAddrs, err := l.client.GetInstanceIPAddresses(ctx, ipHolder.ID)
if err != nil {
return nil, err
}
addrs := make([]string, 0, len(ipHolderAddrs.IPv4.Shared))
for _, addr := range ipHolderAddrs.IPv4.Shared {
addrs = append(addrs, addr.Address)
}
return addrs, nil
}

// shareIPs shares the given list of IP addresses on the given Node
func (l *loadbalancers) shareIPs(ctx context.Context, addrs []string, node *v1.Node) error {
nodeLinodeID, err := parseProviderID(node.Spec.ProviderID)
Expand Down Expand Up @@ -176,25 +156,8 @@
klog.Infof("error getting shared IPs in cluster: %s", err.Error())
return err
}
// 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)
if err != nil {
return err
}
ipHolderAddrs, err := l.getExistingSharedIPs(ctx, ipHolder)
if err != nil {
klog.Infof("error getting shared IPs in cluster: %s", err.Error())
return err
}
addrs := []string{}
for _, i := range inClusterAddrs {
if slices.Contains(ipHolderAddrs, i) {
addrs = append(addrs, i)
}
}
if err = l.shareIPs(ctx, addrs, node); err != nil {

if err = l.shareIPs(ctx, inClusterAddrs, node); err != nil {

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

View check run for this annotation

Codecov / codecov/patch

cloud/linode/cilium_loadbalancers.go#L160 

Added line #L160 was not covered by tests
klog.Infof("error sharing IPs: %s", err.Error())
return err
}
Expand All @@ -205,12 +168,9 @@
// 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)
if err != nil {
return "", err
}

newSharedIP, err := l.client.AddInstanceIPAddress(ctx, ipHolder.ID, true)
newSharedIP, err := l.client.ReserveIPAddress(ctx, linodego.ReserveIPOptions{
Region: l.zone,
})
if err != nil {
return "", err
}
Expand All @@ -221,20 +181,8 @@
if err != nil {
return "", err
}
// 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
ipHolderAddrs, err := l.getExistingSharedIPs(ctx, ipHolder)
if err != nil {
klog.Infof("error getting shared IPs in cluster: %s", err.Error())
return "", err
}
addrs := []string{newSharedIP.Address}
for _, i := range inClusterAddrs {
if slices.Contains(ipHolderAddrs, i) {
addrs = append(addrs, i)
}
}

addrs := append(inClusterAddrs, newSharedIP.Address)

// share the IPs with nodes participating in Cilium BGP peering
if Options.BGPNodeSelector == "" {
Expand Down Expand Up @@ -273,14 +221,9 @@
return err
}
bgpNodes := nodeList.Items
ipHolder, err := l.getIPHolder(ctx)
if err != nil {
// return error or nil if not found since no IP holder means there
// is no IP to reclaim
return IgnoreLinodeAPIError(err, http.StatusNotFound)
}

svcIngress := service.Status.LoadBalancer.Ingress
if len(svcIngress) > 0 && ipHolder != nil {
if len(svcIngress) > 0 {
for _, ingress := range svcIngress {
// delete the shared IP on the Linodes it's shared on
for _, node := range bgpNodes {
Expand All @@ -294,8 +237,8 @@
}
}

// finally delete the shared IP on the ip-holder
err = l.client.DeleteInstanceIPAddress(ctx, ipHolder.ID, ingress.IP)
// finally delete the shared IP
err = l.client.DeleteReservedIPAddress(ctx, ingress.IP)
if IgnoreLinodeAPIError(err, http.StatusNotFound) != nil {
return err
}
Expand All @@ -305,50 +248,6 @@
return nil
}

// 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)
if err != nil {
return nil, err
}
if ipHolder != nil {
return ipHolder, nil
}

ipHolder, err = l.client.CreateInstance(ctx, linodego.InstanceCreateOptions{
Region: l.zone,
Type: "g6-nanode-1",
Label: fmt.Sprintf("%s-%s", ipHolderLabelPrefix, l.zone),
RootPass: uuid.NewString(),
Image: "linode/ubuntu22.04",
Booted: ptr.To(false),
})
if err != nil {
return nil, err
}

return ipHolder, nil
}

func (l *loadbalancers) getIPHolder(ctx context.Context) (*linodego.Instance, error) {
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
linodes, err := l.client.ListInstances(ctx, linodego.NewListOptions(1, string(rawFilter)))
if err != nil {
return nil, err
}
if len(linodes) > 0 {
ipHolder = &linodes[0]
}

return ipHolder, nil
}

func (l *loadbalancers) retrieveCiliumClientset() error {
if l.ciliumClient != nil {
return nil
Expand Down
85 changes: 6 additions & 79 deletions cloud/linode/cilium_loadbalancers_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,7 @@ package linode

import (
"context"
"encoding/json"
"fmt"
"net"
"testing"

k8sClient "github.com/cilium/cilium/pkg/k8s/client"
Expand Down Expand Up @@ -58,14 +56,6 @@ var (
},
},
}
publicIPv4 = net.ParseIP("45.76.101.25")
ipHolderInstance = linodego.Instance{
ID: 12345,
Label: fmt.Sprintf("%s-%s", ipHolderLabelPrefix, zone),
Type: "g6-standard-1",
Region: "us-west",
IPv4: []*net.IP{&publicIPv4},
}
)

func TestCiliumCCMLoadBalancers(t *testing.T) {
Expand All @@ -82,12 +72,8 @@ func TestCiliumCCMLoadBalancers(t *testing.T) {
f: testUnsupportedRegion,
},
{
name: "Create Cilium Load Balancer With explicit loadBalancerClass and existing IP holder nanode",
f: testCreateWithExistingIPHolder,
},
{
name: "Create Cilium Load Balancer With no existing IP holder nanode",
f: testCreateWithNoExistingIPHolder,
name: "Create Cilium Load Balancer With explicit loadBalancerClass",
f: testCreate,
},
{
name: "Delete Cilium Load Balancer",
Expand Down Expand Up @@ -158,17 +144,8 @@ func testNoBGPNodeLabel(t *testing.T, mc *mocks.MockClient) {
addNodes(t, kubeClient, nodes)
lb := &loadbalancers{mc, zone, kubeClient, ciliumClient, ciliumLBType}

filter := map[string]string{"label": fmt.Sprintf("%s-%s", ipHolderLabelPrefix, zone)}
rawFilter, _ := json.Marshal(filter)
mc.EXPECT().ListInstances(gomock.Any(), linodego.NewListOptions(1, string(rawFilter))).Times(1).Return([]linodego.Instance{}, nil)
dummySharedIP := "45.76.101.26"
mc.EXPECT().CreateInstance(gomock.Any(), gomock.Any()).Times(1).Return(&ipHolderInstance, nil)
mc.EXPECT().GetInstanceIPAddresses(gomock.Any(), ipHolderInstance.ID).Times(1).Return(&linodego.InstanceIPAddressResponse{
IPv4: &linodego.InstanceIPv4Response{
Shared: []*linodego.InstanceIP{{Address: dummySharedIP}},
},
}, nil)
mc.EXPECT().AddInstanceIPAddress(gomock.Any(), ipHolderInstance.ID, true).Times(1).Return(&linodego.InstanceIP{Address: dummySharedIP}, nil)
mc.EXPECT().ReserveIPAddress(gomock.Any(), gomock.Any()).Times(1).Return(&linodego.InstanceIP{Address: dummySharedIP}, nil)
mc.EXPECT().ShareIPAddresses(gomock.Any(), linodego.IPAddressesShareOptions{
IPs: []string{dummySharedIP},
LinodeID: 11111,
Expand Down Expand Up @@ -209,45 +186,7 @@ func testUnsupportedRegion(t *testing.T, mc *mocks.MockClient) {
}
}

func testCreateWithExistingIPHolder(t *testing.T, mc *mocks.MockClient) {
Options.BGPNodeSelector = "cilium-bgp-peering=true"
svc := createTestService()

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

filter := map[string]string{"label": fmt.Sprintf("%s-%s", ipHolderLabelPrefix, zone)}
rawFilter, _ := json.Marshal(filter)
mc.EXPECT().ListInstances(gomock.Any(), linodego.NewListOptions(1, string(rawFilter))).Times(1).Return([]linodego.Instance{ipHolderInstance}, nil)
dummySharedIP := "45.76.101.26"
mc.EXPECT().AddInstanceIPAddress(gomock.Any(), ipHolderInstance.ID, true).Times(1).Return(&linodego.InstanceIP{Address: dummySharedIP}, nil)
mc.EXPECT().GetInstanceIPAddresses(gomock.Any(), ipHolderInstance.ID).Times(1).Return(&linodego.InstanceIPAddressResponse{
IPv4: &linodego.InstanceIPv4Response{
Shared: []*linodego.InstanceIP{{Address: dummySharedIP}},
},
}, nil)
mc.EXPECT().ShareIPAddresses(gomock.Any(), linodego.IPAddressesShareOptions{
IPs: []string{dummySharedIP},
LinodeID: 11111,
}).Times(1)
mc.EXPECT().ShareIPAddresses(gomock.Any(), linodego.IPAddressesShareOptions{
IPs: []string{dummySharedIP},
LinodeID: 22222,
}).Times(1)

lbStatus, err := lb.EnsureLoadBalancer(context.TODO(), "linodelb", svc, nodes)
if err != nil {
t.Fatalf("expected a nil error, got %v", err)
}
if lbStatus == nil {
t.Fatal("expected non-nil lbStatus")
}
}

func testCreateWithNoExistingIPHolder(t *testing.T, mc *mocks.MockClient) {
func testCreate(t *testing.T, mc *mocks.MockClient) {
Options.BGPNodeSelector = "cilium-bgp-peering=true"
svc := createTestService()

Expand All @@ -257,17 +196,8 @@ func testCreateWithNoExistingIPHolder(t *testing.T, mc *mocks.MockClient) {
addNodes(t, kubeClient, nodes)
lb := &loadbalancers{mc, zone, kubeClient, ciliumClient, ciliumLBType}

filter := map[string]string{"label": fmt.Sprintf("%s-%s", ipHolderLabelPrefix, zone)}
rawFilter, _ := json.Marshal(filter)
mc.EXPECT().ListInstances(gomock.Any(), linodego.NewListOptions(1, string(rawFilter))).Times(1).Return([]linodego.Instance{}, nil)
dummySharedIP := "45.76.101.26"
mc.EXPECT().CreateInstance(gomock.Any(), gomock.Any()).Times(1).Return(&ipHolderInstance, nil)
mc.EXPECT().GetInstanceIPAddresses(gomock.Any(), ipHolderInstance.ID).Times(1).Return(&linodego.InstanceIPAddressResponse{
IPv4: &linodego.InstanceIPv4Response{
Shared: []*linodego.InstanceIP{{Address: dummySharedIP}},
},
}, nil)
mc.EXPECT().AddInstanceIPAddress(gomock.Any(), ipHolderInstance.ID, true).Times(1).Return(&linodego.InstanceIP{Address: dummySharedIP}, nil)
mc.EXPECT().ReserveIPAddress(gomock.Any(), gomock.Any()).Times(1).Return(&linodego.InstanceIP{Address: dummySharedIP}, nil)
mc.EXPECT().ShareIPAddresses(gomock.Any(), linodego.IPAddressesShareOptions{
IPs: []string{dummySharedIP},
LinodeID: 11111,
Expand Down Expand Up @@ -299,12 +229,9 @@ func testEnsureCiliumLoadBalancerDeleted(t *testing.T, mc *mocks.MockClient) {
dummySharedIP := "45.76.101.26"
svc.Status.LoadBalancer = v1.LoadBalancerStatus{Ingress: []v1.LoadBalancerIngress{{IP: dummySharedIP}}}

filter := map[string]string{"label": fmt.Sprintf("%s-%s", ipHolderLabelPrefix, zone)}
rawFilter, _ := json.Marshal(filter)
mc.EXPECT().ListInstances(gomock.Any(), linodego.NewListOptions(1, string(rawFilter))).Times(1).Return([]linodego.Instance{ipHolderInstance}, nil)
mc.EXPECT().DeleteInstanceIPAddress(gomock.Any(), 11111, dummySharedIP).Times(1).Return(nil)
mc.EXPECT().DeleteInstanceIPAddress(gomock.Any(), 22222, dummySharedIP).Times(1).Return(nil)
mc.EXPECT().DeleteInstanceIPAddress(gomock.Any(), ipHolderInstance.ID, dummySharedIP).Times(1).Return(nil)
mc.EXPECT().DeleteReservedIPAddress(gomock.Any(), dummySharedIP).Times(1).Return(nil)

err := lb.EnsureLoadBalancerDeleted(context.TODO(), "linodelb", svc)
if err != nil {
Expand Down
5 changes: 4 additions & 1 deletion cloud/linode/client/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,13 @@ type Client interface {
CreateInstance(ctx context.Context, opts linodego.InstanceCreateOptions) (*linodego.Instance, error)

GetInstanceIPAddresses(context.Context, int) (*linodego.InstanceIPAddressResponse, error)
AddInstanceIPAddress(ctx context.Context, linodeID int, public bool) (*linodego.InstanceIP, error)
DeleteInstanceIPAddress(ctx context.Context, linodeID int, ipAddress string) error
ShareIPAddresses(ctx context.Context, opts linodego.IPAddressesShareOptions) error

GetReservedIPAddress(context.Context, string) (*linodego.InstanceIP, error)
ReserveIPAddress(context.Context, linodego.ReserveIPOptions) (*linodego.InstanceIP, error)
DeleteReservedIPAddress(context.Context, string) error

UpdateInstanceConfigInterface(context.Context, int, int, int, linodego.InstanceConfigInterfaceUpdateOptions) (*linodego.InstanceConfigInterface, error)

ListVPCs(context.Context, *linodego.ListOptions) ([]linodego.VPC, error)
Expand Down
Loading
Loading