Merge pull request #1201 from savirg/custom-perf

Add support for custom performance

Author: k8s-ci-robot

pkg/cloud_provider/file/fake.go

@@ -159,6 +159,16 @@ func (manager *fakeServiceManager) ResizeInstance(ctx context.Context, obj *Serv
 	return instance, nil
 }
 
+func (manager *fakeServiceManager) UpdateInstancePerformance(ctx context.Context, obj *ServiceInstance, perfConfig *PerformanceConfig) error {
+	if perfConfig == nil {
+		return fmt.Errorf("performance config cannot be nil")
+	}
+	if perfConfig.FixedIOPS <= 0 && perfConfig.IOPSPerTB <= 0 {
+		return fmt.Errorf("performance config must have either FixedIOPS or IOPSPerTB set")
+	}
+	return nil
+}
+
 func (manager *fakeServiceManager) CreateBackup(ctx context.Context, backupInfo *BackupInfo) (*filev1beta1.Backup, error) {
 	if backupInfo.SourceInstanceName == "" || backupInfo.SourceShare == "" || backupInfo.SourceVolumeId == "" || backupInfo.BackupURI == "" {
 		return nil, fmt.Errorf("BackupInfo fields are not set %+v", backupInfo)
@@ -256,6 +266,14 @@ func (m *fakeBlockingServiceManager) HasOperations(ctx context.Context, obj *Ser
 	return false, nil
 }
 
+// UpdateInstancePerformance is a no-op in fake blocking service for testing.
+func (m *fakeBlockingServiceManager) UpdateInstancePerformance(ctx context.Context, obj *ServiceInstance, perfConfig *PerformanceConfig) error {
+	execute := make(chan struct{})
+	m.OperationUnblocker <- execute
+	<-execute
+	return m.fakeServiceManager.UpdateInstancePerformance(ctx, obj, perfConfig)
+}
+
 // Multishare fake functions defined here
 func (manager *fakeServiceManager) GetMultishareInstance(ctx context.Context, obj *MultishareInstance) (*MultishareInstance, error) {
 	instance, ok := manager.createdMultishareInstance[obj.Name]

pkg/cloud_provider/file/file.go

@@ -42,6 +42,12 @@ import (
 	filev1beta1multishare "google.golang.org/api/file/v1beta1"
 )
 
+// PerformanceConfig holds performance parameters for a Filestore instance.
+type PerformanceConfig struct {
+	FixedIOPS int64 // Fixed IOPS (input/output operations per second).
+	IOPSPerTB int64 // IOPS per TiB for density-based provisioning.
+}
+
 const (
 	testEndpoint    = "test-file.sandbox.googleapis.com"
 	stagingEndpoint = "staging-file.sandbox.googleapis.com"
@@ -99,18 +105,19 @@ type ListFilter struct {
 }
 
 type ServiceInstance struct {
-	Project          string
-	Name             string
-	Location         string
-	Tier             string
-	Network          Network
-	Volume           Volume
-	Labels           map[string]string
-	State            string
-	KmsKeyName       string
-	BackupSource     string
-	NfsExportOptions []*NfsExportOptions
-	Protocol         string
+	Project           string
+	Name              string
+	Location          string
+	Tier              string
+	Network           Network
+	Volume            Volume
+	Labels            map[string]string
+	State             string
+	KmsKeyName        string
+	BackupSource      string
+	NfsExportOptions  []*NfsExportOptions
+	Protocol          string
+	PerformanceConfig *PerformanceConfig
 }
 
 type Volume struct {
@@ -166,6 +173,7 @@ type Service interface {
 	GetInstance(ctx context.Context, obj *ServiceInstance) (*ServiceInstance, error)
 	ListInstances(ctx context.Context, obj *ServiceInstance) ([]*ServiceInstance, error)
 	ResizeInstance(ctx context.Context, obj *ServiceInstance) (*ServiceInstance, error)
+	UpdateInstancePerformance(ctx context.Context, obj *ServiceInstance, perfConfig *PerformanceConfig) error
 	GetBackup(ctx context.Context, backupUri string) (*Backup, error)
 	CreateBackup(ctx context.Context, backupInfo *BackupInfo) (*filev1beta1.Backup, error)
 	DeleteBackup(ctx context.Context, backupId string) error
@@ -297,7 +305,26 @@ func (manager *gcfsServiceManager) CreateInstance(ctx context.Context, obj *Serv
 		Protocol:   obj.Protocol,
 	}
 
-	klog.V(4).Infof("Creating instance %q: location %v, tier %q, capacity %v, network %q, ipRange %q, connectMode %q, KmsKeyName %q, labels %v, backup source %q, protocol %v",
+	// Add performance config if provided. Only one of FixedIOPS or IOPSPerTB
+	// may be set at a time.
+	if obj.PerformanceConfig != nil {
+		if obj.PerformanceConfig.FixedIOPS > 0 && obj.PerformanceConfig.IOPSPerTB > 0 {
+			return nil, fmt.Errorf("performance config must have only one of FixedIOPS or IOPSPerTB set")
+		}
+		instance.PerformanceConfig = &filev1beta1.PerformanceConfig{}
+		if obj.PerformanceConfig.FixedIOPS > 0 {
+			instance.PerformanceConfig.FixedIops = &filev1beta1.FixedIOPS{
+				MaxIops: obj.PerformanceConfig.FixedIOPS,
+			}
+		}
+		if obj.PerformanceConfig.IOPSPerTB > 0 {
+			instance.PerformanceConfig.IopsPerTb = &filev1beta1.IOPSPerTB{
+				MaxIopsPerTb: obj.PerformanceConfig.IOPSPerTB,
+			}
+		}
+	}
+
+	klog.V(4).Infof("Creating instance %q: location %v, tier %q, capacity %v, network %q, ipRange %q, connectMode %q, KmsKeyName %q, labels %v, backup source %q, protocol %v, performance config %+v",
 		obj.Name,
 		obj.Location,
 		instance.Tier,
@@ -308,7 +335,8 @@ func (manager *gcfsServiceManager) CreateInstance(ctx context.Context, obj *Serv
 		instance.KmsKeyName,
 		instance.Labels,
 		instance.FileShares[0].SourceBackup,
-		obj.Protocol)
+		obj.Protocol,
+		obj.PerformanceConfig)
 	op, err := manager.instancesService.Create(locationURI(obj.Project, obj.Location), instance).InstanceId(obj.Name).Context(ctx).Do()
 	if err != nil {
 		klog.Errorf("CreateInstance operation failed for instance %v: %v", obj.Name, err)
@@ -356,6 +384,19 @@ func cloudInstanceToServiceInstance(instance *filev1beta1.Instance) (*ServiceIns
 	if len(instance.Networks[0].IpAddresses) > 0 {
 		ip = instance.Networks[0].IpAddresses[0]
 	}
+	// Map performance config if present
+	var perfCfg *PerformanceConfig
+	if instance.PerformanceConfig != nil {
+		perf := &PerformanceConfig{}
+		if instance.PerformanceConfig.FixedIops != nil {
+			perf.FixedIOPS = instance.PerformanceConfig.FixedIops.MaxIops
+		}
+		if instance.PerformanceConfig.IopsPerTb != nil {
+			perf.IOPSPerTB = instance.PerformanceConfig.IopsPerTb.MaxIopsPerTb
+		}
+		perfCfg = perf
+	}
+
 	return &ServiceInstance{
 		Project:  project,
 		Location: location,
@@ -371,11 +412,12 @@ func cloudInstanceToServiceInstance(instance *filev1beta1.Instance) (*ServiceIns
 			ReservedIpRange: instance.Networks[0].ReservedIpRange,
 			ConnectMode:     instance.Networks[0].ConnectMode,
 		},
-		KmsKeyName:   instance.KmsKeyName,
-		Labels:       instance.Labels,
-		State:        instance.State,
-		BackupSource: instance.FileShares[0].SourceBackup,
-		Protocol:     instance.Protocol,
+		KmsKeyName:        instance.KmsKeyName,
+		Labels:            instance.Labels,
+		State:             instance.State,
+		BackupSource:      instance.FileShares[0].SourceBackup,
+		Protocol:          instance.Protocol,
+		PerformanceConfig: perfCfg,
 	}, nil
 }
 
@@ -513,6 +555,56 @@ func (manager *gcfsServiceManager) ResizeInstance(ctx context.Context, obj *Serv
 	return instance, nil
 }
 
+// UpdateInstancePerformance updates the performance configuration of a Filestore instance.
+func (manager *gcfsServiceManager) UpdateInstancePerformance(ctx context.Context, obj *ServiceInstance, perfConfig *PerformanceConfig) error {
+	if perfConfig == nil {
+		return fmt.Errorf("performance config cannot be nil")
+	}
+
+	instanceuri := instanceURI(obj.Project, obj.Location, obj.Name)
+
+	// Validate config and log the intended change
+	if perfConfig.FixedIOPS <= 0 && perfConfig.IOPSPerTB <= 0 {
+		return fmt.Errorf("performance config must have either FixedIOPS or IOPSPerTB set")
+	}
+	if perfConfig.FixedIOPS > 0 && perfConfig.IOPSPerTB > 0 {
+		return fmt.Errorf("performance config must have only one of FixedIOPS or IOPSPerTB set")
+	}
+
+	// Create a file instance for the Patch request with performance config
+	betaObj := &filev1beta1.Instance{
+		PerformanceConfig: &filev1beta1.PerformanceConfig{},
+	}
+
+	if perfConfig.FixedIOPS > 0 {
+		klog.V(4).Infof("Updating instance %q with FixedIOPS: %d", obj.Name, perfConfig.FixedIOPS)
+		betaObj.PerformanceConfig.FixedIops = &filev1beta1.FixedIOPS{
+			MaxIops: perfConfig.FixedIOPS,
+		}
+	}
+	if perfConfig.IOPSPerTB > 0 {
+		klog.V(4).Infof("Updating instance %q with IOPSPerTB: %d", obj.Name, perfConfig.IOPSPerTB)
+		betaObj.PerformanceConfig.IopsPerTb = &filev1beta1.IOPSPerTB{
+			MaxIopsPerTb: perfConfig.IOPSPerTB,
+		}
+	}
+
+	klog.V(4).Infof("Patching instance %q with performance configuration: %+v", obj.Name, perfConfig)
+	op, err := manager.instancesService.Patch(instanceuri, betaObj).UpdateMask("performanceConfig").Context(ctx).Do()
+	if err != nil {
+		return fmt.Errorf("patch operation failed for performance update: %w", err)
+	}
+
+	klog.V(4).Infof("For instance %s, waiting for performance update op %v to complete", instanceuri, op.Name)
+	err = manager.waitForOp(ctx, op)
+	if err != nil {
+		return fmt.Errorf("WaitFor performance update op %s failed: %w", op.Name, err)
+	}
+
+	klog.Infof("Successfully updated performance configuration for instance %q", obj.Name)
+	return nil
+}
+
 func (manager *gcfsServiceManager) GetBackup(ctx context.Context, backupUri string) (*Backup, error) {
 	backup, err := manager.backupService.Get(backupUri).Context(ctx).Do()
 	if err != nil {

pkg/cloud_provider/file/file_test.go

@@ -2,11 +2,16 @@ package file
 
 import (
 	"context"
+	"encoding/json"
 	"fmt"
 	"net/http"
+	"net/http/httptest"
 	"strings"
 	"testing"
 
+	filev1beta1 "google.golang.org/api/file/v1beta1"
+	"google.golang.org/api/option"
+
 	"github.com/googleapis/gax-go/v2/apierror"
 	"google.golang.org/api/googleapi"
 	"google.golang.org/grpc/codes"
@@ -141,6 +146,153 @@ func TestCompareInstances(t *testing.T) {
 	}
 }
 
+func TestUpdateInstancePerformance(t *testing.T) {
+	ctx := context.Background()
+	mgr := &gcfsServiceManager{}
+
+	// nil perfConfig -> error
+	if err := mgr.UpdateInstancePerformance(ctx, &ServiceInstance{}, nil); err == nil {
+		t.Fatalf("expected error for nil perfConfig")
+	}
+
+	// zero values -> error
+	if err := mgr.UpdateInstancePerformance(ctx, &ServiceInstance{}, &PerformanceConfig{}); err == nil {
+		t.Fatalf("expected error for empty perfConfig")
+	}
+
+	// success path using httptest server to mock PATCH and operations GET
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		// return an operation for PATCH and return done=true for GET on operations
+		if r.Method == "PATCH" && strings.Contains(r.URL.Path, "/instances/") {
+			w.Header().Set("Content-Type", "application/json")
+			fmt.Fprint(w, `{"name":"projects/proj/locations/loc/operations/op1","done":false}`)
+			return
+		}
+		if r.Method == "GET" && strings.Contains(r.URL.Path, "/operations/") {
+			w.Header().Set("Content-Type", "application/json")
+			fmt.Fprint(w, `{"name":"projects/proj/locations/loc/operations/op1","done":true}`)
+			return
+		}
+		http.NotFound(w, r)
+	}))
+	defer ts.Close()
+
+	// create file service pointing to test server
+	svc, err := filev1beta1.NewService(ctx, option.WithEndpoint(ts.URL+"/"), option.WithHTTPClient(ts.Client()))
+	if err != nil {
+		t.Fatalf("failed to create file service: %v", err)
+	}
+
+	mgr.instancesService = filev1beta1.NewProjectsLocationsInstancesService(svc)
+	mgr.operationsService = filev1beta1.NewProjectsLocationsOperationsService(svc)
+
+	si := &ServiceInstance{Project: "proj", Location: "loc", Name: "name"}
+	perf := &PerformanceConfig{FixedIOPS: 100}
+
+	if err := mgr.UpdateInstancePerformance(ctx, si, perf); err != nil {
+		t.Fatalf("expected success, got error: %v", err)
+	}
+}
+
+func TestCreateInstance_PerformanceConfig(t *testing.T) {
+	ctx := context.Background()
+	mgr := &gcfsServiceManager{}
+
+	// Mock server to capture Create request and return operation and instance
+	var captured filev1beta1.Instance
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.Method == "POST" && strings.Contains(r.URL.Path, "/instances") {
+			if err := json.NewDecoder(r.Body).Decode(&captured); err != nil {
+				http.Error(w, err.Error(), http.StatusBadRequest)
+				return
+			}
+			w.Header().Set("Content-Type", "application/json")
+			fmt.Fprint(w, `{"name":"projects/proj/locations/loc/operations/op1","done":false}`)
+			return
+		}
+		if r.Method == "GET" && strings.Contains(r.URL.Path, "/operations/") {
+			w.Header().Set("Content-Type", "application/json")
+			fmt.Fprint(w, `{"name":"projects/proj/locations/loc/operations/op1","done":true}`)
+			return
+		}
+		if r.Method == "GET" && strings.Contains(r.URL.Path, "/instances/") {
+			// return the instance including performance config (only FixedIops)
+			inst := filev1beta1.Instance{
+				Name:       "projects/proj/locations/loc/instances/name",
+				Tier:       "tier",
+				FileShares: []*filev1beta1.FileShareConfig{{Name: "vol", CapacityGb: 10}},
+				Networks:   []*filev1beta1.NetworkConfig{{Network: "net"}},
+				PerformanceConfig: &filev1beta1.PerformanceConfig{
+					FixedIops: &filev1beta1.FixedIOPS{MaxIops: 42},
+				},
+			}
+			w.Header().Set("Content-Type", "application/json")
+			_ = json.NewEncoder(w).Encode(&inst)
+			return
+		}
+		http.NotFound(w, r)
+	}))
+	defer ts.Close()
+
+	svc, err := filev1beta1.NewService(ctx, option.WithEndpoint(ts.URL+"/"), option.WithHTTPClient(ts.Client()))
+	if err != nil {
+		t.Fatalf("failed to create file service: %v", err)
+	}
+
+	mgr.instancesService = filev1beta1.NewProjectsLocationsInstancesService(svc)
+	mgr.operationsService = filev1beta1.NewProjectsLocationsOperationsService(svc)
+
+	si := &ServiceInstance{Project: "proj", Location: "loc", Name: "name", PerformanceConfig: &PerformanceConfig{FixedIOPS: 1000}, Volume: Volume{Name: "vol"}, Network: Network{Name: "net"}}
+
+	_, err = mgr.CreateInstance(ctx, si)
+	if err != nil {
+		t.Fatalf("CreateInstance failed: %v", err)
+	}
+
+	if captured.PerformanceConfig == nil {
+		t.Fatalf("expected PerformanceConfig to be sent in Create request")
+	}
+	if captured.PerformanceConfig.FixedIops == nil || captured.PerformanceConfig.FixedIops.MaxIops != 1000 {
+		t.Fatalf("unexpected FixedIops in request: %+v", captured.PerformanceConfig.FixedIops)
+	}
+	if captured.PerformanceConfig.IopsPerTb != nil {
+		t.Fatalf("did not expect IopsPerTb in request: %+v", captured.PerformanceConfig.IopsPerTb)
+	}
+
+	siBoth := &ServiceInstance{Project: "proj", Location: "loc", Name: "name", PerformanceConfig: &PerformanceConfig{FixedIOPS: 1, IOPSPerTB: 2}, Volume: Volume{Name: "vol"}, Network: Network{Name: "net"}}
+	_, err = mgr.CreateInstance(ctx, siBoth)
+	if err == nil {
+		t.Fatalf("expected CreateInstance to fail when both performance params set")
+	}
+}
+
+func TestCloudInstanceToServiceInstance_PerformanceConfig(t *testing.T) {
+	inst := &filev1beta1.Instance{
+		Name:       fmt.Sprintf("projects/%s/locations/%s/instances/%s", "proj", "loc", "name"),
+		Tier:       "tier",
+		FileShares: []*filev1beta1.FileShareConfig{{Name: "vol", CapacityGb: 10}},
+		Networks:   []*filev1beta1.NetworkConfig{{Network: "net", IpAddresses: []string{"1.2.3.4"}}},
+		PerformanceConfig: &filev1beta1.PerformanceConfig{
+			FixedIops: &filev1beta1.FixedIOPS{MaxIops: 1234},
+			IopsPerTb: &filev1beta1.IOPSPerTB{MaxIopsPerTb: 5678},
+		},
+	}
+
+	si, err := cloudInstanceToServiceInstance(inst)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if si.PerformanceConfig == nil {
+		t.Fatalf("expected PerformanceConfig to be populated")
+	}
+	if si.PerformanceConfig.FixedIOPS != 1234 {
+		t.Fatalf("expected FixedIOPS 1234, got %d", si.PerformanceConfig.FixedIOPS)
+	}
+	if si.PerformanceConfig.IOPSPerTB != 5678 {
+		t.Fatalf("expected IOPSPerTB 5678, got %d", si.PerformanceConfig.IOPSPerTB)
+	}
+}
+
 func TestGetInstanceNameFromURI(t *testing.T) {
 	cases := []struct {
 		name      string