diff --git a/go.mod b/go.mod index da11d152f..67fb0f8bc 100644 --- a/go.mod +++ b/go.mod @@ -11,6 +11,7 @@ require ( github.com/gofrs/uuid v4.4.0+incompatible github.com/google/go-cmp v0.7.0 github.com/grpc-ecosystem/grpc-gateway/v2 v2.20.0 + github.com/kubefleet-dev/kubefleet v0.0.1 github.com/onsi/ginkgo/v2 v2.23.4 github.com/onsi/gomega v1.37.0 github.com/prometheus/client_golang v1.22.0 diff --git a/go.sum b/go.sum index 3ab53410e..d2b0f90c2 100644 --- a/go.sum +++ b/go.sum @@ -203,6 +203,8 @@ github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE= github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk= github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= +github.com/kubefleet-dev/kubefleet v0.0.1 h1:Xo/DvRlKq7YLvzjGp8GFD8G6SLAyI41l4Sw4+df534g= +github.com/kubefleet-dev/kubefleet v0.0.1/go.mod h1:EXicokCzLknCzcpQ/8ehtXqBokr0/xUfzsSsU++rReY= github.com/kylelemons/godebug v1.1.0 h1:RPNrshWIDI6G2gRW9EHilWtl7Z6Sb1BR0xunSBf0SNc= github.com/kylelemons/godebug v1.1.0/go.mod h1:9/0rRGxNHcop5bhtWyNeEfOS8JIWk580+fNqagV/RAw= github.com/leodido/go-urn v1.4.0 h1:WT9HwE9SGECu3lg4d/dIA+jxlljEa1/ffXKmRjqdmIQ= diff --git a/pkg/clients/azure/compute/vmsizerecommenderclient_test.go b/pkg/clients/azure/compute/vmsizerecommenderclient_test.go index 029e12c7d..bfb36c60a 100644 --- a/pkg/clients/azure/compute/vmsizerecommenderclient_test.go +++ b/pkg/clients/azure/compute/vmsizerecommenderclient_test.go @@ -7,22 +7,15 @@ package compute import ( "context" - "fmt" - "io" "net/http" - "net/http/httptest" "strings" "testing" "github.com/google/go-cmp/cmp" - "google.golang.org/protobuf/encoding/protojson" "google.golang.org/protobuf/proto" computev1 "go.goms.io/fleet/apis/protos/azure/compute/v1" -) - -const ( - testTenantID = "test-tenant-id" + "go.goms.io/fleet/test/utils/azure/compute" ) func TestNewAttributeBasedVMSizeRecommenderClient(t *testing.T) { @@ -43,7 +36,7 @@ func TestNewAttributeBasedVMSizeRecommenderClient(t *testing.T) { }, { name: "with empty server address", - tenantID: testTenantID, + tenantID: compute.TestTenantID, serverAddress: "", httpClient: http.DefaultClient, wantClient: nil, @@ -51,7 +44,7 @@ func TestNewAttributeBasedVMSizeRecommenderClient(t *testing.T) { }, { name: "with nil HTTP client", - tenantID: testTenantID, + tenantID: compute.TestTenantID, serverAddress: "http://localhost:8080", httpClient: nil, wantClient: nil, @@ -59,11 +52,11 @@ func TestNewAttributeBasedVMSizeRecommenderClient(t *testing.T) { }, { name: "with all fields properly set", - tenantID: testTenantID, + tenantID: compute.TestTenantID, serverAddress: "https://example.com", httpClient: http.DefaultClient, wantClient: &AttributeBasedVMSizeRecommenderClient{ - tenantID: testTenantID, + tenantID: compute.TestTenantID, baseURL: "https://example.com", httpClient: http.DefaultClient, }, @@ -222,59 +215,9 @@ func TestClient_GenerateAttributeBasedRecommendations(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { // Set tenant ID environment variable to create client. - t.Setenv(tenantIDEnvVarName, testTenantID) + t.Setenv(tenantIDEnvVarName, compute.TestTenantID) // Create mock server. - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - // Verify request method. - if r.Method != http.MethodPost { - t.Errorf("got %s, want POST request", r.Method) - } - - // Verify headers. - if r.Header.Get("Content-Type") != "application/json" { - t.Errorf("got %s, want Content-Type: application/json", r.Header.Get("Content-Type")) - } - if r.Header.Get("Accept") != "application/json" { - t.Errorf("got %s, want Accept: application/json", r.Header.Get("Accept")) - } - if r.Header.Get("Grpc-Metadata-subscriptionTenantID") != testTenantID { - t.Errorf("got %s, want Grpc-Metadata-subscriptionTenantID: %s", - r.Header.Get("Grpc-Metadata-subscriptionTenantID"), testTenantID) - } - if r.Header.Get("Grpc-Metadata-clientRequestID") == "" { - t.Error("Grpc-Metadata-clientRequestID header is missing") - } - - // Verify URL path if request is not nil. - if tt.request != nil && tt.request.SubscriptionId != "" && tt.request.Location != "" { - wantPath := fmt.Sprintf(recommendationsPathTemplate, tt.request.SubscriptionId, tt.request.Location) - if r.URL.Path != wantPath { - t.Errorf("got %s, want path %s", r.URL.Path, wantPath) - } - - // Verify request body using protojson for proper proto3 oneof support. - body, err := io.ReadAll(r.Body) - if err != nil { - t.Fatalf("failed to read request body: %v", err) - } - var req computev1.GenerateAttributeBasedRecommendationsRequest - unmarshaler := protojson.UnmarshalOptions{ - DiscardUnknown: true, - } - if err := unmarshaler.Unmarshal(body, &req); err != nil { - t.Fatalf("failed to unmarshal request body: %v", err) - } - if !proto.Equal(tt.request, &req) { - t.Errorf("request body mismatch: got %+v, want %+v", &req, tt.request) - } - } - - // Write mock response. - w.WriteHeader(tt.mockStatusCode) - if _, err := w.Write([]byte(tt.mockResponse)); err != nil { - t.Fatalf("failed to write response: %v", err) - } - })) + server := compute.CreateMockAttributeBasedVMSizeRecommenderServer(t, tt.request, compute.TestTenantID, tt.mockResponse, tt.mockStatusCode) defer server.Close() // Create client. diff --git a/pkg/propertychecker/azure/checker_test.go b/pkg/propertychecker/azure/checker_test.go index db003ce01..dfe80ddb4 100644 --- a/pkg/propertychecker/azure/checker_test.go +++ b/pkg/propertychecker/azure/checker_test.go @@ -6,22 +6,18 @@ package azure import ( "fmt" - "io" "net/http" - "net/http/httptest" "strings" "testing" - "google.golang.org/protobuf/encoding/protojson" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" clusterv1beta1 "go.goms.io/fleet/apis/cluster/v1beta1" placementv1beta1 "go.goms.io/fleet/apis/placement/v1beta1" - computev1 "go.goms.io/fleet/apis/protos/azure/compute/v1" "go.goms.io/fleet/pkg/clients/azure/compute" - "go.goms.io/fleet/pkg/clients/httputil" "go.goms.io/fleet/pkg/propertyprovider/azure" "go.goms.io/fleet/pkg/utils/labels" + testcompute "go.goms.io/fleet/test/utils/azure/compute" ) func TestValidateCapacity(t *testing.T) { @@ -305,6 +301,7 @@ func TestCheckIfMeetSKUCapacityRequirement(t *testing.T) { name string cluster *clusterv1beta1.MemberCluster sku string + targetCapacity uint32 req placementv1beta1.PropertySelectorRequirement mockStatusCode int wantAvailable bool @@ -315,15 +312,17 @@ func TestCheckIfMeetSKUCapacityRequirement(t *testing.T) { name: "valid capacity request", cluster: cluster, sku: validSKU, + targetCapacity: 3, req: validPropertySelectorRequirement, mockStatusCode: http.StatusOK, wantAvailable: true, wantError: false, }, { - name: "unavailable SKU request", - cluster: cluster, - sku: "Standard_D2s_v4", + name: "unavailable SKU request", + cluster: cluster, + sku: "Standard_D2s_v4", + targetCapacity: 1, req: placementv1beta1.PropertySelectorRequirement{ Name: fmt.Sprintf(azure.CapacityPerSKUPropertyTmpl, "Standard_D2s_v4"), Operator: placementv1beta1.PropertySelectorGreaterThanOrEqualTo, @@ -343,6 +342,7 @@ func TestCheckIfMeetSKUCapacityRequirement(t *testing.T) { }, }, sku: validSKU, + targetCapacity: 3, req: validPropertySelectorRequirement, wantError: true, errorSubstring: "failed to extract Azure location label from cluster : label \"fleet.azure.com/location\" not found in cluster", @@ -357,6 +357,7 @@ func TestCheckIfMeetSKUCapacityRequirement(t *testing.T) { }, }, sku: validSKU, + targetCapacity: 3, req: validPropertySelectorRequirement, wantError: true, errorSubstring: "failed to extract Azure location label from cluster", @@ -371,6 +372,7 @@ func TestCheckIfMeetSKUCapacityRequirement(t *testing.T) { }, }, sku: validSKU, + targetCapacity: 3, req: validPropertySelectorRequirement, wantError: true, errorSubstring: "failed to extract Azure subscription ID label from cluster", @@ -379,28 +381,31 @@ func TestCheckIfMeetSKUCapacityRequirement(t *testing.T) { name: "Azure API returns error", cluster: cluster, sku: validSKU, + targetCapacity: 3, req: validPropertySelectorRequirement, mockStatusCode: http.StatusInternalServerError, wantError: true, errorSubstring: "failed to generate VM size recommendations from Azure", }, { - name: "invalid operator in requirement", - cluster: cluster, - sku: validSKU, + name: "invalid operator in requirement", + cluster: cluster, + sku: validSKU, + targetCapacity: 2, req: placementv1beta1.PropertySelectorRequirement{ Name: fmt.Sprintf(azure.CapacityPerSKUPropertyTmpl, validSKU), Operator: placementv1beta1.PropertySelectorEqualTo, - Values: []string{"3"}, + Values: []string{"2"}, }, mockStatusCode: http.StatusOK, wantError: true, errorSubstring: "unsupported operator \"Eq\" for SKU capacity property, only GreaterThan (Gt) and GreaterThanOrEqualTo (Ge) are supported", }, { - name: "unsupported operator in requirement", - cluster: cluster, - sku: validSKU, + name: "unsupported operator in requirement", + cluster: cluster, + sku: validSKU, + targetCapacity: 0, req: placementv1beta1.PropertySelectorRequirement{ Name: fmt.Sprintf(azure.CapacityPerSKUPropertyTmpl, validSKU), Operator: placementv1beta1.PropertySelectorGreaterThanOrEqualTo, @@ -411,9 +416,10 @@ func TestCheckIfMeetSKUCapacityRequirement(t *testing.T) { errorSubstring: "capacity value cannot be zero for operator", }, { - name: "cases-insensitive request - unavailable SKU", - cluster: cluster, - sku: "STANDARD_D2S_V3", + name: "cases-insensitive request - available SKU", + cluster: cluster, + sku: "STANDARD_D2S_V3", + targetCapacity: 1, req: placementv1beta1.PropertySelectorRequirement{ Name: fmt.Sprintf(azure.CapacityPerSKUPropertyTmpl, "STANDARD_D2S_V3"), Operator: placementv1beta1.PropertySelectorGreaterThanOrEqualTo, @@ -426,11 +432,14 @@ func TestCheckIfMeetSKUCapacityRequirement(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - // Create mock server - server := createMockAttributeBasedVMSizeRecommenderServer(t, tt.mockStatusCode) + // Set tenant ID environment variable to create client. + testTenantId := "test-tenant-id" + t.Setenv("AZURE_TENANT_ID", testTenantId) + // Create mock server. + mockRequest := testcompute.GenerateAttributeBasedVMSizeRecommenderRequest(tt.cluster.Labels[labels.AzureSubscriptionIDLabel], tt.cluster.Labels[labels.AzureLocationLabel], tt.sku, tt.targetCapacity) + server := testcompute.CreateMockAttributeBasedVMSizeRecommenderServer(t, mockRequest, testTenantId, testcompute.MockAttributeBasedVMSizeRecommenderResponse, tt.mockStatusCode) defer server.Close() - t.Setenv("AZURE_TENANT_ID", "test-tenant-id") client, err := compute.NewAttributeBasedVMSizeRecommenderClient(server.URL, http.DefaultClient) if err != nil { t.Fatalf("failed to create VM size recommender client: %v", err) @@ -442,12 +451,11 @@ func TestCheckIfMeetSKUCapacityRequirement(t *testing.T) { if err == nil { t.Fatalf("CheckIfMeetSKUCapacityRequirement error () = nil, want error") } else if tt.errorSubstring != "" && !strings.Contains(err.Error(), tt.errorSubstring) { - t.Errorf("CheckIfMeetSKUCapacityRequirement error () = %s, want %v", err, tt.errorSubstring) + t.Fatalf("CheckIfMeetSKUCapacityRequirement error () = %s, want %v", err, tt.errorSubstring) } - return } - if err != nil { + if !tt.wantError && err != nil { t.Fatalf("CheckIfMeetSKUCapacityRequirement error () = %v, want nil", err) } @@ -457,64 +465,3 @@ func TestCheckIfMeetSKUCapacityRequirement(t *testing.T) { }) } } - -// createMockAttributeBasedVMSizeRecommenderServer creates a mock HTTP server for testing AttributeBasedVMSizeRecommenderClient. -func createMockAttributeBasedVMSizeRecommenderServer(t *testing.T, httpStatusCode int) *httptest.Server { - // Create mock server - return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - // Verify request method - if r.Method != http.MethodPost { - t.Errorf("Mock PropertyChecker method () = %s, want POST request", r.Method) - } - - // Verify headers - if r.Header.Get(httputil.HeaderContentTypeKey) != httputil.HeaderContentTypeJSON { - t.Errorf("Mock PropertyChecker content () = %s, want %s", r.Header.Get(httputil.HeaderContentTypeKey), httputil.HeaderContentTypeJSON) - } - if r.Header.Get(httputil.HeaderAcceptKey) != httputil.HeaderContentTypeJSON { - t.Errorf("Mock PropertyChecker accept () = %s, want %s", r.Header.Get(httputil.HeaderAcceptKey), httputil.HeaderContentTypeJSON) - } - - // Verify request body using proto json for proper proto3 one of support - body, err := io.ReadAll(r.Body) - if err != nil { - t.Fatalf("failed to read request body: %v", err) - } - var req computev1.GenerateAttributeBasedRecommendationsRequest - unmarshaler := protojson.UnmarshalOptions{ - DiscardUnknown: true, - } - if err := unmarshaler.Unmarshal(body, &req); err != nil { - t.Fatalf("failed to unmarshal request body: %v", err) - } - - // Write mock response with status code from test case - if httpStatusCode == 0 { - httpStatusCode = http.StatusOK - } - w.Header().Set(httputil.HeaderContentTypeKey, httputil.HeaderContentTypeJSON) - w.WriteHeader(httpStatusCode) - - // Mock the expected response from the Azure API. - mockAzureResponse := `{ - "recommendedVmSizes": { - "regularVmSizes": [ - { - "family": "Dsv3", - "name": "Standard_D2s_v3", - "size": "D2" - }, - { - "family": "Standard", - "name": "Standard_B1s", - "size": "Standard_B1s" - } - ] - } - }` - - if _, err := w.Write([]byte(mockAzureResponse)); err != nil { - t.Fatalf("failed to write mock response: %v", err) - } - })) -} diff --git a/pkg/scheduler/framework/plugins/clusteraffinity/filtering.go b/pkg/scheduler/framework/plugins/clusteraffinity/filtering.go index 246512c94..6b1f34ca2 100644 --- a/pkg/scheduler/framework/plugins/clusteraffinity/filtering.go +++ b/pkg/scheduler/framework/plugins/clusteraffinity/filtering.go @@ -61,6 +61,7 @@ func (p *Plugin) Filter( t := &ps.GetPolicySnapshotSpec().Policy.Affinity.ClusterAffinity.RequiredDuringSchedulingIgnoredDuringExecution.ClusterSelectorTerms[idx] r := clusterRequirement{ ClusterSelectorTerm: *t, + PropertyChecker: p.PropertyChecker, } isMatched, err := r.Matches(cluster) if err != nil { diff --git a/pkg/scheduler/framework/plugins/clusteraffinity/filtering_test.go b/pkg/scheduler/framework/plugins/clusteraffinity/filtering_test.go index cd12fe85a..6a68a1240 100644 --- a/pkg/scheduler/framework/plugins/clusteraffinity/filtering_test.go +++ b/pkg/scheduler/framework/plugins/clusteraffinity/filtering_test.go @@ -18,6 +18,8 @@ package clusteraffinity import ( "context" + "fmt" + "net/http" "testing" "github.com/google/go-cmp/cmp" @@ -28,9 +30,13 @@ import ( clusterv1beta1 "go.goms.io/fleet/apis/cluster/v1beta1" placementv1beta1 "go.goms.io/fleet/apis/placement/v1beta1" + "go.goms.io/fleet/pkg/clients/azure/compute" + checker "go.goms.io/fleet/pkg/propertychecker/azure" "go.goms.io/fleet/pkg/propertyprovider" "go.goms.io/fleet/pkg/propertyprovider/azure" "go.goms.io/fleet/pkg/scheduler/framework" + "go.goms.io/fleet/pkg/utils/labels" + testcompute "go.goms.io/fleet/test/utils/azure/compute" ) const ( @@ -768,3 +774,140 @@ func TestFilter(t *testing.T) { }) } } + +// TestFilter_PropertyChecker tests the Filter extension point of the plugin with a property checker. +func TestFilter_PropertyChecker(t *testing.T) { + // This test ensures that the property checker is invoked correctly. + testCases := []struct { + name string + ps *placementv1beta1.ClusterSchedulingPolicySnapshot + cluster *clusterv1beta1.MemberCluster + vmSize string + targetCapacity uint32 + wantStatus *framework.Status + }{ + { + name: "single cluster capacity based term, matched", + ps: &placementv1beta1.ClusterSchedulingPolicySnapshot{ + Spec: placementv1beta1.SchedulingPolicySnapshotSpec{ + Policy: &placementv1beta1.PlacementPolicy{ + Affinity: &placementv1beta1.Affinity{ + ClusterAffinity: &placementv1beta1.ClusterAffinity{ + RequiredDuringSchedulingIgnoredDuringExecution: &placementv1beta1.ClusterSelector{ + ClusterSelectorTerms: []placementv1beta1.ClusterSelectorTerm{ + { + LabelSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + labels.AzureLocationLabel: regionLabelValue1, + labels.AzureSubscriptionIDLabel: "sub-id-123", + }, + }, + PropertySelector: &placementv1beta1.PropertySelector{ + MatchExpressions: []placementv1beta1.PropertySelectorRequirement{ + { + Name: fmt.Sprintf(azure.CapacityPerSKUPropertyTmpl, "Standard_D2s_v3"), + Operator: placementv1beta1.PropertySelectorGreaterThan, + Values: []string{ + "1", + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + cluster: &clusterv1beta1.MemberCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: clusterName1, + Labels: map[string]string{ + labels.AzureLocationLabel: regionLabelValue1, + labels.AzureSubscriptionIDLabel: "sub-id-123", + }, + }, + }, + vmSize: "Standard_D2s_v3", + targetCapacity: 1, + }, + { + name: "single cluster capacity based term, not matched", + ps: &placementv1beta1.ClusterSchedulingPolicySnapshot{ + Spec: placementv1beta1.SchedulingPolicySnapshotSpec{ + Policy: &placementv1beta1.PlacementPolicy{ + Affinity: &placementv1beta1.Affinity{ + ClusterAffinity: &placementv1beta1.ClusterAffinity{ + RequiredDuringSchedulingIgnoredDuringExecution: &placementv1beta1.ClusterSelector{ + ClusterSelectorTerms: []placementv1beta1.ClusterSelectorTerm{ + { + LabelSelector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + labels.AzureLocationLabel: regionLabelValue1, + labels.AzureSubscriptionIDLabel: "sub-id-123", + }, + }, + PropertySelector: &placementv1beta1.PropertySelector{ + MatchExpressions: []placementv1beta1.PropertySelectorRequirement{ + { + Name: fmt.Sprintf(azure.CapacityPerSKUPropertyTmpl, "Standard_B2ms"), + Operator: placementv1beta1.PropertySelectorGreaterThanOrEqualTo, + Values: []string{ + "4", + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + cluster: &clusterv1beta1.MemberCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: clusterName1, + Labels: map[string]string{ + labels.AzureLocationLabel: regionLabelValue1, + labels.AzureSubscriptionIDLabel: "sub-id-123", + }, + }, + }, + vmSize: "Standard_B2ms", + targetCapacity: 3, + wantStatus: framework.NewNonErrorStatus(framework.ClusterUnschedulable, p.Name(), "cluster does not match with any of the required cluster affinity terms"), + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + // Set tenant ID environment variable to create client. + t.Setenv("AZURE_TENANT_ID", testcompute.TestTenantID) + // Create mock server. + mockRequest := testcompute.GenerateAttributeBasedVMSizeRecommenderRequest(tc.cluster.Labels[labels.AzureSubscriptionIDLabel], tc.cluster.Labels[labels.AzureLocationLabel], tc.vmSize, tc.targetCapacity) + server := testcompute.CreateMockAttributeBasedVMSizeRecommenderServer(t, mockRequest, testcompute.TestTenantID, testcompute.MockAttributeBasedVMSizeRecommenderResponse, http.StatusOK) + defer server.Close() + + client, err := compute.NewAttributeBasedVMSizeRecommenderClient(server.URL, http.DefaultClient) + if err != nil { + t.Fatalf("failed to create VM size recommender client: %v", err) + } + p.PropertyChecker = checker.NewPropertyChecker(*client) + + ctx := context.Background() + state := framework.NewCycleState(nil, nil, nil) + status := p.Filter(ctx, state, tc.ps, tc.cluster) + + if diff := cmp.Diff( + status, tc.wantStatus, + cmp.AllowUnexported(framework.Status{}), + ignoreStatusErrorField, + ); diff != "" { + t.Errorf("Filter() unexpected status (-got, +want):\n%s", diff) + } + }) + } +} diff --git a/pkg/scheduler/framework/plugins/clusteraffinity/plugin.go b/pkg/scheduler/framework/plugins/clusteraffinity/plugin.go index 1d229536f..baf0588f8 100644 --- a/pkg/scheduler/framework/plugins/clusteraffinity/plugin.go +++ b/pkg/scheduler/framework/plugins/clusteraffinity/plugin.go @@ -21,6 +21,7 @@ import ( "errors" "fmt" + "go.goms.io/fleet/pkg/propertychecker/azure" "go.goms.io/fleet/pkg/scheduler/framework" ) @@ -31,6 +32,10 @@ type Plugin struct { // The framework handle. handle framework.Handle + + // Optional PropertyChecker for validating properties not provided by the property provider. + // Currently only supports checking SKU capacity, but can support additional property checks. + PropertyChecker *azure.PropertyChecker } var ( @@ -53,6 +58,9 @@ var ( type clusterAffinityPluginOptions struct { // The name of the plugin. name string + + // The PropertyChecker of the plugin. + PropertyChecker *azure.PropertyChecker } type Option func(*clusterAffinityPluginOptions) @@ -68,6 +76,14 @@ func WithName(name string) Option { } } +// WithPropertyChecker sets the property checker for property validation. +// This enables specific cluster requirement validation. +func WithPropertyChecker(checker *azure.PropertyChecker) Option { + return func(o *clusterAffinityPluginOptions) { + o.PropertyChecker = checker + } +} + // New returns a new Plugin. func New(opts ...Option) Plugin { options := defaultPluginOptions @@ -76,7 +92,8 @@ func New(opts ...Option) Plugin { } return Plugin{ - name: options.name, + name: options.name, + PropertyChecker: options.PropertyChecker, } } diff --git a/pkg/scheduler/framework/plugins/clusteraffinity/types.go b/pkg/scheduler/framework/plugins/clusteraffinity/types.go index a98c03f2f..f3bae73e5 100644 --- a/pkg/scheduler/framework/plugins/clusteraffinity/types.go +++ b/pkg/scheduler/framework/plugins/clusteraffinity/types.go @@ -28,6 +28,7 @@ import ( clusterv1beta1 "go.goms.io/fleet/apis/cluster/v1beta1" placementv1beta1 "go.goms.io/fleet/apis/placement/v1beta1" + "go.goms.io/fleet/pkg/propertychecker/azure" "go.goms.io/fleet/pkg/propertyprovider" ) @@ -36,6 +37,9 @@ import ( type clusterRequirement struct { // Embed the original ClusterSelectorTerm. ClusterSelectorTerm placementv1beta1.ClusterSelectorTerm + + // Optional PropertyChecker for validating properties not provided by the property provider. + PropertyChecker *azure.PropertyChecker } // retrieveResourceUsageFrom retrieves a resource property value from a member cluster. @@ -143,6 +147,22 @@ func (c *clusterRequirement) Matches(cluster *clusterv1beta1.MemberCluster) (boo } for _, exp := range c.ClusterSelectorTerm.PropertySelector.MatchExpressions { + // Check if we have a property checker and if it can handle the property. + if c.PropertyChecker != nil { + handled, available, err := c.MatchPropertiesInPropertyChecker(cluster, exp) + if err != nil { + return false, err + } + if handled { + // Property was handled by the property checker. + if !available { + return false, nil + } + // Property check passed, continue to next expression. + continue + } + // Property was not handled by property checker, fall through to standard validation. + } // Compare the observed value with the expected one using the specified operator. q, err := retrievePropertyValueFrom(cluster, exp.Name) if err != nil { diff --git a/pkg/scheduler/framework/plugins/clusteraffinity/types_azure.go b/pkg/scheduler/framework/plugins/clusteraffinity/types_azure.go new file mode 100644 index 000000000..277fd1b71 --- /dev/null +++ b/pkg/scheduler/framework/plugins/clusteraffinity/types_azure.go @@ -0,0 +1,41 @@ +// +//Copyright (c) Microsoft Corporation. +//Licensed under the MIT license. + +package clusteraffinity + +import ( + clusterv1beta1 "go.goms.io/fleet/apis/cluster/v1beta1" + placementv1beta1 "go.goms.io/fleet/apis/placement/v1beta1" + "go.goms.io/fleet/pkg/propertyprovider/azure" +) + +// MatchPropertiesInPropertyChecker checks if the given property selector requirement +// matches any azure-specific properties that can be handled by the azure property checker. +func (c *clusterRequirement) MatchPropertiesInPropertyChecker(cluster *clusterv1beta1.MemberCluster, req placementv1beta1.PropertySelectorRequirement) (handled bool, available bool, err error) { + // Check if the property is an Azure SKU capacity property. + if sku := isAzureSKUCapacityProperty(req.Name); sku != "" { + // Use the Azure property checker to validate SKU capacity requirement. + available, err := c.PropertyChecker.CheckIfMeetSKUCapacityRequirement(cluster, req, sku) + if err != nil { + return false, false, err + } + return true, available, err + } + + // Property is not handled by this property checker, fallback to standard validation. + return false, false, nil +} + +// isAzureSKUCapacityProperty checks if a property name matches the Azure SKU capacity pattern. +// If it matches, it returns the SKU name; otherwise, it returns an empty string. +func isAzureSKUCapacityProperty(propertyName string) string { + // Validate if the requirement is an Azure SKU capacity property. + // Extract SKU from the property name using regex. + // Expected format: "kubernetes.azure.com/vm-sizes/{sku}/capacity" + matches := azure.CapacityPerSKUPropertyRegex.FindStringSubmatch(propertyName) + if len(matches) == 2 { + return matches[1] + } + return "" +} diff --git a/pkg/scheduler/framework/plugins/clusteraffinity/types_azure_test.go b/pkg/scheduler/framework/plugins/clusteraffinity/types_azure_test.go new file mode 100644 index 000000000..6dceda911 --- /dev/null +++ b/pkg/scheduler/framework/plugins/clusteraffinity/types_azure_test.go @@ -0,0 +1,196 @@ +// +//Copyright (c) Microsoft Corporation. +//Licensed under the MIT license. + +package clusteraffinity + +import ( + "net/http" + "testing" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + clusterv1beta1 "go.goms.io/fleet/apis/cluster/v1beta1" + placementv1beta1 "go.goms.io/fleet/apis/placement/v1beta1" + "go.goms.io/fleet/pkg/clients/azure/compute" + "go.goms.io/fleet/pkg/propertychecker/azure" + "go.goms.io/fleet/pkg/utils/labels" + testcompute "go.goms.io/fleet/test/utils/azure/compute" +) + +func TestIsAzureCapacityProperty(t *testing.T) { + tests := []struct { + name string + property string + want string + }{ + { + name: "valid Azure capacity property", + property: "kubernetes.azure.com/vm-sizes/Standard_D2s_v3/capacity", + want: "Standard_D2s_v3", + }, + { + name: "invalid Azure capacity property - wrong prefix", + property: "kubernetes-fleet.io/vm-sizes/Standard_D2s_v3/capacity", + want: "", + }, + { + name: "invalid Azure capacity property - missing capacity suffix", + property: "kubernetes.azure.com/vm-sizes/Standard_D2s_v3", + want: "", + }, + { + name: "invalid Azure capacity property - empty string", + property: "", + want: "", + }, + { + name: "invalid Azure capacity property - random string", + property: "random-string", + want: "", + }, + { + name: "invalid Azure capacity property - only prefix", + property: "kubernetes.azure.com/vm-sizes", + want: "", + }, + { + name: "invalid Azure capacity property - no VM size", + property: "kubernetes.azure.com/vm-sizes//capacity", + want: "", + }, + { + name: "invalid Azure capacity property - extra segments", + property: "kubernetes.azure.com/vm-sizes/Standard_D2s_v3/extra/capacity", + want: "", + }, + { + name: "invalid Azure capacity property -", + property: "kubernetes.azure.com/vm-sizes.Standard_D2s_v3/capacity", + want: "", + }, + { + name: "invalid capacity property - different suffix", + property: "kubernetes.azure.com/vm-sizes/Standard_D2s_v3/capacity/count", + want: "", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := isAzureSKUCapacityProperty(tt.property) + if got != tt.want { + t.Errorf("isAzureCapacityProperty(%q) = %q, want %q", tt.property, got, tt.want) + } + }) + } +} + +func TestMatchPropertiesInPropertyChecker(t *testing.T) { + cluster := &clusterv1beta1.MemberCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-cluster", + Labels: map[string]string{ + labels.AzureLocationLabel: "eastus", + labels.AzureSubscriptionIDLabel: "1234-5678-9012", + }, + }, + } + + tests := []struct { + name string + cluster *clusterv1beta1.MemberCluster + selector placementv1beta1.PropertySelectorRequirement + vmSize string + targetCapacity uint32 + wantHandled bool + wantAvailable bool + wantErr bool + }{ + { + name: "Azure SKU capacity property not handled", + cluster: cluster, + selector: placementv1beta1.PropertySelectorRequirement{ + Name: "kubernetes.azure.com/vm-sizes/Standard_D2s_v3/count", + Operator: placementv1beta1.PropertySelectorGreaterThanOrEqualTo, + Values: []string{"2"}, + }, + }, + { + name: "Azure SKU capacity property handled and available", + cluster: cluster, + selector: placementv1beta1.PropertySelectorRequirement{ + Name: "kubernetes.azure.com/vm-sizes/Standard_D2s_v3/capacity", + Operator: placementv1beta1.PropertySelectorGreaterThan, + Values: []string{"2"}, + }, + vmSize: "Standard_D2s_v3", + targetCapacity: 2, + wantHandled: true, + wantAvailable: true, + }, + { + name: "Azure SKU capacity property handled but not available", + cluster: cluster, + selector: placementv1beta1.PropertySelectorRequirement{ + Name: "kubernetes.azure.com/vm-sizes/NonExistentSKU/capacity", + Operator: placementv1beta1.PropertySelectorGreaterThanOrEqualTo, + Values: []string{"2"}, + }, + vmSize: "NonExistentSKU", + targetCapacity: 1, + wantHandled: true, + wantAvailable: false, + }, + { + name: "Azure SKU capacity property with invalid operator", + cluster: cluster, + selector: placementv1beta1.PropertySelectorRequirement{ + Name: "kubernetes.azure.com/vm-sizes/Standard_D2s_v3/capacity", + Operator: placementv1beta1.PropertySelectorEqualTo, + Values: []string{"2"}, + }, + wantErr: true, + }, + { + name: "Azure SKU capacity property with non-integer value", + cluster: cluster, + selector: placementv1beta1.PropertySelectorRequirement{ + Name: "kubernetes.azure.com/vm-sizes/Standard_D2s_v3/capacity", + Operator: placementv1beta1.PropertySelectorGreaterThan, + Values: []string{"two"}, + }, + wantErr: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Set tenant ID environment variable to create client. + t.Setenv("AZURE_TENANT_ID", testcompute.TestTenantID) + // Create mock server. + mockRequest := testcompute.GenerateAttributeBasedVMSizeRecommenderRequest(tt.cluster.Labels[labels.AzureSubscriptionIDLabel], tt.cluster.Labels[labels.AzureLocationLabel], tt.vmSize, tt.targetCapacity) + server := testcompute.CreateMockAttributeBasedVMSizeRecommenderServer(t, mockRequest, testcompute.TestTenantID, testcompute.MockAttributeBasedVMSizeRecommenderResponse, http.StatusOK) + defer server.Close() + + client, err := compute.NewAttributeBasedVMSizeRecommenderClient(server.URL, http.DefaultClient) + if err != nil { + t.Fatalf("failed to create VM size recommender client: %v", err) + } + + req := &clusterRequirement{ + placementv1beta1.ClusterSelectorTerm{}, + azure.NewPropertyChecker(*client), + } + handled, available, err := req.MatchPropertiesInPropertyChecker(tt.cluster, tt.selector) + if (err != nil) != tt.wantErr { + t.Fatalf("MatchPropertiesInPropertyChecker() error = %v, wantErr %v", err, tt.wantErr) + } + if handled != tt.wantHandled { + t.Errorf("MatchPropertiesInPropertyChecker() handled = %v, want %v", handled, tt.wantHandled) + } + + if available != tt.wantAvailable { + t.Errorf("MatchPropertiesInPropertyChecker() available = %v, want %v", available, tt.wantAvailable) + } + }) + } +} diff --git a/pkg/scheduler/framework/plugins/clusteraffinity/types_test.go b/pkg/scheduler/framework/plugins/clusteraffinity/types_test.go index dc9858257..648f34b44 100644 --- a/pkg/scheduler/framework/plugins/clusteraffinity/types_test.go +++ b/pkg/scheduler/framework/plugins/clusteraffinity/types_test.go @@ -19,6 +19,7 @@ package clusteraffinity import ( "fmt" "math" + "net/http" "testing" "github.com/google/go-cmp/cmp" @@ -29,7 +30,12 @@ import ( clusterv1beta1 "go.goms.io/fleet/apis/cluster/v1beta1" placementv1beta1 "go.goms.io/fleet/apis/placement/v1beta1" + "go.goms.io/fleet/pkg/clients/azure/compute" + checker "go.goms.io/fleet/pkg/propertychecker/azure" "go.goms.io/fleet/pkg/propertyprovider" + azure "go.goms.io/fleet/pkg/propertyprovider/azure" + "go.goms.io/fleet/pkg/utils/labels" + testcompute "go.goms.io/fleet/test/utils/azure/compute" ) const ( @@ -1215,3 +1221,224 @@ func TestInterpolateWeightFor(t *testing.T) { }) } } + +// TestClusterRequirementMatches_WithPropertyChecker tests the Matches method on clusterRequirement pointers +// with a property checker. +func TestClusterRequirementMatches_WithPropertyChecker(t *testing.T) { + cluster := &clusterv1beta1.MemberCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: clusterName1, + Labels: map[string]string{ + labels.AzureLocationLabel: regionLabelValue1, + labels.AzureSubscriptionIDLabel: "sub-id-1234", + }, + }, + } + + validPropertyName := fmt.Sprintf(azure.CapacityPerSKUPropertyTmpl, "Standard_D2s_v3") + + testCases := []struct { + name string + matchExpression []placementv1beta1.PropertySelectorRequirement + cluster *clusterv1beta1.MemberCluster + sku string + targetCapacity uint32 + want bool + wantError bool + }{ + { + name: "invalid property name (not handled)", + matchExpression: []placementv1beta1.PropertySelectorRequirement{ + { + Name: "kubernetes.azure.com/vm-sizes/Standard_D2s_v3/count", + Operator: placementv1beta1.PropertySelectorEqualTo, + Values: []string{ + "2", + }, + }, + }, + cluster: cluster, + }, + { + name: "property not found", + matchExpression: []placementv1beta1.PropertySelectorRequirement{ + { + Name: nonExistentNonResourcePropertyName, + Operator: placementv1beta1.PropertySelectorGreaterThan, + Values: []string{ + "0", + }, + }, + }, + cluster: cluster, + }, + { + name: "multiple value options", + matchExpression: []placementv1beta1.PropertySelectorRequirement{ + { + Name: validPropertyName, + Operator: placementv1beta1.PropertySelectorGreaterThan, + Values: []string{ + "1", + "2", + }, + }, + }, + cluster: cluster, + wantError: true, + }, + { + name: "invalid value option", + matchExpression: []placementv1beta1.PropertySelectorRequirement{ + { + Name: validPropertyName, + Operator: placementv1beta1.PropertySelectorGreaterThanOrEqualTo, + Values: []string{ + "invalid", + }, + }, + }, + cluster: cluster, + wantError: true, + }, + { + name: "negative value option", + matchExpression: []placementv1beta1.PropertySelectorRequirement{ + { + Name: validPropertyName, + Operator: placementv1beta1.PropertySelectorGreaterThanOrEqualTo, + Values: []string{ + "-1", + }, + }, + }, + cluster: cluster, + wantError: true, + }, + { + name: "non-integer value option", + matchExpression: []placementv1beta1.PropertySelectorRequirement{ + { + Name: validPropertyName, + Operator: placementv1beta1.PropertySelectorGreaterThanOrEqualTo, + Values: []string{ + "2.5", + }, + }, + }, + cluster: cluster, + wantError: true, + }, + { + name: "invalid operator", + matchExpression: []placementv1beta1.PropertySelectorRequirement{ + { + Name: validPropertyName, + Operator: placementv1beta1.PropertySelectorEqualTo, + Values: []string{ + "1", + }, + }, + }, + cluster: cluster, + wantError: true, + }, + { + name: "op >, matched (min limit)", + matchExpression: []placementv1beta1.PropertySelectorRequirement{ + { + Name: validPropertyName, + Operator: placementv1beta1.PropertySelectorGreaterThan, + Values: []string{ + "0", + }, + }, + }, + cluster: cluster, + sku: "Standard_D2s_v3", + targetCapacity: 0, + want: true, + }, + { + name: "op >, not matched", + matchExpression: []placementv1beta1.PropertySelectorRequirement{ + { + Name: fmt.Sprintf(azure.CapacityPerSKUPropertyTmpl, "Standard_D4s_v3"), + Operator: placementv1beta1.PropertySelectorGreaterThan, + Values: []string{ + "8", + }, + }, + }, + cluster: cluster, + sku: "Standard_D4s_v3", + targetCapacity: 8, + }, + { + name: "op >=, matched (max limit)", + matchExpression: []placementv1beta1.PropertySelectorRequirement{ + { + Name: validPropertyName, + Operator: placementv1beta1.PropertySelectorGreaterThanOrEqualTo, + Values: []string{ + "200", + }, + }, + }, + cluster: cluster, + sku: "Standard_D2s_v3", + targetCapacity: 199, + want: true, + }, + { + name: "op >=, not matched", + matchExpression: []placementv1beta1.PropertySelectorRequirement{ + { + Name: fmt.Sprintf(azure.CapacityPerSKUPropertyTmpl, "Standard_D4s_v3"), + Operator: placementv1beta1.PropertySelectorGreaterThanOrEqualTo, + Values: []string{ + "80", + }, + }, + }, + cluster: cluster, + sku: "Standard_D4s_v3", + targetCapacity: 79, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + // Set tenant ID environment variable to create client. + t.Setenv("AZURE_TENANT_ID", testcompute.TestTenantID) + // Create mock server. + mockRequest := testcompute.GenerateAttributeBasedVMSizeRecommenderRequest(tc.cluster.Labels[labels.AzureSubscriptionIDLabel], tc.cluster.Labels[labels.AzureLocationLabel], tc.sku, tc.targetCapacity) + server := testcompute.CreateMockAttributeBasedVMSizeRecommenderServer(t, mockRequest, testcompute.TestTenantID, testcompute.MockAttributeBasedVMSizeRecommenderResponse, http.StatusOK) + defer server.Close() + + client, err := compute.NewAttributeBasedVMSizeRecommenderClient(server.URL, http.DefaultClient) + if err != nil { + t.Fatalf("failed to create VM size recommender client: %v", err) + } + + req := clusterRequirement{ + ClusterSelectorTerm: placementv1beta1.ClusterSelectorTerm{ + PropertySelector: &placementv1beta1.PropertySelector{ + MatchExpressions: tc.matchExpression, + }, + }, + PropertyChecker: checker.NewPropertyChecker(*client), + } + matches, err := req.Matches(tc.cluster) + if tc.wantError { + if err == nil { + t.Fatalf("Matches() (With Property Checker), want error, got nil") + } + } + + if (!tc.wantError && err != nil) || matches != tc.want { + t.Errorf("Matches() (With Property Checker) = %v, %v, want %v, nil", matches, err, tc.want) + } + }) + } +} diff --git a/test/utils/azure/compute/compute.go b/test/utils/azure/compute/compute.go new file mode 100644 index 000000000..091e24290 --- /dev/null +++ b/test/utils/azure/compute/compute.go @@ -0,0 +1,116 @@ +// +//Copyright (c) Microsoft Corporation. +//Licensed under the MIT license. + +// compute package provides utilities for testing Azure Compute services. +package compute + +import ( + "fmt" + "io" + "net/http" + "net/http/httptest" + "testing" + + "google.golang.org/protobuf/encoding/protojson" + "google.golang.org/protobuf/proto" + + computev1 "go.goms.io/fleet/apis/protos/azure/compute/v1" + "go.goms.io/fleet/pkg/clients/httputil" +) + +const ( + TestTenantID = "test-tenant-id" + + MockAttributeBasedVMSizeRecommenderResponse = `{ + "recommendedVmSizes": { + "regularVmSizes": [ + { + "family": "Dsv3", + "name": "Standard_D2s_v3", + "size": "D2" + }, + { + "family": "Standard", + "name": "Standard_B1s", + "size": "Standard_B1s" + } + ] + } + }` +) + +// GenerateAttributeBasedVMSizeRecommenderRequest is a helper function to create a mock request. +func GenerateAttributeBasedVMSizeRecommenderRequest(subscriptionID, location, sku string, targetCapacity uint32) *computev1.GenerateAttributeBasedRecommendationsRequest { + return &computev1.GenerateAttributeBasedRecommendationsRequest{ + SubscriptionId: subscriptionID, + Location: location, + RegularPriorityProfile: &computev1.RegularPriorityProfile{ + CapacityUnitType: computev1.CapacityUnitType_CAPACITY_UNIT_TYPE_VM_INSTANCE_COUNT, + TargetCapacity: targetCapacity, + }, + RecommendationProperties: &computev1.RecommendationProperties{ + RestrictionsFilter: computev1.RecommendationProperties_RESTRICTIONS_FILTER_QUOTA_AND_OFFER_RESTRICTIONS, + }, + ResourceProperties: &computev1.ResourceProperties{ + VmAttributes: &computev1.VMAttributes{ + AllowedVmSizes: []string{sku}, + }, + }, + } +} + +// CreateMockAttributeBasedVMSizeRecommenderServer is a helper function to create a mock server with a generated response. +func CreateMockAttributeBasedVMSizeRecommenderServer(t *testing.T, request *computev1.GenerateAttributeBasedRecommendationsRequest, testTenantID, response string, httpStatusCode int) *httptest.Server { + return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // Verify request method. + if r.Method != http.MethodPost { + t.Errorf("got %s, want POST request", r.Method) + } + + // Verify headers. + if r.Header.Get(httputil.HeaderContentTypeKey) != httputil.HeaderContentTypeJSON { + t.Errorf("got %s, want Content-Type: %s", r.Header.Get(httputil.HeaderContentTypeKey), httputil.HeaderContentTypeJSON) + } + if r.Header.Get(httputil.HeaderAcceptKey) != httputil.HeaderContentTypeJSON { + t.Errorf("got %s, want Accept: %s", r.Header.Get(httputil.HeaderAcceptKey), httputil.HeaderContentTypeJSON) + } + if r.Header.Get(httputil.HeaderAzureSubscriptionTenantIDKey) != testTenantID { + t.Errorf("got %s, want Grpc-Metadata-subscriptionTenantID: %s", + r.Header.Get(httputil.HeaderAzureSubscriptionTenantIDKey), testTenantID) + } + if r.Header.Get(httputil.HeaderAzureClientRequestIDKey) == "" { + t.Error("Grpc-Metadata-clientRequestID header is missing") + } + + // Verify URL path if request is not nil. + if request != nil && request.SubscriptionId != "" && request.Location != "" { + wantPath := fmt.Sprintf("/subscriptions/%s/providers/Microsoft.Compute/locations/%s/vmSizeRecommendations/vmAttributeBased/generate", request.SubscriptionId, request.Location) + if r.URL.Path != wantPath { + t.Errorf("got %s, want path %s", r.URL.Path, wantPath) + } + + // Verify request body using protojson for proper proto3 oneof support. + body, err := io.ReadAll(r.Body) + if err != nil { + t.Fatalf("failed to read request body: %v", err) + } + var req computev1.GenerateAttributeBasedRecommendationsRequest + unmarshaler := protojson.UnmarshalOptions{ + DiscardUnknown: true, + } + if err := unmarshaler.Unmarshal(body, &req); err != nil { + t.Fatalf("failed to unmarshal request body: %v", err) + } + if !proto.Equal(request, &req) { + t.Errorf("request body mismatch: got %+v, want %+v", &req, request) + } + } + + // Write mock response. + w.WriteHeader(httpStatusCode) + if _, err := w.Write([]byte(response)); err != nil { + t.Fatalf("failed to write response: %v", err) + } + })) +}