fix(gcp): select explicitly typed vcpu recommendation amounts

providers/gcp/recommendations_pagination_sdk_test.go

@@ -0,0 +1,131 @@
+package gcp
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"net"
+	"net/http"
+	"net/http/httptest"
+	"strconv"
+	"sync/atomic"
+	"testing"
+	"time"
+
+	"cloud.google.com/go/recommender/apiv1/recommenderpb"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"google.golang.org/api/option"
+	"google.golang.org/grpc"
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/credentials/insecure"
+	"google.golang.org/grpc/status"
+	"google.golang.org/grpc/test/bufconn"
+
+	"github.com/LeanerCloud/cloud-commitments-go/pkg/common"
+)
+
+func TestComputeRecommendationsSDKPageBudget(t *testing.T) {
+	for _, tc := range []struct {
+		name                               string
+		pages, items, dismissed, wantCalls int
+		repeat, cancel, denied, cap        bool
+	}{
+		{name: "exactly20items", pages: 1, items: 20, wantCalls: 1},
+		{name: "moreThan20items", pages: 1, items: 21, wantCalls: 1},
+		{name: "dismissedDoNotConsumeBudget", pages: 1, items: 2, dismissed: 25, wantCalls: 1},
+		{name: "exactly20pages", pages: 20, items: 2, wantCalls: 20},
+		{name: "over20pages", pages: 21, items: 2, wantCalls: 20, cap: true},
+		{name: "empty20pages", pages: 20, wantCalls: 20},
+		{name: "emptyOver20pages", pages: 21, wantCalls: 20, cap: true},
+		{name: "repeatedEmptyToken", pages: 21, repeat: true, wantCalls: 20, cap: true},
+		{name: "cancelSecondPage", pages: 2, items: 2, cancel: true, wantCalls: 2},
+		{name: "deniedSecondPage", pages: 2, items: 2, denied: true, wantCalls: 2},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
+			defer cancel()
+			regions := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				assert.Equal(t, "/compute/v1/projects/recommendation-project/regions", r.URL.Path)
+				w.Header().Set("Content-Type", "application/json")
+				fmt.Fprint(w, `{"items":[{"name":"us-central1","status":"UP"}]}`)
+			}))
+			defer regions.Close()
+			listener := bufconn.Listen(1024 * 1024)
+			defer listener.Close()
+			server := grpc.NewServer()
+			var calls atomic.Int32
+			recommenderpb.RegisterRecommenderServer(server, &recommendationSDKServer{list: func(_ context.Context, req *recommenderpb.ListRecommendationsRequest) (*recommenderpb.ListRecommendationsResponse, error) {
+				page := int(calls.Add(1))
+				assert.Equal(t, "projects/recommendation-project/locations/us-central1/recommenders/google.compute.commitment.UsageCommitmentRecommender", req.GetParent())
+				expectedToken := ""
+				if page > 1 {
+					expectedToken = strconv.Itoa(page)
+				}
+				if page > 1 && tc.repeat {
+					expectedToken = "repeat"
+				}
+				assert.Equal(t, expectedToken, req.GetPageToken())
+				if page == 2 && tc.cancel {
+					cancel()
+					return nil, status.Error(codes.Canceled, "canceled during second page")
+				}
+				if page == 2 && tc.denied {
+					return nil, status.Error(codes.PermissionDenied, "second page denied")
+				}
+				response := &recommenderpb.ListRecommendationsResponse{}
+				for i := 0; i < tc.items+tc.dismissed; i++ {
+					state := recommenderpb.RecommendationStateInfo_ACTIVE
+					if i < tc.dismissed {
+						state = recommenderpb.RecommendationStateInfo_DISMISSED
+					}
+					rec := sdkCostRecommendation(10, state)
+					rec.Name = fmt.Sprintf("page-%d-item-%d", page, i)
+					rec.Content = sdkResourceAmounts([]string{"VCPU", "MEMORY"}, true, "us-central1")
+					response.Recommendations = append(response.Recommendations, rec)
+				}
+				if page < tc.pages {
+					response.NextPageToken = strconv.Itoa(page + 1)
+				}
+				if tc.repeat {
+					response.NextPageToken = "repeat"
+				}
+				// Bound a broken implementation without hiding its extra fetches.
+				if page > 21 {
+					return nil, status.Error(codes.InvalidArgument, "test page safety bound")
+				}
+				return response, nil
+			}})
+			serveDone := make(chan error, 1)
+			go func() { serveDone <- server.Serve(listener) }()
+			defer func() { server.Stop(); assert.NoError(t, <-serveDone) }()
+			provider := NewProviderWithProject(ctx, "recommendation-project", option.WithoutAuthentication(), option.WithEndpoint(regions.URL),
+				option.WithGRPCDialOption(grpc.WithTransportCredentials(insecure.NewCredentials())),
+				option.WithGRPCDialOption(grpc.WithContextDialer(func(ctx context.Context, _ string) (net.Conn, error) { return listener.DialContext(ctx) })))
+			client, err := provider.GetRecommendationsClient(ctx)
+			require.NoError(t, err)
+			recs, err := client.GetRecommendationsForService(ctx, common.ServiceCompute)
+			assert.Equal(t, tc.wantCalls, int(calls.Load()))
+			switch {
+			case tc.cap:
+				require.ErrorContains(t, err, "page cap (20 pages)")
+				assert.Nil(t, recs)
+			case tc.cancel:
+				require.Error(t, err)
+				assert.True(t, errors.Is(err, context.Canceled) || status.Code(err) == codes.Canceled, "cancellation cause: %v", err)
+				assert.Nil(t, recs)
+			case tc.denied:
+				require.Error(t, err)
+				assert.Equal(t, codes.PermissionDenied, status.Code(err))
+				assert.Nil(t, recs)
+			default:
+				require.NoError(t, err)
+				require.Len(t, recs, tc.pages*tc.items)
+				for _, rec := range recs {
+					assert.Equal(t, 4, rec.Count)
+					assert.Equal(t, common.ComputeDetails{MemoryGB: 6}, rec.Details)
+				}
+			}
+		})
+	}
+}

