Skip to content

Commit e3c285a

Browse files
pbenasYashwantGohokar
pbenas
authored and
YashwantGohokar
committed
NLB display name to OCID map
Map the display name of NLB to an OCID to avoid looking it up again in each reconciliation.
1 parent 61b38fa commit e3c285a

File tree

9 files changed

+213
-27
lines changed

9 files changed

+213
-27
lines changed

‎pkg/cloudprovider/providers/oci/ccm.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright 2017 Oracle and/or its affiliates. All rights reserved.
1+
// Copyright (C) 2017, 2025, Oracle and/or its affiliates.
22
//
33
// Licensed under the Apache License, Version 2.0 (the "License");
44
// you may not use this file except in compliance with the License.
@@ -100,7 +100,7 @@ func NewCloudProvider(config *providercfg.Config) (cloudprovider.Interface, erro
100100

101101
rateLimiter := client.NewRateLimiter(logger.Sugar(), config.RateLimiter)
102102

103-
c, err := client.New(logger.Sugar(), cp, &rateLimiter, config.Auth.TenancyID)
103+
c, err := client.New(logger.Sugar(), cp, &rateLimiter, config)
104104
if err != nil {
105105
return nil, err
106106
}

‎pkg/oci/client/client.go‎

Lines changed: 6 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"context"
1919
"time"
2020

21+
providercfg "github.com/oracle/oci-cloud-controller-manager/pkg/cloudprovider/providers/oci/config"
2122
"github.com/oracle/oci-go-sdk/v65/common"
2223
"github.com/oracle/oci-go-sdk/v65/common/auth"
2324
"github.com/oracle/oci-go-sdk/v65/core"
@@ -194,7 +195,7 @@ type client struct {
194195
}
195196

196197
// New constructs an OCI API client.
197-
func New(logger *zap.SugaredLogger, cp common.ConfigurationProvider, opRateLimiter *RateLimiter, targetTenancyID string) (Interface, error) {
198+
func New(logger *zap.SugaredLogger, cp common.ConfigurationProvider, opRateLimiter *RateLimiter, cloudProviderConfig *providercfg.Config) (Interface, error) {
198199

199200
compute, err := core.NewComputeClientWithConfigurationProvider(cp)
200201
if err != nil {
@@ -283,23 +284,15 @@ func New(logger *zap.SugaredLogger, cp common.ConfigurationProvider, opRateLimit
283284
RetryPolicy: newRetryPolicy(),
284285
}
285286

286-
loadbalancer := loadbalancerClientStruct{
287-
loadbalancer: lb,
288-
requestMetadata: requestMetadata,
289-
rateLimiter: *opRateLimiter,
290-
}
291-
networkloadbalancer := networkLoadbalancer{
292-
networkloadbalancer: nlb,
293-
requestMetadata: requestMetadata,
294-
rateLimiter: *opRateLimiter,
295-
}
287+
loadbalancer := NewLBClient(lb, requestMetadata, opRateLimiter)
288+
networkloadbalancer := NewNLBClient(nlb, requestMetadata, opRateLimiter)
296289

297290
c := &client{
298291
compute: &compute,
299292
network: &network,
300293
identity: &identity,
301-
loadbalancer: &loadbalancer,
302-
networkloadbalancer: &networkloadbalancer,
294+
loadbalancer: loadbalancer,
295+
networkloadbalancer: networkloadbalancer,
303296
bs: &bs,
304297
filestorage: &fss,
305298
//compartment: &compartment,

‎pkg/oci/client/client_factory.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright 2019 Oracle and/or its affiliates. All rights reserved.
1+
// Copyright (C) 2019, 2025, Oracle and/or its affiliates.
22
//
33
// Licensed under the Apache License, Version 2.0 (the "License");
44
// you may not use this file except in compliance with the License.
@@ -29,6 +29,6 @@ func GetClient(logger *zap.SugaredLogger, cfg *config.Config) (Interface, error)
2929

3030
rateLimiter := NewRateLimiter(logger, cfg.RateLimiter)
3131

32-
c, err := New(logger, cp, &rateLimiter, cfg.Auth.TenancyID)
32+
c, err := New(logger, cp, &rateLimiter, cfg)
3333
return c, err
3434
}

‎pkg/oci/client/load_balancer.go‎

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright 2018 Oracle and/or its affiliates. All rights reserved.
1+
// Copyright (C) 2018, 2025, Oracle and/or its affiliates.
22
//
33
// Licensed under the Apache License, Version 2.0 (the "License");
44
// you may not use this file except in compliance with the License.
@@ -17,6 +17,7 @@ package client
1717
import (
1818
"context"
1919
"go.uber.org/zap"
20+
"sync"
2021
"time"
2122

2223
"k8s.io/apimachinery/pkg/util/wait"
@@ -33,6 +34,7 @@ const (
3334
)
3435

3536
type loadbalancerClientStruct struct {
37+
nameToOcid sync.Map
3638
loadbalancer loadBalancerClient
3739
requestMetadata common.RequestMetadata
3840
rateLimiter RateLimiter
@@ -64,6 +66,15 @@ type GenericLoadBalancerInterface interface {
6466
UpdateLoadBalancer(ctx context.Context, lbID string, details *GenericUpdateLoadBalancerDetails) (string, error)
6567
}
6668

69+
func NewLBClient(lb loadBalancerClient, rm common.RequestMetadata, lim *RateLimiter) *loadbalancerClientStruct {
70+
l := loadbalancerClientStruct{
71+
loadbalancer: lb,
72+
requestMetadata: rm,
73+
rateLimiter: *lim,
74+
}
75+
return &l
76+
}
77+
6778
func (c *loadbalancerClientStruct) GetLoadBalancer(ctx context.Context, id string) (*GenericLoadBalancer, error) {
6879
if !c.rateLimiter.Reader.TryAccept() {
6980
return nil, RateLimitError(false, "GetLoadBalancer")
@@ -83,6 +94,29 @@ func (c *loadbalancerClientStruct) GetLoadBalancer(ctx context.Context, id strin
8394
}
8495

8596
func (c *loadbalancerClientStruct) GetLoadBalancerByName(ctx context.Context, compartmentID, name string) (*GenericLoadBalancer, error) {
97+
logger := zap.L().Sugar() // TODO refactor after pull-requests/1389
98+
logger = logger.With("lbName", name,
99+
"compartment-id", compartmentID,
100+
"loadBalancerType", "lb",
101+
)
102+
103+
if ocid, ok := c.nameToOcid.Load(name); ok {
104+
var err error
105+
ocidStr, ok := ocid.(string)
106+
if ok {
107+
lb, err := c.GetLoadBalancer(ctx, ocidStr)
108+
if err == nil && *lb.DisplayName == name {
109+
return lb, err
110+
}
111+
}
112+
113+
if !ok || IsNotFound(err) { // Only remove the cached value on 404, not on a 5XX
114+
c.nameToOcid.Delete(name)
115+
}
116+
} else {
117+
logger.Info("LB name to OCID cache miss")
118+
}
119+
86120
var page *string
87121
for {
88122
if !c.rateLimiter.Reader.TryAccept() {
@@ -101,6 +135,7 @@ func (c *loadbalancerClientStruct) GetLoadBalancerByName(ctx context.Context, co
101135
}
102136
for _, lb := range resp.Items {
103137
if *lb.DisplayName == name {
138+
c.nameToOcid.Store(name, *lb.Id)
104139
return c.loadbalancerToGenericLoadbalancer(&lb), nil
105140
}
106141
}

‎pkg/oci/client/network_load_balancer.go‎

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright 2018 Oracle and/or its affiliates. All rights reserved.
1+
// Copyright (C) 2018, 2025, Oracle and/or its affiliates.
22
//
33
// Licensed under the Apache License, Version 2.0 (the "License");
44
// you may not use this file except in compliance with the License.
@@ -16,6 +16,7 @@ package client
1616

1717
import (
1818
"context"
19+
"sync"
1920

2021
"go.uber.org/zap"
2122
"k8s.io/apimachinery/pkg/util/wait"
@@ -26,15 +27,30 @@ import (
2627
)
2728

2829
type networkLoadbalancer struct {
30+
nameToOcid sync.Map
2931
networkloadbalancer networkLoadBalancerClient
3032
requestMetadata common.RequestMetadata
3133
rateLimiter RateLimiter
3234
}
3335

3436
const (
3537
NetworkLoadBalancerEntityType = "NetworkLoadBalancer"
38+
// TODO move to utils?
39+
dns1123LabelFmt = "[a-z0-9]([-a-z0-9]*[a-z0-9])?"
40+
uuidFmt = "[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}"
41+
// <ns>/<svc>/<svc UID>
42+
LBNameRegex = "^" + dns1123LabelFmt + "/" + dns1123LabelFmt + "/" + uuidFmt + "$"
3643
)
3744

45+
func NewNLBClient(nlb networkLoadBalancerClient, rm common.RequestMetadata, lim *RateLimiter) *networkLoadbalancer {
46+
n := networkLoadbalancer{
47+
networkloadbalancer: nlb,
48+
requestMetadata: rm,
49+
rateLimiter: *lim,
50+
}
51+
return &n
52+
}
53+
3854
func (c *networkLoadbalancer) GetLoadBalancer(ctx context.Context, id string) (*GenericLoadBalancer, error) {
3955
if !c.rateLimiter.Reader.TryAccept() {
4056
return nil, RateLimitError(false, "GetLoadBalancer")
@@ -54,6 +70,29 @@ func (c *networkLoadbalancer) GetLoadBalancer(ctx context.Context, id string) (*
5470
}
5571

5672
func (c *networkLoadbalancer) GetLoadBalancerByName(ctx context.Context, compartmentID string, name string) (*GenericLoadBalancer, error) {
73+
logger := zap.L().Sugar() // TODO refactor after pull-requests/1389
74+
logger = logger.With("lbName", name,
75+
"compartment-id", compartmentID,
76+
"loadBalancerType", "nlb",
77+
)
78+
79+
if ocid, ok := c.nameToOcid.Load(name); ok {
80+
var err error
81+
ocidStr, ok := ocid.(string)
82+
if ok {
83+
lb, err := c.GetLoadBalancer(ctx, ocidStr)
84+
if err == nil && *lb.DisplayName == name {
85+
return lb, err
86+
}
87+
}
88+
89+
if !ok || IsNotFound(err) { // Only remove the cached value on 404, not on a 5XX
90+
c.nameToOcid.Delete(name)
91+
}
92+
} else {
93+
logger.Info("NLB name to OCID cache miss")
94+
}
95+
5796
var page *string
5897
for {
5998
if !c.rateLimiter.Reader.TryAccept() {
@@ -72,6 +111,7 @@ func (c *networkLoadbalancer) GetLoadBalancerByName(ctx context.Context, compart
72111
}
73112
for _, lb := range resp.Items {
74113
if *lb.DisplayName == name {
114+
c.nameToOcid.Store(name, *lb.Id)
75115
return c.networkLoadbalancerSummaryToGenericLoadbalancer(&lb), nil
76116
}
77117
}

‎pkg/oci/client/network_load_balancer_test.go‎

Lines changed: 120 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ package client
1717
import (
1818
"context"
1919
"errors"
20+
"fmt"
2021
errors2 "github.com/pkg/errors"
2122
"log"
2223
"strings"
@@ -98,10 +99,112 @@ func TestNLB_AwaitWorkRequest(t *testing.T) {
9899
}
99100
}
100101

102+
var (
103+
fakeNlbOcid1 = "ocid.nlb.fake1"
104+
fakeNlbName1 = "fake display name 1"
105+
fakeNlbOcid2 = "ocid.nlb.fake2"
106+
fakeNlbName2 = "fake display name 2"
107+
fakeSubnetOcid = "ocid.subnet.fake"
108+
109+
NLBMap = map[string]networkloadbalancer.NetworkLoadBalancer{
110+
"ocid.nlb.fake1": networkloadbalancer.NetworkLoadBalancer{
111+
Id: &fakeNlbOcid1,
112+
DisplayName: &fakeNlbName1,
113+
SubnetId: &fakeSubnetOcid,
114+
},
115+
"ocid.nlb.fake2": networkloadbalancer.NetworkLoadBalancer{
116+
Id: &fakeNlbOcid2,
117+
DisplayName: &fakeNlbName2,
118+
SubnetId: &fakeSubnetOcid,
119+
},
120+
}
121+
)
122+
123+
func TestGetLoadBalancerByName(t *testing.T) {
124+
var totalListCalls int
125+
var loadbalancer = NewNLBClient(
126+
&MockNetworkLoadBalancerClient{debug: true, listCalls: &totalListCalls},
127+
common.RequestMetadata{},
128+
&RateLimiter{
129+
Reader: flowcontrol.NewFakeAlwaysRateLimiter(),
130+
Writer: flowcontrol.NewFakeAlwaysRateLimiter(),
131+
})
132+
133+
var tests = []struct {
134+
skip bool // set true to skip a test-case
135+
compartment, name, testname string
136+
want string
137+
wantErr error
138+
wantListCalls int
139+
}{
140+
{
141+
testname: "getFirstNLBFirstTime",
142+
compartment: "ocid.compartment.fake",
143+
name: fakeNlbName1,
144+
want: fakeNlbOcid1,
145+
wantErr: nil,
146+
wantListCalls: 1,
147+
},
148+
{
149+
testname: "getFirstNLBSecondTime",
150+
compartment: "ocid.compartment.fake",
151+
name: fakeNlbName1,
152+
want: fakeNlbOcid1,
153+
wantErr: nil,
154+
wantListCalls: 1, // totals, no new list should be performed
155+
},
156+
{
157+
testname: "getSecondNLBTime",
158+
compartment: "ocid.compartment.fake",
159+
name: fakeNlbName2,
160+
want: fakeNlbOcid2,
161+
wantErr: nil,
162+
wantListCalls: 2,
163+
},
164+
{
165+
testname: "getFirstNLBThirdTime",
166+
compartment: "ocid.compartment.fake",
167+
name: fakeNlbName1,
168+
want: fakeNlbOcid1,
169+
wantErr: nil,
170+
wantListCalls: 2,
171+
},
172+
{
173+
testname: "getSecondNLBSecondTime",
174+
compartment: "ocid.compartment.fake",
175+
name: fakeNlbName2,
176+
want: fakeNlbOcid2,
177+
wantErr: nil,
178+
wantListCalls: 2,
179+
},
180+
}
181+
182+
for _, tt := range tests {
183+
if tt.skip {
184+
continue
185+
}
186+
187+
t.Run(tt.testname, func(t *testing.T) {
188+
log.Println("running test ", tt.testname)
189+
got, err := loadbalancer.GetLoadBalancerByName(context.Background(), tt.compartment, tt.name)
190+
if got == nil || *got.Id != tt.want {
191+
t.Errorf("Expected %v, but got %v", tt.want, got)
192+
}
193+
if !errors.Is(err, tt.wantErr) {
194+
t.Errorf("Expected error = %v, but got %v", tt.wantErr, err)
195+
}
196+
if totalListCalls != tt.wantListCalls {
197+
t.Errorf("Expected the total number of NLB list calls %d, but got %d", tt.wantListCalls, totalListCalls)
198+
}
199+
})
200+
}
201+
}
202+
101203
type MockNetworkLoadBalancerClient struct {
102204
// MockLoadBalancerClient mocks LoadBalancer client implementation.
103-
counter int
104-
debug bool // set true to run tests with debug logs
205+
counter int
206+
debug bool // set true to run tests with debug logs
207+
listCalls *int // number of list operations performed
105208
}
106209

107210
type getNetworkLoadBalancerWorkRequestResponse struct {
@@ -173,12 +276,27 @@ func (c *MockNetworkLoadBalancerClient) GetWorkRequest(ctx context.Context, requ
173276
}
174277

175278
func (c *MockNetworkLoadBalancerClient) GetNetworkLoadBalancer(ctx context.Context, request networkloadbalancer.GetNetworkLoadBalancerRequest) (response networkloadbalancer.GetNetworkLoadBalancerResponse, err error) {
279+
if c.debug {
280+
log.Println(fmt.Sprintf("Getting NLB %v", *request.NetworkLoadBalancerId))
281+
}
282+
283+
response = networkloadbalancer.GetNetworkLoadBalancerResponse{
284+
NetworkLoadBalancer: NLBMap[*request.NetworkLoadBalancerId],
285+
}
176286
return
177287
}
178288
func (c *MockNetworkLoadBalancerClient) ListWorkRequests(ctx context.Context, request networkloadbalancer.ListWorkRequestsRequest) (response networkloadbalancer.ListWorkRequestsResponse, err error) {
179289
return
180290
}
181291
func (c *MockNetworkLoadBalancerClient) ListNetworkLoadBalancers(ctx context.Context, request networkloadbalancer.ListNetworkLoadBalancersRequest) (response networkloadbalancer.ListNetworkLoadBalancersResponse, err error) {
292+
if c.debug {
293+
log.Println(fmt.Sprintf("Lising NLBs in compartment %v", *request.CompartmentId))
294+
}
295+
296+
for _, nlb := range NLBMap {
297+
response.NetworkLoadBalancerCollection.Items = append(response.NetworkLoadBalancerCollection.Items, networkloadbalancer.NetworkLoadBalancerSummary(nlb))
298+
}
299+
*c.listCalls += 1
182300
return
183301
}
184302
func (c *MockNetworkLoadBalancerClient) CreateNetworkLoadBalancer(ctx context.Context, request networkloadbalancer.CreateNetworkLoadBalancerRequest) (response networkloadbalancer.CreateNetworkLoadBalancerResponse, err error) {

‎pkg/volume/provisioner/core/provisioner.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright 2017 Oracle and/or its affiliates. All rights reserved.
1+
// Copyright (C) 2017, 2025, Oracle and/or its affiliates.
22
//
33
// Licensed under the Apache License, Version 2.0 (the "License");
44
// you may not use this file except in compliance with the License.
@@ -124,7 +124,7 @@ func NewOCIProvisioner(logger *zap.SugaredLogger, kubeClient kubernetes.Interfac
124124

125125
rateLimiter := client.NewRateLimiter(logger, cfg.RateLimiter)
126126

127-
client, err := client.New(logger, cp, &rateLimiter, cfg.Auth.TenancyID)
127+
client, err := client.New(logger, cp, &rateLimiter, cfg)
128128
if err != nil {
129129
return nil, errors.Wrapf(err, "unable to construct OCI client")
130130
}

0 commit comments

Comments
 (0)