Await workrequest for UpdateNSG and UpdateLBShape

Signed-off-by: mpagnis <[email protected]>

Author: AkarshES

pkg/cloudprovider/providers/oci/instances_test.go

@@ -460,17 +460,35 @@ func (c *MockLoadBalancerClient) DeleteListener(ctx context.Context, lbID, name
 	return "", nil
 }
 
+var awaitLoadbalancerWorkrequestMap = map[string]error{
+	"failedToGetUpdateNetworkSecurityGroupsWorkRequest": errors.New("internal server error for get workrequest call"),
+}
+
 func (c *MockLoadBalancerClient) AwaitWorkRequest(ctx context.Context, id string) (*client.GenericWorkRequest, error) {
+	if err, ok := awaitLoadbalancerWorkrequestMap[id]; ok {
+		return nil, err
+	}
 	return nil, nil
 }
 
 func (c *MockLoadBalancerClient) UpdateLoadBalancerShape(context.Context, string, *client.GenericUpdateLoadBalancerShapeDetails) (string, error) {
 	return "", nil
 }
 
+var updateNetworkSecurityGroupsLBsFailures = map[string]error{
+	"":                      errors.New("provided LB ID is empty"),
+	"failedToCreateRequest": errors.New("internal server error"),
+}
+var updateNetworkSecurityGroupsLBsWorkRequests = map[string]string{
+	"failedToGetUpdateNetworkSecurityGroupsWorkRequest": "failedToGetUpdateNetworkSecurityGroupsWorkRequest",
+}
+
 func (c *MockLoadBalancerClient) UpdateNetworkSecurityGroups(ctx context.Context, lbId string, nsgIds []string) (string, error) {
-	if lbId == "" {
-		return "", errors.New("provided LB ID is empty")
+	if err, ok := updateNetworkSecurityGroupsLBsFailures[lbId]; ok {
+		return "", err
+	}
+	if wrID, ok := updateNetworkSecurityGroupsLBsWorkRequests[lbId]; ok {
+		return wrID, nil
 	}
 	return "", nil
 }

pkg/cloudprovider/providers/oci/load_balancer.go

@@ -983,23 +983,35 @@ func (clb *CloudLoadBalancerProvider) updateLoadbalancerShape(ctx context.Contex
 			MaximumBandwidthInMbps: spec.FlexMax,
 		}
 	}
-	opcRequestID, err := clb.lbClient.UpdateLoadBalancerShape(ctx, *lb.Id, &shapeDetails)
+	wrID, err := clb.lbClient.UpdateLoadBalancerShape(ctx, *lb.Id, &shapeDetails)
 	if err != nil {
-		return errors.Wrap(err, "failed to update loadbalancer shape")
+		return errors.Wrap(err, "failed to create UpdateLoadBalancerShape request")
 	}
-	clb.logger.With("old-shape", *lb.ShapeName, "new-shape", spec.Shape,
+	logger := clb.logger.With("old-shape", *lb.ShapeName, "new-shape", spec.Shape,
 		"flexMinimumMbps", spec.FlexMin, "flexMaximumMbps", spec.FlexMax,
-		"opc-request-id", opcRequestID, "loadBalancerType", getLoadBalancerType(spec.service)).Info("Successfully created an loadbalancer update shape request")
+		"opc-workrequest-id", wrID, "loadBalancerType", getLoadBalancerType(spec.service))
+	logger.Info("Awaiting UpdateLoadBalancerShape workrequest")
+	_, err = clb.lbClient.AwaitWorkRequest(ctx, wrID)
+	if err != nil {
+		return err
+	}
+	logger.Info("UpdateLoadBalancerShape request completed successfully")
 	return nil
 }
 
 func (clb *CloudLoadBalancerProvider) updateLoadBalancerNetworkSecurityGroups(ctx context.Context, lb *client.GenericLoadBalancer, spec *LBSpec) error {
-	opcRequestID, err := clb.lbClient.UpdateNetworkSecurityGroups(ctx, *lb.Id, spec.NetworkSecurityGroupIds)
+	wrID, err := clb.lbClient.UpdateNetworkSecurityGroups(ctx, *lb.Id, spec.NetworkSecurityGroupIds)
+	if err != nil {
+		return errors.Wrap(err, "failed to create UpdateNetworkSecurityGroups request")
+	}
+	logger := clb.logger.With("existingNSGIds", lb.NetworkSecurityGroupIds, "newNSGIds", spec.NetworkSecurityGroupIds,
+		"opc-workrequest-id", wrID)
+	logger.Info("Awaiting UpdateNetworkSecurityGroups workrequest")
+	_, err = clb.lbClient.AwaitWorkRequest(ctx, wrID)
 	if err != nil {
-		return errors.Wrap(err, "failed to update loadbalancer Network Security Group")
+		return errors.Wrap(err, "failed to await UpdateNetworkSecurityGroups workrequest")
 	}
-	clb.logger.With("existingNSGIds", lb.NetworkSecurityGroupIds, "newNSGIds", spec.NetworkSecurityGroupIds,
-		"opc-request-id", opcRequestID).Info("successfully updated the network security groups")
+	logger.Info("Loadbalancer UpdateNetworkSecurityGroups workrequest completed successfully")
 	return nil
 }
 

