Skip to content

Commit 2e2a189

Browse files
GouthamMLl-technicore
GouthamML
authored and
l-technicore
committed
Storage Backfill Controller to backfill existing storage resource with OKE system tags
1 parent f019aad commit 2e2a189

File tree

10 files changed

+122
-47
lines changed

10 files changed

+122
-47
lines changed

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

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -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)
103+
c, err := client.New(logger.Sugar(), cp, &rateLimiter, config.Auth.TenancyID)
104104
if err != nil {
105105
return nil, err
106106
}
@@ -165,6 +165,7 @@ func init() {
165165
// Initialize passes a Kubernetes clientBuilder interface to the cloud provider.
166166
func (cp *CloudProvider) Initialize(clientBuilder cloudprovider.ControllerClientBuilder, stop <-chan struct{}) {
167167
var err error
168+
//var sbcStopChannel = make(chan struct{})
168169
cp.kubeclient, err = clientBuilder.Client("cloud-controller-manager")
169170
if err != nil {
170171
utilruntime.HandleError(fmt.Errorf("failed to create kubeclient: %v", err))
@@ -199,6 +200,23 @@ func (cp *CloudProvider) Initialize(clientBuilder cloudprovider.ControllerClient
199200

200201
cp.ServiceAccountLister = serviceAccountInformer.Lister()
201202

203+
/* StorageBackfillController not applicable for Open Source CCM
204+
enableStorageBackfillController := GetIsFeatureEnabledFromEnv(cp.logger, resourceTrackingFeatureFlagName, false)
205+
if enableStorageBackfillController {
206+
cp.logger.Info("Starting storage backfill controller")
207+
208+
storageBackfillController := NewStorageBackfillController(
209+
cp.kubeclient,
210+
cp.client,
211+
cp.logger,
212+
cp.metricPusher,
213+
cp.config,
214+
pvInformer.Lister(),
215+
)
216+
go storageBackfillController.Run(sbcStopChannel)
217+
}
218+
*/
219+
202220
cp.securityListManagerFactory = func(mode string) securityListManager {
203221
if cp.config.LoadBalancer.Disabled {
204222
return newSecurityListManagerNOOP()

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

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ package oci
1717
import (
1818
"context"
1919
"errors"
20+
"net/http"
2021
"reflect"
2122
"testing"
2223

@@ -1200,7 +1201,19 @@ func (MockBlockStorageClient) CreateVolume(ctx context.Context, details core.Cre
12001201
return nil, nil
12011202
}
12021203

1204+
var updateVolumeErrors = map[string]error{
1205+
"work-request-fails": errors.New("UpdateVolume request failed: internal server error"),
1206+
"api-returns-too-many-requests": mockServiceError{
1207+
StatusCode: http.StatusTooManyRequests,
1208+
Code: client.HTTP429TooManyRequestsCode,
1209+
Message: "Too many requests",
1210+
},
1211+
}
1212+
12031213
func (c MockBlockStorageClient) UpdateVolume(ctx context.Context, volumeId string, details core.UpdateVolumeDetails) (*core.Volume, error) {
1214+
if err, ok := updateVolumeErrors[volumeId]; ok {
1215+
return nil, err
1216+
}
12041217
return nil, nil
12051218
}
12061219

@@ -1892,3 +1905,29 @@ func (s *mockEndpointSliceNamespaceLister) Get(name string) (ret *v1discovery.En
18921905
}
18931906
return nil, errors.New("get endpointSlice error")
18941907
}
1908+
1909+
type mockServiceError struct {
1910+
StatusCode int
1911+
Code string
1912+
Message string
1913+
OpcRequestID string
1914+
}
1915+
1916+
func (m mockServiceError) GetHTTPStatusCode() int {
1917+
return m.StatusCode
1918+
}
1919+
1920+
func (m mockServiceError) GetMessage() string {
1921+
return m.Message
1922+
}
1923+
1924+
func (m mockServiceError) GetCode() string {
1925+
return m.Code
1926+
}
1927+
1928+
func (m mockServiceError) GetOpcRequestID() string {
1929+
return m.OpcRequestID
1930+
}
1931+
func (m mockServiceError) Error() string {
1932+
return m.Message
1933+
}

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

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -71,15 +71,6 @@ const DefaultNetworkLoadBalancerListenerProtocol = "TCP"
7171
// https://docs.oracle.com/en-us/iaas/Content/General/Concepts/servicelimits.htm#nsg_limits
7272
const MaxNsgPerVnic = 5
7373

74-
const (
75-
OkeSystemTagNamesapce = "orcl-containerengine"
76-
// MaxDefinedTagPerLB is the maximum number of defined tags that be can be associated with the resource
77-
//https://docs.oracle.com/en-us/iaas/Content/Tagging/Concepts/taggingoverview.htm#limits
78-
MaxDefinedTagPerLB = 64
79-
resourceTrackingFeatureFlagName = "CPO_ENABLE_RESOURCE_ATTRIBUTION"
80-
)
81-
82-
var MaxDefinedTagPerLBErr = fmt.Errorf("max limit of defined tags for lb is reached. skip adding tags. sending metric")
8374
var enableOkeSystemTags = false
8475

8576
const (
@@ -416,7 +407,7 @@ func (clb *CloudLoadBalancerProvider) createLoadBalancer(ctx context.Context, sp
416407
IpVersion: spec.IpVersions.LbEndpointIpVersion,
417408
}
418409
// do not block creation if the defined tag limit is reached. defer LB to tracked by backfilling
419-
if len(details.DefinedTags) > MaxDefinedTagPerLB {
410+
if len(details.DefinedTags) > MaxDefinedTagPerResource {
420411
logger.Warnf("the number of defined tags in the LB create request is beyond the limit. removing the resource tracking tags from the details")
421412
delete(details.DefinedTags, OkeSystemTagNamesapce)
422413
}
@@ -1140,7 +1131,7 @@ func (clb *CloudLoadBalancerProvider) updateLoadBalancer(ctx context.Context, lb
11401131
// fail open if the update request fails
11411132
logger.With(zap.Error(err)).Warn("updateLoadBalancer didn't succeed. unable to add oke system tags")
11421133
errType = util.SystemTagErrTypePrefix + util.GetError(err)
1143-
if errors.Is(err, MaxDefinedTagPerLBErr) {
1134+
if errors.Is(err, fmt.Errorf(MaxDefinedTagErrMessage, spec.Type)) {
11441135
errType = util.ErrTagLimitReached
11451136
}
11461137
dimensionsMap[metrics.ComponentDimension] = util.GetMetricDimensionForComponent(errType, util.LoadBalancerType)
@@ -1920,8 +1911,8 @@ func (clb *CloudLoadBalancerProvider) addLoadBalancerOkeSystemTags(ctx context.C
19201911
lbDefinedTagsRequest[OkeSystemTagNamesapce] = spec.SystemTags[OkeSystemTagNamesapce]
19211912

19221913
// update fails if the number of defined tags is more than the service limit i.e 64
1923-
if len(lbDefinedTagsRequest) > MaxDefinedTagPerLB {
1924-
return MaxDefinedTagPerLBErr
1914+
if len(lbDefinedTagsRequest) > MaxDefinedTagPerResource {
1915+
return fmt.Errorf(MaxDefinedTagErrMessage, spec.Type)
19251916
}
19261917

19271918
lbUpdateDetails := &client.GenericUpdateLoadBalancerDetails{

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

Lines changed: 1 addition & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -462,7 +462,7 @@ func NewLBSpec(logger *zap.SugaredLogger, svc *v1.Service, provisionedNodes []*v
462462
IpVersions: ipVersions,
463463
FreeformTags: lbTags.FreeformTags,
464464
DefinedTags: lbTags.DefinedTags,
465-
SystemTags: getResourceTrackingSysTagsFromConfig(logger, initialLBTags),
465+
SystemTags: getResourceTrackingSystemTagsFromConfig(logger, initialLBTags),
466466
ingressIpMode: ingressIpMode,
467467
}, nil
468468
}
@@ -1530,25 +1530,6 @@ func updateSpecWithLbSubnets(spec *LBSpec, lbSubnetId []string) (*LBSpec, error)
15301530
return spec, nil
15311531
}
15321532

1533-
// getResourceTrackingSysTagsFromConfig reads resource tracking tags from config
1534-
// which are specified under common tags
1535-
func getResourceTrackingSysTagsFromConfig(logger *zap.SugaredLogger, initialTags *config.InitialTags) (resourceTrackingTags map[string]map[string]interface{}) {
1536-
resourceTrackingTags = make(map[string]map[string]interface{})
1537-
// TODO: Fix the double negative
1538-
if !(util.IsCommonTagPresent(initialTags) && initialTags.Common.DefinedTags != nil) {
1539-
logger.Error("oke resource tracking system tags are not present in cloud-config.yaml")
1540-
return nil
1541-
}
1542-
1543-
if tag, exists := initialTags.Common.DefinedTags[OkeSystemTagNamesapce]; exists {
1544-
resourceTrackingTags[OkeSystemTagNamesapce] = tag
1545-
return
1546-
}
1547-
1548-
logger.Error("tag config doesn't consist resource tracking tags")
1549-
return nil
1550-
}
1551-
15521533
// getIpFamilies gets ip families based on the field set in the spec
15531534
func getIpFamilies(svc *v1.Service) []string {
15541535
ipFamilies := []string{}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10064,7 +10064,7 @@ func Test_getResourceTrackingSysTagsFromConfig(t *testing.T) {
1006410064
}
1006510065
for name, test := range tests {
1006610066
t.Run(name, func(t *testing.T) {
10067-
tag := getResourceTrackingSysTagsFromConfig(zap.S(), test.initialTags)
10067+
tag := getResourceTrackingSystemTagsFromConfig(zap.S(), test.initialTags)
1006810068
t.Logf("%#v", tag)
1006910069
if !reflect.DeepEqual(test.wantTag, tag) {
1007010070
t.Errorf("wanted %v but got %v", test.wantTag, tag)

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1192,6 +1192,7 @@ func Test_addLoadBalancerOkeSystemTags(t *testing.T) {
11921192
DefinedTags: make(map[string]map[string]interface{}),
11931193
},
11941194
spec: &LBSpec{
1195+
Type: LB,
11951196
SystemTags: map[string]map[string]interface{}{"orcl-containerengine": {"Cluster": "val"}},
11961197
service: &v1.Service{},
11971198
},

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

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,16 +20,29 @@ import (
2020
"strings"
2121
"sync"
2222

23-
"github.com/oracle/oci-cloud-controller-manager/pkg/oci/client"
2423
"github.com/pkg/errors"
2524
"go.uber.org/zap"
2625
api "k8s.io/api/core/v1"
2726
"k8s.io/apimachinery/pkg/labels"
2827
"k8s.io/apimachinery/pkg/util/sets"
2928
listersv1 "k8s.io/client-go/listers/core/v1"
3029
"k8s.io/utils/net"
30+
31+
"github.com/oracle/oci-cloud-controller-manager/pkg/cloudprovider/providers/oci/config"
32+
"github.com/oracle/oci-cloud-controller-manager/pkg/oci/client"
33+
"github.com/oracle/oci-cloud-controller-manager/pkg/util"
3134
)
3235

36+
const (
37+
OkeSystemTagNamesapce = "orcl-containerengine"
38+
// MaxDefinedTagPerResource is the maximum number of defined tags that be can be associated with the resource
39+
//https://docs.oracle.com/en-us/iaas/Content/Tagging/Concepts/taggingoverview.htm#limits
40+
MaxDefinedTagPerResource = 64
41+
resourceTrackingFeatureFlagName = "CPO_ENABLE_RESOURCE_ATTRIBUTION"
42+
)
43+
44+
var MaxDefinedTagErrMessage = "max limit of defined tags for %s is reached. skip adding tags. sending metric"
45+
3346
// Protects Load Balancers against multiple updates in parallel
3447
type loadBalancerLocks struct {
3548
locks sets.String
@@ -152,3 +165,22 @@ func GetIsFeatureEnabledFromEnv(logger *zap.SugaredLogger, featureName string, d
152165
}
153166
return enableFeature
154167
}
168+
169+
// getResourceTrackingSystemTagsFromConfig reads resource tracking tags from config
170+
// which are specified under common tags
171+
func getResourceTrackingSystemTagsFromConfig(logger *zap.SugaredLogger, initialTags *config.InitialTags) (resourceTrackingTags map[string]map[string]interface{}) {
172+
resourceTrackingTags = make(map[string]map[string]interface{})
173+
// TODO: Fix the double negative
174+
if !(util.IsCommonTagPresent(initialTags) && initialTags.Common.DefinedTags != nil) {
175+
logger.Warn("oke resource tracking system tags are not present in cloud-config.yaml")
176+
return nil
177+
}
178+
179+
if tag, exists := initialTags.Common.DefinedTags[OkeSystemTagNamesapce]; exists {
180+
resourceTrackingTags[OkeSystemTagNamesapce] = tag
181+
return
182+
}
183+
184+
logger.Warn("tag config doesn't consist resource tracking tags")
185+
return nil
186+
}

‎pkg/metrics/constants.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,8 @@ const (
4848
PVExpand = "PV_EXPAND"
4949
// PVClone is the OCI metric for PV Clone
5050
PVClone = "PV_CLONE"
51+
// PVUpdate is the OCI metric for PV Update
52+
PVUpdate = "PV_UPDATE"
5153

5254
// FSSProvision is the OCI metric suffix for FSS provision
5355
FSSProvision = "FSS_PROVISION"

‎test/e2e/framework/cloud_provider_framework.go

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -21,20 +21,11 @@ import (
2121
"strings"
2222
"time"
2323

24-
ocicore "github.com/oracle/oci-go-sdk/v65/core"
25-
2624
snapclientset "github.com/kubernetes-csi/external-snapshotter/client/v6/clientset/versioned"
2725
. "github.com/onsi/ginkgo"
2826
. "github.com/onsi/gomega"
29-
"github.com/oracle/oci-cloud-controller-manager/pkg/cloudprovider/providers/oci" // register oci cloud provider
30-
providercfg "github.com/oracle/oci-cloud-controller-manager/pkg/cloudprovider/providers/oci/config"
31-
"github.com/oracle/oci-cloud-controller-manager/pkg/oci/client"
32-
"github.com/oracle/oci-go-sdk/v65/common"
33-
"github.com/oracle/oci-go-sdk/v65/containerengine"
34-
"github.com/oracle/oci-go-sdk/v65/core"
3527
"github.com/pkg/errors"
3628
"go.uber.org/zap"
37-
3829
v1 "k8s.io/api/core/v1"
3930
crdclientset "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset"
4031
apierrors "k8s.io/apimachinery/pkg/api/errors"
@@ -46,6 +37,14 @@ import (
4637
"k8s.io/client-go/tools/cache"
4738
"k8s.io/client-go/tools/clientcmd"
4839
cloudprovider "k8s.io/cloud-provider"
40+
41+
"github.com/oracle/oci-cloud-controller-manager/pkg/cloudprovider/providers/oci" // register oci cloud provider
42+
providercfg "github.com/oracle/oci-cloud-controller-manager/pkg/cloudprovider/providers/oci/config"
43+
"github.com/oracle/oci-cloud-controller-manager/pkg/oci/client"
44+
"github.com/oracle/oci-go-sdk/v65/common"
45+
"github.com/oracle/oci-go-sdk/v65/containerengine"
46+
"github.com/oracle/oci-go-sdk/v65/core"
47+
ocicore "github.com/oracle/oci-go-sdk/v65/core"
4948
)
5049

5150
// CloudProviderFramework is used in the execution of e2e tests.
@@ -371,7 +370,7 @@ func createOCIClient(cloudProviderConfig *providercfg.Config) (client.Interface,
371370
ociClientConfig := common.NewRawConfigurationProvider(cpc.TenancyID, cpc.UserID, cpc.Region, cpc.Fingerprint, cpc.PrivateKey, &cpc.PrivateKeyPassphrase)
372371
logger := zap.L()
373372
rateLimiter := client.NewRateLimiter(logger.Sugar(), cloudProviderConfig.RateLimiter)
374-
ociClient, err := client.New(logger.Sugar(), ociClientConfig, &rateLimiter)
373+
ociClient, err := client.New(logger.Sugar(), ociClientConfig, &rateLimiter, cloudProviderConfig.Auth.TenancyID)
375374
if err != nil {
376375
return nil, errors.Wrapf(err, "Couldn't create oci client from configuration: %s.", cloudConfigFile)
377376
}

‎test/e2e/framework/pvc_util.go

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1748,7 +1748,7 @@ func (j *PVCTestJig) newSecretTemplate(secretName, namespace, saName string) *v1
17481748
return secret
17491749
}
17501750

1751-
func (j *PVCTestJig) CreateSecret(secretName , saName , saNamespace string) error {
1751+
func (j *PVCTestJig) CreateSecret(secretName, saName, saNamespace string) error {
17521752

17531753
secret := j.newSecretTemplate(secretName, saNamespace, saName)
17541754

@@ -1759,3 +1759,15 @@ func (j *PVCTestJig) CreateSecret(secretName , saName , saNamespace string) erro
17591759
fmt.Printf("Secret %s created in namespace %s\n", secretName, saNamespace)
17601760
return nil
17611761
}
1762+
1763+
func (j *PVCTestJig) GetOcidFromPV(pv v1.PersistentVolume) string {
1764+
pvSource := pv.Spec.PersistentVolumeSource
1765+
1766+
if pvSource.CSI != nil {
1767+
return pvSource.CSI.VolumeHandle
1768+
}
1769+
if pvSource.FlexVolume != nil {
1770+
return pv.Name
1771+
}
1772+
return ""
1773+
}

0 commit comments

Comments
 (0)