From da0bcdffb7c1b3bda50e142f9ff266c0ea779b41 Mon Sep 17 00:00:00 2001 From: Grant Spence Date: Fri, 1 May 2026 10:03:27 -0400 Subject: [PATCH] OCPBUGS-84491: Set terminationMessagePolicy on gateway proxy containers Add a deployment overlay to the gatewayclass configuration that sets terminationMessagePolicy=FallbackToLogsOnError on the istio-proxy container, satisfying the OCP platform policy enforced by the termination-message-policy monitor test. https://redhat.atlassian.net/browse/OCPBUGS-84491 Co-Authored-By: Claude Opus 4.6 (1M context) --- .../controller/gatewayclass/controller.go | 3 + .../gatewayclass/controller_test.go | 56 +++++++++---------- .../controller/gatewayclass/istio_olm.go | 4 +- .../gatewayclass/istio_sail_installer.go | 28 +++++++--- test/e2e/gateway_api_test.go | 3 + test/e2e/util_gatewayapi_test.go | 28 ++++++++++ 6 files changed, 83 insertions(+), 39 deletions(-) diff --git a/pkg/operator/controller/gatewayclass/controller.go b/pkg/operator/controller/gatewayclass/controller.go index 28f206abc6..dfd5f11450 100644 --- a/pkg/operator/controller/gatewayclass/controller.go +++ b/pkg/operator/controller/gatewayclass/controller.go @@ -81,6 +81,9 @@ const ( // can be specified. This annotation is only intended for use by // OpenShift developers. istioVersionOverrideAnnotationKey = "unsupported.do-not-use.openshift.io/istio-version" + // gatewayProxyContainerName is the name of the proxy container + // in gateway deployments managed by Istio. + gatewayProxyContainerName = "istio-proxy" // sailLibraryFinalizer is added to GatewayClasses using Sail Library installation. // When a GatewayClass with this finalizer is deleted: // 1. Sail Library mode: Uninstall Istio if this is the last GatewayClass, then remove finalizer diff --git a/pkg/operator/controller/gatewayclass/controller_test.go b/pkg/operator/controller/gatewayclass/controller_test.go index 25256e679b..12ac1b08f1 100644 --- a/pkg/operator/controller/gatewayclass/controller_test.go +++ b/pkg/operator/controller/gatewayclass/controller_test.go @@ -4,7 +4,6 @@ import ( "context" "encoding/json" "fmt" - "strings" "testing" "time" @@ -184,18 +183,15 @@ func Test_Reconcile(t *testing.T) { } 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 - }(), ","))) + payload := make(map[string]json.RawMessage, len(gatewayclasses)) + for _, name := range gatewayclasses { + payload[name] = json.RawMessage(config) + } + b, _ := json.Marshal(payload) + return b } - hpaConfig := func(minReplicas int) string { - return fmt.Sprintf(`{"horizontalPodAutoscaler":{"spec":{"maxReplicas":10,"minReplicas":%d}}}`, minReplicas) + gatewayclassConfig := func(minReplicas int) string { + return fmt.Sprintf(`{"deployment":{"spec":{"template":{"spec":{"containers":[{"name":"istio-proxy","terminationMessagePolicy":"FallbackToLogsOnError"}]}}}},"horizontalPodAutoscaler":{"spec":{"maxReplicas":10,"minReplicas":%d}}}`, minReplicas) } istioRevision := func() *sailv1.IstioRevision { @@ -351,7 +347,7 @@ func Test_Reconcile(t *testing.T) { }, expectCreate: []client.Object{ subscription("redhat-operators", "stable", "servicemeshoperator3.v3.0.1"), - istio("v1.24.4", false, nil, gatewayclassesConfig(hpaConfig(2), "openshift-default")), + istio("v1.24.4", false, nil, gatewayclassesConfig(gatewayclassConfig(2), "openshift-default")), manifests.IstiodAllowNetworkPolicy(), }, expectUpdate: []client.Object{}, @@ -366,7 +362,7 @@ func Test_Reconcile(t *testing.T) { }, expectCreate: []client.Object{ subscription("redhat-operators", "stable", "servicemeshoperator3.v3.0.1"), - istio("v1.24.4", false, nil, gatewayclassesConfig(hpaConfig(1), "openshift-default")), + istio("v1.24.4", false, nil, gatewayclassesConfig(gatewayclassConfig(1), "openshift-default")), manifests.IstiodAllowNetworkPolicy(), }, expectUpdate: []client.Object{}, @@ -390,7 +386,7 @@ func Test_Reconcile(t *testing.T) { }, expectCreate: []client.Object{ subscription("redhat-operators", "stable", "servicemeshoperator3.v3.0.1"), - istio("v1.24.4", false, nil, gatewayclassesConfig(hpaConfig(2), "openshift-default", "openshift-internal")), + istio("v1.24.4", false, nil, gatewayclassesConfig(gatewayclassConfig(2), "openshift-default", "openshift-internal")), manifests.IstiodAllowNetworkPolicy(), }, expectUpdate: []client.Object{}, @@ -406,7 +402,7 @@ func Test_Reconcile(t *testing.T) { }, expectCreate: []client.Object{ subscription("redhat-operators", "stable", "servicemeshoperator3.v3.0.1"), - istio("v1.24.4", false, expectedProxyConfiguration, gatewayclassesConfig(hpaConfig(2), "openshift-default")), + istio("v1.24.4", false, expectedProxyConfiguration, gatewayclassesConfig(gatewayclassConfig(2), "openshift-default")), manifests.IstiodAllowNetworkPolicy(), }, expectUpdate: []client.Object{}, @@ -426,7 +422,7 @@ func Test_Reconcile(t *testing.T) { }, expectCreate: []client.Object{ subscription("redhat-operators", "stable", "servicemeshoperator3.v3.0.1"), - istio("v1.24.4", true, nil, gatewayclassesConfig(hpaConfig(2), "openshift-default")), + istio("v1.24.4", true, nil, gatewayclassesConfig(gatewayclassConfig(2), "openshift-default")), manifests.IstiodAllowNetworkPolicy(), }, expectUpdate: []client.Object{}, @@ -446,7 +442,7 @@ func Test_Reconcile(t *testing.T) { }, expectCreate: []client.Object{ subscription("redhat-operators", "stable", "servicemeshoperator3.v3.0.1"), - istio("v1.24.4", true, nil, gatewayclassesConfig(hpaConfig(2), "openshift-default")), + istio("v1.24.4", true, nil, gatewayclassesConfig(gatewayclassConfig(2), "openshift-default")), manifests.IstiodAllowNetworkPolicy(), }, expectUpdate: []client.Object{}, @@ -463,7 +459,7 @@ func Test_Reconcile(t *testing.T) { }, expectCreate: []client.Object{ subscription("redhat-operators", "stable", "servicemeshoperator3.v3.0.1"), - istio("v1.24-latest", false, nil, gatewayclassesConfig(hpaConfig(2), "openshift-default")), + istio("v1.24-latest", false, nil, gatewayclassesConfig(gatewayclassConfig(2), "openshift-default")), manifests.IstiodAllowNetworkPolicy(), }, expectUpdate: []client.Object{}, @@ -483,7 +479,7 @@ func Test_Reconcile(t *testing.T) { }, expectCreate: []client.Object{ subscription("foo", "bar", "baz"), - istio("quux", false, nil, gatewayclassesConfig(hpaConfig(2), "openshift-default")), + istio("quux", false, nil, gatewayclassesConfig(gatewayclassConfig(2), "openshift-default")), manifests.IstiodAllowNetworkPolicy(), }, expectUpdate: []client.Object{}, @@ -573,11 +569,11 @@ func Test_Reconcile(t *testing.T) { infraConfig(configv1.HighlyAvailableTopologyMode), gatewayClass("openshift-default", true, nil, nil, false), istioCRD(), - istio("v1.24.4", true, nil, gatewayclassesConfig(hpaConfig(2), "openshift-default")), + istio("v1.24.4", true, nil, gatewayclassesConfig(gatewayclassConfig(2), "openshift-default")), }, expectPatched: []client.Object{}, expectDelete: []client.Object{ - istio("v1.24.4", true, nil, gatewayclassesConfig(hpaConfig(2), "openshift-default")), + istio("v1.24.4", true, nil, gatewayclassesConfig(gatewayclassConfig(2), "openshift-default")), }, expectedResult: reconcile.Result{ RequeueAfter: 5 * time.Second, @@ -618,7 +614,7 @@ func Test_Reconcile(t *testing.T) { expectedStatusPatched: []client.Object{ gatewayClass("openshift-default", true, nil, installedConditions(), false), }, - expectedSailLibraryOptions: expectedSailLibraryOptions("v1.24.4", false, nil, gatewayclassesConfig(hpaConfig(2), "openshift-default")), + expectedSailLibraryOptions: expectedSailLibraryOptions("v1.24.4", false, nil, gatewayclassesConfig(gatewayclassConfig(2), "openshift-default")), }, { name: "Sail Library: installs Istio (single replica)", @@ -634,7 +630,7 @@ func Test_Reconcile(t *testing.T) { expectedStatusPatched: []client.Object{ gatewayClass("openshift-default", true, nil, installedConditions(), false), }, - expectedSailLibraryOptions: expectedSailLibraryOptions("v1.24.4", false, nil, gatewayclassesConfig(hpaConfig(1), "openshift-default")), + expectedSailLibraryOptions: expectedSailLibraryOptions("v1.24.4", false, nil, gatewayclassesConfig(gatewayclassConfig(1), "openshift-default")), }, { name: "Sail Library: installs Istio (highly available)", @@ -650,7 +646,7 @@ func Test_Reconcile(t *testing.T) { expectedStatusPatched: []client.Object{ gatewayClass("openshift-default", true, nil, installedConditions(), false), }, - expectedSailLibraryOptions: expectedSailLibraryOptions("v1.24.4", false, nil, gatewayclassesConfig(hpaConfig(2), "openshift-default")), + expectedSailLibraryOptions: expectedSailLibraryOptions("v1.24.4", false, nil, gatewayclassesConfig(gatewayclassConfig(2), "openshift-default")), }, { name: "Sail Library: installs Istio with system proxy configuration", @@ -667,7 +663,7 @@ func Test_Reconcile(t *testing.T) { expectedStatusPatched: []client.Object{ gatewayClass("openshift-default", true, nil, installedConditions(), false), }, - expectedSailLibraryOptions: expectedSailLibraryOptions("v1.24.4", false, expectedProxyConfiguration, gatewayclassesConfig(hpaConfig(2), "openshift-default")), + expectedSailLibraryOptions: expectedSailLibraryOptions("v1.24.4", false, expectedProxyConfiguration, gatewayclassesConfig(gatewayclassConfig(2), "openshift-default")), }, { name: "Sail Library: installs Istio for multiple gatewayclasses in single-topology mode with system proxy configuration", @@ -694,7 +690,7 @@ func Test_Reconcile(t *testing.T) { 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")), + expectedSailLibraryOptions: expectedSailLibraryOptions("v1.24.4", false, expectedProxyConfiguration, gatewayclassesConfig(gatewayclassConfig(1), "openshift-default", "openshift-internal", "openshift-custom")), }, { name: "Sail Library: experimental InferencePool CRD", @@ -715,7 +711,7 @@ func Test_Reconcile(t *testing.T) { expectedStatusPatched: []client.Object{ gatewayClass("openshift-default", true, nil, installedConditions(), false), }, - expectedSailLibraryOptions: expectedSailLibraryOptions("v1.24.4", true, nil, gatewayclassesConfig(hpaConfig(2), "openshift-default")), + expectedSailLibraryOptions: expectedSailLibraryOptions("v1.24.4", true, nil, gatewayclassesConfig(gatewayclassConfig(2), "openshift-default")), }, { name: "Sail Library: stable InferencePool CRD", @@ -736,7 +732,7 @@ func Test_Reconcile(t *testing.T) { expectedStatusPatched: []client.Object{ gatewayClass("openshift-default", true, nil, installedConditions(), false), }, - expectedSailLibraryOptions: expectedSailLibraryOptions("v1.24.4", true, nil, gatewayclassesConfig(hpaConfig(2), "openshift-default")), + expectedSailLibraryOptions: expectedSailLibraryOptions("v1.24.4", true, nil, gatewayclassesConfig(gatewayclassConfig(2), "openshift-default")), }, { name: "Sail Library: Istio version override", @@ -756,7 +752,7 @@ 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, gatewayclassesConfig(hpaConfig(2), "openshift-default")), + expectedSailLibraryOptions: expectedSailLibraryOptions("v1.24-latest", false, nil, gatewayclassesConfig(gatewayclassConfig(2), "openshift-default")), }, { name: "Sail Library: full removal of last GatewayClass", diff --git a/pkg/operator/controller/gatewayclass/istio_olm.go b/pkg/operator/controller/gatewayclass/istio_olm.go index ead8da6412..8757e50c8f 100644 --- a/pkg/operator/controller/gatewayclass/istio_olm.go +++ b/pkg/operator/controller/gatewayclass/istio_olm.go @@ -237,10 +237,10 @@ func desiredIstio(name types.NamespacedName, ownerRef metav1.OwnerReference, ist } if extraConfig.infraConfig != nil { - if hpaConfig, err := buildHorizontalPodAutoscalerConfig(extraConfig.infraConfig, gatewayclasses); err != nil { + if gwClassConfig, err := buildGatewayClassesConfig(extraConfig.infraConfig, gatewayclasses); err != nil { return nil, err } else { - istio.Spec.Values.GatewayClasses = hpaConfig + istio.Spec.Values.GatewayClasses = gwClassConfig } } } diff --git a/pkg/operator/controller/gatewayclass/istio_sail_installer.go b/pkg/operator/controller/gatewayclass/istio_sail_installer.go index beb509c03f..f2f770f8db 100644 --- a/pkg/operator/controller/gatewayclass/istio_sail_installer.go +++ b/pkg/operator/controller/gatewayclass/istio_sail_installer.go @@ -8,6 +8,7 @@ import ( sailv1 "github.com/istio-ecosystem/sail-operator/api/v1" "github.com/istio-ecosystem/sail-operator/pkg/install" + v1 "k8s.io/api/core/v1" gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1" configv1 "github.com/openshift/api/config/v1" @@ -210,10 +211,10 @@ func openshiftValues(enableInferenceExtension bool, operandNamespace string, gat } } if extraConfig.infraConfig != nil { - if hpaConfig, err := buildHorizontalPodAutoscalerConfig(extraConfig.infraConfig, gatewayclasses); err != nil { - return nil, fmt.Errorf("failed to build HPA config: %w", err) + if gwClassConfig, err := buildGatewayClassesConfig(extraConfig.infraConfig, gatewayclasses); err != nil { + return nil, fmt.Errorf("failed to build gateway class config: %w", err) } else { - val.GatewayClasses = hpaConfig + val.GatewayClasses = gwClassConfig } } } @@ -244,10 +245,9 @@ 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) { +// buildGatewayClassesConfig returns Istio per-gatewayclass configuration +// overlays given an infrastructure config and a slice of gatewayclasses. +func buildGatewayClassesConfig(infraConfig *configv1.Infrastructure, gatewayclasses []gatewayapiv1.GatewayClass) (json.RawMessage, error) { const maxReplicas = 10 var minReplicas = 2 @@ -262,6 +262,20 @@ func buildHorizontalPodAutoscalerConfig(infraConfig *configv1.Infrastructure, ga "maxReplicas": maxReplicas, }, }, + "deployment": map[string]any{ + "spec": map[string]any{ + "template": map[string]any{ + "spec": map[string]any{ + "containers": []map[string]any{ + { + "name": gatewayProxyContainerName, + "terminationMessagePolicy": v1.TerminationMessageFallbackToLogsOnError, + }, + }, + }, + }, + }, + }, } gatewayclassesConfig := map[string]any{} for _, gatewayclass := range gatewayclasses { diff --git a/test/e2e/gateway_api_test.go b/test/e2e/gateway_api_test.go index dcd4289ecb..0c33cee3bb 100644 --- a/test/e2e/gateway_api_test.go +++ b/test/e2e/gateway_api_test.go @@ -1575,6 +1575,9 @@ func ensureGatewayObjectSuccess(t *testing.T, ns *corev1.Namespace) []string { } assertHorizontalPodAutoscalerEnabled(t, operatorcontroller.DefaultOperandNamespace, testGatewayName, operatorcontroller.OpenShiftDefaultGatewayClassName, expectedMinReplicas) + t.Log("Verifying terminationMessagePolicy is FallbackToLogsOnError...") + assertTerminationMessagePolicy(t, operatorcontroller.DefaultOperandNamespace, testGatewayName, operatorcontroller.OpenShiftDefaultGatewayClassName) + 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/util_gatewayapi_test.go b/test/e2e/util_gatewayapi_test.go index d3068d64a4..3480172ad4 100644 --- a/test/e2e/util_gatewayapi_test.go +++ b/test/e2e/util_gatewayapi_test.go @@ -811,6 +811,34 @@ func assertHorizontalPodAutoscalerEnabled(t *testing.T, namespaceName, gatewayNa } } +// assertTerminationMessagePolicy verifies that all containers in the gateway +// deployment have terminationMessagePolicy set to FallbackToLogsOnError. +func assertTerminationMessagePolicy(t *testing.T, namespaceName, gatewayName, gatewayclassName string) { + t.Helper() + + deploymentName := types.NamespacedName{ + Namespace: namespaceName, + Name: fmt.Sprintf("%s-%s", gatewayName, gatewayclassName), + } + t.Logf("Verifying terminationMessagePolicy on deployment %s...", deploymentName) + if err := wait.PollUntilContextTimeout(context.Background(), 2*time.Second, 2*time.Minute, false, func(ctx context.Context) (bool, error) { + var dep appsv1.Deployment + if err := kclient.Get(ctx, deploymentName, &dep); err != nil { + t.Logf("Failed to get Deployment %s: %v; retrying...", deploymentName, err) + return false, nil + } + for _, c := range dep.Spec.Template.Spec.Containers { + if c.TerminationMessagePolicy != corev1.TerminationMessageFallbackToLogsOnError { + t.Logf("Container %q has terminationMessagePolicy %q, expected FallbackToLogsOnError; retrying...", c.Name, c.TerminationMessagePolicy) + return false, nil + } + } + return true, nil + }); err != nil { + t.Errorf("Failed to verify terminationMessagePolicy on deployment %s: %v", deploymentName, err) + } +} + func waitForGatewayListenerCondition(t *testing.T, gatewayName types.NamespacedName, listenerName string, conditions ...metav1.Condition) error { t.Helper()