Merge pull request #380 from oracle/oke-119-pending

Await workrequest for UpdateNSG and UpdateLBShape

Author: mrunalpagnis

README.md

@@ -27,6 +27,7 @@ cloud-provider specific code out of the Kubernetes codebase.
 | \>=v 0.11 | v1.16                       | v1.18                        |
 | \>=v 0.12 | v1.18                       | v1.21                        |
 | \>=v 0.13 | v1.19                       | v1.21                        |
+| v1.19.12  | v1.19                       | v1.21                        |
 
 Note: 
 Versions older than v0.13.0 are no longer supported, new features / bug fixes will be available in v0.13.0 and later. 

docs/expand-block-volume-using-csi.md

@@ -6,7 +6,7 @@
 
 ## Create PVC
 
-```bash
+```yaml
 apiVersion: v1
 kind: PersistentVolumeClaim
 metadata:
@@ -22,7 +22,7 @@ spec:
 
 ## Create POD
 
-```bash
+```yaml
 apiVersion: v1
 kind: Pod
 metadata:

docs/load-balancer-annotations.md

@@ -3,14 +3,15 @@
 This file defines a list of [Service][4] `type: LoadBalancer` annotations which are
 supported by the `oci-cloud-controller-manager`.
 
-All annotations are prefixed with `service.beta.kubernetes.io/` or `oci.oraclecloud.com/`. For example:
+All annotations are prefixed with `service.beta.kubernetes.io/` or `oci.oraclecloud.com/` or `oci-network-load-balancer.oraclecloud.com/` (for OCI Network Load Balancer specific annotations). For example:
 
 ```yaml
 kind: Service
 apiVersion: v1
 metadata:
   name: nginx-service
   annotations:
+    oci.oraclecloud.com/load-balancer-type: "lb"
     service.beta.kubernetes.io/oci-load-balancer-shape: "400Mbps"
     service.beta.kubernetes.io/oci-load-balancer-subnet1: "ocid..."
     service.beta.kubernetes.io/oci-load-balancer-subnet2: "ocid..."
@@ -58,6 +59,49 @@ Note:
 - If an invalid mode is passed in the annotation, then the default (`"All"`) mode is configured.
 - If an annotation is not specified, the mode specified in the cloud provider config file is configured.  
 
+## Network Load Balancer
+
+For example:
+
+```yaml
+apiVersion: v1
+kind: Service
+metadata:
+  name: example-nlb
+  annotations:
+    oci-network-load-balancer.oraclecloud.com/security-list-management-mode: "All"
+    oci.oraclecloud.com/load-balancer-type: nlb
+spec:
+  selector:
+    app: example-nlb
+  ports:
+    - port: 8088
+      targetPort: 80
+  type: LoadBalancer
+  externalTrafficPolicy: Local
+```
+
+Note:
+- The only security list management mode allowed when backend protocol is UDP is "None"
+- `externalTrafficPolicy` should be "Local" for preserving source IP
+- We recommend to set the `security-list-management-mode` as "None" and configure NSG / Security rules on your own.
+
+## Network Load Balancer Specific Annotations
+
+| Name                                                                          | Description                                                                                                                                                   | Default
+| -----                                                                         | -----------                                                                                                                                                   | -------                                         
+| `oci-network-load-balancer.oraclecloud.com/internal`                          | Create an [internal network load balancer][1]. Cannot be modified after load balancer creation.                                                               | `false`
+| `oci-network-load-balancer.oraclecloud.com/subnet`                            | The OCID of the required regional or AD specific subnet to attach the network load balancer.	                                                                | Value set for the cluster
+| `oci-network-load-balancer.oraclecloud.com/oci-network-security-groups`       | Specifies Network Security Groups' OCIDs to be associated with the network load balancer.	                                                                    | `""`
+| `oci-network-load-balancer.oraclecloud.com/initial-freeform-tags-override`    | Specifies one or multiple Freeform tags to apply to the OCI Network Load Balancer.                                                                            | `""`
+| `oci-network-load-balancer.oraclecloud.com/initial-defined-tags-override`     | Specifies one or multiple Defined tags to apply to the OCI Network Load Balancer.                                                                             | `""`
+| `oci-network-load-balancer.oraclecloud.com/health-check-retries`              | The number of retries to attempt before a backend server is considered "unhealthy".	                                                                        | `3`
+| `oci-network-load-balancer.oraclecloud.com/health-check-timeout`              | The maximum time, in milliseconds, to wait for a reply to a health check. A health check is successful only if a reply returns within this timeout period.    | `3000 ms`
+| `oci-network-load-balancer.oraclecloud.com/health-check-interval`             | The interval between health checks requests, in milliseconds.		                                                                                            | `3000 ms`
+| `oci-network-load-balancer.oraclecloud.com/backend-policy`                    | The network load balancer policy for the backend set. Valid values: "TWO_TUPLE", "THREE_TUPLE", or "FIVE_TUPLE"		                                        | `"FIVE_TUPLE"`
+| `oci-network-load-balancer.oraclecloud.com/security-list-management-mode`     | Specifies the security list mode ("All", "Frontend","None") to configure how security lists are managed.		                                                | `"None"`
+
+
 [1]: https://kubernetes.io/docs/concepts/services-networking/service/#internal-load-balancer
 [2]: https://docs.us-phoenix-1.oraclecloud.com/Content/Network/Tasks/managingVCNs.htm
 [3]: https://kubernetes.io/docs/concepts/services-networking/ingress/#tls

docs/setting-block-volume-performance-using-csi.md

@@ -0,0 +1,74 @@
+# Block Volume Expansion using CSI
+
+## Setup 
+
+1. Make sure you have installed [CCM](../README.md) and [CSI](../container-storage-interface.md) version v1.19.12 or later
+
+To create a PVC backed by a block volume with a Lower Cost, Balanced, or Higher Performance performance level, set vpusPerGB in the storage class definition as follows:
+
+* for a Lower Cost performance level, set vpusPerGB: "0"
+* for a Balanced performance level, set vpusPerGB: "10"
+* for a Higher Performance performance level, set vpusPerGB: "20"
+
+## Create Storage Class for high performance
+```yaml
+apiVersion: storage.k8s.io/v1
+kind: StorageClass
+metadata:
+  name: oci-high
+provisioner: blockvolume.csi.oraclecloud.com
+parameters:
+  vpusPerGB: "20"
+reclaimPolicy: Delete
+volumeBindingMode: WaitForFirstConsumer
+allowVolumeExpansion: true
+```
+
+The value of vpusPerGB must be "0", "10", or "20". Other values are not supported.
+
+## Create PVC
+
+```yaml
+apiVersion: v1
+kind: PersistentVolumeClaim
+metadata:
+  name: oci-pvc-high
+spec:
+  storageClassName: oci-high
+  accessModes:
+    - ReadWriteOnce
+  resources:
+    requests:
+      storage: 50Gi
+```
+
+## Create POD
+
+```yaml
+apiVersion: v1
+kind: Pod
+metadata:
+  name: app1
+spec:
+  containers:
+    - name: app1
+      image: centos
+      command: ["/bin/sh"]
+      args: ["-c", "while true; do echo $(date -u) >> /data/out.txt; sleep 5; done"]
+      volumeMounts:
+        - name: persistent-storage
+          mountPath: /data
+  volumes:
+    - name: persistent-storage
+      persistentVolumeClaim:
+        claimName: oci-pvc-high
+```
+
+For more information refer [CSI BV Performance Doc][1]
+
+Note: 
+Performance of block volume can be specified at the creation itself. Performance (vpusPerGB) cannot be modified after volume is provisioned.
+CSI version 1.19.12 or later which runs on k8s cluster 1.19 or later supports block volume expansion.
+Flex volume does not support. 
+
+[1]: https://docs.oracle.com/en-us/iaas/Content/ContEng/Tasks/contengcreatingpersistentvolumeclaim.htm#contengcreatingpersistentvolumeclaim_topic_Provisioning_PVCs_on_BV_PV_Volume_performance

docs/using-network-load-balancer.md

@@ -0,0 +1,28 @@
+# Network load balancer
+
+## Setup 
+
+1. Make sure you have installed [CCM](../README.md) version v1.19.12 or later
+
+## Create Service
+
+```yaml
+apiVersion: v1
+kind: Service
+metadata:
+  name: example-nlb
+  annotations:
+    oci-network-load-balancer.oraclecloud.com/security-list-management-mode: "All"
+    oci.oraclecloud.com/load-balancer-type: nlb
+spec:
+  selector:
+    app: example-nlb
+  ports:
+    - port: 8088
+      targetPort: 80
+  type: LoadBalancer
+  externalTrafficPolicy: Local
+```
+For more info please refer [OKE NLB DOC][1]
+
+[1]: https://docs.oracle.com/en-us/iaas/Content/ContEng/Tasks/contengcreatingloadbalancer.htm#contengcreatingnetworkloadbalancers

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()
+}