Adding CIDR reservation functionality

KNOWN_ISSUES.md

@@ -1,14 +1,17 @@
 # v0.1.0
-* IP conflicts: By default, Cloud Filestore creation will pick an unused IP subnet to allocate its
+* IP conflicts: By default, Cloud Filestore creation will pick an unused IP range to allocate its
   service in. This may conflict with other GCP services that do not explicitly
-  reserve IP subnets, such as GKE non-IP alias clusters or GKE TPUs. To avoid
-  IP conflicts, it is recommended to explicitly allocate IP subnets to each GCP
-  service, and this plugin.
-    * IP reservation for this driver is a future enhancement.
-    * GKE Pod and Service CIDRs can be reserved during cluster creation using the
-      `--cluster-ipv4-cidr` and `--services-ipv4-cidr` flags.
-    * GKE TPU CIDRs can be reserved during cluster creation using the
-      `--tpu-ipv4-cidr` flag.
+  reserve IP ranges, such as GKE non-IP alias clusters or GKE TPUs. To avoid
+  IP conflicts, it is recommended to either:
+    * Use a GKE cluster with [Alias IPs](https://cloud.google.com/kubernetes-engine/docs/how-to/alias-ips)
+        * This will prevent IP conflicts with GKE Pod and Service IPs, but not TPUs.
+    * Explicitly allocate IP ranges to each GCP service, and this plugin.
+        * IP range reservation for this driver is a future enhancement.
+        * GKE Pod and Service CIDRs can be reserved during [cluster creation](https://cloud.google.com/sdk/gcloud/reference/container/clusters/create)
+          using the `--cluster-ipv4-cidr` flag.
+        * GKE TPU CIDRs can be reserved during [cluster
+          creation](https://cloud.google.com/sdk/gcloud/reference/beta/container/clusters/create)
+          using the `--tpu-ipv4-cidr` flag.
 * Locality of CSI driver and Cloud Filestore instances: If no location is specified in
   the CreateVolume parameters, then by default the driver will pick the zone
   that it is currently running in. This could result in CreateVolume failures if

pkg/cloud_provider/file/fake.go

@@ -72,3 +72,27 @@ func (manager *fakeServiceManager) GetInstance(ctx context.Context, obj *Service
 		},
 	}
 }
+
+func (manager *fakeServiceManager) ListInstances(ctx context.Context, obj *ServiceInstance) ([]*ServiceInstance, error) {
+	instances := []*ServiceInstance{
+		{
+			Project:  "test-project",
+			Location: "test-location",
+			Name:     "test",
+			Tier:     "test_tier",
+			Network: Network{
+				ReservedIpRange: "192.168.92.32/29",
+			},
+		},
+		{
+			Project:  "test-project",
+			Location: "test-location",
+			Name:     "test",
+			Tier:     "test_tier",
+			Network: Network{
+				ReservedIpRange: "192.168.92.40/29",
+			},
+		},
+	}
+	return instances, nil
+}

pkg/cloud_provider/file/file.go

@@ -56,6 +56,7 @@ type Service interface {
 	CreateInstance(ctx context.Context, obj *ServiceInstance) (*ServiceInstance, error)
 	DeleteInstance(ctx context.Context, obj *ServiceInstance) error
 	GetInstance(ctx context.Context, obj *ServiceInstance) (*ServiceInstance, error)
+	ListInstances(ctx context.Context, obj *ServiceInstance) ([]*ServiceInstance, error)
 }
 
 type gcfsServiceManager struct {
@@ -231,6 +232,25 @@ func (manager *gcfsServiceManager) DeleteInstance(ctx context.Context, obj *Serv
 	return nil
 }
 
+// ListInstances returns a list of active instances in a project at a specific location
+func (manager *gcfsServiceManager) ListInstances(ctx context.Context, obj *ServiceInstance) ([]*ServiceInstance, error) {
+	// Calling cloud provider service to get list of active instances. - indicates we are looking for instances in all the locations for a project
+	instances, err := manager.instancesService.List(locationURI(obj.Project, "-")).Context(ctx).Do()
+	if err != nil {
+		return nil, err
+	}
+
+	var activeInstances []*ServiceInstance
+	for _, activeInstance := range instances.Instances {
+		serviceInstance, err := cloudInstanceToServiceInstance(activeInstance)
+		if err != nil {
+			return nil, err
+		}
+		activeInstances = append(activeInstances, serviceInstance)
+	}
+	return activeInstances, nil
+}
+
 func (manager *gcfsServiceManager) waitForOp(ctx context.Context, op *beta.Operation) error {
 	return wait.Poll(5*time.Second, 5*time.Minute, func() (bool, error) {
 		pollOp, err := manager.operationsService.Get(op.Name).Context(ctx).Do()

pkg/csi_driver/controller.go

@@ -48,9 +48,10 @@ const (
 
 // CreateVolume parameters
 const (
-	paramTier     = "tier"
-	paramLocation = "location"
-	paramNetwork  = "network"
+	paramTier             = "tier"
+	paramLocation         = "location"
+	paramNetwork          = "network"
+	paramReservedIPV4CIDR = "reserved-ipv4-cidr"
 )
 
 // controllerServer handles volume provisioning
@@ -62,9 +63,11 @@ type controllerServerConfig struct {
 	driver      *GCFSDriver
 	fileService file.Service
 	metaService metadata.Service
+	ipAllocator *util.IPAllocator
 }
 
 func newControllerServer(config *controllerServerConfig) csi.ControllerServer {
+	config.ipAllocator = util.NewIPAllocator(make(map[string]bool))
 	return &controllerServer{config: config}
 }
 
@@ -101,6 +104,23 @@ func (s *controllerServer) CreateVolume(ctx context.Context, req *csi.CreateVolu
 			return nil, status.Error(codes.AlreadyExists, err.Error())
 		}
 	} else {
+		// If we are creating a new instance, we need pick an unused /29 range from reserved-ipv4-cidr
+		// If the param was not provided, we default reservedIPRange to "" and cloud provider takes care of the allocation
+		if reservedIPV4CIDR, ok := req.GetParameters()[paramReservedIPV4CIDR]; ok {
+			reservedIPRange, err := s.reserveIPRange(ctx, newFiler, reservedIPV4CIDR)
+
+			// Possible cases are 1) CreateInstanceAborted, 2)CreateInstance running in background
+			// The ListInstances response will contain the reservedIPRange if the operation was started
+			// In case of abort, the /29 IP is released and available for reservation
+			defer s.config.ipAllocator.ReleaseIPRange(reservedIPRange)
+			if err != nil {
+				return nil, err
+			}
+
+			// Adding the reserved IP range to the instance object
+			newFiler.Network.ReservedIpRange = reservedIPRange
+		}
+
 		// Create the instance
 		filer, err = s.config.fileService.CreateInstance(ctx, newFiler)
 		if err != nil {
@@ -110,6 +130,33 @@ func (s *controllerServer) CreateVolume(ctx context.Context, req *csi.CreateVolu
 	return &csi.CreateVolumeResponse{Volume: fileInstanceToCSIVolume(filer, modeInstance)}, nil
 }
 
+// reserveIPRange returns the available IP in the cidr
+func (s *controllerServer) reserveIPRange(ctx context.Context, filer *file.ServiceInstance, cidr string) (string, error) {
+	cloudInstancesReservedIPRanges, err := s.getCloudInstancesReservedIPRanges(ctx, filer)
+	if err != nil {
+		return "", err
+	}
+	unreservedIPBlock, err := s.config.ipAllocator.GetUnreservedIPRange(cidr, cloudInstancesReservedIPRanges)
+	if err != nil {
+		return "", err
+	}
+	return unreservedIPBlock, nil
+}
+
+// getCloudInstancesReservedIPRanges gets the list of reservedIPRanges from cloud instances
+func (s *controllerServer) getCloudInstancesReservedIPRanges(ctx context.Context, filer *file.ServiceInstance) (map[string]bool, error) {
+	instances, err := s.config.fileService.ListInstances(ctx, filer)
+	if err != nil {
+		return nil, status.Error(codes.Aborted, err.Error())
+	}
+	// Initialize an empty reserved list. It will be populated with all the reservedIPRanges obtained from the cloud instances
+	cloudInstancesReservedIPRanges := make(map[string]bool)
+	for _, instance := range instances {
+		cloudInstancesReservedIPRanges[instance.Network.ReservedIpRange] = true
+	}
+	return cloudInstancesReservedIPRanges, nil
+}
+
 // DeleteVolume deletes a GCFS instance
 func (s *controllerServer) DeleteVolume(ctx context.Context, req *csi.DeleteVolumeRequest) (*csi.DeleteVolumeResponse, error) {
 	glog.V(4).Infof("DeleteVolume called with request %v", *req)
@@ -211,7 +258,6 @@ func (s *controllerServer) generateNewFileInstance(name string, capBytes int64,
 	tier := defaultTier
 	network := defaultNetwork
 	location := s.config.metaService.GetZone()
-
 	// Validate parameters (case-insensitive).
 	for k, v := range params {
 		switch strings.ToLower(k) {
@@ -222,7 +268,11 @@ func (s *controllerServer) generateNewFileInstance(name string, capBytes int64,
 			location = v
 		case paramNetwork:
 			network = v
-		// Unused
+
+		// Ignore the cidr flag as it is not passed to the cloud provider
+		// It will be used to get unreserved IP in the reserveIPV4Range function
+		case paramReservedIPV4CIDR:
+			continue
 		case "csiprovisionersecretname", "csiprovisionersecretnamespace":
 		default:
 			return nil, fmt.Errorf("invalid parameter %q", k)
@@ -235,7 +285,6 @@ func (s *controllerServer) generateNewFileInstance(name string, capBytes int64,
 		Tier:     tier,
 		Network: file.Network{
 			Name: network,
-			// ReservedIpRange: "10.3.0.0/29", // TODO
 		},
 		Volume: file.Volume{
 			Name:      newInstanceVolume,

pkg/csi_driver/controller_test.go

@@ -28,12 +28,13 @@ import (
 )
 
 const (
-	testProject   = "test-project"
-	testLocation  = "test-location"
-	testIp        = "test-ip"
-	testCSIVolume = "test-csi"
-	testVolumeId  = "modeInstance/test-location/test-csi/vol1"
-	testBytes     = 1 * util.Tb
+	testProject          = "test-project"
+	testLocation         = "test-location"
+	testIp               = "test-ip"
+	testCSIVolume        = "test-csi"
+	testVolumeId         = "modeInstance/test-location/test-csi/vol1"
+	testReservedIPV4CIDR = "192.168.92.0/26"
+	testBytes            = 1 * util.Tb
 )
 
 func initTestController(t *testing.T) csi.ControllerServer {