diff --git a/pkg/cloudprovider/providers/oci/instances_test.go b/pkg/cloudprovider/providers/oci/instances_test.go index 8c178ab4b5..2e5f62fefa 100644 --- a/pkg/cloudprovider/providers/oci/instances_test.go +++ b/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{} diff --git a/pkg/cloudprovider/providers/oci/load_balancer.go b/pkg/cloudprovider/providers/oci/load_balancer.go index 36ad93bf49..d4e8dfbf27 100644 --- a/pkg/cloudprovider/providers/oci/load_balancer.go +++ b/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 { diff --git a/pkg/cloudprovider/providers/oci/load_balancer_spec.go b/pkg/cloudprovider/providers/oci/load_balancer_spec.go index 53c5e739c5..4ad4f5a697 100644 --- a/pkg/cloudprovider/providers/oci/load_balancer_spec.go +++ b/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 +} diff --git a/pkg/cloudprovider/providers/oci/load_balancer_spec_test.go b/pkg/cloudprovider/providers/oci/load_balancer_spec_test.go index 083a0e2dd6..fcd01316f6 100644 --- a/pkg/cloudprovider/providers/oci/load_balancer_spec_test.go +++ b/pkg/cloudprovider/providers/oci/load_balancer_spec_test.go @@ -65,6 +65,7 @@ func (ssr mockSSLSecretReader) readSSLSecret(ns, name string) (sslSecret *certif } func TestNewLBSpecSuccess(t *testing.T) { + enableOkeSystemTags = true testCases := map[string]struct { defaultSubnetOne string defaultSubnetTwo string @@ -2126,6 +2127,7 @@ func TestNewLBSpecSuccess(t *testing.T) { } func TestNewLBSpecForTags(t *testing.T) { + enableOkeSystemTags = true tests := map[string]struct { defaultSubnetOne string defaultSubnetTwo string @@ -2135,6 +2137,7 @@ func TestNewLBSpecForTags(t *testing.T) { sslConfig *SSLConfig expected *LBSpec clusterTags *providercfg.InitialTags + featureEnabled bool }{ "no resource & cluster level tags but common tags from config": { defaultSubnetOne: "one", @@ -2207,6 +2210,7 @@ func TestNewLBSpecForTags(t *testing.T) { FreeformTags: map[string]string{}, DefinedTags: map[string]map[string]interface{}{"namespace": {"cluster": "CommonCluster", "owner": "CommonClusterOwner"}}, }, + featureEnabled: true, }, "no resource or cluster level tags and no common tags": { defaultSubnetOne: "one", @@ -2270,6 +2274,7 @@ func TestNewLBSpecForTags(t *testing.T) { securityListManager: newSecurityListManagerNOOP(), ManagedNetworkSecurityGroup: &ManagedNetworkSecurityGroup{frontendNsgId: "", backendNsgId: []string{}, nsgRuleManagementMode: ManagementModeNone}, }, + featureEnabled: true, }, "resource level tags with common tags from config": { defaultSubnetOne: "one", @@ -2345,6 +2350,7 @@ func TestNewLBSpecForTags(t *testing.T) { FreeformTags: map[string]string{"cluster": "resource", "unique": "tag", "name": "development_cluster"}, DefinedTags: map[string]map[string]interface{}{"namespace": {"owner": "team", "key": "value"}, "namespace2": {"owner2": "team2", "key2": "value2"}}, }, + featureEnabled: true, }, "resource level defined tags and common defined tags from config with same key": { defaultSubnetOne: "one", @@ -2419,6 +2425,7 @@ func TestNewLBSpecForTags(t *testing.T) { FreeformTags: map[string]string{"cluster": "resource", "unique": "tag", "name": "development_cluster"}, DefinedTags: map[string]map[string]interface{}{"namespace": {"owner2": "team2", "key2": "value2"}}, }, + featureEnabled: true, }, "cluster level tags and common tags": { defaultSubnetOne: "one", @@ -2493,6 +2500,7 @@ func TestNewLBSpecForTags(t *testing.T) { FreeformTags: map[string]string{"lbname": "development_cluster_loadbalancer", "name": "development_cluster"}, DefinedTags: map[string]map[string]interface{}{"namespace": {"owner": "team", "key": "value"}, "namespace2": {"owner2": "team2", "key2": "value2"}}, }, + featureEnabled: true, }, "cluster level defined tags and common defined tags with same key": { defaultSubnetOne: "one", @@ -2567,6 +2575,7 @@ func TestNewLBSpecForTags(t *testing.T) { FreeformTags: map[string]string{"lbname": "development_cluster_loadbalancer", "name": "development_cluster"}, DefinedTags: map[string]map[string]interface{}{"namespace": {"owner2": "team2", "key2": "value2"}}, }, + featureEnabled: true, }, "cluster level tags with no common tags": { defaultSubnetOne: "one", @@ -2637,6 +2646,7 @@ func TestNewLBSpecForTags(t *testing.T) { FreeformTags: map[string]string{"lbname": "development_cluster_loadbalancer"}, DefinedTags: map[string]map[string]interface{}{"namespace": {"owner": "team", "key": "value"}}, }, + featureEnabled: true, }, "no cluster or level tags but common tags from config": { defaultSubnetOne: "one", @@ -2707,6 +2717,76 @@ func TestNewLBSpecForTags(t *testing.T) { FreeformTags: map[string]string{"lbname": "development_cluster_loadbalancer"}, DefinedTags: map[string]map[string]interface{}{"namespace": {"owner": "team", "key": "value"}}, }, + featureEnabled: true, + }, + "when the feature is disabled": { + defaultSubnetOne: "one", + service: &v1.Service{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: "kube-system", + Name: "testservice", + UID: "test-uid", + }, + Spec: v1.ServiceSpec{ + SessionAffinity: v1.ServiceAffinityNone, + Ports: []v1.ServicePort{ + { + Protocol: v1.ProtocolTCP, + Port: int32(80), + }, + }, + }, + }, + clusterTags: &providercfg.InitialTags{ + Common: &providercfg.TagConfig{ + FreeformTags: map[string]string{"lbname": "development_cluster_loadbalancer"}, + DefinedTags: map[string]map[string]interface{}{"namespace": {"owner": "team", "key": "value"}}, + }, + }, + expected: &LBSpec{ + Name: "test-uid", + Type: "lb", + Shape: "100Mbps", + Internal: false, + Subnets: []string{"one"}, + Listeners: map[string]client.GenericListener{ + "TCP-80": { + Name: common.String("TCP-80"), + DefaultBackendSetName: common.String("TCP-80"), + Port: common.Int(80), + Protocol: common.String("TCP"), + }, + }, + BackendSets: map[string]client.GenericBackendSetDetails{ + "TCP-80": { + Backends: []client.GenericBackend{}, + HealthChecker: &client.GenericHealthChecker{ + Protocol: "HTTP", + IsForcePlainText: common.Bool(false), + Port: common.Int(10256), + UrlPath: common.String("/healthz"), + Retries: common.Int(3), + TimeoutInMillis: common.Int(3000), + IntervalInMillis: common.Int(10000), + ReturnCode: common.Int(http.StatusOK), + }, + IsPreserveSource: common.Bool(false), + Policy: common.String("ROUND_ROBIN"), + }, + }, + IsPreserveSource: common.Bool(false), + NetworkSecurityGroupIds: []string{}, + SourceCIDRs: []string{"0.0.0.0/0"}, + Ports: map[string]portSpec{ + "TCP-80": { + ListenerPort: 80, + HealthCheckerPort: 10256, + }, + }, + securityListManager: newSecurityListManagerNOOP(), + ManagedNetworkSecurityGroup: &ManagedNetworkSecurityGroup{frontendNsgId: "", backendNsgId: []string{}, nsgRuleManagementMode: ManagementModeNone}, + }, + featureEnabled: false, }, } cp := &CloudProvider{ @@ -2716,6 +2796,7 @@ func TestNewLBSpecForTags(t *testing.T) { for name, tc := range tests { logger := zap.L() + enableOkeSystemTags = tc.featureEnabled t.Run(name, func(t *testing.T) { // we expect the service to be unchanged tc.expected.service = tc.service @@ -6333,3 +6414,47 @@ func Test_getNetworkLoadbalancerSubnets(t *testing.T) { }) } } + +func Test_getResourceTrackingSysTagsFromConfig(t *testing.T) { + tests := map[string]struct { + initialTags *providercfg.InitialTags + wantTag map[string]map[string]interface{} + }{ + "expect an empty system tag when has no common tags": { + initialTags: &providercfg.InitialTags{}, + wantTag: nil, + }, + "expect an empty system tag when resource tracking tags are not in common tags": { + initialTags: &providercfg.InitialTags{ + LoadBalancer: &providercfg.TagConfig{ + DefinedTags: map[string]map[string]interface{}{"ns": {"key": "val"}}, + }, + Common: &providercfg.TagConfig{ + DefinedTags: map[string]map[string]interface{}{"orcl-not-a-tracking-tag": {"Cluster": "ocid1.cluster.aa..."}}, + }, + }, + wantTag: nil, + }, + "extract tracking system tag from config": { + initialTags: &providercfg.InitialTags{ + LoadBalancer: &providercfg.TagConfig{ + DefinedTags: map[string]map[string]interface{}{"ns": {"key": "val"}}, + }, + Common: &providercfg.TagConfig{ + FreeformTags: map[string]string{"Cluster": "ocid1.cluster.aa..."}, + DefinedTags: map[string]map[string]interface{}{"orcl-containerengine": {"Cluster": "ocid1.cluster.aa..."}}, + }, + }, + wantTag: map[string]map[string]interface{}{"orcl-containerengine": {"Cluster": "ocid1.cluster.aa..."}}, + }, + } + for name, test := range tests { + t.Run(name, func(t *testing.T) { + tag := getResourceTrackingSysTagsFromConfig(zap.S(), test.initialTags) + t.Logf("%#v", tag) + if !reflect.DeepEqual(test.wantTag, tag) { + t.Errorf("wanted %v but got %v", test.wantTag, tag) + } + }) + } +} diff --git a/pkg/cloudprovider/providers/oci/load_balancer_test.go b/pkg/cloudprovider/providers/oci/load_balancer_test.go index 7cd2ae53f6..9c70787c1a 100644 --- a/pkg/cloudprovider/providers/oci/load_balancer_test.go +++ b/pkg/cloudprovider/providers/oci/load_balancer_test.go @@ -19,6 +19,8 @@ import ( "errors" "fmt" "reflect" + "strconv" + "strings" "testing" "time" @@ -1059,6 +1061,122 @@ func TestCloudProvider_EnsureLoadBalancerDeleted(t *testing.T) { } } +func Test_addLoadBalancerOkeSystemTags(t *testing.T) { + tests := map[string]struct { + //config *providercfg.Config + lb *client.GenericLoadBalancer + spec *LBSpec + wantErr error + }{ + "expect an error when spec system tag is nil": { + lb: &client.GenericLoadBalancer{ + Id: common.String("ocid1.loadbalancer."), + }, + spec: &LBSpec{}, + wantErr: errors.New("oke system tag is not found in LB spec. ignoring.."), + }, + "expect an error when spec system tag is empty map": { + lb: &client.GenericLoadBalancer{ + Id: common.String("ocid1.loadbalancer."), + }, + spec: &LBSpec{ + SystemTags: map[string]map[string]interface{}{}, + }, + wantErr: errors.New("oke system tag namespace is not found in LB spec"), + }, + "expect an error when defined tags are limits are reached": { + lb: &client.GenericLoadBalancer{ + Id: common.String("defined tag limit of 64 reached"), + DefinedTags: make(map[string]map[string]interface{}), + }, + spec: &LBSpec{ + SystemTags: map[string]map[string]interface{}{"orcl-containerengine": {"Cluster": "val"}}, + }, + wantErr: errors.New("max limit of defined tags for lb is reached. skip adding tags. sending metric"), + }, + "expect an error when updateLoadBalancer work request fails": { + lb: &client.GenericLoadBalancer{ + Id: common.String("work request fail"), + FreeformTags: map[string]string{"key": "val"}, + DefinedTags: map[string]map[string]interface{}{"ns1": {"key1": "val1"}}, + }, + spec: &LBSpec{ + SystemTags: map[string]map[string]interface{}{"orcl-containerengine": {"Cluster": "val"}}, + }, + wantErr: errors.New("UpdateLoadBalancer request failed: internal server error"), + }, + } + + for name, testcase := range tests { + clb := &CloudLoadBalancerProvider{ + lbClient: &MockLoadBalancerClient{}, + logger: zap.S(), + //config: testcase.config, + } + t.Run(name, func(t *testing.T) { + if strings.Contains(name, "limit") { + // add 64 defined tags + for i := 1; i <= 64; i++ { + testcase.lb.DefinedTags["ns"+strconv.Itoa(i)] = map[string]interface{}{"key": strconv.Itoa(i)} + } + } + err := clb.addLoadBalancerOkeSystemTags(context.Background(), testcase.lb, testcase.spec) + t.Logf(err.Error()) + if !assertError(err, testcase.wantErr) { + t.Errorf("Expected error = %v, but got %v", testcase.wantErr, err) + return + } + }) + } +} + +func Test_doesLbHaveResourceTrackingSystemTags(t *testing.T) { + tests := map[string]struct { + lb *client.GenericLoadBalancer + spec *LBSpec + want bool + }{ + "base case": { + lb: &client.GenericLoadBalancer{ + DefinedTags: map[string]map[string]interface{}{"ns": {"key": "val"}}, + SystemTags: map[string]map[string]interface{}{"orcl-containerengine": {"Cluster": "val"}}, + }, + spec: &LBSpec{ + SystemTags: map[string]map[string]interface{}{"orcl-containerengine": {"Cluster": "val"}}, + }, + want: true, + }, + "system tag exists for different ns in lb": { + lb: &client.GenericLoadBalancer{ + DefinedTags: map[string]map[string]interface{}{"ns": {"key": "val"}}, + SystemTags: map[string]map[string]interface{}{"orcl-free-tier": {"Cluster": "val"}}, + }, + spec: &LBSpec{ + SystemTags: map[string]map[string]interface{}{"orcl-containerengine": {"Cluster": "val"}}, + }, + want: false, + }, + "resource tracking system tag doesnt exists in lb": { + lb: &client.GenericLoadBalancer{ + DefinedTags: map[string]map[string]interface{}{"ns": {"key": "val"}}, + }, + spec: &LBSpec{ + SystemTags: map[string]map[string]interface{}{"orcl-containerengine": {"Cluster": "val"}}, + }, + want: false, + }, + } + for name, test := range tests { + t.Run(name, func(t *testing.T) { + actual := doesLbHaveOkeSystemTags(test.lb, test.spec) + t.Logf("expected %v but got %v", test.want, actual) + if test.want != actual { + t.Errorf("expected %v but got %v", test.want, actual) + } + }) + } +} + func assertError(actual, expected error) bool { if expected == nil || actual == nil { return expected == actual diff --git a/pkg/csi-util/utils.go b/pkg/csi-util/utils.go index 4723d7db7c..f4290b1c17 100644 --- a/pkg/csi-util/utils.go +++ b/pkg/csi-util/utils.go @@ -452,3 +452,17 @@ func ValidateFssId(id string) *FSSVolumeHandler { } return volumeHandler } + +func GetIsFeatureEnabledFromEnv(logger *zap.SugaredLogger, featureName string, defaultValue bool) bool { + enableFeature := defaultValue + enableFeatureEnvVar, ok := os.LookupEnv(featureName) + if ok { + var err error + enableFeature, err = strconv.ParseBool(enableFeatureEnvVar) + if err != nil { + logger.With(zap.Error(err)).Errorf("failed to parse %s envvar, defaulting to %t", featureName, defaultValue) + return defaultValue + } + } + return enableFeature +} diff --git a/pkg/csi/driver/bv_controller.go b/pkg/csi/driver/bv_controller.go index c470927e4a..d927842327 100644 --- a/pkg/csi/driver/bv_controller.go +++ b/pkg/csi/driver/bv_controller.go @@ -65,7 +65,10 @@ const ( multipathEnabled = "multipathEnabled" multipathDevices = "multipathDevices" //device is the consistent device path that would be used for paravirtualized attachment - device = "device" + device = "device" + resourceTrackingFeatureFlagName = "CPO_ENABLE_RESOURCE_ATTRIBUTION" + OkeSystemTagNamesapce = "orcl-containerengine" + MaxDefinedTagPerVolume = 64 ) var ( @@ -77,6 +80,8 @@ var ( } ) +var enableOkeSystemTags = csi_util.GetIsFeatureEnabledFromEnv(zap.S(), resourceTrackingFeatureFlagName, false) + // VolumeParameters holds configuration type VolumeParameters struct { //kmsKey is the KMS key that would be used as CMEK key for BV attachment @@ -412,10 +417,23 @@ func (d *BlockVolumeControllerDriver) CreateVolume(ctx context.Context, req *csi return nil, status.Errorf(codes.InvalidArgument, "invalid available domain: %s or compartment ID: %s", availableDomainShortName, d.config.CompartmentID) } - bvTags := getBVTags(d.config.Tags, volumeParams) + bvTags := getBVTags(log, d.config.Tags, volumeParams) provisionedVolume, err = provision(ctx, log, d.client, volumeName, size, *ad.Name, d.config.CompartmentID, srcSnapshotId, srcVolumeId, volumeParams.diskEncryptionKey, volumeParams.vpusPerGB, bvTags) + + if err != nil && client.IsSystemTagNotFoundOrNotAuthorisedError(log, errors.Unwrap(err)) { + log.With("Ad name", *ad.Name, "Compartment Id", d.config.CompartmentID).With(zap.Error(err)).Warn("New volume creation failed due to oke system tags error. sending metric & retrying without oke system tags") + errorType = util.SystemTagErrTypePrefix + util.GetError(err) + metricDimension = util.GetMetricDimensionForComponent(errorType, metricType) + dimensionsMap[metrics.ComponentDimension] = metricDimension + metrics.SendMetricData(d.metricPusher, metric, time.Since(startTime).Seconds(), dimensionsMap) + + // retry provision without oke system tags + delete(bvTags.DefinedTags, OkeSystemTagNamesapce) + provisionedVolume, err = provision(ctx, log, d.client, volumeName, size, *ad.Name, d.config.CompartmentID, srcSnapshotId, srcVolumeId, + volumeParams.diskEncryptionKey, volumeParams.vpusPerGB, bvTags) + } if err != nil { log.With("Ad name", *ad.Name, "Compartment Id", d.config.CompartmentID).With(zap.Error(err)).Error("New volume creation failed.") errorType = util.GetError(err) @@ -1309,6 +1327,11 @@ func provision(ctx context.Context, log *zap.SugaredLogger, c client.Interface, } if bvTags != nil && bvTags.DefinedTags != nil { volumeDetails.DefinedTags = bvTags.DefinedTags + if len(volumeDetails.DefinedTags) > MaxDefinedTagPerVolume { + log.With("service", "blockstorage", "verb", "create", "resource", "volume", "volumeName", volName). + Warn("the number of defined tags in the volume create request is beyond the limit. removing system tags from the details") + delete(volumeDetails.DefinedTags, OkeSystemTagNamesapce) + } } newVolume, err := c.BlockStorage().CreateVolume(ctx, volumeDetails) @@ -1352,7 +1375,7 @@ func isBlockVolumeAvailable(backup core.VolumeBackup) (bool, error) { return false, nil } -func getBVTags(tags *config.InitialTags, vp VolumeParameters) *config.TagConfig { +func getBVTags(logger *zap.SugaredLogger, tags *config.InitialTags, vp VolumeParameters) *config.TagConfig { bvTags := &config.TagConfig{} if tags != nil && tags.BlockVolume != nil { @@ -1368,7 +1391,7 @@ func getBVTags(tags *config.InitialTags, vp VolumeParameters) *config.TagConfig bvTags = scTags } // merge final tags with common tags - if util.IsCommonTagPresent(tags) { + if enableOkeSystemTags && util.IsCommonTagPresent(tags) { bvTags = util.MergeTagConfig(bvTags, tags.Common) } return bvTags diff --git a/pkg/csi/driver/bv_controller_test.go b/pkg/csi/driver/bv_controller_test.go index f1493f200c..a5b9011d9c 100644 --- a/pkg/csi/driver/bv_controller_test.go +++ b/pkg/csi/driver/bv_controller_test.go @@ -533,6 +533,10 @@ func (c *MockLoadBalancerClient) UpdateNetworkSecurityGroups(context.Context, st return "", nil } +func (c *MockLoadBalancerClient) UpdateLoadBalancer(ctx context.Context, lbID string, details *client.GenericUpdateLoadBalancerDetails) (string, error) { + return "", nil +} + // Networking mocks client VirtualNetwork implementation. func (p *MockProvisionerClient) LoadBalancer(*zap.SugaredLogger, string, string, *authv1.TokenRequest) client.GenericLoadBalancerInterface { return &MockLoadBalancerClient{} @@ -1870,15 +1874,18 @@ func TestGetBVTags(t *testing.T) { emptyTags := &providercfg.InitialTags{} emptyTagConfig := &providercfg.TagConfig{} emptyVolumeParameters := VolumeParameters{} + enableOkeSystemTags = true tests := map[string]struct { initialTags *providercfg.InitialTags volumeParameters VolumeParameters expectedTagConfig *providercfg.TagConfig + featureEnabled bool }{ "no resource tags, no common tags": { initialTags: emptyTags, volumeParameters: emptyVolumeParameters, expectedTagConfig: emptyTagConfig, + featureEnabled: true, }, "no resource tags, but common tags": { initialTags: &providercfg.InitialTags{ @@ -1892,6 +1899,7 @@ func TestGetBVTags(t *testing.T) { FreeformTags: map[string]string{"key1": "value1"}, DefinedTags: map[string]map[string]interface{}{"ns1": {"key1": "value1"}}, }, + featureEnabled: true, }, "resource tags with common tags from config": { initialTags: &providercfg.InitialTags{ @@ -1908,6 +1916,7 @@ func TestGetBVTags(t *testing.T) { FreeformTags: map[string]string{"key1": "value1", "key2": "value2"}, DefinedTags: map[string]map[string]interface{}{"ns1": {"key1": "value1"}, "ns2": {"key2": "value2"}}, }, + featureEnabled: true, }, "resource level tags with common tags from config with same key": { initialTags: &providercfg.InitialTags{ @@ -1924,6 +1933,7 @@ func TestGetBVTags(t *testing.T) { FreeformTags: map[string]string{"key1": "value1"}, DefinedTags: map[string]map[string]interface{}{"ns1": {"key1": "value1"}}, }, + featureEnabled: true, }, "cluster level tags with common tags from config": { initialTags: &providercfg.InitialTags{ @@ -1941,6 +1951,7 @@ func TestGetBVTags(t *testing.T) { FreeformTags: map[string]string{"key1": "value1", "key2": "value2"}, DefinedTags: map[string]map[string]interface{}{"ns1": {"key1": "value1"}, "ns2": {"key2": "value2"}}, }, + featureEnabled: true, }, "cluster level tags with common tags from config with same key": { initialTags: &providercfg.InitialTags{ @@ -1958,6 +1969,7 @@ func TestGetBVTags(t *testing.T) { FreeformTags: map[string]string{"key1": "value2"}, DefinedTags: map[string]map[string]interface{}{"ns1": {"key2": "value2"}}, }, + featureEnabled: true, }, "cluster level tags but no common tags": { initialTags: &providercfg.InitialTags{ @@ -1971,6 +1983,7 @@ func TestGetBVTags(t *testing.T) { FreeformTags: map[string]string{"key1": "value1"}, DefinedTags: map[string]map[string]interface{}{"ns1": {"key1": "value1"}}, }, + featureEnabled: true, }, "no cluster level or resource level tags but common tags": { initialTags: &providercfg.InitialTags{ @@ -1984,13 +1997,30 @@ func TestGetBVTags(t *testing.T) { FreeformTags: map[string]string{"key1": "value1"}, DefinedTags: map[string]map[string]interface{}{"ns1": {"key1": "value1"}}, }, + featureEnabled: true, + }, + "when the feature is disabled": { + initialTags: &providercfg.InitialTags{ + Common: &providercfg.TagConfig{ + FreeformTags: map[string]string{"key1": "value1"}, + DefinedTags: map[string]map[string]interface{}{"ns1": {"key1": "value1"}}, + }, + }, + volumeParameters: VolumeParameters{ + freeformTags: map[string]string{"key2": "value2"}, + definedTags: map[string]map[string]interface{}{"ns2": {"key2": "value2"}}, + }, + expectedTagConfig: &providercfg.TagConfig{ + FreeformTags: map[string]string{"key2": "value2"}, + DefinedTags: map[string]map[string]interface{}{"ns2": {"key2": "value2"}}, + }, + featureEnabled: false, }, } for name, testcase := range tests { + enableOkeSystemTags = testcase.featureEnabled t.Run(name, func(t *testing.T) { - actualTagConfig := getBVTags(testcase.initialTags, testcase.volumeParameters) - t.Logf("%v", actualTagConfig) - t.Logf("%v", testcase.expectedTagConfig) + actualTagConfig := getBVTags(zap.S(), testcase.initialTags, testcase.volumeParameters) if !reflect.DeepEqual(actualTagConfig, testcase.expectedTagConfig) { t.Errorf("Expected tagconfig %v but got %v", testcase.expectedTagConfig, actualTagConfig) } diff --git a/pkg/oci/client/client.go b/pkg/oci/client/client.go index 5653c70570..313d76b45a 100644 --- a/pkg/oci/client/client.go +++ b/pkg/oci/client/client.go @@ -104,6 +104,7 @@ type loadBalancerClient interface { DeleteListener(ctx context.Context, request loadbalancer.DeleteListenerRequest) (response loadbalancer.DeleteListenerResponse, err error) UpdateLoadBalancerShape(ctx context.Context, request loadbalancer.UpdateLoadBalancerShapeRequest) (response loadbalancer.UpdateLoadBalancerShapeResponse, err error) UpdateNetworkSecurityGroups(ctx context.Context, request loadbalancer.UpdateNetworkSecurityGroupsRequest) (response loadbalancer.UpdateNetworkSecurityGroupsResponse, err error) + UpdateLoadBalancer(ctx context.Context, request loadbalancer.UpdateLoadBalancerRequest) (response loadbalancer.UpdateLoadBalancerResponse, err error) } type networkLoadBalancerClient interface { @@ -120,6 +121,7 @@ type networkLoadBalancerClient interface { UpdateListener(ctx context.Context, request networkloadbalancer.UpdateListenerRequest) (response networkloadbalancer.UpdateListenerResponse, err error) DeleteListener(ctx context.Context, request networkloadbalancer.DeleteListenerRequest) (response networkloadbalancer.DeleteListenerResponse, err error) UpdateNetworkSecurityGroups(ctx context.Context, request networkloadbalancer.UpdateNetworkSecurityGroupsRequest) (response networkloadbalancer.UpdateNetworkSecurityGroupsResponse, err error) + UpdateNetworkLoadBalancer(ctx context.Context, request networkloadbalancer.UpdateNetworkLoadBalancerRequest) (response networkloadbalancer.UpdateNetworkLoadBalancerResponse, err error) } type filestorageClient interface { diff --git a/pkg/oci/client/errors.go b/pkg/oci/client/errors.go index 2919452da2..7183c5c50e 100644 --- a/pkg/oci/client/errors.go +++ b/pkg/oci/client/errors.go @@ -18,14 +18,22 @@ import ( "context" "math" "net/http" + "regexp" "time" "github.com/oracle/oci-go-sdk/v65/common" "github.com/pkg/errors" + "go.uber.org/zap" ) var errNotFound = errors.New("not found") +/* +Addition of system tags can fail due to permission issue while API returns error code: RelatedResourceNotAuthorizedOrNotFound & +the error message "The following tag namespaces / keys are not authorized or not found: 'orcl-containerengine'" +*/ +var regexSystemTagNotFoundNotAuthorised = regexp.MustCompile(".*tag namespace.*orcl-containerengine.*") + // HTTP Error Types const ( HTTP400RelatedResourceNotAuthorizedOrNotFoundCode = "RelatedResourceNotAuthorizedOrNotFound" @@ -111,3 +119,20 @@ func NewRetryPolicyWithMaxAttempts(retryAttempts uint) *common.RetryPolicy { ) return &policy } + +func IsSystemTagNotFoundOrNotAuthorisedError(logger *zap.SugaredLogger, err error) bool { + + var ociServiceError common.ServiceError + + // unwrap till ociServiceError is found + if errors.As(err, &ociServiceError) { + logger.Debugf("API error code: %s", ociServiceError.GetCode()) + logger.Debugf("service error message: %s", ociServiceError.GetMessage()) + + if ociServiceError.GetHTTPStatusCode() == http.StatusBadRequest && + ociServiceError.GetCode() == HTTP400RelatedResourceNotAuthorizedOrNotFoundCode { + return regexSystemTagNotFoundNotAuthorised.MatchString(ociServiceError.GetMessage()) + } + } + return false +} diff --git a/pkg/oci/client/errors_test.go b/pkg/oci/client/errors_test.go index 4593fbcc65..2ebf63fbdc 100644 --- a/pkg/oci/client/errors_test.go +++ b/pkg/oci/client/errors_test.go @@ -15,6 +15,9 @@ package client import ( + "fmt" + "github.com/pkg/errors" + "go.uber.org/zap" "net/http" "testing" @@ -43,6 +46,9 @@ func (m mockServiceError) GetCode() string { func (m mockServiceError) GetOpcRequestID() string { return m.OpcRequestID } +func (m mockServiceError) Error() string { + return m.Message +} func TestIsRetryableServiceError(t *testing.T) { testCases := map[string]struct { @@ -117,3 +123,64 @@ func TestIsRetryableServiceError(t *testing.T) { } } + +func TestIsSystemTagNotFoundOrNotAuthorisedError(t *testing.T) { + systemTagError := mockServiceError{ + StatusCode: http.StatusBadRequest, + Code: HTTP400RelatedResourceNotAuthorizedOrNotFoundCode, + Message: "The following tag namespaces / keys are not authorized or not found: 'orcl-containerengine'", + } + systemTagError2 := mockServiceError{ + StatusCode: http.StatusBadRequest, + Code: HTTP400RelatedResourceNotAuthorizedOrNotFoundCode, + Message: "The following tag namespaces / keys are not authorized or not found: TagDefinition cluster_foobar in TagNamespace orcl-containerengine does not exists.\\n", + } + userDefinedTagError1 := mockServiceError{ + StatusCode: http.StatusBadRequest, + Code: HTTP400RelatedResourceNotAuthorizedOrNotFoundCode, + Message: "The following tag namespaces / keys are not authorized or not found: 'foobar-namespace'", + } + userDefinedTagError2 := mockServiceError{ + StatusCode: http.StatusBadRequest, + Code: HTTP400RelatedResourceNotAuthorizedOrNotFoundCode, + Message: "TagNamespace orcl-foobar does not exists.\\nTagNamespace orcl-foobar-name does not exists.\\n", + } + tests := map[string]struct { + se mockServiceError + wrappedError error + expectIsTagError bool + }{ + "base case": { + wrappedError: errors.WithMessage(systemTagError, "taggin failure"), + expectIsTagError: true, + }, + "three layer wrapping - resource tracking system tag error": { + wrappedError: errors.Wrap(errors.Wrap(errors.WithMessage(systemTagError, "taggin failure"), "first layer"), "second layer"), + expectIsTagError: true, + }, + "wrapping with stack trace - resource tracking system tag error": { + wrappedError: errors.WithStack(errors.Wrap(errors.WithMessage(systemTagError2, "taggin failure"), "first layer")), + expectIsTagError: true, + }, + "three layer wrapping - user defined tag error": { + wrappedError: errors.Wrap(errors.Wrap(errors.WithMessage(userDefinedTagError1, "taggin failure"), "first layer"), "second layer"), + expectIsTagError: false, + }, + "wrapping with stack trace - user defined tag error": { + wrappedError: errors.WithStack(errors.Wrap(errors.WithMessage(userDefinedTagError2, "taggin failure"), "first layer")), + expectIsTagError: false, + }, + "not a service error": { + wrappedError: errors.Wrap(fmt.Errorf("not a service error"), "precheck error"), + expectIsTagError: false, + }, + } + for name, test := range tests { + t.Run(name, func(t *testing.T) { + actualResult := IsSystemTagNotFoundOrNotAuthorisedError(zap.S(), test.wrappedError) + if actualResult != test.expectIsTagError { + t.Errorf("expected %t but got %t", actualResult, test.expectIsTagError) + } + }) + } +} diff --git a/pkg/oci/client/generic_load_balancer_types.go b/pkg/oci/client/generic_load_balancer_types.go index f206736e3f..d2b108fe5a 100644 --- a/pkg/oci/client/generic_load_balancer_types.go +++ b/pkg/oci/client/generic_load_balancer_types.go @@ -146,6 +146,7 @@ type GenericLoadBalancer struct { FreeformTags map[string]string DefinedTags map[string]map[string]interface{} + SystemTags map[string]map[string]interface{} } type GenericWorkRequest struct { @@ -162,3 +163,8 @@ type GenericWorkRequest struct { type GenericUpdateNetworkSecurityGroupsDetails struct { NetworkSecurityGroupIds []string } + +type GenericUpdateLoadBalancerDetails struct { + FreeformTags map[string]string + DefinedTags map[string]map[string]interface{} +} diff --git a/pkg/oci/client/load_balancer.go b/pkg/oci/client/load_balancer.go index 5571fa1340..8608bdf08d 100644 --- a/pkg/oci/client/load_balancer.go +++ b/pkg/oci/client/load_balancer.go @@ -61,6 +61,7 @@ type GenericLoadBalancerInterface interface { AwaitWorkRequest(ctx context.Context, id string) (*GenericWorkRequest, error) ListWorkRequests(ctx context.Context, compartmentId, lbId string) ([]*GenericWorkRequest, error) + UpdateLoadBalancer(ctx context.Context, lbID string, details *GenericUpdateLoadBalancerDetails) (string, error) } func (c *loadbalancerClientStruct) GetLoadBalancer(ctx context.Context, id string) (*GenericLoadBalancer, error) { @@ -515,6 +516,25 @@ func (c *loadbalancerClientStruct) UpdateNetworkSecurityGroups(ctx context.Conte return *resp.OpcWorkRequestId, nil } +func (c *loadbalancerClientStruct) UpdateLoadBalancer(ctx context.Context, lbID string, details *GenericUpdateLoadBalancerDetails) (string, error) { + if !c.rateLimiter.Writer.TryAccept() { + return "", RateLimitError(true, "UpdateLoadBalancer") + } + + resp, err := c.loadbalancer.UpdateLoadBalancer(ctx, loadbalancer.UpdateLoadBalancerRequest{ + UpdateLoadBalancerDetails: loadbalancer.UpdateLoadBalancerDetails{ + FreeformTags: details.FreeformTags, + DefinedTags: details.DefinedTags, + }, + LoadBalancerId: &lbID, + RequestMetadata: c.requestMetadata, + }) + incRequestCounter(err, updateVerb, loadBalancerResource) + if err != nil { + return "", errors.WithStack(err) + } + return *resp.OpcWorkRequestId, nil +} func (c *loadbalancerClientStruct) loadbalancerToGenericLoadbalancer(lb *loadbalancer.LoadBalancer) *GenericLoadBalancer { if lb == nil { return nil @@ -536,6 +556,7 @@ func (c *loadbalancerClientStruct) loadbalancerToGenericLoadbalancer(lb *loadbal BackendSets: c.backendSetsToGenericBackendSetDetails(lb.BackendSets), FreeformTags: lb.FreeformTags, DefinedTags: lb.DefinedTags, + SystemTags: lb.SystemTags, } } diff --git a/pkg/oci/client/load_balancer_test.go b/pkg/oci/client/load_balancer_test.go index 49dc269783..c2a420388a 100644 --- a/pkg/oci/client/load_balancer_test.go +++ b/pkg/oci/client/load_balancer_test.go @@ -220,6 +220,9 @@ func (c *MockLoadBalancerClient) UpdateLoadBalancerShape(ctx context.Context, re func (c *MockLoadBalancerClient) UpdateNetworkSecurityGroups(ctx context.Context, request loadbalancer.UpdateNetworkSecurityGroupsRequest) (response loadbalancer.UpdateNetworkSecurityGroupsResponse, err error) { return } +func (c *MockLoadBalancerClient) UpdateLoadBalancer(ctx context.Context, request loadbalancer.UpdateLoadBalancerRequest) (response loadbalancer.UpdateLoadBalancerResponse, err error) { + return +} func assertError(actual, expected error) bool { if expected == nil || actual == nil { diff --git a/pkg/oci/client/network_load_balancer.go b/pkg/oci/client/network_load_balancer.go index 94e93572a2..10a2edeaef 100644 --- a/pkg/oci/client/network_load_balancer.go +++ b/pkg/oci/client/network_load_balancer.go @@ -385,6 +385,27 @@ func (c *networkLoadbalancer) UpdateNetworkSecurityGroups(ctx context.Context, l return *resp.OpcWorkRequestId, nil } +func (c *networkLoadbalancer) UpdateLoadBalancer(ctx context.Context, lbID string, details *GenericUpdateLoadBalancerDetails) (string, error) { + if !c.rateLimiter.Writer.TryAccept() { + return "", RateLimitError(true, "UpdateLoadBalancer") + } + + resp, err := c.networkloadbalancer.UpdateNetworkLoadBalancer(ctx, networkloadbalancer.UpdateNetworkLoadBalancerRequest{ + UpdateNetworkLoadBalancerDetails: networkloadbalancer.UpdateNetworkLoadBalancerDetails{ + FreeformTags: details.FreeformTags, + DefinedTags: details.DefinedTags, + }, + NetworkLoadBalancerId: &lbID, + }) + incRequestCounter(err, updateVerb, networkLoadBalancerResource) + + if err != nil { + return "", errors.WithStack(err) + } + + return *resp.OpcWorkRequestId, nil +} + func backendsToBackendDetails(backends []GenericBackend) []networkloadbalancer.BackendDetails { backendDetails := make([]networkloadbalancer.BackendDetails, 0) for _, backend := range backends { @@ -430,6 +451,7 @@ func (c *networkLoadbalancer) networkLoadbalancerToGenericLoadbalancer(nlb *netw BackendSets: c.backendSetsToGenericBackendSetDetails(nlb.BackendSets), FreeformTags: nlb.FreeformTags, DefinedTags: nlb.DefinedTags, + SystemTags: nlb.SystemTags, } } @@ -448,6 +470,7 @@ func (c *networkLoadbalancer) networkLoadbalancerSummaryToGenericLoadbalancer(nl BackendSets: c.backendSetsToGenericBackendSetDetails(nlb.BackendSets), FreeformTags: nlb.FreeformTags, DefinedTags: nlb.DefinedTags, + SystemTags: nlb.SystemTags, } } diff --git a/pkg/oci/client/network_load_balancer_test.go b/pkg/oci/client/network_load_balancer_test.go index d4a2af28cc..d677a6aceb 100644 --- a/pkg/oci/client/network_load_balancer_test.go +++ b/pkg/oci/client/network_load_balancer_test.go @@ -208,3 +208,7 @@ func (c *MockNetworkLoadBalancerClient) DeleteListener(ctx context.Context, requ func (c *MockNetworkLoadBalancerClient) UpdateNetworkSecurityGroups(ctx context.Context, request networkloadbalancer.UpdateNetworkSecurityGroupsRequest) (response networkloadbalancer.UpdateNetworkSecurityGroupsResponse, err error) { return } +func (c *MockNetworkLoadBalancerClient) UpdateNetworkLoadBalancer(ctx context.Context, request networkloadbalancer.UpdateNetworkLoadBalancerRequest) (response networkloadbalancer.UpdateNetworkLoadBalancerResponse, err error) { + //TODO implement me + return +} diff --git a/pkg/util/commons.go b/pkg/util/commons.go index 4a1b9dcb98..7f6c8f0e4f 100644 --- a/pkg/util/commons.go +++ b/pkg/util/commons.go @@ -18,11 +18,11 @@ import ( "context" "errors" "fmt" - "github.com/oracle/oci-cloud-controller-manager/pkg/cloudprovider/providers/oci/config" "regexp" "strconv" "strings" + "github.com/oracle/oci-cloud-controller-manager/pkg/cloudprovider/providers/oci/config" "github.com/oracle/oci-go-sdk/v65/common" metricErrors "github.com/pkg/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -35,14 +35,15 @@ const ( CompartmentIDAnnotation = "oci.oraclecloud.com/compartment-id" // Error codes - Err429 = "429" - Err4XX = "4XX" - Err5XX = "5XX" - ErrValidation = "VALIDATION_ERROR" - ErrLimitExceeded = "LIMIT_EXCEEDED" - ErrCtxTimeout = "CTX_TIMEOUT" - Success = "SUCCESS" - BackupCreating = "CREATING" + Err429 = "429" + Err4XX = "4XX" + Err5XX = "5XX" + ErrValidation = "VALIDATION_ERROR" + ErrLimitExceeded = "LIMIT_EXCEEDED" + ErrCtxTimeout = "CTX_TIMEOUT" + ErrTagLimitReached = "TAG_LIMIT_REACHED" + Success = "SUCCESS" + BackupCreating = "CREATING" // Components generating errors // Load Balancer @@ -51,6 +52,9 @@ const ( // storage types CSIStorageType = "CSI" FVDStorageType = "FVD" + + // Errorcode prefixes + SystemTagErrTypePrefix = "SYSTEM_TAG_" ) // LookupNodeCompartment returns the compartment OCID for the given nodeName. @@ -153,7 +157,6 @@ func MergeTagConfig(srcTagConfig, dstTagConfig *config.TagConfig) *config.TagCon // IsCommonTagPresent return true if Common tags are initialised in config func IsCommonTagPresent(initialTags *config.InitialTags) bool { - // TODO: perform feature enabled check return initialTags != nil && initialTags.Common != nil } diff --git a/pkg/util/commons_test.go b/pkg/util/commons_test.go index f6432ee3a8..a81fd1638f 100644 --- a/pkg/util/commons_test.go +++ b/pkg/util/commons_test.go @@ -16,10 +16,11 @@ package util import ( "errors" - "github.com/oracle/oci-cloud-controller-manager/pkg/cloudprovider/providers/oci/config" "reflect" "testing" + "github.com/oracle/oci-cloud-controller-manager/pkg/cloudprovider/providers/oci/config" + errors2 "github.com/pkg/errors" "k8s.io/apimachinery/pkg/util/wait" ) diff --git a/pkg/volume/provisioner/block/block_test.go b/pkg/volume/provisioner/block/block_test.go index 1e4eb37cf8..3c05b3c41d 100644 --- a/pkg/volume/provisioner/block/block_test.go +++ b/pkg/volume/provisioner/block/block_test.go @@ -457,6 +457,10 @@ func (c *MockLoadBalancerClient) UpdateNetworkSecurityGroups(context.Context, st return "", nil } +func (c *MockLoadBalancerClient) UpdateLoadBalancer(ctx context.Context, lbID string, details *client.GenericUpdateLoadBalancerDetails) (string, error) { + return "", nil +} + func (c *MockVirtualNetworkClient) AddNetworkSecurityGroupSecurityRules(ctx context.Context, id string, details core.AddNetworkSecurityGroupSecurityRulesDetails) (*core.AddNetworkSecurityGroupSecurityRulesResponse, error) { return nil, nil } diff --git a/pkg/volume/provisioner/fss/fss_test.go b/pkg/volume/provisioner/fss/fss_test.go index 949f5f3a09..23eb97bf67 100644 --- a/pkg/volume/provisioner/fss/fss_test.go +++ b/pkg/volume/provisioner/fss/fss_test.go @@ -455,6 +455,9 @@ func (c *MockLoadBalancerClient) AwaitWorkRequest(ctx context.Context, id string func (c *MockLoadBalancerClient) UpdateNetworkSecurityGroups(context.Context, string, []string) (string, error) { return "", nil } +func (c *MockLoadBalancerClient) UpdateLoadBalancer(ctx context.Context, lbID string, details *client.GenericUpdateLoadBalancerDetails) (string, error) { + return "", nil +} func (c *MockVirtualNetworkClient) AddNetworkSecurityGroupSecurityRules(ctx context.Context, id string, details core.AddNetworkSecurityGroupSecurityRulesDetails) (*core.AddNetworkSecurityGroupSecurityRulesResponse, error) { return nil, nil