Skip to content

Commit 65ca117

Browse files
gcs278claude
gcs278
and
claude
committed
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) <noreply@anthropic.com>
1 parent 9e5261e commit 65ca117

5 files changed

Lines changed: 79 additions & 39 deletions

File tree

‎pkg/operator/controller/gatewayclass/controller_test.go‎

Lines changed: 26 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import (
44
"context"
55
"encoding/json"
66
"fmt"
7-
"strings"
87
"testing"
98
"time"
109

@@ -184,18 +183,15 @@ func Test_Reconcile(t *testing.T) {
184183
}
185184

186185
gatewayclassesConfig := func(config string, gatewayclasses ...string) json.RawMessage {
187-
return json.RawMessage(fmt.Appendf(nil, `{%s}`, strings.Join(func() []string {
188-
var result []string
189-
190-
for _, name := range gatewayclasses {
191-
result = append(result, fmt.Sprintf(`"%s":%s`, name, config))
192-
}
193-
194-
return result
195-
}(), ",")))
186+
payload := make(map[string]json.RawMessage, len(gatewayclasses))
187+
for _, name := range gatewayclasses {
188+
payload[name] = json.RawMessage(config)
189+
}
190+
b, _ := json.Marshal(payload)
191+
return b
196192
}
197-
hpaConfig := func(minReplicas int) string {
198-
return fmt.Sprintf(`{"horizontalPodAutoscaler":{"spec":{"maxReplicas":10,"minReplicas":%d}}}`, minReplicas)
193+
gatewayclassConfig := func(minReplicas int) string {
194+
return fmt.Sprintf(`{"deployment":{"spec":{"template":{"spec":{"containers":[{"name":"istio-proxy","terminationMessagePolicy":"FallbackToLogsOnError"}]}}}},"horizontalPodAutoscaler":{"spec":{"maxReplicas":10,"minReplicas":%d}}}`, minReplicas)
199195
}
200196

201197
istioRevision := func() *sailv1.IstioRevision {
@@ -351,7 +347,7 @@ func Test_Reconcile(t *testing.T) {
351347
},
352348
expectCreate: []client.Object{
353349
subscription("redhat-operators", "stable", "servicemeshoperator3.v3.0.1"),
354-
istio("v1.24.4", false, nil, gatewayclassesConfig(hpaConfig(2), "openshift-default")),
350+
istio("v1.24.4", false, nil, gatewayclassesConfig(gatewayclassConfig(2), "openshift-default")),
355351
manifests.IstiodAllowNetworkPolicy(),
356352
},
357353
expectUpdate: []client.Object{},
@@ -366,7 +362,7 @@ func Test_Reconcile(t *testing.T) {
366362
},
367363
expectCreate: []client.Object{
368364
subscription("redhat-operators", "stable", "servicemeshoperator3.v3.0.1"),
369-
istio("v1.24.4", false, nil, gatewayclassesConfig(hpaConfig(1), "openshift-default")),
365+
istio("v1.24.4", false, nil, gatewayclassesConfig(gatewayclassConfig(1), "openshift-default")),
370366
manifests.IstiodAllowNetworkPolicy(),
371367
},
372368
expectUpdate: []client.Object{},
@@ -390,7 +386,7 @@ func Test_Reconcile(t *testing.T) {
390386
},
391387
expectCreate: []client.Object{
392388
subscription("redhat-operators", "stable", "servicemeshoperator3.v3.0.1"),
393-
istio("v1.24.4", false, nil, gatewayclassesConfig(hpaConfig(2), "openshift-default", "openshift-internal")),
389+
istio("v1.24.4", false, nil, gatewayclassesConfig(gatewayclassConfig(2), "openshift-default", "openshift-internal")),
394390
manifests.IstiodAllowNetworkPolicy(),
395391
},
396392
expectUpdate: []client.Object{},
@@ -406,7 +402,7 @@ func Test_Reconcile(t *testing.T) {
406402
},
407403
expectCreate: []client.Object{
408404
subscription("redhat-operators", "stable", "servicemeshoperator3.v3.0.1"),
409-
istio("v1.24.4", false, expectedProxyConfiguration, gatewayclassesConfig(hpaConfig(2), "openshift-default")),
405+
istio("v1.24.4", false, expectedProxyConfiguration, gatewayclassesConfig(gatewayclassConfig(2), "openshift-default")),
410406
manifests.IstiodAllowNetworkPolicy(),
411407
},
412408
expectUpdate: []client.Object{},
@@ -426,7 +422,7 @@ func Test_Reconcile(t *testing.T) {
426422
},
427423
expectCreate: []client.Object{
428424
subscription("redhat-operators", "stable", "servicemeshoperator3.v3.0.1"),
429-
istio("v1.24.4", true, nil, gatewayclassesConfig(hpaConfig(2), "openshift-default")),
425+
istio("v1.24.4", true, nil, gatewayclassesConfig(gatewayclassConfig(2), "openshift-default")),
430426
manifests.IstiodAllowNetworkPolicy(),
431427
},
432428
expectUpdate: []client.Object{},
@@ -446,7 +442,7 @@ func Test_Reconcile(t *testing.T) {
446442
},
447443
expectCreate: []client.Object{
448444
subscription("redhat-operators", "stable", "servicemeshoperator3.v3.0.1"),
449-
istio("v1.24.4", true, nil, gatewayclassesConfig(hpaConfig(2), "openshift-default")),
445+
istio("v1.24.4", true, nil, gatewayclassesConfig(gatewayclassConfig(2), "openshift-default")),
450446
manifests.IstiodAllowNetworkPolicy(),
451447
},
452448
expectUpdate: []client.Object{},
@@ -463,7 +459,7 @@ func Test_Reconcile(t *testing.T) {
463459
},
464460
expectCreate: []client.Object{
465461
subscription("redhat-operators", "stable", "servicemeshoperator3.v3.0.1"),
466-
istio("v1.24-latest", false, nil, gatewayclassesConfig(hpaConfig(2), "openshift-default")),
462+
istio("v1.24-latest", false, nil, gatewayclassesConfig(gatewayclassConfig(2), "openshift-default")),
467463
manifests.IstiodAllowNetworkPolicy(),
468464
},
469465
expectUpdate: []client.Object{},
@@ -483,7 +479,7 @@ func Test_Reconcile(t *testing.T) {
483479
},
484480
expectCreate: []client.Object{
485481
subscription("foo", "bar", "baz"),
486-
istio("quux", false, nil, gatewayclassesConfig(hpaConfig(2), "openshift-default")),
482+
istio("quux", false, nil, gatewayclassesConfig(gatewayclassConfig(2), "openshift-default")),
487483
manifests.IstiodAllowNetworkPolicy(),
488484
},
489485
expectUpdate: []client.Object{},
@@ -573,11 +569,11 @@ func Test_Reconcile(t *testing.T) {
573569
infraConfig(configv1.HighlyAvailableTopologyMode),
574570
gatewayClass("openshift-default", true, nil, nil, false),
575571
istioCRD(),
576-
istio("v1.24.4", true, nil, gatewayclassesConfig(hpaConfig(2), "openshift-default")),
572+
istio("v1.24.4", true, nil, gatewayclassesConfig(gatewayclassConfig(2), "openshift-default")),
577573
},
578574
expectPatched: []client.Object{},
579575
expectDelete: []client.Object{
580-
istio("v1.24.4", true, nil, gatewayclassesConfig(hpaConfig(2), "openshift-default")),
576+
istio("v1.24.4", true, nil, gatewayclassesConfig(gatewayclassConfig(2), "openshift-default")),
581577
},
582578
expectedResult: reconcile.Result{
583579
RequeueAfter: 5 * time.Second,
@@ -618,7 +614,7 @@ func Test_Reconcile(t *testing.T) {
618614
expectedStatusPatched: []client.Object{
619615
gatewayClass("openshift-default", true, nil, installedConditions(), false),
620616
},
621-
expectedSailLibraryOptions: expectedSailLibraryOptions("v1.24.4", false, nil, gatewayclassesConfig(hpaConfig(2), "openshift-default")),
617+
expectedSailLibraryOptions: expectedSailLibraryOptions("v1.24.4", false, nil, gatewayclassesConfig(gatewayclassConfig(2), "openshift-default")),
622618
},
623619
{
624620
name: "Sail Library: installs Istio (single replica)",
@@ -634,7 +630,7 @@ func Test_Reconcile(t *testing.T) {
634630
expectedStatusPatched: []client.Object{
635631
gatewayClass("openshift-default", true, nil, installedConditions(), false),
636632
},
637-
expectedSailLibraryOptions: expectedSailLibraryOptions("v1.24.4", false, nil, gatewayclassesConfig(hpaConfig(1), "openshift-default")),
633+
expectedSailLibraryOptions: expectedSailLibraryOptions("v1.24.4", false, nil, gatewayclassesConfig(gatewayclassConfig(1), "openshift-default")),
638634
},
639635
{
640636
name: "Sail Library: installs Istio (highly available)",
@@ -650,7 +646,7 @@ func Test_Reconcile(t *testing.T) {
650646
expectedStatusPatched: []client.Object{
651647
gatewayClass("openshift-default", true, nil, installedConditions(), false),
652648
},
653-
expectedSailLibraryOptions: expectedSailLibraryOptions("v1.24.4", false, nil, gatewayclassesConfig(hpaConfig(2), "openshift-default")),
649+
expectedSailLibraryOptions: expectedSailLibraryOptions("v1.24.4", false, nil, gatewayclassesConfig(gatewayclassConfig(2), "openshift-default")),
654650
},
655651
{
656652
name: "Sail Library: installs Istio with system proxy configuration",
@@ -667,7 +663,7 @@ func Test_Reconcile(t *testing.T) {
667663
expectedStatusPatched: []client.Object{
668664
gatewayClass("openshift-default", true, nil, installedConditions(), false),
669665
},
670-
expectedSailLibraryOptions: expectedSailLibraryOptions("v1.24.4", false, expectedProxyConfiguration, gatewayclassesConfig(hpaConfig(2), "openshift-default")),
666+
expectedSailLibraryOptions: expectedSailLibraryOptions("v1.24.4", false, expectedProxyConfiguration, gatewayclassesConfig(gatewayclassConfig(2), "openshift-default")),
671667
},
672668
{
673669
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) {
694690
expectedStatusPatched: []client.Object{
695691
gatewayClass("openshift-default", true, nil, installedConditions(), false),
696692
},
697-
expectedSailLibraryOptions: expectedSailLibraryOptions("v1.24.4", false, expectedProxyConfiguration, gatewayclassesConfig(hpaConfig(1), "openshift-default", "openshift-internal", "openshift-custom")),
693+
expectedSailLibraryOptions: expectedSailLibraryOptions("v1.24.4", false, expectedProxyConfiguration, gatewayclassesConfig(gatewayclassConfig(1), "openshift-default", "openshift-internal", "openshift-custom")),
698694
},
699695
{
700696
name: "Sail Library: experimental InferencePool CRD",
@@ -715,7 +711,7 @@ func Test_Reconcile(t *testing.T) {
715711
expectedStatusPatched: []client.Object{
716712
gatewayClass("openshift-default", true, nil, installedConditions(), false),
717713
},
718-
expectedSailLibraryOptions: expectedSailLibraryOptions("v1.24.4", true, nil, gatewayclassesConfig(hpaConfig(2), "openshift-default")),
714+
expectedSailLibraryOptions: expectedSailLibraryOptions("v1.24.4", true, nil, gatewayclassesConfig(gatewayclassConfig(2), "openshift-default")),
719715
},
720716
{
721717
name: "Sail Library: stable InferencePool CRD",
@@ -736,7 +732,7 @@ func Test_Reconcile(t *testing.T) {
736732
expectedStatusPatched: []client.Object{
737733
gatewayClass("openshift-default", true, nil, installedConditions(), false),
738734
},
739-
expectedSailLibraryOptions: expectedSailLibraryOptions("v1.24.4", true, nil, gatewayclassesConfig(hpaConfig(2), "openshift-default")),
735+
expectedSailLibraryOptions: expectedSailLibraryOptions("v1.24.4", true, nil, gatewayclassesConfig(gatewayclassConfig(2), "openshift-default")),
740736
},
741737
{
742738
name: "Sail Library: Istio version override",
@@ -756,7 +752,7 @@ func Test_Reconcile(t *testing.T) {
756752
"unsupported.do-not-use.openshift.io/istio-version": "v1.24-latest",
757753
}, installedConditions(), false),
758754
},
759-
expectedSailLibraryOptions: expectedSailLibraryOptions("v1.24-latest", false, nil, gatewayclassesConfig(hpaConfig(2), "openshift-default")),
755+
expectedSailLibraryOptions: expectedSailLibraryOptions("v1.24-latest", false, nil, gatewayclassesConfig(gatewayclassConfig(2), "openshift-default")),
760756
},
761757
{
762758
name: "Sail Library: full removal of last GatewayClass",

‎pkg/operator/controller/gatewayclass/istio_olm.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -237,10 +237,10 @@ func desiredIstio(name types.NamespacedName, ownerRef metav1.OwnerReference, ist
237237
}
238238

239239
if extraConfig.infraConfig != nil {
240-
if hpaConfig, err := buildHorizontalPodAutoscalerConfig(extraConfig.infraConfig, gatewayclasses); err != nil {
240+
if gwClassConfig, err := buildGatewayClassesConfig(extraConfig.infraConfig, gatewayclasses); err != nil {
241241
return nil, err
242242
} else {
243-
istio.Spec.Values.GatewayClasses = hpaConfig
243+
istio.Spec.Values.GatewayClasses = gwClassConfig
244244
}
245245
}
246246
}

‎pkg/operator/controller/gatewayclass/istio_sail_installer.go‎

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -210,10 +210,10 @@ func openshiftValues(enableInferenceExtension bool, operandNamespace string, gat
210210
}
211211
}
212212
if extraConfig.infraConfig != nil {
213-
if hpaConfig, err := buildHorizontalPodAutoscalerConfig(extraConfig.infraConfig, gatewayclasses); err != nil {
214-
return nil, fmt.Errorf("failed to build HPA config: %w", err)
213+
if gwClassConfig, err := buildGatewayClassesConfig(extraConfig.infraConfig, gatewayclasses); err != nil {
214+
return nil, fmt.Errorf("failed to build gateway class config: %w", err)
215215
} else {
216-
val.GatewayClasses = hpaConfig
216+
val.GatewayClasses = gwClassConfig
217217
}
218218
}
219219
}
@@ -244,10 +244,9 @@ func buildProxyMetadata(proxyConfig *configv1.Proxy) map[string]string {
244244
return proxyMetadata
245245
}
246246

247-
// buildHorizontalPodAutoscalerConfig returns Istio configuration for the
248-
// horizontal pod autoscaler given an infrastructure config and a slice of
249-
// gatewayclasses.
250-
func buildHorizontalPodAutoscalerConfig(infraConfig *configv1.Infrastructure, gatewayclasses []gatewayapiv1.GatewayClass) (json.RawMessage, error) {
247+
// buildGatewayClassesConfig returns Istio per-gatewayclass configuration
248+
// overlays given an infrastructure config and a slice of gatewayclasses.
249+
func buildGatewayClassesConfig(infraConfig *configv1.Infrastructure, gatewayclasses []gatewayapiv1.GatewayClass) (json.RawMessage, error) {
251250
const maxReplicas = 10
252251

253252
var minReplicas = 2
@@ -262,6 +261,20 @@ func buildHorizontalPodAutoscalerConfig(infraConfig *configv1.Infrastructure, ga
262261
"maxReplicas": maxReplicas,
263262
},
264263
},
264+
"deployment": map[string]any{
265+
"spec": map[string]any{
266+
"template": map[string]any{
267+
"spec": map[string]any{
268+
"containers": []map[string]any{
269+
{
270+
"name": "istio-proxy",
271+
"terminationMessagePolicy": "FallbackToLogsOnError",
272+
},
273+
},
274+
},
275+
},
276+
},
277+
},
265278
}
266279
gatewayclassesConfig := map[string]any{}
267280
for _, gatewayclass := range gatewayclasses {

‎test/e2e/gateway_api_test.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1575,6 +1575,9 @@ func ensureGatewayObjectSuccess(t *testing.T, ns *corev1.Namespace) []string {
15751575
}
15761576
assertHorizontalPodAutoscalerEnabled(t, operatorcontroller.DefaultOperandNamespace, testGatewayName, operatorcontroller.OpenShiftDefaultGatewayClassName, expectedMinReplicas)
15771577

1578+
t.Log("Verifying terminationMessagePolicy is FallbackToLogsOnError...")
1579+
assertTerminationMessagePolicy(t, operatorcontroller.DefaultOperandNamespace, testGatewayName, operatorcontroller.OpenShiftDefaultGatewayClassName)
1580+
15781581
t.Log("Making sure the httproute is created and accepted...")
15791582
_, err = assertHttpRouteSuccessful(t, ns.Name, "test-httproute", gateway)
15801583
if err != nil {

‎test/e2e/util_gatewayapi_test.go‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -811,6 +811,34 @@ func assertHorizontalPodAutoscalerEnabled(t *testing.T, namespaceName, gatewayNa
811811
}
812812
}
813813

814+
// assertTerminationMessagePolicy verifies that all containers in the gateway
815+
// deployment have terminationMessagePolicy set to FallbackToLogsOnError.
816+
func assertTerminationMessagePolicy(t *testing.T, namespaceName, gatewayName, gatewayclassName string) {
817+
t.Helper()
818+
819+
deploymentName := types.NamespacedName{
820+
Namespace: namespaceName,
821+
Name: fmt.Sprintf("%s-%s", gatewayName, gatewayclassName),
822+
}
823+
t.Logf("Verifying terminationMessagePolicy on deployment %s...", deploymentName)
824+
if err := wait.PollUntilContextTimeout(context.Background(), 2*time.Second, 2*time.Minute, false, func(ctx context.Context) (bool, error) {
825+
var dep appsv1.Deployment
826+
if err := kclient.Get(ctx, deploymentName, &dep); err != nil {
827+
t.Logf("Failed to get Deployment %s: %v; retrying...", deploymentName, err)
828+
return false, nil
829+
}
830+
for _, c := range dep.Spec.Template.Spec.Containers {
831+
if c.TerminationMessagePolicy != corev1.TerminationMessageFallbackToLogsOnError {
832+
t.Logf("Container %q has terminationMessagePolicy %q, expected FallbackToLogsOnError; retrying...", c.Name, c.TerminationMessagePolicy)
833+
return false, nil
834+
}
835+
}
836+
return true, nil
837+
}); err != nil {
838+
t.Errorf("Failed to verify terminationMessagePolicy on deployment %s: %v", deploymentName, err)
839+
}
840+
}
841+
814842
func waitForGatewayListenerCondition(t *testing.T, gatewayName types.NamespacedName, listenerName string, conditions ...metav1.Condition) error {
815843
t.Helper()
816844

0 commit comments

Comments
 (0)