From be2fc6e148724542d12000f56a5041f5cde74177 Mon Sep 17 00:00:00 2001 From: nicole-lihui Date: Fri, 26 Dec 2025 14:30:55 +0800 Subject: [PATCH] fix iop autoscaling disable by default --- internal/istio/iop.go | 12 +- internal/istio/iop_test.go | 373 +++++++++++++++++++++++++++++++++++++ 2 files changed, 380 insertions(+), 5 deletions(-) diff --git a/internal/istio/iop.go b/internal/istio/iop.go index e5c08c6..c3d26e3 100644 --- a/internal/istio/iop.go +++ b/internal/istio/iop.go @@ -290,20 +290,22 @@ func (r *IstioOperatorReconciler) convertIopToHelmApp(ctx context.Context, in *i // spec.values.gateways.istio-ingressgateway.autoscaleEnabled // spec.values.gateways.istio-egressgateway.autoscaleEnabled + // Default autoscaling is disabled to ensure it can be explicitly disabled + autoscaling := map[string]interface{}{ + "enabled": false, + } autoscaleEnabledKey := fmt.Sprintf("spec.values.%s.autoscaleEnabled", cInfo.Component.ToHelmValuesTreeRoot) minReplicasKey := fmt.Sprintf("spec.values.%s.autoscaleMin", cInfo.Component.ToHelmValuesTreeRoot) // Configure autoscaling settings if enabled := cInfo.Values.GetPathBool(autoscaleEnabledKey); enabled { - autoscaling := map[string]interface{}{ - "enabled": enabled, - } + autoscaling["enabled"] = true if minReplicas := cInfo.Values.GetPathString(minReplicasKey); minReplicas != "" { autoscaling["minReplicas"] = minReplicas } - - componentValues["autoscaling"] = autoscaling } + // If autoscaleEnabled is explicitly set to false or not set, autoscaling remains disabled (default) + componentValues["autoscaling"] = autoscaling // compatible iop gateway template appValue := "istio-ingressgateway" diff --git a/internal/istio/iop_test.go b/internal/istio/iop_test.go index a576c8f..6a9d2f4 100644 --- a/internal/istio/iop_test.go +++ b/internal/istio/iop_test.go @@ -9,6 +9,7 @@ import ( "istio.io/istio/operator/pkg/render" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" istiov1alpha1 "pluma.io/api/istio/v1alpha1" + operatorv1alpha1 "pluma.io/api/operator/v1alpha1" ) func TestIstioOperatorReconciler_convertIopToHelmApp_WithSchemaValidation(t *testing.T) { @@ -296,3 +297,375 @@ func TestIsEgressGateway(t *testing.T) { }) } } + +func TestIstioOperatorReconciler_convertIopToHelmApp_Autoscaling(t *testing.T) { + tests := []struct { + name string + iop *istiov1alpha1.IstioOperator + expectedAutoscaling map[string]interface{} + gatewayComponentName string + description string + }{ + { + name: "autoscaling default disabled when not set", + description: "When autoscaleEnabled is not set, autoscaling should be disabled by default", + gatewayComponentName: "istio-ingressgateway", + iop: &istiov1alpha1.IstioOperator{ + TypeMeta: metav1.TypeMeta{ + Kind: "IstioOperator", + APIVersion: "install.istio.io/v1alpha1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-iop-default", + Namespace: "istio-system", + }, + Spec: &istiov1alpha1.IstioOperatorSpec{ + Tag: &structpb.Value{ + Kind: &structpb.Value_StringValue{ + StringValue: "1.25.5", + }, + }, + Values: &structpb.Struct{ + Fields: map[string]*structpb.Value{ + "global": { + Kind: &structpb.Value_StructValue{ + StructValue: &structpb.Struct{ + Fields: map[string]*structpb.Value{ + "istioNamespace": { + Kind: &structpb.Value_StringValue{ + StringValue: "istio-system", + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + expectedAutoscaling: map[string]interface{}{ + "enabled": false, + }, + }, + { + name: "autoscaling enabled when autoscaleEnabled is true", + description: "When autoscaleEnabled is explicitly set to true, autoscaling should be enabled", + gatewayComponentName: "istio-ingressgateway", + iop: &istiov1alpha1.IstioOperator{ + TypeMeta: metav1.TypeMeta{ + Kind: "IstioOperator", + APIVersion: "install.istio.io/v1alpha1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-iop-enabled", + Namespace: "istio-system", + }, + Spec: &istiov1alpha1.IstioOperatorSpec{ + Tag: &structpb.Value{ + Kind: &structpb.Value_StringValue{ + StringValue: "1.25.5", + }, + }, + Values: &structpb.Struct{ + Fields: map[string]*structpb.Value{ + "global": { + Kind: &structpb.Value_StructValue{ + StructValue: &structpb.Struct{ + Fields: map[string]*structpb.Value{ + "istioNamespace": { + Kind: &structpb.Value_StringValue{ + StringValue: "istio-system", + }, + }, + }, + }, + }, + }, + "gateways": { + Kind: &structpb.Value_StructValue{ + StructValue: &structpb.Struct{ + Fields: map[string]*structpb.Value{ + "istio-ingressgateway": { + Kind: &structpb.Value_StructValue{ + StructValue: &structpb.Struct{ + Fields: map[string]*structpb.Value{ + "autoscaleEnabled": { + Kind: &structpb.Value_BoolValue{ + BoolValue: true, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + expectedAutoscaling: map[string]interface{}{ + "enabled": true, + }, + }, + { + name: "autoscaling disabled when autoscaleEnabled is false", + description: "When autoscaleEnabled is explicitly set to false, autoscaling should be disabled", + gatewayComponentName: "istio-ingressgateway", + iop: &istiov1alpha1.IstioOperator{ + TypeMeta: metav1.TypeMeta{ + Kind: "IstioOperator", + APIVersion: "install.istio.io/v1alpha1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-iop-disabled", + Namespace: "istio-system", + }, + Spec: &istiov1alpha1.IstioOperatorSpec{ + Tag: &structpb.Value{ + Kind: &structpb.Value_StringValue{ + StringValue: "1.25.5", + }, + }, + Values: &structpb.Struct{ + Fields: map[string]*structpb.Value{ + "global": { + Kind: &structpb.Value_StructValue{ + StructValue: &structpb.Struct{ + Fields: map[string]*structpb.Value{ + "istioNamespace": { + Kind: &structpb.Value_StringValue{ + StringValue: "istio-system", + }, + }, + }, + }, + }, + }, + "gateways": { + Kind: &structpb.Value_StructValue{ + StructValue: &structpb.Struct{ + Fields: map[string]*structpb.Value{ + "istio-ingressgateway": { + Kind: &structpb.Value_StructValue{ + StructValue: &structpb.Struct{ + Fields: map[string]*structpb.Value{ + "autoscaleEnabled": { + Kind: &structpb.Value_BoolValue{ + BoolValue: false, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + expectedAutoscaling: map[string]interface{}{ + "enabled": false, + }, + }, + { + name: "autoscaling enabled with minReplicas when both are set", + description: "When autoscaleEnabled is true and autoscaleMin is set, both should be included", + gatewayComponentName: "istio-ingressgateway", + iop: &istiov1alpha1.IstioOperator{ + TypeMeta: metav1.TypeMeta{ + Kind: "IstioOperator", + APIVersion: "install.istio.io/v1alpha1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-iop-with-min", + Namespace: "istio-system", + }, + Spec: &istiov1alpha1.IstioOperatorSpec{ + Tag: &structpb.Value{ + Kind: &structpb.Value_StringValue{ + StringValue: "1.25.5", + }, + }, + Values: &structpb.Struct{ + Fields: map[string]*structpb.Value{ + "global": { + Kind: &structpb.Value_StructValue{ + StructValue: &structpb.Struct{ + Fields: map[string]*structpb.Value{ + "istioNamespace": { + Kind: &structpb.Value_StringValue{ + StringValue: "istio-system", + }, + }, + }, + }, + }, + }, + "gateways": { + Kind: &structpb.Value_StructValue{ + StructValue: &structpb.Struct{ + Fields: map[string]*structpb.Value{ + "istio-ingressgateway": { + Kind: &structpb.Value_StructValue{ + StructValue: &structpb.Struct{ + Fields: map[string]*structpb.Value{ + "autoscaleEnabled": { + Kind: &structpb.Value_BoolValue{ + BoolValue: true, + }, + }, + "autoscaleMin": { + Kind: &structpb.Value_StringValue{ + StringValue: "3", + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + expectedAutoscaling: map[string]interface{}{ + "enabled": true, + "minReplicas": "3", + }, + }, + { + name: "egress gateway autoscaling default disabled", + description: "Egress gateway should also have autoscaling disabled by default", + gatewayComponentName: "istio-egressgateway", + iop: &istiov1alpha1.IstioOperator{ + TypeMeta: metav1.TypeMeta{ + Kind: "IstioOperator", + APIVersion: "install.istio.io/v1alpha1", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "test-iop-egress", + Namespace: "istio-system", + }, + Spec: &istiov1alpha1.IstioOperatorSpec{ + Tag: &structpb.Value{ + Kind: &structpb.Value_StringValue{ + StringValue: "1.25.5", + }, + }, + Values: &structpb.Struct{ + Fields: map[string]*structpb.Value{ + "global": { + Kind: &structpb.Value_StructValue{ + StructValue: &structpb.Struct{ + Fields: map[string]*structpb.Value{ + "istioNamespace": { + Kind: &structpb.Value_StringValue{ + StringValue: "istio-system", + }, + }, + }, + }, + }, + }, + "gateways": { + Kind: &structpb.Value_StructValue{ + StructValue: &structpb.Struct{ + Fields: map[string]*structpb.Value{ + "istio-egressgateway": { + Kind: &structpb.Value_StructValue{ + StructValue: &structpb.Struct{ + Fields: map[string]*structpb.Value{}, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + expectedAutoscaling: map[string]interface{}{ + "enabled": false, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Create a mock reconciler + reconciler := &IstioOperatorReconciler{} + + // Convert IstioOperator to HelmApp + helmApp, err := reconciler.convertIopToHelmApp(context.Background(), tt.iop) + if err != nil { + t.Fatalf("convertIopToHelmApp() error = %v", err) + } + + // Find the gateway component (should be unique by Chart name) + var gatewayComponent *operatorv1alpha1.HelmComponent + for _, component := range helmApp.Spec.Components { + if component.Chart == "gateway" { + gatewayComponent = component + break + } + } + + if gatewayComponent == nil { + t.Fatalf("Gateway component not found in HelmApp") + } + + // Extract autoscaling from component values + componentValuesMap := structToMap(gatewayComponent.ComponentValues) + if componentValuesMap == nil { + t.Fatalf("Failed to convert component values to map") + } + autoscaling, ok := componentValuesMap["autoscaling"] + if !ok { + t.Fatalf("autoscaling not found in component values") + } + + autoscalingMap, ok := autoscaling.(map[string]interface{}) + if !ok { + t.Fatalf("autoscaling is not a map[string]interface{}, got %T", autoscaling) + } + + // Check enabled field + enabled, ok := autoscalingMap["enabled"] + if !ok { + t.Errorf("autoscaling.enabled not found") + } else { + expectedEnabled := tt.expectedAutoscaling["enabled"] + if enabled != expectedEnabled { + t.Errorf("autoscaling.enabled = %v, want %v", enabled, expectedEnabled) + } + } + + // Check minReplicas field if expected + if expectedMinReplicas, ok := tt.expectedAutoscaling["minReplicas"]; ok { + minReplicas, ok := autoscalingMap["minReplicas"] + if !ok { + t.Errorf("autoscaling.minReplicas not found, want %v", expectedMinReplicas) + } else if minReplicas != expectedMinReplicas { + t.Errorf("autoscaling.minReplicas = %v, want %v", minReplicas, expectedMinReplicas) + } + } else { + // If minReplicas is not expected, it should not be present + if minReplicas, ok := autoscalingMap["minReplicas"]; ok { + t.Errorf("autoscaling.minReplicas should not be present, got %v", minReplicas) + } + } + }) + } +}