Skip to content

[cleanup] refactoring packages #438

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

Merged
AshleyDumaine merged 2 commits into from
Aug 15, 2025
Merged

[cleanup] refactoring packages #438

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
b64ad42
massive refactor to deal with cyclic imports if we want to use the in…
AshleyDumaine Aug 14, 2025
d33b183
make package for linode services
AshleyDumaine Aug 15, 2025
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 Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ fmt:
.PHONY: test
# we say code is not worth testing unless it's formatted
test: fmt codegen
go test -v -coverpkg=./sentry,./cloud/linode/client,./cloud/linode/firewall,./cloud/linode,./cloud/nodeipam,./cloud/nodeipam/ipam -coverprofile ./coverage.out -cover ./sentry/... ./cloud/... $(TEST_ARGS)
go test -v -coverpkg=./sentry,./cloud/linode/client,./cloud/linode,./cloud/linode/utils,./cloud/linode/services,./cloud/nodeipam,./cloud/nodeipam/ipam -coverprofile ./coverage.out -cover ./sentry/... ./cloud/... $(TEST_ARGS)

.PHONY: build-linux
build-linux: codegen
Expand Down
34 changes: 18 additions & 16 deletions cloud/linode/cilium_loadbalancers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ import (
"k8s.io/utils/ptr"

"github.com/linode/linode-cloud-controller-manager/cloud/annotations"
"github.com/linode/linode-cloud-controller-manager/cloud/linode/options"
ccmUtils "github.com/linode/linode-cloud-controller-manager/cloud/linode/utils"
)

