Skip to content

Fix e2e failures related to default proxy #8103

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
mgs255 wants to merge 1 commit into from
Closed

Fix e2e failures related to default proxy #8103

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 1 addition & 13 deletions internal/gatewayapi/contexts.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ func (g *GatewayContext) attachEnvoyProxy(resources *resource.Resources, epMap m
// Priority order (highest to lowest):
// 1. Gateway-level EnvoyProxy (via parametersRef)
// 2. GatewayClass-level EnvoyProxy
// 3. Default EnvoyProxySpec from EnvoyGateway configuration

if g.Spec.Infrastructure != nil && g.Spec.Infrastructure.ParametersRef != nil && !IsMergeGatewaysEnabled(resources) {
ref := g.Spec.Infrastructure.ParametersRef
Expand All @@ -69,18 +68,7 @@ func (g *GatewayContext) attachEnvoyProxy(resources *resource.Resources, epMap m
}

// Use GatewayClass-level EnvoyProxy if available
if resources.EnvoyProxyForGatewayClass != nil {
g.envoyProxy = resources.EnvoyProxyForGatewayClass
return
}

// Fall back to default EnvoyProxySpec from EnvoyGateway configuration
if resources.EnvoyProxyDefaultSpec != nil {
// Create a synthetic EnvoyProxy object from the default spec
g.envoyProxy = &egv1a1.EnvoyProxy{
Spec: *resources.EnvoyProxyDefaultSpec,
}
}
g.envoyProxy = resources.EnvoyProxyForGatewayClass
}

// ListenerContext wraps a Listener and provides helper methods for
Expand Down
245 changes: 0 additions & 245 deletions internal/gatewayapi/contexts_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,7 @@ import (

"github.com/stretchr/testify/require"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/utils/ptr"
gwapiv1 "sigs.k8s.io/gateway-api/apis/v1"

egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1"
"github.com/envoyproxy/gateway/internal/gatewayapi/resource"
"github.com/envoyproxy/gateway/internal/gatewayapi/status"
)

Expand Down Expand Up @@ -154,243 +149,3 @@ func TestContextsStaleListener(t *testing.T) {
expectedGCtxListeners := []*ListenerContext{httpsListenerCtx}
require.Equal(t, expectedGCtxListeners, gCtx.listeners)
}

func TestAttachEnvoyProxy(t *testing.T) {
testCases := []struct {
name string
gatewayParametersRef *gwapiv1.LocalParametersReference
envoyProxyForGateway *egv1a1.EnvoyProxy
envoyProxyForGWClass *egv1a1.EnvoyProxy
envoyProxyDefaultSpec *egv1a1.EnvoyProxySpec
expectedMergeGateways *bool
expectedConcurrency *int32
expectEnvoyProxyNil bool
}{
{
name: "no envoy proxy at any level",
expectEnvoyProxyNil: true,
},
{
name: "only default spec - should use default",
envoyProxyDefaultSpec: &egv1a1.EnvoyProxySpec{
Concurrency: ptr.To[int32](4),
},
expectedConcurrency: ptr.To[int32](4),
},
{
name: "gatewayclass envoy proxy overrides default spec",
envoyProxyForGWClass: &egv1a1.EnvoyProxy{
ObjectMeta: metav1.ObjectMeta{
Namespace: "envoy-gateway-system",
Name: "gc-proxy",
},
Spec: egv1a1.EnvoyProxySpec{
Concurrency: ptr.To[int32](8),
},
},
envoyProxyDefaultSpec: &egv1a1.EnvoyProxySpec{
Concurrency: ptr.To[int32](4),
},
expectedConcurrency: ptr.To[int32](8),
},
{
name: "gateway envoy proxy overrides gatewayclass",
gatewayParametersRef: &gwapiv1.LocalParametersReference{
Group: gwapiv1.Group(egv1a1.GroupVersion.Group),
Kind: gwapiv1.Kind(egv1a1.KindEnvoyProxy),
Name: "gw-proxy",
},
envoyProxyForGateway: &egv1a1.EnvoyProxy{
ObjectMeta: metav1.ObjectMeta{
Namespace: "default",
Name: "gw-proxy",
},
Spec: egv1a1.EnvoyProxySpec{
Concurrency: ptr.To[int32](16),
},
},
envoyProxyForGWClass: &egv1a1.EnvoyProxy{
ObjectMeta: metav1.ObjectMeta{
Namespace: "envoy-gateway-system",
Name: "gc-proxy",
},
Spec: egv1a1.EnvoyProxySpec{
Concurrency: ptr.To[int32](8),
},
},
envoyProxyDefaultSpec: &egv1a1.EnvoyProxySpec{
Concurrency: ptr.To[int32](4),
},
expectedConcurrency: ptr.To[int32](16),
},
{
name: "default spec with merge gateways enabled",
envoyProxyDefaultSpec: &egv1a1.EnvoyProxySpec{
MergeGateways: ptr.To(true),
Concurrency: ptr.To[int32](4),
},
expectedMergeGateways: ptr.To(true),
expectedConcurrency: ptr.To[int32](4),
},
{
name: "gatewayclass overrides default merge gateways setting",
envoyProxyForGWClass: &egv1a1.EnvoyProxy{
ObjectMeta: metav1.ObjectMeta{
Namespace: "envoy-gateway-system",
Name: "gc-proxy",
},
Spec: egv1a1.EnvoyProxySpec{
MergeGateways: ptr.To(false),
},
},
envoyProxyDefaultSpec: &egv1a1.EnvoyProxySpec{
MergeGateways: ptr.To(true),
},
expectedMergeGateways: ptr.To(false),
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
// Create gateway
gateway := &gwapiv1.Gateway{
ObjectMeta: metav1.ObjectMeta{
Namespace: "default",
Name: "test-gateway",
},
Spec: gwapiv1.GatewaySpec{
GatewayClassName: "test-gc",
},
}
if tc.gatewayParametersRef != nil {
gateway.Spec.Infrastructure = &gwapiv1.GatewayInfrastructure{
ParametersRef: tc.gatewayParametersRef,
}
}

gCtx := &GatewayContext{Gateway: gateway}

// Build resources
resources := &resource.Resources{
EnvoyProxyForGatewayClass: tc.envoyProxyForGWClass,
EnvoyProxyDefaultSpec: tc.envoyProxyDefaultSpec,
}

// Build envoy proxy map for gateway-level proxies
epMap := make(map[types.NamespacedName]*egv1a1.EnvoyProxy)
if tc.envoyProxyForGateway != nil {
key := types.NamespacedName{
Namespace: tc.envoyProxyForGateway.Namespace,
Name: tc.envoyProxyForGateway.Name,
}
epMap[key] = tc.envoyProxyForGateway
}

// Call attachEnvoyProxy
gCtx.attachEnvoyProxy(resources, epMap)

// Verify results
if tc.expectEnvoyProxyNil {
require.Nil(t, gCtx.envoyProxy)
return
}

require.NotNil(t, gCtx.envoyProxy)

if tc.expectedConcurrency != nil {
require.NotNil(t, gCtx.envoyProxy.Spec.Concurrency)
require.Equal(t, *tc.expectedConcurrency, *gCtx.envoyProxy.Spec.Concurrency)
}

if tc.expectedMergeGateways != nil {
require.NotNil(t, gCtx.envoyProxy.Spec.MergeGateways)
require.Equal(t, *tc.expectedMergeGateways, *gCtx.envoyProxy.Spec.MergeGateways)
}
})
}
}