pkg/cloudprovider/providers/oci/load_balancer_test.go

@@ -24,10 +24,10 @@ import (
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
+	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/v50/common"
 	"github.com/oracle/oci-go-sdk/v50/core"
-	"github.com/oracle/oci-cloud-controller-manager/pkg/oci/client"
-	providercfg "github.com/oracle/oci-cloud-controller-manager/pkg/cloudprovider/providers/oci/config"
 )
 
 func Test_getDefaultLBSubnets(t *testing.T) {
@@ -476,7 +476,7 @@ func TestUpdateLoadBalancerNetworkSecurityGroups(t *testing.T) {
 		loadbalancer *client.GenericLoadBalancer
 		wantErr      error
 	}{
-		"Update NSG when there's an issue with LB": {
+		"lb id is missing": {
 			spec: &LBSpec{
 				Name:                    "test",
 				NetworkSecurityGroupIds: []string{"ocid1"},
@@ -485,7 +485,29 @@ func TestUpdateLoadBalancerNetworkSecurityGroups(t *testing.T) {
 				Id:          common.String(""),
 				DisplayName: common.String("privateLB"),
 			},
-			wantErr: errors.New("failed to update loadbalancer Network Security Group: provided LB ID is empty"),
+			wantErr: errors.New("failed to create UpdateNetworkSecurityGroups request: provided LB ID is empty"),
+		},
+		"failed to create workrequest": {
+			spec: &LBSpec{
+				Name:                    "test",
+				NetworkSecurityGroupIds: []string{"ocid1"},
+			},
+			loadbalancer: &client.GenericLoadBalancer{
+				Id:          common.String("failedToCreateRequest"),
+				DisplayName: common.String("privateLB"),
+			},
+			wantErr: errors.New("failed to create UpdateNetworkSecurityGroups request: internal server error"),
+		},
+		"failed to get workrequest": {
+			spec: &LBSpec{
+				Name:                    "test",
+				NetworkSecurityGroupIds: []string{"ocid1"},
+			},
+			loadbalancer: &client.GenericLoadBalancer{
+				Id:          common.String("failedToGetUpdateNetworkSecurityGroupsWorkRequest"),
+				DisplayName: common.String("privateLB"),
+			},
+			wantErr: errors.New("failed to await UpdateNetworkSecurityGroups workrequest: internal server error for get workrequest call"),
 		},
 		"Update NSG to existing LB": {
 			spec: &LBSpec{
@@ -507,8 +529,8 @@ func TestUpdateLoadBalancerNetworkSecurityGroups(t *testing.T) {
 	for name, tt := range tests {
 		t.Run(name, func(t *testing.T) {
 			err := cp.updateLoadBalancerNetworkSecurityGroups(context.Background(), tt.loadbalancer, tt.spec)
-			if err != nil && err.Error() != tt.wantErr.Error() {
-				t.Errorf("Expected error = %v, but got %v", err, tt.wantErr)
+			if !assertError(err, tt.wantErr) {
+				t.Errorf("Expected error = %v, but got %v", tt.wantErr, err)
 				return
 			}
 		})
@@ -626,3 +648,10 @@ func TestCloudProvider_EnsureLoadBalancerDeleted(t *testing.T) {
 		})
 	}
 }
+
+func assertError(actual, expected error) bool {
+	if expected == nil || actual == nil {
+		return expected == actual
+	}
+	return actual.Error() == expected.Error()
+}