pilot: no specialized slice for management listeners (istio#15939)

* pilot: no specialize slice for management listeners Signed-off-by: Yuchen Dai <[email protected]> * fix bug introduced by rebase also fix typo in istio#15639 Signed-off-by: Yuchen Dai <[email protected]>
mfilipiak-ccg · Jul 31, 2019 · 89cb553 · 89cb553
1 parent aa330ce
commit 89cb553
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 92 deletions.
diff --git a/pilot/pkg/networking/core/v1alpha3/envoyfilter/listener_patch.go b/pilot/pkg/networking/core/v1alpha3/envoyfilter/listener_patch.go
@@ -29,15 +29,15 @@ import (
 
 // ApplyListenerPatches applies patches to LDS output
 func ApplyListenerPatches(patchContext networking.EnvoyFilter_PatchContext,
-	proxy *model.Proxy, push *model.PushContext, listeners []*xdsapi.Listener, skipAdds bool) []*xdsapi.Listener {
+	proxy *model.Proxy, push *model.PushContext, listeners []*xdsapi.Listener) []*xdsapi.Listener {
 
 	envoyFilterWrappers := push.EnvoyFilters(proxy)
-	return doListenerListOperation(proxy, patchContext, envoyFilterWrappers, listeners, skipAdds)
+	return doListenerListOperation(proxy, patchContext, envoyFilterWrappers, listeners)
 }
 
 func doListenerListOperation(proxy *model.Proxy, patchContext networking.EnvoyFilter_PatchContext,
 	envoyFilterWrappers []*model.EnvoyFilterWrapper,
-	listeners []*xdsapi.Listener, skipAdds bool) []*xdsapi.Listener {
+	listeners []*xdsapi.Listener) []*xdsapi.Listener {
 	listenersRemoved := false
 	for _, efw := range envoyFilterWrappers {
 		// do all the changes for a single envoy filter crd object. [including adds]
@@ -51,10 +51,6 @@ func doListenerListOperation(proxy *model.Proxy, patchContext networking.EnvoyFi
 			}
 			doListenerOperation(proxy, patchContext, efw.Patches, listener, &listenersRemoved)
 		}
-		// adds at listener level if enabled
-		if skipAdds {
-			continue
-		}
 		for _, cp := range efw.Patches[networking.EnvoyFilter_LISTENER] {
 			if cp.Operation == networking.EnvoyFilter_Patch_ADD {
 				if !commonConditionMatch(proxy, patchContext, cp) {

diff --git a/pilot/pkg/networking/core/v1alpha3/envoyfilter/listener_patch_test.go b/pilot/pkg/networking/core/v1alpha3/envoyfilter/listener_patch_test.go
@@ -314,58 +314,6 @@ func TestApplyListenerPatches(t *testing.T) {
 		},
 	}
 
-	sidecarOutboundInNoAdd := []*xdsapi.Listener{
-		{
-			Name: "12345",
-			Address: core.Address{
-				Address: &core.Address_SocketAddress{
-					SocketAddress: &core.SocketAddress{
-						PortSpecifier: &core.SocketAddress_PortValue{
-							PortValue: 12345,
-						},
-					},
-				},
-			},
-			FilterChains: []listener.FilterChain{
-				{
-					Filters: []listener.Filter{
-						{Name: "filter1"},
-						{Name: "filter2"},
-					},
-				},
-			},
-		},
-		{
-			Name: "another-listener",
-		},
-	}
-
-	sidecarOutboundOutNoAdd := []*xdsapi.Listener{
-		{
-			Name: "12345",
-			Address: core.Address{
-				Address: &core.Address_SocketAddress{
-					SocketAddress: &core.SocketAddress{
-						PortSpecifier: &core.SocketAddress_PortValue{
-							PortValue: 12345,
-						},
-					},
-				},
-			},
-			FilterChains: []listener.FilterChain{
-				{
-					Filters: []listener.Filter{
-						{Name: "filter0"},
-						{Name: "filter1"},
-					},
-				},
-			},
-		},
-		{
-			Name: "another-listener",
-		},
-	}
-
 	sidecarInboundIn := []*xdsapi.Listener{
 		{
 			Name: "12345",
@@ -575,7 +523,6 @@ func TestApplyListenerPatches(t *testing.T) {
 		proxy        *model.Proxy
 		push         *model.PushContext
 		listeners    []*xdsapi.Listener
-		skipAdds     bool
 	}
 	tests := []struct {
 		name string
@@ -589,7 +536,6 @@ func TestApplyListenerPatches(t *testing.T) {
 				proxy:        sidecarProxy,
 				push:         push,
 				listeners:    sidecarInboundIn,
-				skipAdds:     false,
 			},
 			want: sidecarInboundOut,
 		},
@@ -600,7 +546,6 @@ func TestApplyListenerPatches(t *testing.T) {
 				proxy:        gatewayProxy,
 				push:         push,
 				listeners:    gatewayIn,
-				skipAdds:     false,
 			},
 			want: gatewayOut,
 		},
@@ -611,26 +556,14 @@ func TestApplyListenerPatches(t *testing.T) {
 				proxy:        sidecarProxy,
 				push:         push,
 				listeners:    sidecarOutboundIn,
-				skipAdds:     false,
 			},
 			want: sidecarOutboundOut,
 		},
-		{
-			name: "sidecar outbound lds - skip adds",
-			args: args{
-				patchContext: networking.EnvoyFilter_SIDECAR_OUTBOUND,
-				proxy:        sidecarProxy,
-				push:         push,
-				listeners:    sidecarOutboundInNoAdd,
-				skipAdds:     true,
-			},
-			want: sidecarOutboundOutNoAdd,
-		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			got := ApplyListenerPatches(tt.args.patchContext, tt.args.proxy, tt.args.push,
-				tt.args.listeners, tt.args.skipAdds)
+				tt.args.listeners)
 			if diff := cmp.Diff(tt.want, got); diff != "" {
 				t.Errorf("ApplyListenerPatches(): %s mismatch (-want +got):\n%s", tt.name, diff)
 			}

diff --git a/pilot/pkg/networking/core/v1alpha3/listener_builder.go b/pilot/pkg/networking/core/v1alpha3/listener_builder.go
@@ -43,7 +43,6 @@ type ListenerBuilder struct {
 	gatewayListeners       []*xdsapi.Listener
 	inboundListeners       []*xdsapi.Listener
 	outboundListeners      []*xdsapi.Listener
-	managementListeners    []*xdsapi.Listener
 	virtualListener        *xdsapi.Listener
 	virtualInboundListener *xdsapi.Listener
 }
@@ -112,7 +111,7 @@ func (builder *ListenerBuilder) buildManagementListeners(_ *ConfigGeneratorImpl,
 		} else {
 			// dedup management listeners as well
 			addresses[addressString] = m
-			builder.managementListeners = append(builder.managementListeners, m)
+			builder.inboundListeners = append(builder.inboundListeners, m)
 		}
 
 	}
@@ -194,59 +193,56 @@ func (builder *ListenerBuilder) buildVirtualInboundListener(env *model.Environme
 
 func (builder *ListenerBuilder) patchListeners(push *model.PushContext) {
 	if builder.node.Type == model.Router {
-		envoyfilter.ApplyListenerPatches(networking.EnvoyFilter_GATEWAY, builder.node, push, builder.gatewayListeners, false)
+		envoyfilter.ApplyListenerPatches(networking.EnvoyFilter_GATEWAY, builder.node, push, builder.gatewayListeners)
 		return
 	}
 
-	patchOneListener := func(listener *xdsapi.Listener) *xdsapi.Listener {
+	patchOneListener := func(listener *xdsapi.Listener, ctx networking.EnvoyFilter_PatchContext) *xdsapi.Listener {
 		if listener == nil {
 			return nil
 		}
 		tempArray := []*xdsapi.Listener{listener}
-		tempArray = envoyfilter.ApplyListenerPatches(networking.EnvoyFilter_SIDECAR_OUTBOUND, builder.node, push, tempArray, true)
+		tempArray = envoyfilter.ApplyListenerPatches(ctx, builder.node, push, tempArray)
 		// temp array will either be empty [if virtual listener was removed] or will have a modified listener
 		if len(tempArray) == 0 {
 			return nil
 		}
 		return tempArray[0]
 	}
-	builder.virtualListener = patchOneListener(builder.virtualListener)
-	builder.virtualInboundListener = patchOneListener(builder.virtualInboundListener)
-	builder.managementListeners = envoyfilter.ApplyListenerPatches(networking.EnvoyFilter_SIDECAR_INBOUND, builder.node,
-		push, builder.managementListeners, true)
+	builder.virtualListener = patchOneListener(builder.virtualListener, networking.EnvoyFilter_SIDECAR_OUTBOUND)
+	builder.virtualInboundListener = patchOneListener(builder.virtualInboundListener, networking.EnvoyFilter_SIDECAR_INBOUND)
 	builder.inboundListeners = envoyfilter.ApplyListenerPatches(networking.EnvoyFilter_SIDECAR_INBOUND, builder.node,
-		push, builder.inboundListeners, false)
-	builder.outboundListeners = envoyfilter.ApplyListenerPatches(networking.EnvoyFilter_SIDECAR_INBOUND, builder.node,
-		push, builder.outboundListeners, false)
+		push, builder.inboundListeners)
+	builder.outboundListeners = envoyfilter.ApplyListenerPatches(networking.EnvoyFilter_SIDECAR_OUTBOUND, builder.node,
+		push, builder.outboundListeners)
 }
 
 func (builder *ListenerBuilder) getListeners() []*xdsapi.Listener {
 	if builder.node.Type == model.SidecarProxy {
-		nInbound, nOutbound, nManagement := len(builder.inboundListeners), len(builder.outboundListeners), len(builder.managementListeners)
+		nInbound, nOutbound := len(builder.inboundListeners), len(builder.outboundListeners)
 		nVirtual, nVirtualInbound := 0, 0
 		if builder.virtualListener != nil {
 			nVirtual = 1
 		}
 		if builder.virtualInboundListener != nil {
 			nVirtualInbound = 1
 		}
-		nListener := nInbound + nOutbound + nManagement + nVirtual + nVirtualInbound
+		nListener := nInbound + nOutbound + nVirtual + nVirtualInbound
 
 		listeners := make([]*xdsapi.Listener, 0, nListener)
 		listeners = append(listeners, builder.inboundListeners...)
 		listeners = append(listeners, builder.outboundListeners...)
-		listeners = append(listeners, builder.managementListeners...)
 		if builder.virtualListener != nil {
 			listeners = append(listeners, builder.virtualListener)
 		}
 		if builder.virtualInboundListener != nil {
 			listeners = append(listeners, builder.virtualInboundListener)
 		}
 
-		log.Debugf("Build %d listeners for node %s including %d inbound, %d outbound, %d management, %d virtual and %d virtual inbound listeners",
+		log.Debugf("Build %d listeners for node %s including %d inbound, %d outbound, %d virtual and %d virtual inbound listeners",
 			nListener,
 			builder.node.ID,
-			nInbound, nOutbound, nManagement,
+			nInbound, nOutbound,
 			nVirtual, nVirtualInbound)
 		return listeners
 	}