func TestIsMergeGatewaysEnabled(t *testing.T) {
testCases := []struct {
name string
envoyProxyForGWClass *egv1a1.EnvoyProxy
envoyProxyDefaultSpec *egv1a1.EnvoyProxySpec
expected bool
}{
{
name: "no envoy proxy configured",
expected: false,
},
{
name: "default spec with merge gateways true",
envoyProxyDefaultSpec: &egv1a1.EnvoyProxySpec{
MergeGateways: ptr.To(true),
},
expected: true,
},
{
name: "default spec with merge gateways false",
envoyProxyDefaultSpec: &egv1a1.EnvoyProxySpec{
MergeGateways: ptr.To(false),
},
expected: false,
},
{
name: "gatewayclass proxy with merge gateways true",
envoyProxyForGWClass: &egv1a1.EnvoyProxy{
Spec: egv1a1.EnvoyProxySpec{
MergeGateways: ptr.To(true),
},
},
expected: true,
},
{
name: "gatewayclass proxy overrides default - gc true, default false",
envoyProxyForGWClass: &egv1a1.EnvoyProxy{
Spec: egv1a1.EnvoyProxySpec{
MergeGateways: ptr.To(true),
},
},
envoyProxyDefaultSpec: &egv1a1.EnvoyProxySpec{
MergeGateways: ptr.To(false),
},
expected: true,
},
{
name: "gatewayclass proxy overrides default - gc false, default true",
envoyProxyForGWClass: &egv1a1.EnvoyProxy{
Spec: egv1a1.EnvoyProxySpec{
MergeGateways: ptr.To(false),
},
},
envoyProxyDefaultSpec: &egv1a1.EnvoyProxySpec{
MergeGateways: ptr.To(true),
},
expected: false,
},
{
name: "gatewayclass proxy nil merge gateways falls back to default",
envoyProxyForGWClass: &egv1a1.EnvoyProxy{
Spec: egv1a1.EnvoyProxySpec{
Concurrency: ptr.To[int32](4), // some other setting
},
},
envoyProxyDefaultSpec: &egv1a1.EnvoyProxySpec{
MergeGateways: ptr.To(true),
},
expected: true,
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
resources := &resource.Resources{
EnvoyProxyForGatewayClass: tc.envoyProxyForGWClass,
EnvoyProxyDefaultSpec: tc.envoyProxyDefaultSpec,
}

result := IsMergeGatewaysEnabled(resources)
require.Equal(t, tc.expected, result)
})
}
}
Loading