From fc6e70e0073410a750b5782b0ef385c2ff0eee4a Mon Sep 17 00:00:00 2001 From: Miciah Dashiel Butler Masters Date: Tue, 16 Dec 2025 16:35:31 -0500 Subject: [PATCH] gatewayclass: Enable Horizontal Pod Autoscaling Enable Horizontal Pod Autoscaling (HPA) on Istio. Hard-code the autoscaling parameters based on the cluster infrastructure config: - If the infrastructure topology is "SingleReplica", set minimum to 1. - Otherwise, set minimum to 2. - In any case, set maximum to 10. This commit implements NE-2396. https://redhat.atlassian.net/browse/NE-2396 * pkg/operator/controller/gatewayclass/controller.go (extraIstioConfig): Add infraConfig field. (NewUnmanaged): Watch infrastructures. (reconcileWithOLM): Get the cluster infrastructure config resource, list gatewayclasses with our controller name, and pass the config resource and gatewayclasses list to ensureIstioOLM. (reconcileWithSailLibrary): Likewise get the cluster infrastructure config resource, list gatewayclasses with our controller name, and pass these values ensureIstio. * pkg/operator/controller/gatewayclass/controller_test.go (Test_Reconcile): Add test cases for missing cluster infrastructure config. Add test cases for SingleReplica topology mode. Add test cases with multiple gatewayclasses. Add the cluster infrastructure config object to existingObjects for the existing test cases. Add the expected HPA configuration to the expected Istio CRs or Sail library options in test cases. * pkg/operator/controller/gatewayclass/istio_olm.go (ensureIstioOLM): Add gatewayclasses and infraConfig parameters. Use the infraConfig argument to initialize extraIstioConfig, and pass the gatewayclasses argument to desiredIstio. Check for errors from desiredIstio as json marshaling of the HPA configuration could generate an error. (desiredIstio): Add a gatewayclasses parameter, and pass the argument as well as infraConfig from extraIstioConfig to buildHorizontalPodAutoscalerConfig. Check for errors from the same. * pkg/operator/controller/gatewayclass/istio_sail_installer.go (ensureIstio): Add gatewayclasses and infraConfig parameters. Use the infraConfig argument to initialize extraIstioConfig, and pass the gatewayclasses argument to buildInstallerOptions. Check for errors from the same. (buildInstallerOptions): Add a gatewayclasses parameter, and pass the argument to openshiftValues. Check for errors from the same. (openshiftValues): Add a gatewayclasses parameter. Use the argument and infraConfig from extraConfig to look up the infrastructure topology mode and configure HPA accordingly, using the new buildHorizontalPodAutoscalerConfig helper. (buildHorizontalPodAutoscalerConfig): New function. Return the HPA configuration for Istio based on the given infraConfig and gatewayclasses list. * pkg/operator/controller/gatewayclass/istio_sail_installer_test.go (TestOLMAndSailLibraryValuesMatch): Add gatewayclasses and infraConfig to the test inputs. Add a test case for single-replica topology. Assert that openshiftValues and desiredIstio return no errors. * pkg/operator/controller/status/controller.go (Reconcile): Add horizontalpodautoscalers to relatedObjects. * test/e2e/operator_test.go (TestClusterOperatorStatusRelatedObjects): Add horizontalpodautoscalers to the expected list of related objects. * test/e2e/gateway_api_test.go (ensureGatewayObjectSuccess): Call new assertHorizontalPodAutoscalerEnabled helper. * test/e2e/util_gatewayapi_test.go (assertHorizontalPodAutoscalerEnabled): New helper. Assert that an HPA is configured with the expected minReplicas for the given gateway. --- .../controller/gatewayclass/controller.go | 31 ++- .../gatewayclass/controller_test.go | 213 ++++++++++++++++-- .../controller/gatewayclass/istio_olm.go | 26 ++- .../gatewayclass/istio_sail_installer.go | 70 +++++- .../gatewayclass/istio_sail_installer_test.go | 45 +++- pkg/operator/controller/status/controller.go | 7 + test/e2e/gateway_api_test.go | 10 + test/e2e/operator_test.go | 4 + test/e2e/util_gatewayapi_test.go | 40 ++++ 9 files changed, 403 insertions(+), 43 deletions(-) diff --git a/pkg/operator/controller/gatewayclass/controller.go b/pkg/operator/controller/gatewayclass/controller.go index 4fb046a3ec..12c1def03b 100644 --- a/pkg/operator/controller/gatewayclass/controller.go +++ b/pkg/operator/controller/gatewayclass/controller.go @@ -98,6 +98,7 @@ const ( type extraIstioConfig struct { proxyConfig *configv1.Proxy + infraConfig *configv1.Infrastructure } var log = logf.Logger.WithName(controllerName) @@ -245,6 +246,12 @@ func NewUnmanaged(mgr manager.Manager, config Config) (controller.Controller, er } } + // Watch the cluster infrastructure config in case the infrastructure + // topology changes. + if err := c.Watch(source.Kind[client.Object](operatorCache, &configv1.Infrastructure{}, reconciler.enqueueRequestForSomeGatewayClass())); err != nil { + return nil, err + } + gatewayClassController = c return c, nil } @@ -418,6 +425,11 @@ func (r *reconciler) reconcileWithOLM(ctx context.Context, request reconcile.Req log.Info("reconciling with OLM", "request", request) var errs []error + var infraConfig configv1.Infrastructure + if err := r.cache.Get(ctx, types.NamespacedName{Name: "cluster"}, &infraConfig); err != nil { + return reconcile.Result{}, err + } + gatewayclass := &gatewayapiv1.GatewayClass{} if err := r.cache.Get(ctx, request.NamespacedName, gatewayclass); err != nil { if errors.IsNotFound(err) { @@ -466,7 +478,12 @@ func (r *reconciler) reconcileWithOLM(ctx context.Context, request reconcile.Req if v, ok := gatewayclass.Annotations[istioVersionOverrideAnnotationKey]; ok { istioVersion = v } - if _, _, err := r.ensureIstioOLM(ctx, gatewayclass, istioVersion); err != nil { + var gatewayclasses gatewayapiv1.GatewayClassList + if err := r.cache.List(ctx, &gatewayclasses, client.MatchingFields{operatorcontroller.GatewayClassIndexFieldName: operatorcontroller.OpenShiftGatewayClassControllerName}); err != nil { + return reconcile.Result{}, err + } + + if _, _, err := r.ensureIstioOLM(ctx, gatewayclass, istioVersion, gatewayclasses.Items, &infraConfig); err != nil { errs = append(errs, err) } else { // The OSSM operator installs the istios.sailoperator.io CRD. @@ -492,6 +509,11 @@ func (r *reconciler) reconcileWithOLM(ctx context.Context, request reconcile.Req func (r *reconciler) reconcileWithSailLibrary(ctx context.Context, request reconcile.Request) (reconcile.Result, error) { log.Info("reconciling with sail library", "request", request) + var infraConfig configv1.Infrastructure + if err := r.cache.Get(ctx, types.NamespacedName{Name: "cluster"}, &infraConfig); err != nil { + return reconcile.Result{}, err + } + gatewayClass := &gatewayapiv1.GatewayClass{} if err := r.cache.Get(ctx, request.NamespacedName, gatewayClass); err != nil { if errors.IsNotFound(err) { @@ -528,7 +550,12 @@ func (r *reconciler) reconcileWithSailLibrary(ctx context.Context, request recon istioVersion = v } - if err := r.ensureIstio(ctx, istioVersion); err != nil { + var gatewayclasses gatewayapiv1.GatewayClassList + if err := r.cache.List(ctx, &gatewayclasses, client.MatchingFields{operatorcontroller.GatewayClassIndexFieldName: operatorcontroller.OpenShiftGatewayClassControllerName}); err != nil { + return reconcile.Result{}, err + } + + if err := r.ensureIstio(ctx, istioVersion, gatewayclasses.Items, &infraConfig); err != nil { log.Error(err, "failed to ensure Istio") errs = append(errs, err) } diff --git a/pkg/operator/controller/gatewayclass/controller_test.go b/pkg/operator/controller/gatewayclass/controller_test.go index 4c3f09244a..bcc7e0aa44 100644 --- a/pkg/operator/controller/gatewayclass/controller_test.go +++ b/pkg/operator/controller/gatewayclass/controller_test.go @@ -2,7 +2,9 @@ package gatewayclass import ( "context" + "encoding/json" "fmt" + "strings" "testing" "time" @@ -49,6 +51,15 @@ func Test_Reconcile(t *testing.T) { } } + infraConfig := func(infraTopologyMode configv1.TopologyMode) *configv1.Infrastructure { + return &configv1.Infrastructure{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Status: configv1.InfrastructureStatus{ + InfrastructureTopology: infraTopologyMode, + }, + } + } + subscription := func(catalog, channel, version string) *operatorsv1alpha1.Subscription { return &operatorsv1alpha1.Subscription{ ObjectMeta: metav1.ObjectMeta{ @@ -94,7 +105,7 @@ func Test_Reconcile(t *testing.T) { } } - istio := func(version string, gieEnabled bool, proxyconfig map[string]string) *sailv1.Istio { + istio := func(version string, gieEnabled bool, proxyconfig map[string]string, gatewayclassesConfig json.RawMessage) *sailv1.Istio { ret := &sailv1.Istio{ ObjectMeta: metav1.ObjectMeta{ Name: "openshift-gateway", @@ -165,10 +176,26 @@ func Test_Reconcile(t *testing.T) { if gieEnabled { ret.Spec.Values.Pilot.Env["ENABLE_GATEWAY_API_INFERENCE_EXTENSION"] = "true" } + ret.Spec.Values.GatewayClasses = gatewayclassesConfig return ret } + gatewayclassesConfig := func(config string, gatewayclasses ...string) json.RawMessage { + return json.RawMessage(fmt.Appendf(nil, `{%s}`, strings.Join(func() []string { + var result []string + + for _, name := range gatewayclasses { + result = append(result, fmt.Sprintf(`"%s":%s`, name, config)) + } + + return result + }(), ","))) + } + hpaConfig := func(minReplicas int) string { + return fmt.Sprintf(`{"horizontalPodAutoscaler":{"spec":{"maxReplicas":10,"minReplicas":%d}}}`, minReplicas) + } + istioRevision := func() *sailv1.IstioRevision { return &sailv1.IstioRevision{ ObjectMeta: metav1.ObjectMeta{ @@ -224,7 +251,7 @@ func Test_Reconcile(t *testing.T) { } } - expectedSailLibraryOptions := func(version string, gieEnabled bool, proxyConfig map[string]string) *install.Options { + expectedSailLibraryOptions := func(version string, gieEnabled bool, proxyConfig map[string]string, gatewayclassesConfig json.RawMessage) *install.Options { // Start with sail-operator's Gateway API defaults aka trust upstream defaults values := install.GatewayAPIDefaults() @@ -253,6 +280,7 @@ func Test_Reconcile(t *testing.T) { ProxyMetadata: proxyConfig, }, }, + GatewayClasses: gatewayclassesConfig, } values = install.MergeValues(values, openshiftOverrides) @@ -282,23 +310,83 @@ func Test_Reconcile(t *testing.T) { expectSailLibraryUninstallCalled bool }{ { - name: "OLM mode: Nonexistent gatewayclass", + name: "OLM mode: Missing cluster infrastructure config and nonexistent gatewayclass", request: req("openshift-default"), existingObjects: []client.Object{}, expectCreate: []client.Object{}, expectUpdate: []client.Object{}, expectDelete: []client.Object{}, + expectError: `infrastructures.config.openshift.io "cluster" not found`, + }, + { + name: "OLM mode: Nonexistent gatewayclass", + request: req("openshift-default"), + existingObjects: []client.Object{ + infraConfig(configv1.HighlyAvailableTopologyMode), + }, + expectCreate: []client.Object{}, + expectUpdate: []client.Object{}, + expectDelete: []client.Object{}, //expectError: `"openshift-default" not found`, // We should not expect an error when a class is not found }, + { + name: "OLM mode: Missing cluster infrastructure config", + request: req("openshift-default"), + existingObjects: []client.Object{ + gatewayClass("openshift-default", false, nil, nil, false), + }, + expectCreate: []client.Object{}, + expectUpdate: []client.Object{}, + expectDelete: []client.Object{}, + expectError: `infrastructures.config.openshift.io "cluster" not found`, + }, { name: "OLM mode: Minimal gatewayclass", request: req("openshift-default"), existingObjects: []client.Object{ + infraConfig(configv1.HighlyAvailableTopologyMode), + gatewayClass("openshift-default", false, nil, nil, false), + }, + expectCreate: []client.Object{ + subscription("redhat-operators", "stable", "servicemeshoperator3.v3.0.1"), + istio("v1.24.4", false, nil, gatewayclassesConfig(hpaConfig(2), "openshift-default")), + }, + expectUpdate: []client.Object{}, + expectDelete: []client.Object{}, + }, + { + name: "OLM mode: Minimal gatewayclass with single-node topology", + request: req("openshift-default"), + existingObjects: []client.Object{ + infraConfig(configv1.SingleReplicaTopologyMode), + gatewayClass("openshift-default", false, nil, nil, false), + }, + expectCreate: []client.Object{ + subscription("redhat-operators", "stable", "servicemeshoperator3.v3.0.1"), + istio("v1.24.4", false, nil, gatewayclassesConfig(hpaConfig(1), "openshift-default")), + }, + expectUpdate: []client.Object{}, + expectDelete: []client.Object{}, + }, + { + name: "OLM mode: Minimal gatewayclass on a cluster with multiple gatewayclasses", + request: req("openshift-default"), + existingObjects: []client.Object{ + infraConfig(configv1.HighlyAvailableTopologyMode), gatewayClass("openshift-default", false, nil, nil, false), + gatewayClass("openshift-internal", false, nil, nil, false), + &gatewayapiv1.GatewayClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: "istio", + }, + Spec: gatewayapiv1.GatewayClassSpec{ + ControllerName: gatewayapiv1.GatewayController("istio"), + }, + }, }, expectCreate: []client.Object{ subscription("redhat-operators", "stable", "servicemeshoperator3.v3.0.1"), - istio("v1.24.4", false, nil), + istio("v1.24.4", false, nil, gatewayclassesConfig(hpaConfig(2), "openshift-default", "openshift-internal")), }, expectUpdate: []client.Object{}, expectDelete: []client.Object{}, @@ -307,12 +395,13 @@ func Test_Reconcile(t *testing.T) { name: "OLM mode: Minimal gatewayclass and system proxy", request: req("openshift-default"), existingObjects: []client.Object{ + infraConfig(configv1.HighlyAvailableTopologyMode), gatewayClass("openshift-default", false, nil, nil, false), proxyConfig("http://some.proxy.tld:8080", "https://another.proxy.tld", ".cluster.local,.ec2.internal,.svc,10.0.0.0/16,10.128.0.0/14"), }, expectCreate: []client.Object{ subscription("redhat-operators", "stable", "servicemeshoperator3.v3.0.1"), - istio("v1.24.4", false, expectedProxyConfiguration), + istio("v1.24.4", false, expectedProxyConfiguration, gatewayclassesConfig(hpaConfig(2), "openshift-default")), }, expectUpdate: []client.Object{}, expectDelete: []client.Object{}, @@ -321,6 +410,7 @@ func Test_Reconcile(t *testing.T) { name: "OLM mode: Minimal gatewayclass with experimental InferencePool CRD", request: req("openshift-default"), existingObjects: []client.Object{ + infraConfig(configv1.HighlyAvailableTopologyMode), gatewayClass("openshift-default", false, nil, nil, false), &apiextensionsv1.CustomResourceDefinition{ ObjectMeta: metav1.ObjectMeta{ @@ -330,7 +420,7 @@ func Test_Reconcile(t *testing.T) { }, expectCreate: []client.Object{ subscription("redhat-operators", "stable", "servicemeshoperator3.v3.0.1"), - istio("v1.24.4", true, nil), + istio("v1.24.4", true, nil, gatewayclassesConfig(hpaConfig(2), "openshift-default")), }, expectUpdate: []client.Object{}, expectDelete: []client.Object{}, @@ -339,6 +429,7 @@ func Test_Reconcile(t *testing.T) { name: "OLM mode: Minimal gatewayclass with stable InferencePool CRD", request: req("openshift-default"), existingObjects: []client.Object{ + infraConfig(configv1.HighlyAvailableTopologyMode), gatewayClass("openshift-default", false, nil, nil, false), &apiextensionsv1.CustomResourceDefinition{ ObjectMeta: metav1.ObjectMeta{ @@ -348,7 +439,7 @@ func Test_Reconcile(t *testing.T) { }, expectCreate: []client.Object{ subscription("redhat-operators", "stable", "servicemeshoperator3.v3.0.1"), - istio("v1.24.4", true, nil), + istio("v1.24.4", true, nil, gatewayclassesConfig(hpaConfig(2), "openshift-default")), }, expectUpdate: []client.Object{}, expectDelete: []client.Object{}, @@ -357,13 +448,14 @@ func Test_Reconcile(t *testing.T) { name: "OLM mode: Gatewayclass with Istio version override", request: req("openshift-default"), existingObjects: []client.Object{ + infraConfig(configv1.HighlyAvailableTopologyMode), gatewayClass("openshift-default", false, map[string]string{ "unsupported.do-not-use.openshift.io/istio-version": "v1.24-latest", }, nil, false), }, expectCreate: []client.Object{ subscription("redhat-operators", "stable", "servicemeshoperator3.v3.0.1"), - istio("v1.24-latest", false, nil), + istio("v1.24-latest", false, nil, gatewayclassesConfig(hpaConfig(2), "openshift-default")), }, expectUpdate: []client.Object{}, expectDelete: []client.Object{}, @@ -372,6 +464,7 @@ func Test_Reconcile(t *testing.T) { name: "OLM mode: Gatewayclass with OSSM and Istio overrides", request: req("openshift-default"), existingObjects: []client.Object{ + infraConfig(configv1.HighlyAvailableTopologyMode), gatewayClass("openshift-default", false, map[string]string{ "unsupported.do-not-use.openshift.io/ossm-catalog": "foo", "unsupported.do-not-use.openshift.io/ossm-channel": "bar", @@ -381,26 +474,53 @@ func Test_Reconcile(t *testing.T) { }, expectCreate: []client.Object{ subscription("foo", "bar", "baz"), - istio("quux", false, nil), + istio("quux", false, nil, gatewayclassesConfig(hpaConfig(2), "openshift-default")), }, expectUpdate: []client.Object{}, expectDelete: []client.Object{}, }, + { + name: "Sail Library: Missing cluster infrastructure config and nonexistent gatewayclass", + fakeSailInstaller: &fakeSailInstaller{}, + request: req("openshift-default"), + existingObjects: []client.Object{}, + expectCreate: []client.Object{}, + expectUpdate: []client.Object{}, + expectDelete: []client.Object{}, + expectError: `infrastructures.config.openshift.io "cluster" not found`, + expectedSailLibraryOptions: &install.Options{}, + }, { name: "Sail Library: nonexistent GatewayClass", fakeSailInstaller: &fakeSailInstaller{}, request: req("openshift-default"), - existingObjects: []client.Object{}, - expectCreate: []client.Object{}, - expectUpdate: []client.Object{}, - expectDelete: []client.Object{}, + existingObjects: []client.Object{ + infraConfig(configv1.HighlyAvailableTopologyMode), + }, + expectCreate: []client.Object{}, + expectUpdate: []client.Object{}, + expectDelete: []client.Object{}, //expectError: `"openshift-default" not found`, // We should not expect an error when a class is not found expectedSailLibraryOptions: &install.Options{}, }, + { + name: "Sail Library: Missing cluster infrastructure config", + fakeSailInstaller: &fakeSailInstaller{}, + request: req("openshift-default"), + existingObjects: []client.Object{ + gatewayClass("openshift-default", false, nil, nil, false), + }, + expectCreate: []client.Object{}, + expectUpdate: []client.Object{}, + expectDelete: []client.Object{}, + expectError: `infrastructures.config.openshift.io "cluster" not found`, + expectedSailLibraryOptions: &install.Options{}, + }, { name: "Sail Library: disabled - removes finalizer and status", request: req("openshift-default"), existingObjects: []client.Object{ + infraConfig(configv1.HighlyAvailableTopologyMode), gatewayClass("openshift-default", true, nil, []metav1.Condition{ { Type: ControllerInstalledConditionType, @@ -426,6 +546,7 @@ func Test_Reconcile(t *testing.T) { fakeSailInstaller: &fakeSailInstaller{}, request: req("openshift-default"), existingObjects: []client.Object{ + infraConfig(configv1.HighlyAvailableTopologyMode), gatewayClass("openshift-default", false, nil, nil, false), }, expectPatched: []client.Object{ @@ -439,13 +560,14 @@ func Test_Reconcile(t *testing.T) { fakeSailInstaller: &fakeSailInstaller{}, request: req("openshift-default"), existingObjects: []client.Object{ + infraConfig(configv1.HighlyAvailableTopologyMode), gatewayClass("openshift-default", true, nil, nil, false), istioCRD(), - istio("v1.24.4", true, nil), + istio("v1.24.4", true, nil, gatewayclassesConfig(hpaConfig(2), "openshift-default")), }, expectPatched: []client.Object{}, expectDelete: []client.Object{ - istio("v1.24.4", true, nil), + istio("v1.24.4", true, nil, gatewayclassesConfig(hpaConfig(2), "openshift-default")), }, expectedResult: reconcile.Result{ RequeueAfter: 5 * time.Second, @@ -457,6 +579,7 @@ func Test_Reconcile(t *testing.T) { fakeSailInstaller: &fakeSailInstaller{}, request: req("openshift-default"), existingObjects: []client.Object{ + infraConfig(configv1.HighlyAvailableTopologyMode), gatewayClass("openshift-default", true, nil, nil, false), istioCRD(), istioRevisionCRD(), @@ -474,6 +597,7 @@ func Test_Reconcile(t *testing.T) { fakeSailInstaller: &fakeSailInstaller{}, request: req("openshift-default"), existingObjects: []client.Object{ + infraConfig(configv1.HighlyAvailableTopologyMode), gatewayClass("openshift-default", true, nil, nil, false), istioCRD(), istioRevisionCRD(), @@ -481,38 +605,78 @@ func Test_Reconcile(t *testing.T) { expectedStatusPatched: []client.Object{ gatewayClass("openshift-default", true, nil, installedConditions(), false), }, - expectedSailLibraryOptions: expectedSailLibraryOptions("v1.24.4", false, nil), + expectedSailLibraryOptions: expectedSailLibraryOptions("v1.24.4", false, nil, gatewayclassesConfig(hpaConfig(2), "openshift-default")), }, { - name: "Sail Library: installs Istio", + name: "Sail Library: installs Istio (single replica)", fakeSailInstaller: &fakeSailInstaller{}, request: req("openshift-default"), existingObjects: []client.Object{ + infraConfig(configv1.SingleReplicaTopologyMode), gatewayClass("openshift-default", true, nil, nil, false), }, expectedStatusPatched: []client.Object{ gatewayClass("openshift-default", true, nil, installedConditions(), false), }, - expectedSailLibraryOptions: expectedSailLibraryOptions("v1.24.4", false, nil), + expectedSailLibraryOptions: expectedSailLibraryOptions("v1.24.4", false, nil, gatewayclassesConfig(hpaConfig(1), "openshift-default")), + }, + { + name: "Sail Library: installs Istio (highly available)", + fakeSailInstaller: &fakeSailInstaller{}, + request: req("openshift-default"), + existingObjects: []client.Object{ + infraConfig(configv1.HighlyAvailableTopologyMode), + gatewayClass("openshift-default", true, nil, nil, false), + }, + expectedStatusPatched: []client.Object{ + gatewayClass("openshift-default", true, nil, installedConditions(), false), + }, + expectedSailLibraryOptions: expectedSailLibraryOptions("v1.24.4", false, nil, gatewayclassesConfig(hpaConfig(2), "openshift-default")), }, { name: "Sail Library: installs Istio with system proxy configuration", fakeSailInstaller: &fakeSailInstaller{}, request: req("openshift-default"), existingObjects: []client.Object{ + infraConfig(configv1.HighlyAvailableTopologyMode), gatewayClass("openshift-default", true, nil, nil, false), proxyConfig("http://some.proxy.tld:8080", "https://another.proxy.tld", ".cluster.local,.ec2.internal,.svc,10.0.0.0/16,10.128.0.0/14"), }, expectedStatusPatched: []client.Object{ gatewayClass("openshift-default", true, nil, installedConditions(), false), }, - expectedSailLibraryOptions: expectedSailLibraryOptions("v1.24.4", false, expectedProxyConfiguration), + expectedSailLibraryOptions: expectedSailLibraryOptions("v1.24.4", false, expectedProxyConfiguration, gatewayclassesConfig(hpaConfig(2), "openshift-default")), + }, + { + name: "Sail Library: installs Istio for multiple gatewayclasses in single-topology mode with system proxy configuration", + fakeSailInstaller: &fakeSailInstaller{}, + request: req("openshift-default"), + existingObjects: []client.Object{ + infraConfig(configv1.SingleReplicaTopologyMode), + gatewayClass("openshift-default", true, nil, nil, false), + gatewayClass("openshift-internal", true, nil, nil, false), + &gatewayapiv1.GatewayClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: "istio", + }, + Spec: gatewayapiv1.GatewayClassSpec{ + ControllerName: gatewayapiv1.GatewayController("istio"), + }, + }, + gatewayClass("openshift-custom", true, nil, nil, false), + proxyConfig("http://some.proxy.tld:8080", "https://another.proxy.tld", ".cluster.local,.ec2.internal,.svc,10.0.0.0/16,10.128.0.0/14"), + }, + expectedStatusPatched: []client.Object{ + gatewayClass("openshift-default", true, nil, installedConditions(), false), + }, + expectedSailLibraryOptions: expectedSailLibraryOptions("v1.24.4", false, expectedProxyConfiguration, gatewayclassesConfig(hpaConfig(1), "openshift-default", "openshift-internal", "openshift-custom")), }, { name: "Sail Library: experimental InferencePool CRD", fakeSailInstaller: &fakeSailInstaller{}, request: req("openshift-default"), existingObjects: []client.Object{ + infraConfig(configv1.HighlyAvailableTopologyMode), gatewayClass("openshift-default", true, nil, nil, false), &apiextensionsv1.CustomResourceDefinition{ ObjectMeta: metav1.ObjectMeta{ @@ -523,13 +687,14 @@ func Test_Reconcile(t *testing.T) { expectedStatusPatched: []client.Object{ gatewayClass("openshift-default", true, nil, installedConditions(), false), }, - expectedSailLibraryOptions: expectedSailLibraryOptions("v1.24.4", true, nil), + expectedSailLibraryOptions: expectedSailLibraryOptions("v1.24.4", true, nil, gatewayclassesConfig(hpaConfig(2), "openshift-default")), }, { name: "Sail Library: stable InferencePool CRD", fakeSailInstaller: &fakeSailInstaller{}, request: req("openshift-default"), existingObjects: []client.Object{ + infraConfig(configv1.HighlyAvailableTopologyMode), gatewayClass("openshift-default", true, nil, nil, false), &apiextensionsv1.CustomResourceDefinition{ ObjectMeta: metav1.ObjectMeta{ @@ -540,13 +705,14 @@ func Test_Reconcile(t *testing.T) { expectedStatusPatched: []client.Object{ gatewayClass("openshift-default", true, nil, installedConditions(), false), }, - expectedSailLibraryOptions: expectedSailLibraryOptions("v1.24.4", true, nil), + expectedSailLibraryOptions: expectedSailLibraryOptions("v1.24.4", true, nil, gatewayclassesConfig(hpaConfig(2), "openshift-default")), }, { name: "Sail Library: Istio version override", fakeSailInstaller: &fakeSailInstaller{}, request: req("openshift-default"), existingObjects: []client.Object{ + infraConfig(configv1.HighlyAvailableTopologyMode), gatewayClass("openshift-default", true, map[string]string{ "unsupported.do-not-use.openshift.io/istio-version": "v1.24-latest", }, nil, false), @@ -556,13 +722,14 @@ func Test_Reconcile(t *testing.T) { "unsupported.do-not-use.openshift.io/istio-version": "v1.24-latest", }, installedConditions(), false), }, - expectedSailLibraryOptions: expectedSailLibraryOptions("v1.24-latest", false, nil), + expectedSailLibraryOptions: expectedSailLibraryOptions("v1.24-latest", false, nil, gatewayclassesConfig(hpaConfig(2), "openshift-default")), }, { name: "Sail Library: full removal of last GatewayClass", fakeSailInstaller: &fakeSailInstaller{}, request: req("openshift-default"), existingObjects: []client.Object{ + infraConfig(configv1.HighlyAvailableTopologyMode), gatewayClass("openshift-default", true, nil, nil, true), }, expectPatched: []client.Object{ @@ -576,6 +743,7 @@ func Test_Reconcile(t *testing.T) { fakeSailInstaller: &fakeSailInstaller{}, request: req("openshift-default"), existingObjects: []client.Object{ + infraConfig(configv1.HighlyAvailableTopologyMode), gatewayClass("openshift-default", true, nil, nil, true), gatewayClass("openshift-secondary", true, nil, nil, false), }, @@ -592,6 +760,7 @@ func Test_Reconcile(t *testing.T) { }, request: req("openshift-default"), existingObjects: []client.Object{ + infraConfig(configv1.HighlyAvailableTopologyMode), gatewayClass("openshift-default", true, nil, nil, true), }, expectError: "failed to uninstall Istio: failed to cleanup resources", diff --git a/pkg/operator/controller/gatewayclass/istio_olm.go b/pkg/operator/controller/gatewayclass/istio_olm.go index 10860c1ec0..ead8da6412 100644 --- a/pkg/operator/controller/gatewayclass/istio_olm.go +++ b/pkg/operator/controller/gatewayclass/istio_olm.go @@ -27,7 +27,7 @@ const systemClusterCriticalPriorityClassName = "system-cluster-critical" // ensureIstioOLM attempts to ensure that an Istio CR is present and returns a // Boolean indicating whether it exists, the CR if it exists, and an error // value. -func (r *reconciler) ensureIstioOLM(ctx context.Context, gatewayclass *gatewayapiv1.GatewayClass, istioVersion string) (bool, *sailv1.Istio, error) { +func (r *reconciler) ensureIstioOLM(ctx context.Context, gatewayclass *gatewayapiv1.GatewayClass, istioVersion string, gatewayclasses []gatewayapiv1.GatewayClass, infraConfig *configv1.Infrastructure) (bool, *sailv1.Istio, error) { name := controller.IstioName(r.config.OperandNamespace) have, current, err := r.currentIstio(ctx, name) if err != nil { @@ -53,9 +53,13 @@ func (r *reconciler) ensureIstioOLM(ctx context.Context, gatewayclass *gatewayap return have, current, fmt.Errorf("error verifying cluster proxy configuration: %w", err) } - desired := desiredIstio(name, ownerRef, istioVersion, enableInferenceExtension, &extraIstioConfig{ + desired, err := desiredIstio(name, ownerRef, istioVersion, enableInferenceExtension, gatewayclasses, &extraIstioConfig{ proxyConfig: &proxyConfig, + infraConfig: infraConfig, }) + if err != nil { + return have, current, err + } switch { case !have: @@ -163,7 +167,7 @@ func gatewayAPIPilotEnv(enableInferenceExtension bool) map[string]string { } // desiredIstio returns the desired Istio CR. -func desiredIstio(name types.NamespacedName, ownerRef metav1.OwnerReference, istioVersion string, enableInferenceExtension bool, extraConfig *extraIstioConfig) *sailv1.Istio { +func desiredIstio(name types.NamespacedName, ownerRef metav1.OwnerReference, istioVersion string, enableInferenceExtension bool, gatewayclasses []gatewayapiv1.GatewayClass, extraConfig *extraIstioConfig) (*sailv1.Istio, error) { pilotContainerEnv := gatewayAPIPilotEnv(enableInferenceExtension) istio := &sailv1.Istio{ ObjectMeta: metav1.ObjectMeta{ @@ -226,12 +230,22 @@ func desiredIstio(name types.NamespacedName, ownerRef metav1.OwnerReference, ist } if extraConfig != nil { - if proxyMetadata := buildProxyMetadata(extraConfig.proxyConfig); proxyMetadata != nil { - istio.Spec.Values.MeshConfig.DefaultConfig.ProxyMetadata = proxyMetadata + if extraConfig.proxyConfig != nil { + if proxyMetadata := buildProxyMetadata(extraConfig.proxyConfig); proxyMetadata != nil { + istio.Spec.Values.MeshConfig.DefaultConfig.ProxyMetadata = proxyMetadata + } + } + + if extraConfig.infraConfig != nil { + if hpaConfig, err := buildHorizontalPodAutoscalerConfig(extraConfig.infraConfig, gatewayclasses); err != nil { + return nil, err + } else { + istio.Spec.Values.GatewayClasses = hpaConfig + } } } - return istio + return istio, nil } // currentIstio returns the current istio CR. diff --git a/pkg/operator/controller/gatewayclass/istio_sail_installer.go b/pkg/operator/controller/gatewayclass/istio_sail_installer.go index a1dd16d69c..beb509c03f 100644 --- a/pkg/operator/controller/gatewayclass/istio_sail_installer.go +++ b/pkg/operator/controller/gatewayclass/istio_sail_installer.go @@ -2,11 +2,13 @@ package gatewayclass import ( "context" + "encoding/json" "fmt" "strings" sailv1 "github.com/istio-ecosystem/sail-operator/api/v1" "github.com/istio-ecosystem/sail-operator/pkg/install" + gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" configv1 "github.com/openshift/api/config/v1" "github.com/openshift/cluster-ingress-operator/pkg/operator/controller" @@ -116,7 +118,7 @@ func (r *reconciler) overwriteOLMManagedCRDFunc(ctx context.Context, crd *apiext // ensureIstio installs or updates Istio using the Sail Library. // It returns an error if the installation fails. -func (r *reconciler) ensureIstio(ctx context.Context, istioVersion string) error { +func (r *reconciler) ensureIstio(ctx context.Context, istioVersion string, gatewayclasses []gatewayapiv1.GatewayClass, infraConfig *configv1.Infrastructure) error { sailInstaller := r.sailInstaller enableInferenceExtension, err := r.inferencepoolCrdExists(ctx) @@ -132,9 +134,13 @@ func (r *reconciler) ensureIstio(ctx context.Context, istioVersion string) error } // Build options from current state - opts := r.buildInstallerOptions(enableInferenceExtension, istioVersion, &extraIstioConfig{ + opts, err := r.buildInstallerOptions(enableInferenceExtension, istioVersion, gatewayclasses, &extraIstioConfig{ proxyConfig: &proxyConfig, + infraConfig: infraConfig, }) + if err != nil { + return err + } opts.OverwriteOLMManagedCRD = r.overwriteOLMManagedCRDFunc @@ -148,12 +154,15 @@ func (r *reconciler) ensureIstio(ctx context.Context, istioVersion string) error // buildInstallerOptions creates Sail Library installation options by merging // Gateway API defaults with OpenShift-specific overrides -func (r *reconciler) buildInstallerOptions(enableInferenceExtension bool, istioVersion string, extraConfig *extraIstioConfig) install.Options { +func (r *reconciler) buildInstallerOptions(enableInferenceExtension bool, istioVersion string, gatewayclasses []gatewayapiv1.GatewayClass, extraConfig *extraIstioConfig) (install.Options, error) { // Start with Gateway API defaults values := install.GatewayAPIDefaults() // Apply OpenShift-specific overrides - openshiftOverrides := openshiftValues(enableInferenceExtension, r.config.OperandNamespace, extraConfig) + openshiftOverrides, err := openshiftValues(enableInferenceExtension, r.config.OperandNamespace, gatewayclasses, extraConfig) + if err != nil { + return install.Options{}, err + } values = install.MergeValues(values, openshiftOverrides) return install.Options{ @@ -163,12 +172,12 @@ func (r *reconciler) buildInstallerOptions(enableInferenceExtension bool, istioV Version: istioVersion, ManageCRDs: ptr.To(true), IncludeAllCRDs: ptr.To(true), - } + }, nil } // openshiftValues returns the OpenShift-specific value overrides for Istio. // These values are merged on top of the gateway-api preset defaults. -func openshiftValues(enableInferenceExtension bool, operandNamespace string, extraConfig *extraIstioConfig) *sailv1.Values { +func openshiftValues(enableInferenceExtension bool, operandNamespace string, gatewayclasses []gatewayapiv1.GatewayClass, extraConfig *extraIstioConfig) (*sailv1.Values, error) { pilotEnv := gatewayAPIPilotEnv(enableInferenceExtension) val := &sailv1.Values{ @@ -194,12 +203,21 @@ func openshiftValues(enableInferenceExtension bool, operandNamespace string, ext }, } - if extraConfig != nil && extraConfig.proxyConfig != nil { - if proxyMetadata := buildProxyMetadata(extraConfig.proxyConfig); proxyMetadata != nil { - val.MeshConfig.DefaultConfig.ProxyMetadata = proxyMetadata + if extraConfig != nil { + if extraConfig.proxyConfig != nil { + if proxyMetadata := buildProxyMetadata(extraConfig.proxyConfig); proxyMetadata != nil { + val.MeshConfig.DefaultConfig.ProxyMetadata = proxyMetadata + } + } + if extraConfig.infraConfig != nil { + if hpaConfig, err := buildHorizontalPodAutoscalerConfig(extraConfig.infraConfig, gatewayclasses); err != nil { + return nil, fmt.Errorf("failed to build HPA config: %w", err) + } else { + val.GatewayClasses = hpaConfig + } } } - return val + return val, nil } func buildProxyMetadata(proxyConfig *configv1.Proxy) map[string]string { @@ -225,3 +243,35 @@ func buildProxyMetadata(proxyConfig *configv1.Proxy) map[string]string { } return proxyMetadata } + +// buildHorizontalPodAutoscalerConfig returns Istio configuration for the +// horizontal pod autoscaler given an infrastructure config and a slice of +// gatewayclasses. +func buildHorizontalPodAutoscalerConfig(infraConfig *configv1.Infrastructure, gatewayclasses []gatewayapiv1.GatewayClass) (json.RawMessage, error) { + const maxReplicas = 10 + + var minReplicas = 2 + if infraConfig.Status.InfrastructureTopology == configv1.SingleReplicaTopologyMode { + minReplicas = 1 + } + + gatewayclassConfig := map[string]any{ + "horizontalPodAutoscaler": map[string]any{ + "spec": map[string]any{ + "minReplicas": minReplicas, + "maxReplicas": maxReplicas, + }, + }, + } + gatewayclassesConfig := map[string]any{} + for _, gatewayclass := range gatewayclasses { + gatewayclassesConfig[gatewayclass.Name] = gatewayclassConfig + } + + gatewayclassesConfigJson, err := json.Marshal(gatewayclassesConfig) + if err != nil { + return nil, err + } + + return gatewayclassesConfigJson, nil +} diff --git a/pkg/operator/controller/gatewayclass/istio_sail_installer_test.go b/pkg/operator/controller/gatewayclass/istio_sail_installer_test.go index 6f3b3559b8..cf17efe4fa 100644 --- a/pkg/operator/controller/gatewayclass/istio_sail_installer_test.go +++ b/pkg/operator/controller/gatewayclass/istio_sail_installer_test.go @@ -10,6 +10,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/istio-ecosystem/sail-operator/pkg/install" + gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" configv1 "github.com/openshift/api/config/v1" testutil "github.com/openshift/cluster-ingress-operator/pkg/operator/controller/test/util" @@ -540,21 +541,55 @@ func Test_mapStatusToConditions(t *testing.T) { // is identical between the OLM path (desiredIstio) and Sail Library path (openshiftValues) // during the transition period. This test can be removed once the OLM path is deleted. func TestOLMAndSailLibraryValuesMatch(t *testing.T) { + defaultGatewayClass := []gatewayapiv1.GatewayClass{{ + ObjectMeta: metav1.ObjectMeta{ + Name: "openshift-default", + }, + }} + infraConfig := func(infraTopologyMode configv1.TopologyMode) *configv1.Infrastructure { + return &configv1.Infrastructure{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Status: configv1.InfrastructureStatus{ + InfrastructureTopology: infraTopologyMode, + }, + } + } + singleReplicaInfraConfig := infraConfig(configv1.SingleReplicaTopologyMode) + highlyAvailableInfraConfig := infraConfig(configv1.HighlyAvailableTopologyMode) + testCases := []struct { name string + gatewayclasses []gatewayapiv1.GatewayClass enableInferenceExtension bool extraconfig *extraIstioConfig }{ { name: "without inference extension", + gatewayclasses: defaultGatewayClass, enableInferenceExtension: false, + extraconfig: &extraIstioConfig{ + infraConfig: highlyAvailableInfraConfig, + }, }, { name: "with inference extension", + gatewayclasses: defaultGatewayClass, enableInferenceExtension: true, + extraconfig: &extraIstioConfig{ + infraConfig: highlyAvailableInfraConfig, + }, + }, + { + name: "with single-node topology", + gatewayclasses: defaultGatewayClass, + enableInferenceExtension: true, + extraconfig: &extraIstioConfig{ + infraConfig: singleReplicaInfraConfig, + }, }, { - name: "with system proxy config enabled", + name: "with system proxy config enabled", + gatewayclasses: defaultGatewayClass, extraconfig: &extraIstioConfig{ proxyConfig: &configv1.Proxy{ Status: configv1.ProxyStatus{ @@ -563,6 +598,7 @@ func TestOLMAndSailLibraryValuesMatch(t *testing.T) { NoProxy: ".cluster.local,.ec2.internal,.svc,10.0.0.0/16,10.128.0.0/14", }, }, + infraConfig: highlyAvailableInfraConfig, }, }, } @@ -571,17 +607,20 @@ func TestOLMAndSailLibraryValuesMatch(t *testing.T) { t.Run(tc.name, func(t *testing.T) { // Get values from Sail Library path (GatewayAPIDefaults + OpenShift overrides) sailValues := install.GatewayAPIDefaults() - openshiftOverrides := openshiftValues(tc.enableInferenceExtension, "openshift-ingress", tc.extraconfig) + openshiftOverrides, err := openshiftValues(tc.enableInferenceExtension, "openshift-ingress", tc.gatewayclasses, tc.extraconfig) + assert.Nil(t, err) sailValues = install.MergeValues(sailValues, openshiftOverrides) // Get values from OLM path - olmIstio := desiredIstio( + olmIstio, err := desiredIstio( types.NamespacedName{Name: "test", Namespace: "test-ns"}, metav1.OwnerReference{}, "v1.24.4", tc.enableInferenceExtension, + tc.gatewayclasses, tc.extraconfig, ) + assert.Nil(t, err) cmpOpts := []cmp.Option{ cmpopts.EquateEmpty(), diff --git a/pkg/operator/controller/status/controller.go b/pkg/operator/controller/status/controller.go index 930a7abd9b..e5fe23c0f1 100644 --- a/pkg/operator/controller/status/controller.go +++ b/pkg/operator/controller/status/controller.go @@ -23,6 +23,7 @@ import ( "github.com/openshift/cluster-ingress-operator/pkg/operator/controller/ingress" oputil "github.com/openshift/cluster-ingress-operator/pkg/util" + autoscalingv2 "k8s.io/api/autoscaling/v2" corev1 "k8s.io/api/core/v1" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" @@ -275,6 +276,12 @@ func (r *reconciler) Reconcile(ctx context.Context, request reconcile.Request) ( Group: iov1.GroupVersion.Group, Resource: "dnsrecords", Namespace: state.IngressNamespace.Name, + }, configv1.ObjectReference{ + Group: autoscalingv2.GroupName, + Resource: "horizontalpodautoscalers", + // The operator configures HPA for gateways in the + // operand namespace. + Namespace: state.IngressNamespace.Name, }) } if state.CanaryNamespace != nil { diff --git a/test/e2e/gateway_api_test.go b/test/e2e/gateway_api_test.go index 88704f48ba..dcd4289ecb 100644 --- a/test/e2e/gateway_api_test.go +++ b/test/e2e/gateway_api_test.go @@ -1565,6 +1565,16 @@ func ensureGatewayObjectSuccess(t *testing.T, ns *corev1.Namespace) []string { errs = append(errs, error.Error(err)) } + // Keep going even if there was an error; the gateway might be partially + // configured, and the test should report what is or isn't as expected. + + t.Log("Verifying HPA is enabled...") + var expectedMinReplicas = 2 + if infraConfig.Status.InfrastructureTopology == configv1.SingleReplicaTopologyMode { + expectedMinReplicas = 1 + } + assertHorizontalPodAutoscalerEnabled(t, operatorcontroller.DefaultOperandNamespace, testGatewayName, operatorcontroller.OpenShiftDefaultGatewayClassName, expectedMinReplicas) + t.Log("Making sure the httproute is created and accepted...") _, err = assertHttpRouteSuccessful(t, ns.Name, "test-httproute", gateway) if err != nil { diff --git a/test/e2e/operator_test.go b/test/e2e/operator_test.go index bff490d00e..447578cead 100644 --- a/test/e2e/operator_test.go +++ b/test/e2e/operator_test.go @@ -232,6 +232,10 @@ func TestClusterOperatorStatusRelatedObjects(t *testing.T) { Group: iov1.GroupVersion.Group, Resource: "dnsrecords", Namespace: "openshift-ingress", + }, { + Group: "autoscaling", + Resource: "horizontalpodautoscalers", + Namespace: "openshift-ingress", }, { Resource: "namespaces", diff --git a/test/e2e/util_gatewayapi_test.go b/test/e2e/util_gatewayapi_test.go index b21fa323e0..d3068d64a4 100644 --- a/test/e2e/util_gatewayapi_test.go +++ b/test/e2e/util_gatewayapi_test.go @@ -26,6 +26,7 @@ import ( "github.com/google/go-cmp/cmp" admissionregistrationv1 "k8s.io/api/admissionregistration/v1" appsv1 "k8s.io/api/apps/v1" + autoscalingv2 "k8s.io/api/autoscaling/v2" corev1 "k8s.io/api/core/v1" rbacv1 "k8s.io/api/rbac/v1" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" @@ -771,6 +772,45 @@ func assertGatewaySuccessful(t *testing.T, namespace, name string) (*gatewayapiv return gw, nil } +// assertHorizontalPodAutoscalerEnabled verifies that HPA is enabled and +// configured appropriately for the given gateway. +func assertHorizontalPodAutoscalerEnabled(t *testing.T, namespaceName, gatewayName, gatewayclassName string, expectedMinReplicas int) { + t.Helper() + + var hpa autoscalingv2.HorizontalPodAutoscaler + hpaName := types.NamespacedName{ + Namespace: namespaceName, + Name: fmt.Sprintf("%s-%s", gatewayName, gatewayclassName), + } + gatewayNamespacedName := types.NamespacedName{ + Namespace: namespaceName, + Name: gatewayName, + } + t.Logf("Getting HorizontalPodAutoscaler %s for Gateway %s...", hpaName, gatewayNamespacedName) + if err := wait.PollUntilContextTimeout(context.Background(), 1*time.Second, 1*time.Minute, false, func(context context.Context) (bool, error) { + if err := kclient.Get(context, hpaName, &hpa); err != nil { + t.Logf("Failed to get HorizontalPodAutoscaler %s: %v; retrying...", hpaName, err) + + return false, nil + } + + if hpa.Spec.MinReplicas == nil { + t.Logf("HorizontalPodAutoscaler %s has no minReplicas set yet; retrying...", hpaName) + + return false, nil + } + + if int(*hpa.Spec.MinReplicas) != expectedMinReplicas { + t.Logf("HorizontalPodAutoscaler %s has minReplicas %d; expected %d; retrying...", hpaName, *hpa.Spec.MinReplicas, expectedMinReplicas) + return false, nil + } + + return true, nil + }); err != nil { + t.Errorf("Failed to verify that gateway %s has the expected HorizontalPodAutoscaler: %v", gatewayNamespacedName, err) + } +} + func waitForGatewayListenerCondition(t *testing.T, gatewayName types.NamespacedName, listenerName string, conditions ...metav1.Condition) error { t.Helper()