Skip to content

Commit ee8fb73

Browse files
AshleyDumaineAshleyDumaine
authored
Merge pull request #438 from linode/package-refactor
[cleanup] refactoring packages
2 parents 36df8ec + d33b183 commit ee8fb73

25 files changed

+714
-676
lines changed

‎Makefile‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ fmt:
8080
.PHONY: test
8181
# we say code is not worth testing unless it's formatted
8282
test: fmt codegen
83-
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)
83+
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)
8484

8585
.PHONY: build-linux
8686
build-linux: codegen

‎cloud/linode/cilium_loadbalancers.go‎

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@ import (
2424
"k8s.io/utils/ptr"
2525

2626
"github.com/linode/linode-cloud-controller-manager/cloud/annotations"
27+
"github.com/linode/linode-cloud-controller-manager/cloud/linode/options"
28+
ccmUtils "github.com/linode/linode-cloud-controller-manager/cloud/linode/utils"
2729
)
2830

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

112114
// shareIPs shares the given list of IP addresses on the given Node
113115
func (l *loadbalancers) shareIPs(ctx context.Context, addrs []string, node *v1.Node) error {
114-
nodeLinodeID, err := parseProviderID(node.Spec.ProviderID)
116+
nodeLinodeID, err := ccmUtils.ParseProviderID(node.Spec.ProviderID)
115117
if err != nil {
116118
return err
117119
}
@@ -159,8 +161,8 @@ func (l *loadbalancers) handleIPSharing(ctx context.Context, node *v1.Node, ipHo
159161
}
160162
// If performing Service load-balancing via IP sharing + BGP, check for a special annotation
161163
// added by the CCM gets set when load-balancer IPs have been successfully shared on the node
162-
if Options.BGPNodeSelector != "" {
163-
kv := strings.Split(Options.BGPNodeSelector, "=")
164+
if options.Options.BGPNodeSelector != "" {
165+
kv := strings.Split(options.Options.BGPNodeSelector, "=")
164166
// Check if node should be participating in IP sharing via the given selector
165167
if val, ok := node.Labels[kv[0]]; !ok || len(kv) != 2 || val != kv[1] {
166168
// not a selected Node
@@ -243,7 +245,7 @@ func (l *loadbalancers) createSharedIP(ctx context.Context, nodes []*v1.Node, ip
243245
}
244246

245247
// share the IPs with nodes participating in Cilium BGP peering
246-
if Options.BGPNodeSelector == "" {
248+
if options.Options.BGPNodeSelector == "" {
247249
for _, node := range nodes {
248250
if _, ok := node.Labels[commonControlPlaneLabel]; !ok {
249251
if err = l.shareIPs(ctx, addrs, node); err != nil {
@@ -252,7 +254,7 @@ func (l *loadbalancers) createSharedIP(ctx context.Context, nodes []*v1.Node, ip
252254
}
253255
}
254256
} else {
255-
kv := strings.Split(Options.BGPNodeSelector, "=")
257+
kv := strings.Split(options.Options.BGPNodeSelector, "=")
256258
for _, node := range nodes {
257259
if val, ok := node.Labels[kv[0]]; ok && len(kv) == 2 && val == kv[1] {
258260
if err = l.shareIPs(ctx, addrs, node); err != nil {
@@ -273,7 +275,7 @@ func (l *loadbalancers) deleteSharedIP(ctx context.Context, service *v1.Service)
273275
return err
274276
}
275277
nodeList, err := l.kubeClient.CoreV1().Nodes().List(ctx, metav1.ListOptions{
276-
LabelSelector: Options.BGPNodeSelector,
278+
LabelSelector: options.Options.BGPNodeSelector,
277279
})
278280
if err != nil {
279281
return err
@@ -282,16 +284,16 @@ func (l *loadbalancers) deleteSharedIP(ctx context.Context, service *v1.Service)
282284

283285
serviceNn := getServiceNn(service)
284286
var ipHolderSuffix string
285-
if Options.IpHolderSuffix != "" {
286-
ipHolderSuffix = Options.IpHolderSuffix
287+
if options.Options.IpHolderSuffix != "" {
288+
ipHolderSuffix = options.Options.IpHolderSuffix
287289
klog.V(3).Infof("using parameter-based IP Holder suffix %s for Service %s", ipHolderSuffix, serviceNn)
288290
}
289291

290292
ipHolder, err := l.getIPHolder(ctx, ipHolderSuffix)
291293
if err != nil {
292294
// return error or nil if not found since no IP holder means there
293295
// is no IP to reclaim
294-
return IgnoreLinodeAPIError(err, http.StatusNotFound)
296+
return ccmUtils.IgnoreLinodeAPIError(err, http.StatusNotFound)
295297
}
296298
svcIngress := service.Status.LoadBalancer.Ingress
297299
if len(svcIngress) > 0 && ipHolder != nil {
@@ -300,19 +302,19 @@ func (l *loadbalancers) deleteSharedIP(ctx context.Context, service *v1.Service)
300302
for _, ingress := range svcIngress {
301303
// delete the shared IP on the Linodes it's shared on
302304
for _, node := range bgpNodes {
303-
nodeLinodeID, err = parseProviderID(node.Spec.ProviderID)
305+
nodeLinodeID, err = ccmUtils.ParseProviderID(node.Spec.ProviderID)
304306
if err != nil {
305307
return err
306308
}
307309
err = l.client.DeleteInstanceIPAddress(ctx, nodeLinodeID, ingress.IP)
308-
if IgnoreLinodeAPIError(err, http.StatusNotFound) != nil {
310+
if ccmUtils.IgnoreLinodeAPIError(err, http.StatusNotFound) != nil {
309311
return err
310312
}
311313
}
312314

313315
// finally delete the shared IP on the ip-holder
314316
err = l.client.DeleteInstanceIPAddress(ctx, ipHolder.ID, ingress.IP)
315-
if IgnoreLinodeAPIError(err, http.StatusNotFound) != nil {
317+
if ccmUtils.IgnoreLinodeAPIError(err, http.StatusNotFound) != nil {
316318
return err
317319
}
318320
}
@@ -415,7 +417,7 @@ func (l *loadbalancers) retrieveCiliumClientset() error {
415417
kubeConfig *rest.Config
416418
err error
417419
)
418-
kubeconfigFlag := Options.KubeconfigFlag
420+
kubeconfigFlag := options.Options.KubeconfigFlag
419421
if kubeconfigFlag == nil || kubeconfigFlag.Value.String() == "" {
420422
kubeConfig, err = rest.InClusterConfig()
421423
} else {
@@ -513,7 +515,7 @@ func (l *loadbalancers) ensureCiliumBGPPeeringPolicy(ctx context.Context) error
513515
// otherwise create it
514516
var nodeSelector slimv1.LabelSelector
515517
// If no BGPNodeSelector is specified, select all worker nodes.
516-
if Options.BGPNodeSelector == "" {
518+
if options.Options.BGPNodeSelector == "" {
517519
nodeSelector = slimv1.LabelSelector{
518520
MatchExpressions: []slimv1.LabelSelectorRequirement{
519521
{
@@ -523,9 +525,9 @@ func (l *loadbalancers) ensureCiliumBGPPeeringPolicy(ctx context.Context) error
523525
},
524526
}
525527
} else {
526-
kv := strings.Split(Options.BGPNodeSelector, "=")
528+
kv := strings.Split(options.Options.BGPNodeSelector, "=")
527529
if len(kv) != BGPNodeSelectorFlagInputLen {
528-
return fmt.Errorf("invalid node selector %s", Options.BGPNodeSelector)
530+
return fmt.Errorf("invalid node selector %s", options.Options.BGPNodeSelector)
529531
}
530532

531533
nodeSelector = slimv1.LabelSelector{MatchLabels: map[string]string{kv[0]: kv[1]}}

‎cloud/linode/cilium_loadbalancers_test.go‎

Lines changed: 35 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ import (
1515
"k8s.io/client-go/kubernetes"
1616

1717
"github.com/linode/linode-cloud-controller-manager/cloud/linode/client/mocks"
18+
"github.com/linode/linode-cloud-controller-manager/cloud/linode/options"
19+
ccmUtils "github.com/linode/linode-cloud-controller-manager/cloud/linode/utils"
1820
)
1921

2022
const (
@@ -32,7 +34,7 @@ var (
3234
Labels: map[string]string{"cilium-bgp-peering": "true"},
3335
},
3436
Spec: v1.NodeSpec{
35-
ProviderID: fmt.Sprintf("%s%d", providerIDPrefix, 11111),
37+
ProviderID: fmt.Sprintf("%s%d", ccmUtils.ProviderIDPrefix, 11111),
3638
},
3739
},
3840
{
@@ -41,15 +43,15 @@ var (
4143
Labels: map[string]string{"cilium-bgp-peering": "true"},
4244
},
4345
Spec: v1.NodeSpec{
44-
ProviderID: fmt.Sprintf("%s%d", providerIDPrefix, 22222),
46+
ProviderID: fmt.Sprintf("%s%d", ccmUtils.ProviderIDPrefix, 22222),
4547
},
4648
},
4749
{
4850
ObjectMeta: metav1.ObjectMeta{
4951
Name: "node-3",
5052
},
5153
Spec: v1.NodeSpec{
52-
ProviderID: fmt.Sprintf("%s%d", providerIDPrefix, 33333),
54+
ProviderID: fmt.Sprintf("%s%d", ccmUtils.ProviderIDPrefix, 33333),
5355
},
5456
},
5557
{
@@ -60,7 +62,7 @@ var (
6062
},
6163
},
6264
Spec: v1.NodeSpec{
63-
ProviderID: fmt.Sprintf("%s%d", providerIDPrefix, 44444),
65+
ProviderID: fmt.Sprintf("%s%d", ccmUtils.ProviderIDPrefix, 44444),
6466
},
6567
},
6668
}
@@ -71,7 +73,7 @@ var (
7173
Labels: map[string]string{"cilium-bgp-peering": "true"},
7274
},
7375
Spec: v1.NodeSpec{
74-
ProviderID: fmt.Sprintf("%s%d", providerIDPrefix, 55555),
76+
ProviderID: fmt.Sprintf("%s%d", ccmUtils.ProviderIDPrefix, 55555),
7577
},
7678
},
7779
}
@@ -202,7 +204,7 @@ func addNodes(t *testing.T, kubeClient kubernetes.Interface, nodes []*v1.Node) {
202204
func createNewIpHolderInstance() linodego.Instance {
203205
return linodego.Instance{
204206
ID: 123456,
205-
Label: generateClusterScopedIPHolderLinodeName(zone, Options.IpHolderSuffix),
207+
Label: generateClusterScopedIPHolderLinodeName(zone, options.Options.IpHolderSuffix),
206208
Type: "g6-standard-1",
207209
Region: "us-west",
208210
IPv4: []*net.IP{&publicIPv4},
@@ -212,8 +214,8 @@ func createNewIpHolderInstance() linodego.Instance {
212214
func testNoBGPNodeLabel(t *testing.T, mc *mocks.MockClient) {
213215
t.Helper()
214216

215-
Options.BGPNodeSelector = ""
216-
Options.IpHolderSuffix = clusterName
217+
options.Options.BGPNodeSelector = ""
218+
options.Options.IpHolderSuffix = clusterName
217219
t.Setenv("BGP_PEER_PREFIX", "2600:3cef")
218220
svc := createTestService()
219221
newIpHolderInstance = createNewIpHolderInstance()
@@ -231,7 +233,7 @@ func testNoBGPNodeLabel(t *testing.T, mc *mocks.MockClient) {
231233
}
232234

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

274-
Options.BGPNodeSelector = nodeSelector
276+
options.Options.BGPNodeSelector = nodeSelector
275277
svc := createTestService()
276278

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

305-
Options.BGPNodeSelector = nodeSelector
307+
options.Options.BGPNodeSelector = nodeSelector
306308
svc := createTestService()
307309
newIpHolderInstance = createNewIpHolderInstance()
308310

@@ -346,8 +348,8 @@ func testCreateWithExistingIPHolderWithOldIpHolderNamingConvention(t *testing.T,
346348
func testCreateWithExistingIPHolderWithNewIpHolderNamingConvention(t *testing.T, mc *mocks.MockClient) {
347349
t.Helper()
348350

349-
Options.BGPNodeSelector = nodeSelector
350-
Options.IpHolderSuffix = clusterName
351+
options.Options.BGPNodeSelector = nodeSelector
352+
options.Options.IpHolderSuffix = clusterName
351353
svc := createTestService()
352354
newIpHolderInstance = createNewIpHolderInstance()
353355

@@ -391,8 +393,8 @@ func testCreateWithExistingIPHolderWithNewIpHolderNamingConvention(t *testing.T,
391393
func testCreateWithExistingIPHolderWithNewIpHolderNamingConventionUsingLongSuffix(t *testing.T, mc *mocks.MockClient) {
392394
t.Helper()
393395

394-
Options.BGPNodeSelector = nodeSelector
395-
Options.IpHolderSuffix = "OaTJrRuufacHVougjwkpBpmstiqvswvBNEMWXsRYfMBTCkKIUTXpbGIcIbDWSQp"
396+
options.Options.BGPNodeSelector = nodeSelector
397+
options.Options.IpHolderSuffix = "OaTJrRuufacHVougjwkpBpmstiqvswvBNEMWXsRYfMBTCkKIUTXpbGIcIbDWSQp"
396398
svc := createTestService()
397399
newIpHolderInstance = createNewIpHolderInstance()
398400

@@ -436,8 +438,8 @@ func testCreateWithExistingIPHolderWithNewIpHolderNamingConventionUsingLongSuffi
436438
func testCreateWithNoExistingIPHolderUsingNoSuffix(t *testing.T, mc *mocks.MockClient) {
437439
t.Helper()
438440

439-
Options.BGPNodeSelector = nodeSelector
440-
Options.IpHolderSuffix = ""
441+
options.Options.BGPNodeSelector = nodeSelector
442+
options.Options.IpHolderSuffix = ""
441443
svc := createTestService()
442444
newIpHolderInstance = createNewIpHolderInstance()
443445

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

491-
Options.BGPNodeSelector = nodeSelector
492-
Options.IpHolderSuffix = clusterName
493+
options.Options.BGPNodeSelector = nodeSelector
494+
options.Options.IpHolderSuffix = clusterName
493495
svc := createTestService()
494496
newIpHolderInstance = createNewIpHolderInstance()
495497

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

543-
Options.BGPNodeSelector = nodeSelector
544-
Options.IpHolderSuffix = "OaTJrRuufacHVougjwkpBpmstiqvswvBNEMWXsRYfMBTCkKIUTXpbGIcIbDWSQp"
545+
options.Options.BGPNodeSelector = nodeSelector
546+
options.Options.IpHolderSuffix = "OaTJrRuufacHVougjwkpBpmstiqvswvBNEMWXsRYfMBTCkKIUTXpbGIcIbDWSQp"
545547
svc := createTestService()
546548
newIpHolderInstance = createNewIpHolderInstance()
547549

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

595-
Options.BGPNodeSelector = nodeSelector
597+
options.Options.BGPNodeSelector = nodeSelector
596598
svc := createTestService()
597599

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

626-
Options.BGPNodeSelector = nodeSelector
627-
Options.IpHolderSuffix = clusterName
628+
options.Options.BGPNodeSelector = nodeSelector
629+
options.Options.IpHolderSuffix = clusterName
628630
svc := createTestService()
629631
newIpHolderInstance = createNewIpHolderInstance()
630632

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

665-
Options.BGPNodeSelector = nodeSelector
667+
options.Options.BGPNodeSelector = nodeSelector
666668
svc := createTestService()
667669

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

726-
Options.BGPNodeSelector = nodeSelector
727-
Options.IpHolderSuffix = clusterName
728+
options.Options.BGPNodeSelector = nodeSelector
729+
options.Options.IpHolderSuffix = clusterName
728730
svc := createTestService()
729731
newIpHolderInstance = createNewIpHolderInstance()
730732

@@ -740,7 +742,7 @@ func testCiliumUpdateLoadBalancerAddNodeWithNewIpHolderNamingConvention(t *testi
740742
t.Errorf("json marshal error: %v", err)
741743
}
742744
mc.EXPECT().ListInstances(gomock.Any(), linodego.NewListOptions(1, string(rawFilter))).Times(1).Return([]linodego.Instance{}, nil)
743-
filter = map[string]string{"label": generateClusterScopedIPHolderLinodeName(zone, Options.IpHolderSuffix)}
745+
filter = map[string]string{"label": generateClusterScopedIPHolderLinodeName(zone, options.Options.IpHolderSuffix)}
744746
rawFilter, err = json.Marshal(filter)
745747
if err != nil {
746748
t.Errorf("json marshal error: %v", err)
@@ -777,7 +779,7 @@ func testCiliumUpdateLoadBalancerAddNodeWithNewIpHolderNamingConvention(t *testi
777779
t.Errorf("json marshal error: %v", err)
778780
}
779781
mc.EXPECT().ListInstances(gomock.Any(), linodego.NewListOptions(1, string(rawFilter))).Times(1).Return([]linodego.Instance{}, nil)
780-
filter = map[string]string{"label": generateClusterScopedIPHolderLinodeName(zone, Options.IpHolderSuffix)}
782+
filter = map[string]string{"label": generateClusterScopedIPHolderLinodeName(zone, options.Options.IpHolderSuffix)}
781783
rawFilter, err = json.Marshal(filter)
782784
if err != nil {
783785
t.Errorf("json marshal error: %v", err)

0 commit comments

Comments
 (0)