const (
Expand Down Expand Up @@ -111,7 +113,7 @@ func (l *loadbalancers) getExistingSharedIPs(ctx context.Context, ipHolder *lino

// 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)
nodeLinodeID, err := ccmUtils.ParseProviderID(node.Spec.ProviderID)
if err != nil {
return err
}
Expand Down Expand Up @@ -159,8 +161,8 @@ func (l *loadbalancers) handleIPSharing(ctx context.Context, node *v1.Node, ipHo
}
// If performing Service load-balancing via IP sharing + BGP, check for a special annotation
// added by the CCM gets set when load-balancer IPs have been successfully shared on the node
if Options.BGPNodeSelector != "" {
kv := strings.Split(Options.BGPNodeSelector, "=")
if options.Options.BGPNodeSelector != "" {
kv := strings.Split(options.Options.BGPNodeSelector, "=")
// Check if node should be participating in IP sharing via the given selector
if val, ok := node.Labels[kv[0]]; !ok || len(kv) != 2 || val != kv[1] {
// not a selected Node
Expand Down Expand Up @@ -243,7 +245,7 @@ func (l *loadbalancers) createSharedIP(ctx context.Context, nodes []*v1.Node, ip
}

// share the IPs with nodes participating in Cilium BGP peering
if Options.BGPNodeSelector == "" {
if options.Options.BGPNodeSelector == "" {
for _, node := range nodes {
if _, ok := node.Labels[commonControlPlaneLabel]; !ok {
if err = l.shareIPs(ctx, addrs, node); err != nil {
Expand All @@ -252,7 +254,7 @@ func (l *loadbalancers) createSharedIP(ctx context.Context, nodes []*v1.Node, ip
}
}
} else {
kv := strings.Split(Options.BGPNodeSelector, "=")
kv := strings.Split(options.Options.BGPNodeSelector, "=")
for _, node := range nodes {
if val, ok := node.Labels[kv[0]]; ok && len(kv) == 2 && val == kv[1] {
if err = l.shareIPs(ctx, addrs, node); err != nil {
Expand All @@ -273,7 +275,7 @@ func (l *loadbalancers) deleteSharedIP(ctx context.Context, service *v1.Service)
return err
}
nodeList, err := l.kubeClient.CoreV1().Nodes().List(ctx, metav1.ListOptions{
LabelSelector: Options.BGPNodeSelector,
LabelSelector: options.Options.BGPNodeSelector,
})
if err != nil {
return err
Expand All @@ -282,16 +284,16 @@ func (l *loadbalancers) deleteSharedIP(ctx context.Context, service *v1.Service)

serviceNn := getServiceNn(service)
var ipHolderSuffix string
if Options.IpHolderSuffix != "" {
ipHolderSuffix = Options.IpHolderSuffix
if options.Options.IpHolderSuffix != "" {
ipHolderSuffix = options.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
return IgnoreLinodeAPIError(err, http.StatusNotFound)
return ccmUtils.IgnoreLinodeAPIError(err, http.StatusNotFound)
}
svcIngress := service.Status.LoadBalancer.Ingress
if len(svcIngress) > 0 && ipHolder != nil {
Expand All @@ -300,19 +302,19 @@ func (l *loadbalancers) deleteSharedIP(ctx context.Context, service *v1.Service)
for _, ingress := range svcIngress {
// delete the shared IP on the Linodes it's shared on
for _, node := range bgpNodes {
nodeLinodeID, err = parseProviderID(node.Spec.ProviderID)
nodeLinodeID, err = ccmUtils.ParseProviderID(node.Spec.ProviderID)
if err != nil {
return err
}
err = l.client.DeleteInstanceIPAddress(ctx, nodeLinodeID, ingress.IP)
if IgnoreLinodeAPIError(err, http.StatusNotFound) != nil {
if ccmUtils.IgnoreLinodeAPIError(err, http.StatusNotFound) != nil {
return err
}
}

// finally delete the shared IP on the ip-holder
err = l.client.DeleteInstanceIPAddress(ctx, ipHolder.ID, ingress.IP)
if IgnoreLinodeAPIError(err, http.StatusNotFound) != nil {
if ccmUtils.IgnoreLinodeAPIError(err, http.StatusNotFound) != nil {
return err
}
}
Expand Down Expand Up @@ -415,7 +417,7 @@ func (l *loadbalancers) retrieveCiliumClientset() error {
kubeConfig *rest.Config
err error
)
kubeconfigFlag := Options.KubeconfigFlag
kubeconfigFlag := options.Options.KubeconfigFlag
if kubeconfigFlag == nil || kubeconfigFlag.Value.String() == "" {
kubeConfig, err = rest.InClusterConfig()
} else {
Expand Down Expand Up @@ -513,7 +515,7 @@ func (l *loadbalancers) ensureCiliumBGPPeeringPolicy(ctx context.Context) error
// otherwise create it
var nodeSelector slimv1.LabelSelector
// If no BGPNodeSelector is specified, select all worker nodes.
if Options.BGPNodeSelector == "" {
if options.Options.BGPNodeSelector == "" {
nodeSelector = slimv1.LabelSelector{
MatchExpressions: []slimv1.LabelSelectorRequirement{
{
Expand All @@ -523,9 +525,9 @@ func (l *loadbalancers) ensureCiliumBGPPeeringPolicy(ctx context.Context) error
},
}
} else {
kv := strings.Split(Options.BGPNodeSelector, "=")
kv := strings.Split(options.Options.BGPNodeSelector, "=")
if len(kv) != BGPNodeSelectorFlagInputLen {
return fmt.Errorf("invalid node selector %s", Options.BGPNodeSelector)
return fmt.Errorf("invalid node selector %s", options.Options.BGPNodeSelector)
}

nodeSelector = slimv1.LabelSelector{MatchLabels: map[string]string{kv[0]: kv[1]}}
Expand Down
68 changes: 35 additions & 33 deletions cloud/linode/cilium_loadbalancers_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ import (
"k8s.io/client-go/kubernetes"

"github.com/linode/linode-cloud-controller-manager/cloud/linode/client/mocks"
"github.com/linode/linode-cloud-controller-manager/cloud/linode/options"
ccmUtils "github.com/linode/linode-cloud-controller-manager/cloud/linode/utils"
)

const (
Expand All @@ -32,7 +34,7 @@ var (
Labels: map[string]string{"cilium-bgp-peering": "true"},
},
Spec: v1.NodeSpec{
ProviderID: fmt.Sprintf("%s%d", providerIDPrefix, 11111),
ProviderID: fmt.Sprintf("%s%d", ccmUtils.ProviderIDPrefix, 11111),
},
},
{
Expand All @@ -41,15 +43,15 @@ var (
Labels: map[string]string{"cilium-bgp-peering": "true"},
},
Spec: v1.NodeSpec{
ProviderID: fmt.Sprintf("%s%d", providerIDPrefix, 22222),
ProviderID: fmt.Sprintf("%s%d", ccmUtils.ProviderIDPrefix, 22222),
},
},
{
ObjectMeta: metav1.ObjectMeta{
Name: "node-3",
},
Spec: v1.NodeSpec{
ProviderID: fmt.Sprintf("%s%d", providerIDPrefix, 33333),
ProviderID: fmt.Sprintf("%s%d", ccmUtils.ProviderIDPrefix, 33333),
},
},
{
Expand All @@ -60,7 +62,7 @@ var (
},
},
Spec: v1.NodeSpec{
ProviderID: fmt.Sprintf("%s%d", providerIDPrefix, 44444),
ProviderID: fmt.Sprintf("%s%d", ccmUtils.ProviderIDPrefix, 44444),
},
},
}
Expand All @@ -71,7 +73,7 @@ var (
Labels: map[string]string{"cilium-bgp-peering": "true"},
},
Spec: v1.NodeSpec{
ProviderID: fmt.Sprintf("%s%d", providerIDPrefix, 55555),
ProviderID: fmt.Sprintf("%s%d", ccmUtils.ProviderIDPrefix, 55555),
},
},
}
Expand Down Expand Up @@ -202,7 +204,7 @@ func addNodes(t *testing.T, kubeClient kubernetes.Interface, nodes []*v1.Node) {
func createNewIpHolderInstance() linodego.Instance {
return linodego.Instance{
ID: 123456,
Label: generateClusterScopedIPHolderLinodeName(zone, Options.IpHolderSuffix),
Label: generateClusterScopedIPHolderLinodeName(zone, options.Options.IpHolderSuffix),
Type: "g6-standard-1",
Region: "us-west",
IPv4: []*net.IP{&publicIPv4},
Expand All @@ -212,8 +214,8 @@ func createNewIpHolderInstance() linodego.Instance {
func testNoBGPNodeLabel(t *testing.T, mc *mocks.MockClient) {
t.Helper()

Options.BGPNodeSelector = ""
Options.IpHolderSuffix = clusterName
options.Options.BGPNodeSelector = ""
options.Options.IpHolderSuffix = clusterName
t.Setenv("BGP_PEER_PREFIX", "2600:3cef")
svc := createTestService()
newIpHolderInstance = createNewIpHolderInstance()
Expand All @@ -231,7 +233,7 @@ func testNoBGPNodeLabel(t *testing.T, mc *mocks.MockClient) {
}

mc.EXPECT().ListInstances(gomock.Any(), linodego.NewListOptions(1, string(rawFilter))).Times(1).Return([]linodego.Instance{}, nil)
filter = map[string]string{"label": generateClusterScopedIPHolderLinodeName(zone, Options.IpHolderSuffix)}
filter = map[string]string{"label": generateClusterScopedIPHolderLinodeName(zone, options.Options.IpHolderSuffix)}
rawFilter, err = json.Marshal(filter)
if err != nil {
t.Errorf("json marshal error: %v", err)
Expand Down Expand Up @@ -271,7 +273,7 @@ func testNoBGPNodeLabel(t *testing.T, mc *mocks.MockClient) {
func testUnsupportedRegion(t *testing.T, mc *mocks.MockClient) {
t.Helper()

Options.BGPNodeSelector = nodeSelector
options.Options.BGPNodeSelector = nodeSelector
svc := createTestService()

kubeClient, _ := k8sClient.NewFakeClientset()
Expand Down Expand Up @@ -302,7 +304,7 @@ func testUnsupportedRegion(t *testing.T, mc *mocks.MockClient) {
func testCreateWithExistingIPHolderWithOldIpHolderNamingConvention(t *testing.T, mc *mocks.MockClient) {
t.Helper()

Options.BGPNodeSelector = nodeSelector
options.Options.BGPNodeSelector = nodeSelector
svc := createTestService()
newIpHolderInstance = createNewIpHolderInstance()

Expand Down Expand Up @@ -346,8 +348,8 @@ func testCreateWithExistingIPHolderWithOldIpHolderNamingConvention(t *testing.T,
func testCreateWithExistingIPHolderWithNewIpHolderNamingConvention(t *testing.T, mc *mocks.MockClient) {
t.Helper()

Options.BGPNodeSelector = nodeSelector
Options.IpHolderSuffix = clusterName
options.Options.BGPNodeSelector = nodeSelector
options.Options.IpHolderSuffix = clusterName
svc := createTestService()
newIpHolderInstance = createNewIpHolderInstance()

Expand Down Expand Up @@ -391,8 +393,8 @@ func testCreateWithExistingIPHolderWithNewIpHolderNamingConvention(t *testing.T,
func testCreateWithExistingIPHolderWithNewIpHolderNamingConventionUsingLongSuffix(t *testing.T, mc *mocks.MockClient) {
t.Helper()

Options.BGPNodeSelector = nodeSelector
Options.IpHolderSuffix = "OaTJrRuufacHVougjwkpBpmstiqvswvBNEMWXsRYfMBTCkKIUTXpbGIcIbDWSQp"
options.Options.BGPNodeSelector = nodeSelector
options.Options.IpHolderSuffix = "OaTJrRuufacHVougjwkpBpmstiqvswvBNEMWXsRYfMBTCkKIUTXpbGIcIbDWSQp"
svc := createTestService()
newIpHolderInstance = createNewIpHolderInstance()

Expand Down Expand Up @@ -436,8 +438,8 @@ func testCreateWithExistingIPHolderWithNewIpHolderNamingConventionUsingLongSuffi
func testCreateWithNoExistingIPHolderUsingNoSuffix(t *testing.T, mc *mocks.MockClient) {
t.Helper()

Options.BGPNodeSelector = nodeSelector
Options.IpHolderSuffix = ""
options.Options.BGPNodeSelector = nodeSelector
options.Options.IpHolderSuffix = ""
svc := createTestService()
newIpHolderInstance = createNewIpHolderInstance()

Expand All @@ -453,7 +455,7 @@ func testCreateWithNoExistingIPHolderUsingNoSuffix(t *testing.T, mc *mocks.MockC
t.Errorf("json marshal error: %v", err)
}
mc.EXPECT().ListInstances(gomock.Any(), linodego.NewListOptions(1, string(rawFilter))).Times(1).Return([]linodego.Instance{}, nil)
filter = map[string]string{"label": generateClusterScopedIPHolderLinodeName(zone, Options.IpHolderSuffix)}
filter = map[string]string{"label": generateClusterScopedIPHolderLinodeName(zone, options.Options.IpHolderSuffix)}
rawFilter, err = json.Marshal(filter)
if err != nil {
t.Errorf("json marshal error: %v", err)
Expand Down Expand Up @@ -488,8 +490,8 @@ func testCreateWithNoExistingIPHolderUsingNoSuffix(t *testing.T, mc *mocks.MockC
func testCreateWithNoExistingIPHolderUsingShortSuffix(t *testing.T, mc *mocks.MockClient) {
t.Helper()

Options.BGPNodeSelector = nodeSelector
Options.IpHolderSuffix = clusterName
options.Options.BGPNodeSelector = nodeSelector
options.Options.IpHolderSuffix = clusterName
svc := createTestService()
newIpHolderInstance = createNewIpHolderInstance()

Expand All @@ -505,7 +507,7 @@ func testCreateWithNoExistingIPHolderUsingShortSuffix(t *testing.T, mc *mocks.Mo
t.Errorf("json marshal error: %v", err)
}
mc.EXPECT().ListInstances(gomock.Any(), linodego.NewListOptions(1, string(rawFilter))).Times(1).Return([]linodego.Instance{}, nil)
filter = map[string]string{"label": generateClusterScopedIPHolderLinodeName(zone, Options.IpHolderSuffix)}
filter = map[string]string{"label": generateClusterScopedIPHolderLinodeName(zone, options.Options.IpHolderSuffix)}
rawFilter, err = json.Marshal(filter)
if err != nil {
t.Errorf("json marshal error: %v", err)
Expand Down Expand Up @@ -540,8 +542,8 @@ func testCreateWithNoExistingIPHolderUsingShortSuffix(t *testing.T, mc *mocks.Mo
func testCreateWithNoExistingIPHolderUsingLongSuffix(t *testing.T, mc *mocks.MockClient) {
t.Helper()

Options.BGPNodeSelector = nodeSelector
Options.IpHolderSuffix = "OaTJrRuufacHVougjwkpBpmstiqvswvBNEMWXsRYfMBTCkKIUTXpbGIcIbDWSQp"
options.Options.BGPNodeSelector = nodeSelector
options.Options.IpHolderSuffix = "OaTJrRuufacHVougjwkpBpmstiqvswvBNEMWXsRYfMBTCkKIUTXpbGIcIbDWSQp"
svc := createTestService()
newIpHolderInstance = createNewIpHolderInstance()

Expand All @@ -557,7 +559,7 @@ func testCreateWithNoExistingIPHolderUsingLongSuffix(t *testing.T, mc *mocks.Moc
t.Errorf("json marshal error: %v", err)
}
mc.EXPECT().ListInstances(gomock.Any(), linodego.NewListOptions(1, string(rawFilter))).Times(1).Return([]linodego.Instance{}, nil)
filter = map[string]string{"label": generateClusterScopedIPHolderLinodeName(zone, Options.IpHolderSuffix)}
filter = map[string]string{"label": generateClusterScopedIPHolderLinodeName(zone, options.Options.IpHolderSuffix)}
rawFilter, err = json.Marshal(filter)
if err != nil {
t.Errorf("json marshal error: %v", err)
Expand Down Expand Up @@ -592,7 +594,7 @@ func testCreateWithNoExistingIPHolderUsingLongSuffix(t *testing.T, mc *mocks.Moc
func testEnsureCiliumLoadBalancerDeletedWithOldIpHolderNamingConvention(t *testing.T, mc *mocks.MockClient) {
t.Helper()

Options.BGPNodeSelector = nodeSelector
options.Options.BGPNodeSelector = nodeSelector
svc := createTestService()

kubeClient, _ := k8sClient.NewFakeClientset()
Expand Down Expand Up @@ -623,8 +625,8 @@ func testEnsureCiliumLoadBalancerDeletedWithOldIpHolderNamingConvention(t *testi
func testEnsureCiliumLoadBalancerDeletedWithNewIpHolderNamingConvention(t *testing.T, mc *mocks.MockClient) {
t.Helper()

Options.BGPNodeSelector = nodeSelector
Options.IpHolderSuffix = clusterName
options.Options.BGPNodeSelector = nodeSelector
options.Options.IpHolderSuffix = clusterName
svc := createTestService()
newIpHolderInstance = createNewIpHolderInstance()

Expand All @@ -643,7 +645,7 @@ func testEnsureCiliumLoadBalancerDeletedWithNewIpHolderNamingConvention(t *testi
t.Errorf("json marshal error: %v", err)
}
mc.EXPECT().ListInstances(gomock.Any(), linodego.NewListOptions(1, string(rawFilter))).Times(1).Return([]linodego.Instance{}, nil)
filter = map[string]string{"label": generateClusterScopedIPHolderLinodeName(zone, Options.IpHolderSuffix)}
filter = map[string]string{"label": generateClusterScopedIPHolderLinodeName(zone, options.Options.IpHolderSuffix)}
rawFilter, err = json.Marshal(filter)
if err != nil {
t.Errorf("json marshal error: %v", err)
Expand All @@ -662,7 +664,7 @@ func testEnsureCiliumLoadBalancerDeletedWithNewIpHolderNamingConvention(t *testi
func testCiliumUpdateLoadBalancerAddNodeWithOldIpHolderNamingConvention(t *testing.T, mc *mocks.MockClient) {
t.Helper()

Options.BGPNodeSelector = nodeSelector
options.Options.BGPNodeSelector = nodeSelector
svc := createTestService()

kubeClient, _ := k8sClient.NewFakeClientset()
Expand Down Expand Up @@ -723,8 +725,8 @@ func testCiliumUpdateLoadBalancerAddNodeWithOldIpHolderNamingConvention(t *testi
func testCiliumUpdateLoadBalancerAddNodeWithNewIpHolderNamingConvention(t *testing.T, mc *mocks.MockClient) {
t.Helper()

Options.BGPNodeSelector = nodeSelector
Options.IpHolderSuffix = clusterName
options.Options.BGPNodeSelector = nodeSelector
options.Options.IpHolderSuffix = clusterName
svc := createTestService()
newIpHolderInstance = createNewIpHolderInstance()

Expand All @@ -740,7 +742,7 @@ func testCiliumUpdateLoadBalancerAddNodeWithNewIpHolderNamingConvention(t *testi
t.Errorf("json marshal error: %v", err)
}
mc.EXPECT().ListInstances(gomock.Any(), linodego.NewListOptions(1, string(rawFilter))).Times(1).Return([]linodego.Instance{}, nil)
filter = map[string]string{"label": generateClusterScopedIPHolderLinodeName(zone, Options.IpHolderSuffix)}
filter = map[string]string{"label": generateClusterScopedIPHolderLinodeName(zone, options.Options.IpHolderSuffix)}
rawFilter, err = json.Marshal(filter)
if err != nil {
t.Errorf("json marshal error: %v", err)
Expand Down Expand Up @@ -777,7 +779,7 @@ func testCiliumUpdateLoadBalancerAddNodeWithNewIpHolderNamingConvention(t *testi
t.Errorf("json marshal error: %v", err)
}
mc.EXPECT().ListInstances(gomock.Any(), linodego.NewListOptions(1, string(rawFilter))).Times(1).Return([]linodego.Instance{}, nil)
filter = map[string]string{"label": generateClusterScopedIPHolderLinodeName(zone, Options.IpHolderSuffix)}
filter = map[string]string{"label": generateClusterScopedIPHolderLinodeName(zone, options.Options.IpHolderSuffix)}
rawFilter, err = json.Marshal(filter)
if err != nil {
t.Errorf("json marshal error: %v", err)
Expand Down
Loading
Loading