Skip to content

Commit 3b68f93

Browse files
michaelbeaumontzirain
authored and
davem-git
committed
fix: DualStack NodePort Gateway's status.addresses should contain addresses from both IP families (envoyproxy#6372)
* fix: DualStack NodePort Gateway's `status.addresses` should contain both families Signed-off-by: Mike Beaumont <[email protected]> * test: update existing Signed-off-by: Mike Beaumont <[email protected]> * test: new cases Signed-off-by: Mike Beaumont <[email protected]> * chore: changelog Signed-off-by: Mike Beaumont <[email protected]> --------- Signed-off-by: Mike Beaumont <[email protected]> Co-authored-by: zirain <[email protected]>
1 parent 79b0bac commit 3b68f93

File tree

6 files changed

+150
-32
lines changed

6 files changed

+150
-32
lines changed

‎internal/gatewayapi/status/gateway.go‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ package status
77

88
import (
99
"fmt"
10+
"slices"
1011
"time"
1112

1213
appsv1 "k8s.io/api/apps/v1"
@@ -43,10 +44,15 @@ func GatewayAccepted(gw *gwapiv1.Gateway) bool {
4344
return !GatewayNotAccepted(gw)
4445
}
4546

47+
type NodeAddresses struct {
48+
IPv4 []string
49+
IPv6 []string
50+
}
51+
4652
// UpdateGatewayStatusProgrammedCondition updates the status addresses for the provided gateway
4753
// based on the status IP/Hostname of svc and updates the Programmed condition based on the
4854
// service and deployment or daemonset state.
49-
func UpdateGatewayStatusProgrammedCondition(gw *gwapiv1.Gateway, svc *corev1.Service, envoyObj client.Object, nodeAddresses ...string) {
55+
func UpdateGatewayStatusProgrammedCondition(gw *gwapiv1.Gateway, svc *corev1.Service, envoyObj client.Object, nodeAddresses NodeAddresses) {
5056
var addresses, hostnames []string
5157
// Update the status addresses field.
5258
if svc != nil {
@@ -92,7 +98,14 @@ func UpdateGatewayStatusProgrammedCondition(gw *gwapiv1.Gateway, svc *corev1.Ser
9298
}
9399

94100
if svc.Spec.Type == corev1.ServiceTypeNodePort {
95-
addresses = nodeAddresses
101+
var relevantAddresses []string
102+
if slices.Contains(svc.Spec.IPFamilies, corev1.IPv4Protocol) {
103+
relevantAddresses = append(relevantAddresses, nodeAddresses.IPv4...)
104+
}
105+
if slices.Contains(svc.Spec.IPFamilies, corev1.IPv6Protocol) {
106+
relevantAddresses = append(relevantAddresses, nodeAddresses.IPv6...)
107+
}
108+
addresses = relevantAddresses
96109
}
97110
}
98111

‎internal/gatewayapi/status/gateway_test.go‎

Lines changed: 60 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ func TestUpdateGatewayStatusProgrammedCondition(t *testing.T) {
2727
gw *gwapiv1.Gateway
2828
svc *corev1.Service
2929
deployment *appsv1.Deployment
30-
nodeAddresses []string
30+
nodeAddresses NodeAddresses
3131
}
3232
tests := []struct {
3333
name string
@@ -52,6 +52,9 @@ func TestUpdateGatewayStatusProgrammedCondition(t *testing.T) {
5252
Spec: corev1.ServiceSpec{
5353
ClusterIPs: []string{"127.0.0.1"},
5454
Type: corev1.ServiceTypeLoadBalancer,
55+
IPFamilies: []corev1.IPFamily{
56+
corev1.IPv4Protocol,
57+
},
5558
},
5659
Status: corev1.ServiceStatus{
5760
LoadBalancer: corev1.LoadBalancerStatus{
@@ -81,6 +84,9 @@ func TestUpdateGatewayStatusProgrammedCondition(t *testing.T) {
8184
Spec: corev1.ServiceSpec{
8285
ClusterIPs: []string{"127.0.0.1"},
8386
Type: corev1.ServiceTypeLoadBalancer,
87+
IPFamilies: []corev1.IPFamily{
88+
corev1.IPv4Protocol,
89+
},
8490
},
8591
Status: corev1.ServiceStatus{
8692
LoadBalancer: corev1.LoadBalancerStatus{
@@ -114,6 +120,9 @@ func TestUpdateGatewayStatusProgrammedCondition(t *testing.T) {
114120
Spec: corev1.ServiceSpec{
115121
ClusterIPs: []string{"127.0.0.1"},
116122
Type: corev1.ServiceTypeClusterIP,
123+
IPFamilies: []corev1.IPFamily{
124+
corev1.IPv4Protocol,
125+
},
117126
},
118127
},
119128
},
@@ -127,13 +136,18 @@ func TestUpdateGatewayStatusProgrammedCondition(t *testing.T) {
127136
{
128137
name: "Nodeport svc",
129138
args: args{
130-
gw: &gwapiv1.Gateway{},
131-
nodeAddresses: []string{"1", "2"},
139+
gw: &gwapiv1.Gateway{},
140+
nodeAddresses: NodeAddresses{
141+
IPv4: []string{"1", "2"},
142+
},
132143
svc: &corev1.Service{
133144
TypeMeta: metav1.TypeMeta{},
134145
ObjectMeta: metav1.ObjectMeta{},
135146
Spec: corev1.ServiceSpec{
136147
Type: corev1.ServiceTypeNodePort,
148+
IPFamilies: []corev1.IPFamily{
149+
corev1.IPv4Protocol,
150+
},
137151
},
138152
},
139153
},
@@ -153,9 +167,9 @@ func TestUpdateGatewayStatusProgrammedCondition(t *testing.T) {
153167
args: args{
154168
gw: &gwapiv1.Gateway{},
155169
// 20 node addresses
156-
nodeAddresses: func() (addr []string) {
170+
nodeAddresses: func() (addr NodeAddresses) {
157171
for i := 0; i < 20; i++ {
158-
addr = append(addr, strconv.Itoa(i))
172+
addr.IPv4 = append(addr.IPv4, strconv.Itoa(i))
159173
}
160174
return
161175
}(),
@@ -164,6 +178,9 @@ func TestUpdateGatewayStatusProgrammedCondition(t *testing.T) {
164178
ObjectMeta: metav1.ObjectMeta{},
165179
Spec: corev1.ServiceSpec{
166180
Type: corev1.ServiceTypeNodePort,
181+
IPFamilies: []corev1.IPFamily{
182+
corev1.IPv4Protocol,
183+
},
167184
},
168185
},
169186
},
@@ -185,6 +202,9 @@ func TestUpdateGatewayStatusProgrammedCondition(t *testing.T) {
185202
svc: &corev1.Service{
186203
Spec: corev1.ServiceSpec{
187204
Type: corev1.ServiceTypeLoadBalancer,
205+
IPFamilies: []corev1.IPFamily{
206+
corev1.IPv6Protocol,
207+
},
188208
},
189209
Status: corev1.ServiceStatus{
190210
LoadBalancer: corev1.LoadBalancerStatus{
@@ -210,6 +230,9 @@ func TestUpdateGatewayStatusProgrammedCondition(t *testing.T) {
210230
Spec: corev1.ServiceSpec{
211231
ClusterIPs: []string{"2001:db8::2"},
212232
Type: corev1.ServiceTypeClusterIP,
233+
IPFamilies: []corev1.IPFamily{
234+
corev1.IPv6Protocol,
235+
},
213236
},
214237
},
215238
},
@@ -223,11 +246,16 @@ func TestUpdateGatewayStatusProgrammedCondition(t *testing.T) {
223246
{
224247
name: "Nodeport svc with IPv6 node addresses",
225248
args: args{
226-
gw: &gwapiv1.Gateway{},
227-
nodeAddresses: []string{"2001:db8::3", "2001:db8::4"},
249+
gw: &gwapiv1.Gateway{},
250+
nodeAddresses: NodeAddresses{
251+
IPv6: []string{"2001:db8::3", "2001:db8::4"},
252+
},
228253
svc: &corev1.Service{
229254
Spec: corev1.ServiceSpec{
230255
Type: corev1.ServiceTypeNodePort,
256+
IPFamilies: []corev1.IPFamily{
257+
corev1.IPv6Protocol,
258+
},
231259
},
232260
},
233261
},
@@ -281,10 +309,34 @@ func TestUpdateGatewayStatusProgrammedCondition(t *testing.T) {
281309
},
282310
wantAddresses: []gwapiv1.GatewayStatusAddress{},
283311
},
312+
{
313+
name: "Nodeport svc Ipv6 with dual stack node addresses",
314+
args: args{
315+
gw: &gwapiv1.Gateway{},
316+
nodeAddresses: NodeAddresses{
317+
IPv4: []string{"10.0.0.1"},
318+
IPv6: []string{"2001:db8::4"},
319+
},
320+
svc: &corev1.Service{
321+
Spec: corev1.ServiceSpec{
322+
Type: corev1.ServiceTypeNodePort,
323+
IPFamilies: []corev1.IPFamily{
324+
corev1.IPv6Protocol,
325+
},
326+
},
327+
},
328+
},
329+
wantAddresses: []gwapiv1.GatewayStatusAddress{
330+
{
331+
Type: ptr.To(gwapiv1.IPAddressType),
332+
Value: "2001:db8::4",
333+
},
334+
},
335+
},
284336
}
285337
for _, tt := range tests {
286338
t.Run(tt.name, func(t *testing.T) {
287-
UpdateGatewayStatusProgrammedCondition(tt.args.gw, tt.args.svc, tt.args.deployment, tt.args.nodeAddresses...)
339+
UpdateGatewayStatusProgrammedCondition(tt.args.gw, tt.args.svc, tt.args.deployment, tt.args.nodeAddresses)
288340
assert.True(t, reflect.DeepEqual(tt.wantAddresses, tt.args.gw.Status.Addresses))
289341
})
290342
}

‎internal/provider/kubernetes/status.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -576,7 +576,7 @@ func (r *gatewayAPIReconciler) updateStatusForGateway(ctx context.Context, gtw *
576576
// to true in the Gateway API translator
577577
status.UpdateGatewayStatusAccepted(gtw)
578578
// update address field and programmed condition
579-
status.UpdateGatewayStatusProgrammedCondition(gtw, svc, envoyObj, r.store.listNodeAddresses()...)
579+
status.UpdateGatewayStatusProgrammedCondition(gtw, svc, envoyObj, r.store.listNodeAddresses())
580580
}
581581

582582
key := utils.NamespacedName(gtw)

‎internal/provider/kubernetes/store.go‎

Lines changed: 26 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,17 @@
66
package kubernetes
77

88
import (
9+
"net"
910
"sync"
1011

1112
corev1 "k8s.io/api/core/v1"
13+
14+
"github.com/envoyproxy/gateway/internal/gatewayapi/status"
1215
)
1316

1417
type nodeDetails struct {
15-
name string
16-
address string
18+
name string
19+
addresses status.NodeAddresses
1720
}
1821

1922
// kubernetesProviderStore holds cached information for the kubernetes provider.
@@ -34,23 +37,31 @@ func newProviderStore() *kubernetesProviderStore {
3437
func (p *kubernetesProviderStore) addNode(n *corev1.Node) {
3538
details := nodeDetails{name: n.Name}
3639

37-
var internalIP, externalIP string
40+
var internalIPs, externalIPs status.NodeAddresses
3841
for _, addr := range n.Status.Addresses {
39-
if addr.Type == corev1.NodeExternalIP {
40-
externalIP = addr.Address
42+
var addrs *status.NodeAddresses
43+
switch addr.Type {
44+
case corev1.NodeExternalIP:
45+
addrs = &externalIPs
46+
case corev1.NodeInternalIP:
47+
addrs = &internalIPs
48+
default:
49+
continue
4150
}
42-
if addr.Type == corev1.NodeInternalIP {
43-
internalIP = addr.Address
51+
if net.ParseIP(addr.Address).To4() != nil {
52+
addrs.IPv4 = append(addrs.IPv4, addr.Address)
53+
} else {
54+
addrs.IPv6 = append(addrs.IPv6, addr.Address)
4455
}
4556
}
4657

4758
// In certain scenarios (like in local KinD clusters), the Node
4859
// externalIP is not provided, in that case we default back
4960
// to the internalIP of the Node.
50-
if externalIP != "" {
51-
details.address = externalIP
52-
} else if internalIP != "" {
53-
details.address = internalIP
61+
if len(externalIPs.IPv4) > 0 || len(externalIPs.IPv6) > 0 {
62+
details.addresses = externalIPs
63+
} else if len(internalIPs.IPv4) > 0 || len(internalIPs.IPv6) > 0 {
64+
details.addresses = internalIPs
5465
}
5566
p.mu.Lock()
5667
defer p.mu.Unlock()
@@ -63,14 +74,13 @@ func (p *kubernetesProviderStore) removeNode(n *corev1.Node) {
6374
delete(p.nodes, n.Name)
6475
}
6576

66-
func (p *kubernetesProviderStore) listNodeAddresses() []string {
67-
addrs := []string{}
77+
func (p *kubernetesProviderStore) listNodeAddresses() status.NodeAddresses {
6878
p.mu.Lock()
6979
defer p.mu.Unlock()
80+
addrs := status.NodeAddresses{}
7081
for _, n := range p.nodes {
71-
if n.address != "" {
72-
addrs = append(addrs, n.address)
73-
}
82+
addrs.IPv4 = append(addrs.IPv4, n.addresses.IPv4...)
83+
addrs.IPv6 = append(addrs.IPv6, n.addresses.IPv6...)
7484
}
7585
return addrs
7686
}

‎internal/provider/kubernetes/store_test.go‎

Lines changed: 47 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,22 +11,24 @@ import (
1111
"github.com/stretchr/testify/assert"
1212
corev1 "k8s.io/api/core/v1"
1313
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
14+
15+
"github.com/envoyproxy/gateway/internal/gatewayapi/status"
1416
)
1517

1618
func TestNodeDetailsAddressStore(t *testing.T) {
1719
store := newProviderStore()
1820
testCases := []struct {
1921
name string
2022
nodeObject *corev1.Node
21-
expectedAddresses []string
23+
expectedAddresses status.NodeAddresses
2224
}{
2325
{
2426
name: "No node addresses",
2527
nodeObject: &corev1.Node{
2628
ObjectMeta: metav1.ObjectMeta{Name: "node1"},
2729
Status: corev1.NodeStatus{Addresses: []corev1.NodeAddress{{}}},
2830
},
29-
expectedAddresses: []string{},
31+
expectedAddresses: status.NodeAddresses{},
3032
},
3133
{
3234
name: "only external address",
@@ -37,7 +39,9 @@ func TestNodeDetailsAddressStore(t *testing.T) {
3739
Type: corev1.NodeExternalIP,
3840
}}},
3941
},
40-
expectedAddresses: []string{"1.1.1.1"},
42+
expectedAddresses: status.NodeAddresses{
43+
IPv4: []string{"1.1.1.1"},
44+
},
4145
},
4246
{
4347
name: "only internal address",
@@ -48,7 +52,9 @@ func TestNodeDetailsAddressStore(t *testing.T) {
4852
Type: corev1.NodeInternalIP,
4953
}}},
5054
},
51-
expectedAddresses: []string{"1.1.1.1"},
55+
expectedAddresses: status.NodeAddresses{
56+
IPv4: []string{"1.1.1.1"},
57+
},
5258
},
5359
{
5460
name: "prefer external address",
@@ -65,7 +71,43 @@ func TestNodeDetailsAddressStore(t *testing.T) {
6571
},
6672
}},
6773
},
68-
expectedAddresses: []string{"1.1.1.1"},
74+
expectedAddresses: status.NodeAddresses{
75+
IPv4: []string{"1.1.1.1"},
76+
},
77+
},
78+
{
79+
name: "all external addresses",
80+
nodeObject: &corev1.Node{
81+
ObjectMeta: metav1.ObjectMeta{Name: "node1"},
82+
Status: corev1.NodeStatus{Addresses: []corev1.NodeAddress{{
83+
Address: "1.1.1.1",
84+
Type: corev1.NodeExternalIP,
85+
}, {
86+
Address: "2606:4700:4700::1111",
87+
Type: corev1.NodeExternalIP,
88+
}}},
89+
},
90+
expectedAddresses: status.NodeAddresses{
91+
IPv4: []string{"1.1.1.1"},
92+
IPv6: []string{"2606:4700:4700::1111"},
93+
},
94+
},
95+
{
96+
name: "all internal addresses",
97+
nodeObject: &corev1.Node{
98+
ObjectMeta: metav1.ObjectMeta{Name: "node1"},
99+
Status: corev1.NodeStatus{Addresses: []corev1.NodeAddress{{
100+
Address: "1.1.1.1",
101+
Type: corev1.NodeInternalIP,
102+
}, {
103+
Address: "2606:4700:4700::1111",
104+
Type: corev1.NodeInternalIP,
105+
}}},
106+
},
107+
expectedAddresses: status.NodeAddresses{
108+
IPv4: []string{"1.1.1.1"},
109+
IPv6: []string{"2606:4700:4700::1111"},
110+
},
69111
},
70112
}
71113

‎release-notes/current.yaml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ bug fixes: |
4747
Add ConfigMap indexers for EnvoyExtensionPolicies to reconcile Lua changes
4848
Fixed issue that default accesslog format not working.
4949
Fixed validation errors when the rateLimit url for Redis in the EnvoyGateway API includes multiple comma separated hosts.
50+
Fixes addresses in status of DualStack NodePort Gateways.
5051
5152
# Enhancements that improve performance.
5253
performance improvements: |

0 commit comments

Comments
 (0)