System tag support in CPO. Backfilling existing LB & NLB

Author: GouthamML

pkg/cloudprovider/providers/oci/instances_test.go

@@ -497,6 +497,18 @@ func (c *MockLoadBalancerClient) DeleteListener(ctx context.Context, lbID, name
 	return "", nil
 }
 
+var updateLoadBalancerErrors = map[string]error{
+	"work request fail": errors.New("internal server error"),
+}
+
+func (c *MockLoadBalancerClient) UpdateLoadBalancer(ctx context.Context, lbID string, details *client.GenericUpdateLoadBalancerDetails) (string, error) {
+	if err, ok := updateLoadBalancerErrors[lbID]; ok {
+		return "", err
+	}
+
+	return "", nil
+}
+
 var awaitLoadbalancerWorkrequestMap = map[string]error{
 	"failedToGetUpdateNetworkSecurityGroupsWorkRequest": errors.New("internal server error for get workrequest call"),
 }
@@ -612,6 +624,10 @@ func (c *MockNetworkLoadBalancerClient) UpdateNetworkSecurityGroups(ctx context.
 	return "", nil
 }
 
+func (c *MockNetworkLoadBalancerClient) UpdateLoadBalancer(ctx context.Context, lbID string, details *client.GenericUpdateLoadBalancerDetails) (string, error) {
+	return "", nil
+}
+
 // MockBlockStorageClient mocks BlockStorage client implementation
 type MockBlockStorageClient struct{}
 

pkg/cloudprovider/providers/oci/load_balancer.go

