diff --git a/pkg/cloudprovider/providers/oci/ccm.go b/pkg/cloudprovider/providers/oci/ccm.go index 6af041abe3..d6a620e9f6 100644 --- a/pkg/cloudprovider/providers/oci/ccm.go +++ b/pkg/cloudprovider/providers/oci/ccm.go @@ -1,4 +1,4 @@ -// Copyright 2017 Oracle and/or its affiliates. All rights reserved. +// Copyright (C) 2017, 2025, Oracle and/or its affiliates. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -100,7 +100,7 @@ func NewCloudProvider(config *providercfg.Config) (cloudprovider.Interface, erro rateLimiter := client.NewRateLimiter(logger.Sugar(), config.RateLimiter) - c, err := client.New(logger.Sugar(), cp, &rateLimiter, config.Auth.TenancyID) + c, err := client.New(logger.Sugar(), cp, &rateLimiter, config) if err != nil { return nil, err } diff --git a/pkg/oci/client/client.go b/pkg/oci/client/client.go index 405f96ed1b..65a213c667 100644 --- a/pkg/oci/client/client.go +++ b/pkg/oci/client/client.go @@ -18,6 +18,7 @@ import ( "context" "time" + providercfg "github.com/oracle/oci-cloud-controller-manager/pkg/cloudprovider/providers/oci/config" "github.com/oracle/oci-go-sdk/v65/common" "github.com/oracle/oci-go-sdk/v65/common/auth" "github.com/oracle/oci-go-sdk/v65/core" @@ -194,7 +195,7 @@ type client struct { } // New constructs an OCI API client. -func New(logger *zap.SugaredLogger, cp common.ConfigurationProvider, opRateLimiter *RateLimiter, targetTenancyID string) (Interface, error) { +func New(logger *zap.SugaredLogger, cp common.ConfigurationProvider, opRateLimiter *RateLimiter, cloudProviderConfig *providercfg.Config) (Interface, error) { compute, err := core.NewComputeClientWithConfigurationProvider(cp) if err != nil { @@ -283,23 +284,15 @@ func New(logger *zap.SugaredLogger, cp common.ConfigurationProvider, opRateLimit RetryPolicy: newRetryPolicy(), } - loadbalancer := loadbalancerClientStruct{ - loadbalancer: lb, - requestMetadata: requestMetadata, - rateLimiter: *opRateLimiter, - } - networkloadbalancer := networkLoadbalancer{ - networkloadbalancer: nlb, - requestMetadata: requestMetadata, - rateLimiter: *opRateLimiter, - } + loadbalancer := NewLBClient(lb, requestMetadata, opRateLimiter) + networkloadbalancer := NewNLBClient(nlb, requestMetadata, opRateLimiter) c := &client{ compute: &compute, network: &network, identity: &identity, - loadbalancer: &loadbalancer, - networkloadbalancer: &networkloadbalancer, + loadbalancer: loadbalancer, + networkloadbalancer: networkloadbalancer, bs: &bs, filestorage: &fss, //compartment: &compartment, diff --git a/pkg/oci/client/client_factory.go b/pkg/oci/client/client_factory.go index 1d7410e7aa..626917ec9c 100644 --- a/pkg/oci/client/client_factory.go +++ b/pkg/oci/client/client_factory.go @@ -1,4 +1,4 @@ -// Copyright 2019 Oracle and/or its affiliates. All rights reserved. +// Copyright (C) 2019, 2025, Oracle and/or its affiliates. // // Licensed under the Apache License, Version 2.0 (the "License"); // 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) rateLimiter := NewRateLimiter(logger, cfg.RateLimiter) - c, err := New(logger, cp, &rateLimiter, cfg.Auth.TenancyID) + c, err := New(logger, cp, &rateLimiter, cfg) return c, err } diff --git a/pkg/oci/client/load_balancer.go b/pkg/oci/client/load_balancer.go index f72337b197..85b935a87c 100644 --- a/pkg/oci/client/load_balancer.go +++ b/pkg/oci/client/load_balancer.go @@ -1,4 +1,4 @@ -// Copyright 2018 Oracle and/or its affiliates. All rights reserved. +// Copyright (C) 2018, 2025, Oracle and/or its affiliates. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -17,6 +17,7 @@ package client import ( "context" "go.uber.org/zap" + "sync" "time" "k8s.io/apimachinery/pkg/util/wait" @@ -33,6 +34,7 @@ const ( ) type loadbalancerClientStruct struct { + nameToOcid sync.Map loadbalancer loadBalancerClient requestMetadata common.RequestMetadata rateLimiter RateLimiter @@ -64,6 +66,15 @@ type GenericLoadBalancerInterface interface { UpdateLoadBalancer(ctx context.Context, lbID string, details *GenericUpdateLoadBalancerDetails) (string, error) } +func NewLBClient(lb loadBalancerClient, rm common.RequestMetadata, lim *RateLimiter) *loadbalancerClientStruct { + l := loadbalancerClientStruct{ + loadbalancer: lb, + requestMetadata: rm, + rateLimiter: *lim, + } + return &l +} + func (c *loadbalancerClientStruct) GetLoadBalancer(ctx context.Context, id string) (*GenericLoadBalancer, error) { if !c.rateLimiter.Reader.TryAccept() { return nil, RateLimitError(false, "GetLoadBalancer") @@ -83,6 +94,29 @@ func (c *loadbalancerClientStruct) GetLoadBalancer(ctx context.Context, id strin } func (c *loadbalancerClientStruct) GetLoadBalancerByName(ctx context.Context, compartmentID, name string) (*GenericLoadBalancer, error) { + logger := zap.L().Sugar() // TODO refactor after pull-requests/1389 + logger = logger.With("lbName", name, + "compartment-id", compartmentID, + "loadBalancerType", "lb", + ) + + if ocid, ok := c.nameToOcid.Load(name); ok { + var err error + ocidStr, ok := ocid.(string) + if ok { + lb, err := c.GetLoadBalancer(ctx, ocidStr) + if err == nil && *lb.DisplayName == name { + return lb, err + } + } + + if !ok || IsNotFound(err) { // Only remove the cached value on 404, not on a 5XX + c.nameToOcid.Delete(name) + } + } else { + logger.Info("LB name to OCID cache miss") + } + var page *string for { if !c.rateLimiter.Reader.TryAccept() { @@ -101,6 +135,7 @@ func (c *loadbalancerClientStruct) GetLoadBalancerByName(ctx context.Context, co } for _, lb := range resp.Items { if *lb.DisplayName == name { + c.nameToOcid.Store(name, *lb.Id) return c.loadbalancerToGenericLoadbalancer(&lb), nil } } diff --git a/pkg/oci/client/network_load_balancer.go b/pkg/oci/client/network_load_balancer.go index e95dab3bf4..d560b34942 100644 --- a/pkg/oci/client/network_load_balancer.go +++ b/pkg/oci/client/network_load_balancer.go @@ -1,4 +1,4 @@ -// Copyright 2018 Oracle and/or its affiliates. All rights reserved. +// Copyright (C) 2018, 2025, Oracle and/or its affiliates. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -16,6 +16,7 @@ package client import ( "context" + "sync" "go.uber.org/zap" "k8s.io/apimachinery/pkg/util/wait" @@ -26,6 +27,7 @@ import ( ) type networkLoadbalancer struct { + nameToOcid sync.Map networkloadbalancer networkLoadBalancerClient requestMetadata common.RequestMetadata rateLimiter RateLimiter @@ -33,8 +35,22 @@ type networkLoadbalancer struct { const ( NetworkLoadBalancerEntityType = "NetworkLoadBalancer" + // TODO move to utils? + dns1123LabelFmt = "[a-z0-9]([-a-z0-9]*[a-z0-9])?" + uuidFmt = "[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}" + // // + LBNameRegex = "^" + dns1123LabelFmt + "/" + dns1123LabelFmt + "/" + uuidFmt + "$" ) +func NewNLBClient(nlb networkLoadBalancerClient, rm common.RequestMetadata, lim *RateLimiter) *networkLoadbalancer { + n := networkLoadbalancer{ + networkloadbalancer: nlb, + requestMetadata: rm, + rateLimiter: *lim, + } + return &n +} + func (c *networkLoadbalancer) GetLoadBalancer(ctx context.Context, id string) (*GenericLoadBalancer, error) { if !c.rateLimiter.Reader.TryAccept() { return nil, RateLimitError(false, "GetLoadBalancer") @@ -54,6 +70,29 @@ func (c *networkLoadbalancer) GetLoadBalancer(ctx context.Context, id string) (* } func (c *networkLoadbalancer) GetLoadBalancerByName(ctx context.Context, compartmentID string, name string) (*GenericLoadBalancer, error) { + logger := zap.L().Sugar() // TODO refactor after pull-requests/1389 + logger = logger.With("lbName", name, + "compartment-id", compartmentID, + "loadBalancerType", "nlb", + ) + + if ocid, ok := c.nameToOcid.Load(name); ok { + var err error + ocidStr, ok := ocid.(string) + if ok { + lb, err := c.GetLoadBalancer(ctx, ocidStr) + if err == nil && *lb.DisplayName == name { + return lb, err + } + } + + if !ok || IsNotFound(err) { // Only remove the cached value on 404, not on a 5XX + c.nameToOcid.Delete(name) + } + } else { + logger.Info("NLB name to OCID cache miss") + } + var page *string for { if !c.rateLimiter.Reader.TryAccept() { @@ -72,6 +111,7 @@ func (c *networkLoadbalancer) GetLoadBalancerByName(ctx context.Context, compart } for _, lb := range resp.Items { if *lb.DisplayName == name { + c.nameToOcid.Store(name, *lb.Id) return c.networkLoadbalancerSummaryToGenericLoadbalancer(&lb), nil } } diff --git a/pkg/oci/client/network_load_balancer_test.go b/pkg/oci/client/network_load_balancer_test.go index 711d33d434..4e60679491 100644 --- a/pkg/oci/client/network_load_balancer_test.go +++ b/pkg/oci/client/network_load_balancer_test.go @@ -17,6 +17,7 @@ package client import ( "context" "errors" + "fmt" errors2 "github.com/pkg/errors" "log" "strings" @@ -98,10 +99,112 @@ func TestNLB_AwaitWorkRequest(t *testing.T) { } } +var ( + fakeNlbOcid1 = "ocid.nlb.fake1" + fakeNlbName1 = "fake display name 1" + fakeNlbOcid2 = "ocid.nlb.fake2" + fakeNlbName2 = "fake display name 2" + fakeSubnetOcid = "ocid.subnet.fake" + + NLBMap = map[string]networkloadbalancer.NetworkLoadBalancer{ + "ocid.nlb.fake1": networkloadbalancer.NetworkLoadBalancer{ + Id: &fakeNlbOcid1, + DisplayName: &fakeNlbName1, + SubnetId: &fakeSubnetOcid, + }, + "ocid.nlb.fake2": networkloadbalancer.NetworkLoadBalancer{ + Id: &fakeNlbOcid2, + DisplayName: &fakeNlbName2, + SubnetId: &fakeSubnetOcid, + }, + } +) + +func TestGetLoadBalancerByName(t *testing.T) { + var totalListCalls int + var loadbalancer = NewNLBClient( + &MockNetworkLoadBalancerClient{debug: true, listCalls: &totalListCalls}, + common.RequestMetadata{}, + &RateLimiter{ + Reader: flowcontrol.NewFakeAlwaysRateLimiter(), + Writer: flowcontrol.NewFakeAlwaysRateLimiter(), + }) + + var tests = []struct { + skip bool // set true to skip a test-case + compartment, name, testname string + want string + wantErr error + wantListCalls int + }{ + { + testname: "getFirstNLBFirstTime", + compartment: "ocid.compartment.fake", + name: fakeNlbName1, + want: fakeNlbOcid1, + wantErr: nil, + wantListCalls: 1, + }, + { + testname: "getFirstNLBSecondTime", + compartment: "ocid.compartment.fake", + name: fakeNlbName1, + want: fakeNlbOcid1, + wantErr: nil, + wantListCalls: 1, // totals, no new list should be performed + }, + { + testname: "getSecondNLBTime", + compartment: "ocid.compartment.fake", + name: fakeNlbName2, + want: fakeNlbOcid2, + wantErr: nil, + wantListCalls: 2, + }, + { + testname: "getFirstNLBThirdTime", + compartment: "ocid.compartment.fake", + name: fakeNlbName1, + want: fakeNlbOcid1, + wantErr: nil, + wantListCalls: 2, + }, + { + testname: "getSecondNLBSecondTime", + compartment: "ocid.compartment.fake", + name: fakeNlbName2, + want: fakeNlbOcid2, + wantErr: nil, + wantListCalls: 2, + }, + } + + for _, tt := range tests { + if tt.skip { + continue + } + + t.Run(tt.testname, func(t *testing.T) { + log.Println("running test ", tt.testname) + got, err := loadbalancer.GetLoadBalancerByName(context.Background(), tt.compartment, tt.name) + if got == nil || *got.Id != tt.want { + t.Errorf("Expected %v, but got %v", tt.want, got) + } + if !errors.Is(err, tt.wantErr) { + t.Errorf("Expected error = %v, but got %v", tt.wantErr, err) + } + if totalListCalls != tt.wantListCalls { + t.Errorf("Expected the total number of NLB list calls %d, but got %d", tt.wantListCalls, totalListCalls) + } + }) + } +} + type MockNetworkLoadBalancerClient struct { // MockLoadBalancerClient mocks LoadBalancer client implementation. - counter int - debug bool // set true to run tests with debug logs + counter int + debug bool // set true to run tests with debug logs + listCalls *int // number of list operations performed } type getNetworkLoadBalancerWorkRequestResponse struct { @@ -173,12 +276,27 @@ func (c *MockNetworkLoadBalancerClient) GetWorkRequest(ctx context.Context, requ } func (c *MockNetworkLoadBalancerClient) GetNetworkLoadBalancer(ctx context.Context, request networkloadbalancer.GetNetworkLoadBalancerRequest) (response networkloadbalancer.GetNetworkLoadBalancerResponse, err error) { + if c.debug { + log.Println(fmt.Sprintf("Getting NLB %v", *request.NetworkLoadBalancerId)) + } + + response = networkloadbalancer.GetNetworkLoadBalancerResponse{ + NetworkLoadBalancer: NLBMap[*request.NetworkLoadBalancerId], + } return } func (c *MockNetworkLoadBalancerClient) ListWorkRequests(ctx context.Context, request networkloadbalancer.ListWorkRequestsRequest) (response networkloadbalancer.ListWorkRequestsResponse, err error) { return } func (c *MockNetworkLoadBalancerClient) ListNetworkLoadBalancers(ctx context.Context, request networkloadbalancer.ListNetworkLoadBalancersRequest) (response networkloadbalancer.ListNetworkLoadBalancersResponse, err error) { + if c.debug { + log.Println(fmt.Sprintf("Lising NLBs in compartment %v", *request.CompartmentId)) + } + + for _, nlb := range NLBMap { + response.NetworkLoadBalancerCollection.Items = append(response.NetworkLoadBalancerCollection.Items, networkloadbalancer.NetworkLoadBalancerSummary(nlb)) + } + *c.listCalls += 1 return } func (c *MockNetworkLoadBalancerClient) CreateNetworkLoadBalancer(ctx context.Context, request networkloadbalancer.CreateNetworkLoadBalancerRequest) (response networkloadbalancer.CreateNetworkLoadBalancerResponse, err error) { diff --git a/pkg/volume/provisioner/core/provisioner.go b/pkg/volume/provisioner/core/provisioner.go index e1bb67872e..3cd8cf2d64 100644 --- a/pkg/volume/provisioner/core/provisioner.go +++ b/pkg/volume/provisioner/core/provisioner.go @@ -1,4 +1,4 @@ -// Copyright 2017 Oracle and/or its affiliates. All rights reserved. +// Copyright (C) 2017, 2025, Oracle and/or its affiliates. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -124,7 +124,7 @@ func NewOCIProvisioner(logger *zap.SugaredLogger, kubeClient kubernetes.Interfac rateLimiter := client.NewRateLimiter(logger, cfg.RateLimiter) - client, err := client.New(logger, cp, &rateLimiter, cfg.Auth.TenancyID) + client, err := client.New(logger, cp, &rateLimiter, cfg) if err != nil { return nil, errors.Wrapf(err, "unable to construct OCI client") } diff --git a/test/e2e/cloud-provider-oci/load_balancer.go b/test/e2e/cloud-provider-oci/load_balancer.go index fc5c6bfbda..5fb96b4be3 100644 --- a/test/e2e/cloud-provider-oci/load_balancer.go +++ b/test/e2e/cloud-provider-oci/load_balancer.go @@ -693,8 +693,8 @@ var _ = Describe("IpMode [Slow]", func() { } Context("[cloudprovider][ccm][lb][ipMode]", func() { It("traffic should work from pods via load balancer", func() { - if sharedfw.CompareVersions(f.OkeClusterK8sVersion, "v1.30") < 0 { - Skip("Cluster K8s Version " + f.OkeClusterK8sVersion + " is less than v1.30, skipping test for Load Balancer ingress ipMode=\"Proxy\"") + if f.OkeClusterK8sVersion != "v1.30" { + Skip("Cluster K8s Version " + f.OkeClusterK8sVersion + " is less than v1.30, skipping test for ESIPP since it relies on ipMode=\"Proxy\"") } for _, test := range esippTestsArray { By("Running test for: " + test.lbType) @@ -784,7 +784,7 @@ var _ = Describe("ESIPP - IpMode Proxy [Slow]", func() { } Context("[cloudprovider][ccm][lb][esipp]", func() { It("should preserve source IP of pod with ipMode Proxy", func() { - if sharedfw.CompareVersions(f.OkeClusterK8sVersion, "v1.30") < 0 { + if f.OkeClusterK8sVersion != "v1.30" { Skip("Cluster K8s Version " + f.OkeClusterK8sVersion + " is less than v1.30, skipping test for ESIPP since it relies on ipMode=\"Proxy\"") } for _, test := range esippTestsArray { diff --git a/test/e2e/framework/cloud_provider_framework.go b/test/e2e/framework/cloud_provider_framework.go index 1d12f5215f..31eb3adc59 100644 --- a/test/e2e/framework/cloud_provider_framework.go +++ b/test/e2e/framework/cloud_provider_framework.go @@ -370,7 +370,7 @@ func createOCIClient(cloudProviderConfig *providercfg.Config) (client.Interface, ociClientConfig := common.NewRawConfigurationProvider(cpc.TenancyID, cpc.UserID, cpc.Region, cpc.Fingerprint, cpc.PrivateKey, &cpc.PrivateKeyPassphrase) logger := zap.L() rateLimiter := client.NewRateLimiter(logger.Sugar(), cloudProviderConfig.RateLimiter) - ociClient, err := client.New(logger.Sugar(), ociClientConfig, &rateLimiter, cloudProviderConfig.Auth.TenancyID) + ociClient, err := client.New(logger.Sugar(), ociClientConfig, &rateLimiter, cloudProviderConfig) if err != nil { return nil, errors.Wrapf(err, "Couldn't create oci client from configuration: %s.", cloudConfigFile) }