Storage Backfill Controller to backfill existing storage resource with OKE system tags

Author: GouthamML

pkg/cloudprovider/providers/oci/ccm.go

@@ -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)
+	c, err := client.New(logger.Sugar(), cp, &rateLimiter, config.Auth.TenancyID)
 	if err != nil {
 		return nil, err
 	}
@@ -165,6 +165,7 @@ func init() {
 // Initialize passes a Kubernetes clientBuilder interface to the cloud provider.
 func (cp *CloudProvider) Initialize(clientBuilder cloudprovider.ControllerClientBuilder, stop <-chan struct{}) {
 	var err error
+	//var sbcStopChannel = make(chan struct{})
 	cp.kubeclient, err = clientBuilder.Client("cloud-controller-manager")
 	if err != nil {
 		utilruntime.HandleError(fmt.Errorf("failed to create kubeclient: %v", err))
@@ -199,6 +200,22 @@ func (cp *CloudProvider) Initialize(clientBuilder cloudprovider.ControllerClient
 
 	cp.ServiceAccountLister = serviceAccountInformer.Lister()
 
+	// StorageBackfillController not applicable for Open Source CCM
+	//enableStorageBackfillController := GetIsFeatureEnabledFromEnv(cp.logger, resourceTrackingFeatureFlagName, false)
+	//if enableStorageBackfillController {
+	//	cp.logger.Info("Starting storage backfill controller")
+	//
+	//	storageBackfillController := NewStorageBackfillController(
+	//		cp.kubeclient,
+	//		cp.client,
+	//		cp.logger,
+	//		cp.metricPusher,
+	//		cp.config,
+	//		pvInformer.Lister(),
+	//	)
+	//	go storageBackfillController.Run(sbcStopChannel)
+	//}
+
 	cp.securityListManagerFactory = func(mode string) securityListManager {
 		if cp.config.LoadBalancer.Disabled {
 			return newSecurityListManagerNOOP()

pkg/cloudprovider/providers/oci/instances_test.go

@@ -17,6 +17,7 @@ package oci
 import (
 	"context"
 	"errors"
+	"net/http"
 	"reflect"
 	"testing"
 
@@ -1200,7 +1201,19 @@ func (MockBlockStorageClient) CreateVolume(ctx context.Context, details core.Cre
 	return nil, nil
 }
 
+var updateVolumeErrors = map[string]error{
+	"work-request-fails": errors.New("UpdateVolume request failed: internal server error"),
+	"api-returns-too-many-requests": mockServiceError{
+		StatusCode: http.StatusTooManyRequests,
+		Code:       client.HTTP429TooManyRequestsCode,
+		Message:    "Too many requests",
+	},
+}
+
 func (c MockBlockStorageClient) UpdateVolume(ctx context.Context, volumeId string, details core.UpdateVolumeDetails) (*core.Volume, error) {
+	if err, ok := updateVolumeErrors[volumeId]; ok {
+		return nil, err
+	}
 	return nil, nil
 }
 
@@ -1892,3 +1905,29 @@ func (s *mockEndpointSliceNamespaceLister) Get(name string) (ret *v1discovery.En
 	}
 	return nil, errors.New("get endpointSlice error")
 }
+
+type mockServiceError struct {
+	StatusCode   int
+	Code         string
+	Message      string
+	OpcRequestID string
+}
+
+func (m mockServiceError) GetHTTPStatusCode() int {
+	return m.StatusCode
+}
+
+func (m mockServiceError) GetMessage() string {
+	return m.Message
+}
+
+func (m mockServiceError) GetCode() string {
+	return m.Code
+}
+
+func (m mockServiceError) GetOpcRequestID() string {
+	return m.OpcRequestID
+}
+func (m mockServiceError) Error() string {
+	return m.Message
+}

pkg/cloudprovider/providers/oci/load_balancer.go

@@ -71,15 +71,6 @@ const DefaultNetworkLoadBalancerListenerProtocol = "TCP"
 // https://docs.oracle.com/en-us/iaas/Content/General/Concepts/servicelimits.htm#nsg_limits
 const MaxNsgPerVnic = 5
 
-const (
-	OkeSystemTagNamesapce = "orcl-containerengine"
-	// MaxDefinedTagPerLB is the maximum number of defined tags that be can be associated with the resource
-	//https://docs.oracle.com/en-us/iaas/Content/Tagging/Concepts/taggingoverview.htm#limits
-	MaxDefinedTagPerLB              = 64
-	resourceTrackingFeatureFlagName = "CPO_ENABLE_RESOURCE_ATTRIBUTION"
-)
-
-var MaxDefinedTagPerLBErr = fmt.Errorf("max limit of defined tags for lb is reached. skip adding tags. sending metric")
 var enableOkeSystemTags = false
 
 const (
@@ -416,7 +407,7 @@ func (clb *CloudLoadBalancerProvider) createLoadBalancer(ctx context.Context, sp
 		IpVersion:               spec.IpVersions.LbEndpointIpVersion,
 	}
 	// do not block creation if the defined tag limit is reached. defer LB to tracked by backfilling
-	if len(details.DefinedTags) > MaxDefinedTagPerLB {
+	if len(details.DefinedTags) > MaxDefinedTagPerResource {
 		logger.Warnf("the number of defined tags in the LB create request is beyond the limit. removing the resource tracking tags from the details")
 		delete(details.DefinedTags, OkeSystemTagNamesapce)
 	}
@@ -1140,7 +1131,7 @@ func (clb *CloudLoadBalancerProvider) updateLoadBalancer(ctx context.Context, lb
 			// fail open if the update request fails
 			logger.With(zap.Error(err)).Warn("updateLoadBalancer didn't succeed. unable to add oke system tags")
 			errType = util.SystemTagErrTypePrefix + util.GetError(err)
-			if errors.Is(err, MaxDefinedTagPerLBErr) {
+			if errors.Is(err, fmt.Errorf(MaxDefinedTagErrMessage, spec.Type)) {
 				errType = util.ErrTagLimitReached
 			}
 			dimensionsMap[metrics.ComponentDimension] = util.GetMetricDimensionForComponent(errType, util.LoadBalancerType)
@@ -1920,8 +1911,8 @@ func (clb *CloudLoadBalancerProvider) addLoadBalancerOkeSystemTags(ctx context.C
 	lbDefinedTagsRequest[OkeSystemTagNamesapce] = spec.SystemTags[OkeSystemTagNamesapce]
 
 	// update fails if the number of defined tags is more than the service limit i.e 64
-	if len(lbDefinedTagsRequest) > MaxDefinedTagPerLB {
-		return MaxDefinedTagPerLBErr
+	if len(lbDefinedTagsRequest) > MaxDefinedTagPerResource {
+		return fmt.Errorf(MaxDefinedTagErrMessage, spec.Type)
 	}
 
 	lbUpdateDetails := &client.GenericUpdateLoadBalancerDetails{

pkg/cloudprovider/providers/oci/load_balancer_spec.go

@@ -462,7 +462,7 @@ func NewLBSpec(logger *zap.SugaredLogger, svc *v1.Service, provisionedNodes []*v
 		IpVersions:                  ipVersions,
 		FreeformTags:                lbTags.FreeformTags,
 		DefinedTags:                 lbTags.DefinedTags,
-		SystemTags:                  getResourceTrackingSysTagsFromConfig(logger, initialLBTags),
+		SystemTags:                  getResourceTrackingSystemTagsFromConfig(logger, initialLBTags),
 		ingressIpMode:               ingressIpMode,
 	}, nil
 }
@@ -1530,25 +1530,6 @@ func updateSpecWithLbSubnets(spec *LBSpec, lbSubnetId []string) (*LBSpec, error)
 	return spec, nil
 }
 
-// getResourceTrackingSysTagsFromConfig reads resource tracking tags from config
-// which are specified under common tags
-func getResourceTrackingSysTagsFromConfig(logger *zap.SugaredLogger, initialTags *config.InitialTags) (resourceTrackingTags map[string]map[string]interface{}) {
-	resourceTrackingTags = make(map[string]map[string]interface{})
-	// TODO: Fix the double negative
-	if !(util.IsCommonTagPresent(initialTags) && initialTags.Common.DefinedTags != nil) {
-		logger.Error("oke resource tracking system tags are not present in cloud-config.yaml")
-		return nil
-	}
-
-	if tag, exists := initialTags.Common.DefinedTags[OkeSystemTagNamesapce]; exists {
-		resourceTrackingTags[OkeSystemTagNamesapce] = tag
-		return
-	}
-
-	logger.Error("tag config doesn't consist resource tracking tags")
-	return nil
-}
-
 // getIpFamilies gets ip families based on the field set in the spec
 func getIpFamilies(svc *v1.Service) []string {
 	ipFamilies := []string{}

pkg/cloudprovider/providers/oci/load_balancer_spec_test.go

@@ -10064,7 +10064,7 @@ func Test_getResourceTrackingSysTagsFromConfig(t *testing.T) {
 	}
 	for name, test := range tests {
 		t.Run(name, func(t *testing.T) {
-			tag := getResourceTrackingSysTagsFromConfig(zap.S(), test.initialTags)
+			tag := getResourceTrackingSystemTagsFromConfig(zap.S(), test.initialTags)
 			t.Logf("%#v", tag)
 			if !reflect.DeepEqual(test.wantTag, tag) {
 				t.Errorf("wanted %v but got %v", test.wantTag, tag)

pkg/cloudprovider/providers/oci/load_balancer_test.go

@@ -1192,6 +1192,7 @@ func Test_addLoadBalancerOkeSystemTags(t *testing.T) {
 				DefinedTags: make(map[string]map[string]interface{}),
 			},
 			spec: &LBSpec{
+				Type:       LB,
 				SystemTags: map[string]map[string]interface{}{"orcl-containerengine": {"Cluster": "val"}},
 				service:    &v1.Service{},
 			},

pkg/cloudprovider/providers/oci/util.go

@@ -20,16 +20,29 @@ import (
 	"strings"
 	"sync"
 
-	"github.com/oracle/oci-cloud-controller-manager/pkg/oci/client"
 	"github.com/pkg/errors"
 	"go.uber.org/zap"
 	api "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/util/sets"
 	listersv1 "k8s.io/client-go/listers/core/v1"
 	"k8s.io/utils/net"
+
+	"github.com/oracle/oci-cloud-controller-manager/pkg/cloudprovider/providers/oci/config"
+	"github.com/oracle/oci-cloud-controller-manager/pkg/oci/client"
+	"github.com/oracle/oci-cloud-controller-manager/pkg/util"
 )
 
+const (
+	OkeSystemTagNamesapce = "orcl-containerengine"
+	// MaxDefinedTagPerResource is the maximum number of defined tags that be can be associated with the resource
+	//https://docs.oracle.com/en-us/iaas/Content/Tagging/Concepts/taggingoverview.htm#limits
+	MaxDefinedTagPerResource        = 64
+	resourceTrackingFeatureFlagName = "CPO_ENABLE_RESOURCE_ATTRIBUTION"
+)
+
+var MaxDefinedTagErrMessage = "max limit of defined tags for %s is reached. skip adding tags. sending metric"
+
 // Protects Load Balancers against multiple updates in parallel
 type loadBalancerLocks struct {
 	locks sets.String
@@ -152,3 +165,22 @@ func GetIsFeatureEnabledFromEnv(logger *zap.SugaredLogger, featureName string, d
 	}
 	return enableFeature
 }
+
+// getResourceTrackingSystemTagsFromConfig reads resource tracking tags from config
+// which are specified under common tags
+func getResourceTrackingSystemTagsFromConfig(logger *zap.SugaredLogger, initialTags *config.InitialTags) (resourceTrackingTags map[string]map[string]interface{}) {
+	resourceTrackingTags = make(map[string]map[string]interface{})
+	// TODO: Fix the double negative
+	if !(util.IsCommonTagPresent(initialTags) && initialTags.Common.DefinedTags != nil) {
+		logger.Warn("oke resource tracking system tags are not present in cloud-config.yaml")
+		return nil
+	}
+
+	if tag, exists := initialTags.Common.DefinedTags[OkeSystemTagNamesapce]; exists {
+		resourceTrackingTags[OkeSystemTagNamesapce] = tag
+		return
+	}
+
+	logger.Warn("tag config doesn't consist resource tracking tags")
+	return nil
+}

pkg/metrics/constants.go

@@ -48,6 +48,8 @@ const (
 	PVExpand = "PV_EXPAND"
 	// PVClone is the OCI metric for PV Clone
 	PVClone = "PV_CLONE"
+	// PVUpdate is the OCI metric for PV Update
+	PVUpdate = "PV_UPDATE"
 
 	// FSSProvision is the OCI metric suffix for FSS provision
 	FSSProvision = "FSS_PROVISION"

test/e2e/framework/cloud_provider_framework.go

@@ -21,20 +21,11 @@ import (
 	"strings"
 	"time"
 
-	ocicore "github.com/oracle/oci-go-sdk/v65/core"
-
 	snapclientset "github.com/kubernetes-csi/external-snapshotter/client/v6/clientset/versioned"
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
-	"github.com/oracle/oci-cloud-controller-manager/pkg/cloudprovider/providers/oci" // register oci cloud provider
-	providercfg "github.com/oracle/oci-cloud-controller-manager/pkg/cloudprovider/providers/oci/config"
-	"github.com/oracle/oci-cloud-controller-manager/pkg/oci/client"
-	"github.com/oracle/oci-go-sdk/v65/common"
-	"github.com/oracle/oci-go-sdk/v65/containerengine"
-	"github.com/oracle/oci-go-sdk/v65/core"
 	"github.com/pkg/errors"
 	"go.uber.org/zap"
-
 	v1 "k8s.io/api/core/v1"
 	crdclientset "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
@@ -46,6 +37,14 @@ import (
 	"k8s.io/client-go/tools/cache"
 	"k8s.io/client-go/tools/clientcmd"
 	cloudprovider "k8s.io/cloud-provider"
+
+	"github.com/oracle/oci-cloud-controller-manager/pkg/cloudprovider/providers/oci" // register oci cloud provider
+	providercfg "github.com/oracle/oci-cloud-controller-manager/pkg/cloudprovider/providers/oci/config"
+	"github.com/oracle/oci-cloud-controller-manager/pkg/oci/client"
+	"github.com/oracle/oci-go-sdk/v65/common"
+	"github.com/oracle/oci-go-sdk/v65/containerengine"
+	"github.com/oracle/oci-go-sdk/v65/core"
+	ocicore "github.com/oracle/oci-go-sdk/v65/core"
 )
 
 // CloudProviderFramework is used in the execution of e2e tests.
@@ -371,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)
+	ociClient, err := client.New(logger.Sugar(), ociClientConfig, &rateLimiter, cloudProviderConfig.Auth.TenancyID)
 	if err != nil {
 		return nil, errors.Wrapf(err, "Couldn't create oci client from configuration: %s.", cloudConfigFile)
 	}

test/e2e/framework/pvc_util.go

@@ -1748,7 +1748,7 @@ func (j *PVCTestJig) newSecretTemplate(secretName, namespace, saName string) *v1
 	return secret
 }
 
-func (j *PVCTestJig) CreateSecret(secretName , saName , saNamespace string) error {
+func (j *PVCTestJig) CreateSecret(secretName, saName, saNamespace string) error {
 
 	secret := j.newSecretTemplate(secretName, saNamespace, saName)
 
@@ -1759,3 +1759,15 @@ func (j *PVCTestJig) CreateSecret(secretName , saName , saNamespace string) erro
 	fmt.Printf("Secret %s created in namespace %s\n", secretName, saNamespace)
 	return nil
 }
+
+func (j *PVCTestJig) GetOcidFromPV(pv v1.PersistentVolume) string {
+	pvSource := pv.Spec.PersistentVolumeSource
+
+	if pvSource.CSI != nil {
+		return pvSource.CSI.VolumeHandle
+	}
+	if pvSource.FlexVolume != nil {
+		return pv.Name
+	}
+	return ""
+}