@@ -68,6 +68,17 @@ 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 (
 	// Fallback value if annotation on service is not set
 	lbDefaultShape = "100Mbps"
@@ -373,6 +384,11 @@ func (clb *CloudLoadBalancerProvider) createLoadBalancer(ctx context.Context, sp
 		FreeformTags:            spec.FreeformTags,
 		DefinedTags:             spec.DefinedTags,
 	}
+	// do not block creation if the defined tag limit is reached. defer LB to tracked by backfilling
+	if len(details.DefinedTags) > MaxDefinedTagPerLB {
+		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)
+	}
 
 	if spec.Shape == flexible {
 		details.ShapeDetails = &client.GenericShapeDetails{
@@ -714,6 +730,19 @@ func (cp *CloudProvider) EnsureLoadBalancer(ctx context.Context, clusterName str
 
 	if !lbExists {
 		lbStatus, newLBOCID, err := lbProvider.createLoadBalancer(ctx, spec)
+		if err != nil && client.IsSystemTagNotFoundOrNotAuthorisedError(logger, err) {
+			logger.Warn("LB creation failed due to error in adding system tags. sending metric & retrying without system tags")
+
+			// send resource track tagging failure metrics
+			errorType = util.SystemTagErrTypePrefix + util.GetError(err)
+			lbMetricDimension = util.GetMetricDimensionForComponent(errorType, util.LoadBalancerType)
+			dimensionsMap[metrics.ComponentDimension] = lbMetricDimension
+			metrics.SendMetricData(cp.metricPusher, getMetric(loadBalancerType, Create), time.Since(startTime).Seconds(), dimensionsMap)
+
+			// retry create without resource tracking system tags
+			delete(spec.DefinedTags, OkeSystemTagNamesapce)
+			lbStatus, newLBOCID, err = lbProvider.createLoadBalancer(ctx, spec)
+		}
 		if err != nil {
 			logger.With(zap.Error(err)).Error("Failed to provision LoadBalancer")
 			errorType = util.GetError(err)
@@ -884,7 +913,7 @@ func (cp *CloudProvider) getLoadBalancerSubnets(ctx context.Context, logger *zap
 
 func (clb *CloudLoadBalancerProvider) updateLoadBalancer(ctx context.Context, lb *client.GenericLoadBalancer, spec *LBSpec) error {
 	lbID := *lb.Id
-
+	start := time.Now()
 	logger := clb.logger.With("loadBalancerID", lbID, "compartmentID", clb.config.CompartmentID, "loadBalancerType", getLoadBalancerType(spec.service), "serviceName", spec.service.Name)
 
 	var actualPublicReservedIP *string
@@ -983,6 +1012,24 @@ func (clb *CloudLoadBalancerProvider) updateLoadBalancer(ctx context.Context, lb
 			return errors.Errorf("The Load Balancer service reserved IP cannot be updated after the Load Balancer is created.")
 		}
 	}
+
+	dimensionsMap := make(map[string]string)
+	var errType string
+	if enableOkeSystemTags && !doesLbHaveOkeSystemTags(lb, spec) {
+		logger.Info("detected loadbalancer without oke system tags. proceeding to add")
+		err = clb.addLoadBalancerOkeSystemTags(ctx, lb, spec)
+		if err != nil {
+			// 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) {
+				errType = util.ErrTagLimitReached
+			}
+			dimensionsMap[metrics.ComponentDimension] = util.GetMetricDimensionForComponent(errType, util.LoadBalancerType)
+			dimensionsMap[metrics.ResourceOCIDDimension] = *lb.Id
+			metrics.SendMetricData(clb.metricPusher, getMetric(spec.Type, Update), time.Since(start).Seconds(), dimensionsMap)
+		}
+	}
 	return nil
 }
 
@@ -1648,6 +1695,56 @@ func (clb *CloudLoadBalancerProvider) updateLoadBalancerNetworkSecurityGroups(ct
 	return nil
 }
 
+func doesLbHaveOkeSystemTags(lb *client.GenericLoadBalancer, spec *LBSpec) bool {
+	if lb.SystemTags == nil || spec.SystemTags == nil {
+		return false
+	}
+	if okeSystemTag, okeSystemTagNsExists := lb.SystemTags[OkeSystemTagNamesapce]; okeSystemTagNsExists {
+		return reflect.DeepEqual(okeSystemTag, spec.SystemTags[OkeSystemTagNamesapce])
+	}
+	return false
+}
+func (clb *CloudLoadBalancerProvider) addLoadBalancerOkeSystemTags(ctx context.Context, lb *client.GenericLoadBalancer, spec *LBSpec) error {
+	lbDefinedTagsRequest := make(map[string]map[string]interface{})
+
+	if spec.SystemTags == nil {
+		return fmt.Errorf("oke system tag is not found in LB spec. ignoring..")
+	}
+	if _, exists := spec.SystemTags[OkeSystemTagNamesapce]; !exists {
+		return fmt.Errorf("oke system tag namespace is not found in LB spec")
+	}
+
+	if lb.DefinedTags != nil {
+		lbDefinedTagsRequest = lb.DefinedTags
+	}
+
+	// no overwriting customer tags as customer can not have a tag namespace with prefix 'orcl-'
+	// system tags are passed as defined tags in the request
+	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
+	}
+
+	lbUpdateDetails := &client.GenericUpdateLoadBalancerDetails{
+		FreeformTags: lb.FreeformTags,
+		DefinedTags:  lbDefinedTagsRequest,
+	}
+	wrID, err := clb.lbClient.UpdateLoadBalancer(ctx, *lb.Id, lbUpdateDetails)
+	if err != nil {
+		return errors.Wrap(err, "UpdateLoadBalancer request failed")
+	}
+	_, err = clb.lbClient.AwaitWorkRequest(ctx, wrID)
+	if err != nil {
+		return errors.Wrap(err, "failed to await updateloadbalancer work request")
+	}
+
+	logger := clb.logger.With("opc-workrequest-id", wrID, "loadBalancerID", lb.Id)
+	logger.Info("UpdateLoadBalancer request to add oke system tags completed successfully")
+	return nil
+}
+
 // Given an OCI load balancer, return a LoadBalancerStatus
 func loadBalancerToStatus(lb *client.GenericLoadBalancer) (*v1.LoadBalancerStatus, error) {
 	if len(lb.IpAddresses) == 0 {

pkg/cloudprovider/providers/oci/load_balancer_spec.go

@@ -17,7 +17,6 @@ package oci
 import (
 	"encoding/json"
 	"fmt"
-	"github.com/oracle/oci-cloud-controller-manager/pkg/util"
 	"net"
 	"net/http"
 	"strconv"
@@ -30,6 +29,7 @@ import (
 
 	"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"
 	"github.com/oracle/oci-go-sdk/v65/common"
 	"github.com/pkg/errors"
 	helper "k8s.io/cloud-provider/service/helpers"
@@ -318,6 +318,7 @@ type LBSpec struct {
 	NetworkSecurityGroupIds     []string
 	FreeformTags                map[string]string
 	DefinedTags                 map[string]map[string]interface{}
+	SystemTags                  map[string]map[string]interface{}
 
 	service *v1.Service
 	nodes   []*v1.Node
@@ -379,7 +380,7 @@ func NewLBSpec(logger *zap.SugaredLogger, svc *v1.Service, nodes []*v1.Node, sub
 		return nil, err
 	}
 	// merge lbtags with common tags if present
-	if util.IsCommonTagPresent(initialLBTags) {
+	if enableOkeSystemTags && util.IsCommonTagPresent(initialLBTags) {
 		lbTags = util.MergeTagConfig(lbTags, initialLBTags.Common)
 	}
 
@@ -421,6 +422,7 @@ func NewLBSpec(logger *zap.SugaredLogger, svc *v1.Service, nodes []*v1.Node, sub
 		securityListManager:         secListFactory(ruleManagementMode),
 		FreeformTags:                lbTags.FreeformTags,
 		DefinedTags:                 lbTags.DefinedTags,
+		SystemTags:                  getResourceTrackingSysTagsFromConfig(logger, initialLBTags),
 	}, nil
 }
 
@@ -1351,3 +1353,22 @@ 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
+}