providers/gcp/recommendations_sdk_test.go

@@ -22,6 +22,7 @@ import (
 	"google.golang.org/grpc/metadata"
 	"google.golang.org/grpc/status"
 	"google.golang.org/grpc/test/bufconn"
+	"google.golang.org/protobuf/types/known/structpb"
 
 	"github.com/LeanerCloud/cloud-commitments-go/pkg/common"
 )
@@ -39,14 +40,31 @@ func TestComputeRecommendationsThroughSDK(t *testing.T) {
 	const central = "projects/recommendation-project/locations/us-central1/recommenders/google.compute.commitment.UsageCommitmentRecommender"
 	const west = "projects/recommendation-project/locations/europe-west1/recommenders/google.compute.commitment.UsageCommitmentRecommender"
 	cases := []struct {
-		name    string
-		empty   bool
-		failure map[string]bool
+		name       string
+		empty      bool
+		failure    map[string]bool
+		resources  []string
+		stringVCPU bool
+		invalid    bool
 	}{
 		{name: "regional recommendations and pagination"},
 		{name: "empty success", empty: true},
 		{name: "all regions denied", failure: map[string]bool{central: true, west: true}},
 		{name: "partial regional success", failure: map[string]bool{west: true}},
+		{name: "accelerator memory cpu", resources: []string{"ACCELERATOR", "MEMORY", "VCPU"}},
+		{name: "accelerator cpu memory", resources: []string{"ACCELERATOR", "VCPU", "MEMORY"}},
+		{name: "memory accelerator cpu", resources: []string{"MEMORY", "ACCELERATOR", "VCPU"}},
+		{name: "memory cpu accelerator", resources: []string{"MEMORY", "VCPU", "ACCELERATOR"}},
+		{name: "cpu accelerator memory", resources: []string{"VCPU", "ACCELERATOR", "MEMORY"}},
+		{name: "cpu memory accelerator", resources: []string{"VCPU", "MEMORY", "ACCELERATOR"}},
+		{name: "local SSD before cpu", resources: []string{"LOCAL_SSD", "VCPU", "MEMORY"}},
+		{name: "local SSD after cpu", resources: []string{"VCPU", "MEMORY", "LOCAL_SSD"}},
+		{name: "string cpu", resources: []string{"ACCELERATOR", "VCPU", "MEMORY"}, stringVCPU: true},
+		{name: "lowercase memory", resources: []string{"memory", "VCPU"}},
+		{name: "legacy lowercase memory", resources: []string{"memory_mb", "VCPU"}},
+		{name: "unknown resource", resources: []string{"UNKNOWN", "VCPU", "MEMORY"}, invalid: true},
+		{name: "unknown after cpu", resources: []string{"VCPU", "MEMORY", "UNKNOWN"}, invalid: true},
+		{name: "missing cpu with overview", resources: []string{"ACCELERATOR", "MEMORY"}, invalid: true},
 	}
 	for _, tc := range cases {
 		t.Run(tc.name, func(t *testing.T) {
@@ -72,6 +90,13 @@ func TestComputeRecommendationsThroughSDK(t *testing.T) {
 			listener := bufconn.Listen(1024 * 1024)
 			defer listener.Close()
 			server := grpc.NewServer()
+			recommendation := func(savings int64, state recommenderpb.RecommendationStateInfo_State, region string) *recommenderpb.Recommendation {
+				rec := sdkCostRecommendation(savings, state)
+				if tc.resources != nil {
+					rec.Content = sdkResourceAmounts(tc.resources, tc.stringVCPU, region)
+				}
+				return rec
+			}
 			recommenderpb.RegisterRecommenderServer(server, &recommendationSDKServer{
 				list: func(ctx context.Context, req *recommenderpb.ListRecommendationsRequest) (*recommenderpb.ListRecommendationsResponse, error) {
 					mu.Lock()
@@ -89,19 +114,19 @@ func TestComputeRecommendationsThroughSDK(t *testing.T) {
 						return &recommenderpb.ListRecommendationsResponse{}, nil
 					}
 					if req.GetParent() == west {
-						return &recommenderpb.ListRecommendationsResponse{Recommendations: []*recommenderpb.Recommendation{sdkCostRecommendation(30, recommenderpb.RecommendationStateInfo_ACTIVE)}}, nil
+						return &recommenderpb.ListRecommendationsResponse{Recommendations: []*recommenderpb.Recommendation{recommendation(30, recommenderpb.RecommendationStateInfo_ACTIVE, "europe-west1")}}, nil
 					}
 					switch req.GetPageToken() {
 					case "":
 						return &recommenderpb.ListRecommendationsResponse{
 							Recommendations: []*recommenderpb.Recommendation{
-								sdkCostRecommendation(10, recommenderpb.RecommendationStateInfo_ACTIVE),
-								sdkCostRecommendation(999, recommenderpb.RecommendationStateInfo_DISMISSED),
+								recommendation(10, recommenderpb.RecommendationStateInfo_ACTIVE, "us-central1"),
+								recommendation(999, recommenderpb.RecommendationStateInfo_DISMISSED, "us-central1"),
 							},
 							NextPageToken: "second-page",
 						}, nil
 					case "second-page":
-						return &recommenderpb.ListRecommendationsResponse{Recommendations: []*recommenderpb.Recommendation{sdkCostRecommendation(20, recommenderpb.RecommendationStateInfo_ACTIVE)}}, nil
+						return &recommenderpb.ListRecommendationsResponse{Recommendations: []*recommenderpb.Recommendation{recommendation(20, recommenderpb.RecommendationStateInfo_ACTIVE, "us-central1")}}, nil
 					default:
 						return nil, status.Error(codes.InvalidArgument, "unexpected page token")
 					}
@@ -124,9 +149,13 @@ func TestComputeRecommendationsThroughSDK(t *testing.T) {
 			adapter, err := NewProviderWithProject(ctx, "recommendation-project", options...).GetRecommendationsClient(ctx)
 			require.NoError(t, err)
 			recs, err := adapter.GetRecommendationsForService(ctx, common.ServiceCompute)
-			if len(tc.failure) == 2 {
+			if len(tc.failure) == 2 || tc.invalid {
 				require.Error(t, err)
-				assert.Equal(t, codes.PermissionDenied, status.Code(err))
+				if !tc.invalid {
+					assert.Equal(t, codes.PermissionDenied, status.Code(err))
+				} else {
+					assert.Contains(t, err.Error(), "VCPU")
+				}
 				assert.Contains(t, err.Error(), "all 2 GCP recommendation service calls failed across 2 regions")
 				assert.Nil(t, recs)
 			} else {
@@ -136,6 +165,13 @@ func TestComputeRecommendationsThroughSDK(t *testing.T) {
 					assert.Equal(t, common.ProviderGCP, rec.Provider)
 					assert.Equal(t, common.ServiceCompute, rec.Service)
 					assert.Equal(t, "recommendation-project", rec.Account)
+					assert.Empty(t, rec.ResourceType)
+					if tc.resources != nil {
+						assert.Equal(t, 4, rec.Count)
+						assert.Equal(t, common.ComputeDetails{MemoryGB: 6}, rec.Details)
+					} else {
+						assert.Zero(t, rec.Count)
+					}
 					if rec.EstimatedSavings == 30 {
 						assert.Equal(t, "europe-west1", rec.Region)
 					} else {
@@ -156,14 +192,35 @@ func TestComputeRecommendationsThroughSDK(t *testing.T) {
 			defer mu.Unlock()
 			assert.Equal(t, 1, regionsCalls)
 			expectedRequests := []string{central + "|", west + "|"}
-			if !tc.empty && !tc.failure[central] {
+			if !tc.empty && !tc.failure[central] && !tc.invalid {
 				expectedRequests = append(expectedRequests, central+"|second-page")
 			}
 			assert.ElementsMatch(t, expectedRequests, requests)
 		})
 	}
 }
 
+// Synthetic explicit-filter operations exercise the generic API contract, not a captured cloud response.
+func sdkResourceAmounts(kinds []string, stringVCPU bool, region string) *recommenderpb.RecommendationContent {
+	group := &recommenderpb.OperationGroup{}
+	for _, kind := range kinds {
+		amount := structpb.NewNumberValue(map[string]float64{"VCPU": 4, "MEMORY": 6144, "memory": 6144, "memory_mb": 6144, "ACCELERATOR": 2, "LOCAL_SSD": 375, "UNKNOWN": 99}[kind])
+		if kind == "VCPU" && stringVCPU {
+			amount = structpb.NewStringValue("4")
+		}
+		group.Operations = append(group.Operations, &recommenderpb.Operation{
+			Action: "AdD", ResourceType: "compute.googleapis.com/Commitment",
+			Resource: "//compute.googleapis.com/projects/recommendation-project/regions/" + region + "/commitments/cud-001",
+			Path:     "/resources/*/amount", PathFilters: map[string]*structpb.Value{"/resources/*/type": structpb.NewStringValue(kind)},
+			PathValue: &recommenderpb.Operation_Value{Value: amount},
+		})
+	}
+	return &recommenderpb.RecommendationContent{
+		OperationGroups: []*recommenderpb.OperationGroup{group},
+		Overview:        &structpb.Struct{Fields: map[string]*structpb.Value{"numericValue": structpb.NewNumberValue(999)}},
+	}
+}
+
 // A cost-only fixture verifies listing without inventing purchasable machine resources.
 func sdkCostRecommendation(savings int64, state recommenderpb.RecommendationStateInfo_State) *recommenderpb.Recommendation {
 	return &recommenderpb.Recommendation{