Skip to content

Commit

Permalink
feat(gateway) only advertise LoadBalancer addrs
Browse files Browse the repository at this point in the history
Exclude ClusterIP addresses from Gateway address status. Only
LoadBalancer addresses, if present, will be advertised.
  • Loading branch information
rainest committed Jun 14, 2022
1 parent e39fdd0 commit 31b7eff
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 39 deletions.
26 changes: 10 additions & 16 deletions internal/controllers/gateway/gateway_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -421,24 +421,18 @@ func (r *GatewayReconciler) determineL4ListenersFromService(
corev1.ProtocolTCP: {Group: &gatewayV1alpha2Group, Kind: gatewayv1alpha2.Kind("TCPRoute")},
corev1.ProtocolUDP: {Group: &gatewayV1alpha2Group, Kind: gatewayv1alpha2.Kind("UDPRoute")},
}
for _, clusterIP := range svc.Spec.ClusterIPs {
addresses = append(addresses, gatewayv1alpha2.GatewayAddress{
Type: &gatewayIPAddrType,
Value: clusterIP,
})

for _, port := range svc.Spec.Ports {
listeners = append(listeners, gatewayv1alpha2.Listener{
Name: gatewayv1alpha2.SectionName(port.Name),
Protocol: gatewayv1alpha2.ProtocolType(port.Protocol),
Port: gatewayv1alpha2.PortNumber(port.Port),
AllowedRoutes: &gatewayv1alpha2.AllowedRoutes{
Kinds: []gatewayv1alpha2.RouteGroupKind{
protocolToRouteGroupKind[port.Protocol],
},
for _, port := range svc.Spec.Ports {
listeners = append(listeners, gatewayv1alpha2.Listener{
Name: gatewayv1alpha2.SectionName(port.Name),
Protocol: gatewayv1alpha2.ProtocolType(port.Protocol),
Port: gatewayv1alpha2.PortNumber(port.Port),
AllowedRoutes: &gatewayv1alpha2.AllowedRoutes{
Kinds: []gatewayv1alpha2.RouteGroupKind{
protocolToRouteGroupKind[port.Protocol],
},
})
}
},
})
}

// for LoadBalancer service types we'll also capture the LB IP or Host
Expand Down
40 changes: 17 additions & 23 deletions test/integration/gateway_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,29 +106,23 @@ func TestUnmanagedGatewayBasics(t *testing.T) {
require.Eventually(t, func() bool {
gw, err = c.GatewayV1alpha2().Gateways(ns.Name).Get(ctx, gw.Name, metav1.GetOptions{})
require.NoError(t, err)
// TODO 2557 we include ClusterIPs in gw status addrs. these are NOT in LB status (though they are in the spec)
// is this correct? it seems like we should use external only and require publish-addr be provided manually otherwise
//if len(gw.Spec.Addresses) == len(pubsvc.Status.LoadBalancer.Ingress) {
// addrs := make(map[string]bool, len(pubsvc.Status.LoadBalancer.Ingress))
// for _, ing := range pubsvc.Status.LoadBalancer.Ingress {
// // taking a slight shortcut by using the same map for both types. value lookups will still work
// // and the test isn't concerned with the weird case where you've somehow wound up with
// // LB Hostname 10.0.0.1 and GW IP 10.0.0.1. the GW type is also optional, so we don't always know
// addrs[ing.IP] = true
// addrs[ing.Hostname] = true
// }
// for _, addr := range gw.Spec.Addresses {
// if _, ok := addrs[addr.Value]; !ok {
// return false
// }
// }
// return true
//}
//return false

// currently a noop pending confirmation of whether we want both. if so, we need to combine the lists when
// checking. if not, the above should work once the clusterIP is removed
return pubsvc != nil
if len(gw.Spec.Addresses) == len(pubsvc.Status.LoadBalancer.Ingress) {
addrs := make(map[string]bool, len(pubsvc.Status.LoadBalancer.Ingress))
for _, ing := range pubsvc.Status.LoadBalancer.Ingress {
// taking a slight shortcut by using the same map for both types. value lookups will still work
// and the test isn't concerned with the weird case where you've somehow wound up with
// LB Hostname 10.0.0.1 and GW IP 10.0.0.1. the GW type is also optional, so we don't always know
addrs[ing.IP] = true
addrs[ing.Hostname] = true
}
for _, addr := range gw.Spec.Addresses {
if _, ok := addrs[addr.Value]; !ok {
return false
}
}
return true
}
return false
}, gatewayUpdateWaitTime, time.Second)

t.Log("verifying that the gateway status gets updated to match the publish service")
Expand Down

0 comments on commit 31b7eff

Please sign in to comment.