gcnv san: fix for zonal pools and host group handling (#2485)

Author: zachatnetapp

storage_drivers/gcp/api/gcnv.go

@@ -55,6 +55,7 @@ var (
 	volumeNameRegex       = regexp.MustCompile(`^projects/(?P<projectNumber>[^/]+)/locations/(?P<location>[^/]+)/volumes/(?P<volume>[^/]+)$`)
 	snapshotNameRegex     = regexp.MustCompile(`^projects/(?P<projectNumber>[^/]+)/locations/(?P<location>[^/]+)/volumes/(?P<volume>[^/]+)/snapshots/(?P<snapshot>[^/]+)$`)
 	networkNameRegex      = regexp.MustCompile(`^projects/(?P<projectNumber>[^/]+)/global/networks/(?P<network>[^/]+)$`)
+	zoneSuffixRegex       = regexp.MustCompile(`-[a-z]$`)
 )
 
 // ClientConfig holds configuration data for the API driver object.
@@ -368,14 +369,16 @@ func (c Client) findAllLocationsFromCapacityPool(flexPoolsCount int) map[string]
 	return locations
 }
 
-// getLocationFromCapacityPools returns the location from the first available capacity pool.
-// Returns an error if no capacity pools are available.
-func (c Client) getLocationFromCapacityPools() (string, error) {
-	pools := c.CapacityPools()
-	if pools == nil || len(*pools) == 0 {
-		return "", fmt.Errorf("no capacity pools available")
+// getRegionalLocation returns the regional location from the backend config.
+// Host groups are regional resources and must use a regional location (e.g., "us-east4"), even when
+// the backend config specifies a zonal location. The zone suffix is stripped if present.
+// Zonal locations like "us-east4-a" are trimmed to "us-east4".
+// Regional locations like "us-east4" are returned as-is.
+func (c Client) getRegionalLocation() (string, error) {
+	if c.config.Location == "" {
+		return "", fmt.Errorf("backend config location is not set")
 	}
-	return (*pools)[0].Location, nil
+	return zoneSuffixRegex.ReplaceAllString(c.config.Location, ""), nil
 }
 
 // ///////////////////////////////////////////////////////////////////////////////
@@ -1878,9 +1881,9 @@ func (c Client) HostGroups(ctx context.Context) ([]*HostGroup, error) {
 
 	Logc(ctx).WithFields(logFields).Debug("Listing host groups via v1 protobuf SDK.")
 
-	location, err := c.getLocationFromCapacityPools()
+	location, err := c.getRegionalLocation()
 	if err != nil {
-		return []*HostGroup{}, nil
+		return nil, err
 	}
 
 	sdkCtx, cancel := context.WithTimeout(ctx, c.config.SDKTimeout)
@@ -1917,7 +1920,7 @@ func (c Client) HostGroupByName(ctx context.Context, name string) (*HostGroup, e
 
 	Logc(ctx).WithFields(logFields).Debug("Fetching host group by name via v1 protobuf SDK.")
 
-	location, err := c.getLocationFromCapacityPools()
+	location, err := c.getRegionalLocation()
 	if err != nil {
 		return nil, err
 	}
@@ -1955,7 +1958,7 @@ func (c Client) CreateHostGroup(ctx context.Context, request *HostGroupCreateReq
 
 	Logc(ctx).WithFields(logFields).Debug("Creating host group via v1 protobuf SDK.")
 
-	location, err := c.getLocationFromCapacityPools()
+	location, err := c.getRegionalLocation()
 	if err != nil {
 		return nil, err
 	}

storage_drivers/gcp/api/gcnv_test.go

@@ -1943,3 +1943,41 @@ func TestListComputeZones_Success(t *testing.T) {
 	// making it difficult to mock without a full gRPC mock setup.
 	t.Skip("ListComputeZones requires compute client mocking which is complex")
 }
+
+func TestGetRegionalLocation(t *testing.T) {
+	t.Run("regional config location returned as-is", func(t *testing.T) {
+		sdk := getFakeSDK(false)
+		sdk.config.Location = "us-east4"
+
+		location, err := sdk.getRegionalLocation()
+		assert.NoError(t, err)
+		assert.Equal(t, "us-east4", location)
+	})
+
+	t.Run("zonal config location stripped to region", func(t *testing.T) {
+		sdk := getFakeSDK(false)
+		sdk.config.Location = "us-east4-a"
+
+		location, err := sdk.getRegionalLocation()
+		assert.NoError(t, err)
+		assert.Equal(t, "us-east4", location)
+	})
+
+	t.Run("different region zonal stripped", func(t *testing.T) {
+		sdk := getFakeSDK(false)
+		sdk.config.Location = "europe-west1-b"
+
+		location, err := sdk.getRegionalLocation()
+		assert.NoError(t, err)
+		assert.Equal(t, "europe-west1", location)
+	})
+
+	t.Run("empty location returns error", func(t *testing.T) {
+		sdk := getFakeSDK(false)
+		sdk.config.Location = ""
+
+		_, err := sdk.getRegionalLocation()
+		assert.Error(t, err)
+		assert.Contains(t, err.Error(), "backend config location is not set")
+	})
+}

storage_drivers/gcp/gcp_gcnv_san_test.go

@@ -2914,6 +2914,22 @@ func TestSANDriver_ReconcileNodeAccess_KeepOrphanedHostGroupsWithVolumes(t *test
 	assert.NoError(t, err, "ReconcileNodeAccess should succeed")
 }
 
+func TestSANDriver_ReconcileNodeAccess_HostGroupsError(t *testing.T) {
+	mockAPI, driver := newMockSANDriver(t)
+
+	nodes := []*models.Node{
+		{Name: "node-1", IQN: "iqn.xxx.node-1"},
+	}
+
+	// Mock expectations - HostGroups returns error (e.g., location not configured)
+	mockAPI.EXPECT().RefreshGCNVResources(ctx).Return(nil).Times(1)
+	mockAPI.EXPECT().HostGroups(ctx).Return(nil, fmt.Errorf("backend config location is not set"))
+
+	err := driver.ReconcileNodeAccess(ctx, nodes, "", "")
+	assert.Error(t, err, "ReconcileNodeAccess should fail when HostGroups fails")
+	assert.Contains(t, err.Error(), "could not list host groups", "Error should mention host groups")
+}
+
 // ============================================================================
 // Snapshot tests
 // ============================================================================