Skip to content

Commit 29f1a4e

Browse files
authored
fix: set the Gateway status reasons Gateway API 1.6 requires (backport apache/apisix-ingress-controller#2843) (#466)
1 parent 95605a2 commit 29f1a4e

3 files changed

Lines changed: 253 additions & 3 deletions

File tree

‎internal/controller/gateway_controller.go‎

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -163,10 +163,12 @@ func (r *GatewayReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
163163
r.Log.Info("gateway has been accepted", "gateway", gateway.GetName())
164164
type conditionStatus struct {
165165
status bool
166+
reason gatewayv1.GatewayConditionReason
166167
msg string
167168
}
168169
acceptStatus := conditionStatus{
169170
status: true,
171+
reason: gatewayv1.GatewayReasonAccepted,
170172
msg: acceptedMessage("gateway"),
171173
}
172174

@@ -177,6 +179,7 @@ func (r *GatewayReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
177179
if err := r.processInfrastructure(tctx, gateway); err != nil {
178180
acceptStatus = conditionStatus{
179181
status: false,
182+
reason: gatewayv1.GatewayReasonInvalidParameters,
180183
msg: err.Error(),
181184
}
182185
}
@@ -187,8 +190,16 @@ func (r *GatewayReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
187190

188191
gatewayProxy, ok := tctx.GatewayProxies[rk]
189192
if !ok {
193+
// InvalidParameters is only the right answer when the Gateway actually
194+
// names a parametersRef that cannot be resolved. A Gateway that names
195+
// none is simply not configured yet.
196+
reason := gatewayv1.GatewayReasonPending
197+
if gateway.Spec.Infrastructure != nil && gateway.Spec.Infrastructure.ParametersRef != nil {
198+
reason = gatewayv1.GatewayReasonInvalidParameters
199+
}
190200
acceptStatus = conditionStatus{
191201
status: false,
202+
reason: reason,
192203
msg: "gateway proxy not found",
193204
}
194205
} else {
@@ -215,11 +226,24 @@ func (r *GatewayReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
215226
if err := r.Provider.Update(ctx, tctx, gateway); err != nil {
216227
acceptStatus = conditionStatus{
217228
status: false,
229+
reason: gatewayv1.GatewayReasonAccepted,
218230
msg: err.Error(),
219231
}
220232
}
221233

222-
accepted := SetGatewayConditionAccepted(gateway, acceptStatus.status, acceptStatus.msg)
234+
// A listener the Gateway cannot serve is the most specific thing to report,
235+
// so it wins over whatever else was found: the Gateway says
236+
// ListenersNotValid, and the status separates a Gateway that still serves
237+
// some listeners from one that serves none.
238+
if status, invalid := gatewayAcceptanceFromListeners(listenerStatuses); invalid {
239+
acceptStatus = conditionStatus{
240+
status: status,
241+
reason: gatewayv1.GatewayReasonListenersNotValid,
242+
msg: "one or more listeners are not accepted",
243+
}
244+
}
245+
246+
accepted := SetGatewayConditionAccepted(gateway, acceptStatus.status, acceptStatus.reason, acceptStatus.msg)
223247
programmed := SetGatewayConditionProgrammed(gateway, conditionProgrammedStatus, conditionProgrammedMsg)
224248
if accepted || programmed || len(addrs) > 0 || len(listenerStatuses) > 0 {
225249
if len(addrs) > 0 {

‎internal/controller/utils.go‎

Lines changed: 57 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -168,11 +168,11 @@ func IsConditionPresentAndEqual(conditions []metav1.Condition, condition metav1.
168168
return false
169169
}
170170

171-
func SetGatewayConditionAccepted(gw *gatewayv1.Gateway, status bool, message string) (ok bool) {
171+
func SetGatewayConditionAccepted(gw *gatewayv1.Gateway, status bool, reason gatewayv1.GatewayConditionReason, message string) (ok bool) {
172172
condition := metav1.Condition{
173173
Type: string(gatewayv1.GatewayConditionAccepted),
174174
Status: ConditionStatus(status),
175-
Reason: string(gatewayv1.GatewayReasonAccepted),
175+
Reason: string(reason),
176176
ObservedGeneration: gw.GetGeneration(),
177177
Message: message,
178178
LastTransitionTime: metav1.Now(),
@@ -599,6 +599,38 @@ func routeKindsForProtocol(protocol gatewayv1.ProtocolType) []gatewayv1.RouteGro
599599
return []gatewayv1.RouteGroupKind{}
600600
}
601601

602+
// isSupportedProtocol reports whether a listener protocol is one this
603+
// implementation serves. routeKindsForProtocol returning nothing is the same
604+
// question asked the other way round.
605+
func isSupportedProtocol(protocol gatewayv1.ProtocolType) bool {
606+
return len(routeKindsForProtocol(protocol)) > 0
607+
}
608+
609+
// gatewayAcceptanceFromListeners derives the Gateway's Accepted condition from
610+
// its listeners. Gateway API asks for ListenersNotValid as soon as one listener
611+
// is not accepted, with the status separating "some listeners work" from "none
612+
// do". invalid is false when every listener is accepted, leaving the caller's
613+
// own verdict alone.
614+
func gatewayAcceptanceFromListeners(listeners []gatewayv1.ListenerStatus) (status bool, invalid bool) {
615+
var accepted, rejected int
616+
for _, listener := range listeners {
617+
for _, condition := range listener.Conditions {
618+
if condition.Type != string(gatewayv1.ListenerConditionAccepted) {
619+
continue
620+
}
621+
if condition.Status == metav1.ConditionTrue {
622+
accepted++
623+
} else {
624+
rejected++
625+
}
626+
}
627+
}
628+
if rejected == 0 {
629+
return true, false
630+
}
631+
return accepted > 0, true
632+
}
633+
602634
func SetApisixCRDConditionAccepted(status *apiv2.ApisixStatus, generation int64, err error) {
603635
var condition = metav1.Condition{
604636
Type: string(apiv2.ConditionTypeAccepted),
@@ -988,6 +1020,29 @@ func getListenerStatus(
9881020
supportedKinds = []gatewayv1.RouteGroupKind{}
9891021
)
9901022

1023+
// A protocol this implementation does not serve is rejected outright:
1024+
// accepting it would advertise a listener that can never carry traffic.
1025+
if !isSupportedProtocol(listener.Protocol) {
1026+
conditionAccepted.Status = metav1.ConditionFalse
1027+
conditionAccepted.Reason = string(gatewayv1.ListenerReasonUnsupportedProtocol)
1028+
conditionAccepted.Message = fmt.Sprintf("protocol %q is not supported", listener.Protocol)
1029+
conditionProgrammed.Status = metav1.ConditionFalse
1030+
conditionProgrammed.Reason = string(gatewayv1.ListenerReasonInvalid)
1031+
1032+
statusArray = append(statusArray, reuseUnchangedListenerStatus(gateway, i, gatewayv1.ListenerStatus{
1033+
Name: listener.Name,
1034+
Conditions: []metav1.Condition{
1035+
conditionProgrammed,
1036+
conditionAccepted,
1037+
conditionConflicted,
1038+
conditionResolvedRefs,
1039+
},
1040+
SupportedKinds: supportedKinds,
1041+
AttachedRoutes: attachedRoutes,
1042+
}))
1043+
continue
1044+
}
1045+
9911046
// A port serving more than one TLS mode cannot be programmed, so the
9921047
// listener is rejected rather than accepted with undefined behaviour.
9931048
if listener.Protocol == gatewayv1.TLSProtocolType && tlsModeConflictPorts[listener.Port] {
Lines changed: 171 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,171 @@
1+
// Licensed to the Apache Software Foundation (ASF) under one
2+
// or more contributor license agreements. See the NOTICE file
3+
// distributed with this work for additional information
4+
// regarding copyright ownership. The ASF licenses this file
5+
// to you under the Apache License, Version 2.0 (the
6+
// "License"); you may not use this file except in compliance
7+
// with the License. You may obtain a copy of the License at
8+
//
9+
// http://www.apache.org/licenses/LICENSE-2.0
10+
//
11+
// Unless required by applicable law or agreed to in writing,
12+
// software distributed under the License is distributed on an
13+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
// KIND, either express or implied. See the License for the
15+
// specific language governing permissions and limitations
16+
// under the License.
17+
18+
package controller
19+
20+
import (
21+
"context"
22+
"testing"
23+
24+
"github.com/stretchr/testify/assert"
25+
"github.com/stretchr/testify/require"
26+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
27+
"sigs.k8s.io/controller-runtime/pkg/client"
28+
"sigs.k8s.io/controller-runtime/pkg/client/fake"
29+
gatewayv1 "sigs.k8s.io/gateway-api/apis/v1"
30+
31+
"github.com/apache/apisix-ingress-controller/internal/controller/indexer"
32+
)
33+
34+
// A listener whose protocol the implementation does not serve must say so with
35+
// UnsupportedProtocol and advertise no route kinds, rather than be accepted and
36+
// then quietly serve nothing.
37+
func TestGetListenerStatus_UnsupportedProtocol(t *testing.T) {
38+
scheme := parentRefTestScheme(t)
39+
40+
for _, tc := range []struct {
41+
name string
42+
listener gatewayv1.Listener
43+
wantStatus metav1.ConditionStatus
44+
wantReason gatewayv1.ListenerConditionReason
45+
wantKinds int
46+
}{
47+
{
48+
name: "unknown protocol",
49+
listener: gatewayv1.Listener{
50+
Name: "invalid", Port: 1111, Protocol: gatewayv1.ProtocolType("INVALID"),
51+
},
52+
wantStatus: metav1.ConditionFalse,
53+
wantReason: gatewayv1.ListenerReasonUnsupportedProtocol,
54+
wantKinds: 0,
55+
},
56+
{
57+
name: "known protocol",
58+
listener: gatewayv1.Listener{
59+
Name: "http", Port: 80, Protocol: gatewayv1.HTTPProtocolType,
60+
},
61+
wantStatus: metav1.ConditionTrue,
62+
wantReason: gatewayv1.ListenerReasonAccepted,
63+
wantKinds: 2,
64+
},
65+
} {
66+
t.Run(tc.name, func(t *testing.T) {
67+
gw := &gatewayv1.Gateway{
68+
ObjectMeta: metav1.ObjectMeta{Namespace: "default", Name: "gw"},
69+
Spec: gatewayv1.GatewaySpec{
70+
GatewayClassName: "apisix",
71+
Listeners: []gatewayv1.Listener{tc.listener},
72+
},
73+
}
74+
cli := fake.NewClientBuilder().WithScheme(scheme).
75+
WithObjects(newParentRefGatewayClass(), gw).
76+
// An HTTP listener counts both route kinds it can serve, so both
77+
// indexes have to exist even though no route does.
78+
WithIndex(&gatewayv1.HTTPRoute{}, indexer.ParentRefs,
79+
func(client.Object) []string { return nil }).
80+
WithIndex(&gatewayv1.GRPCRoute{}, indexer.ParentRefs,
81+
func(client.Object) []string { return nil }).
82+
Build()
83+
84+
statuses, err := getListenerStatus(context.Background(), cli, gw)
85+
require.NoError(t, err)
86+
require.Len(t, statuses, 1)
87+
88+
var accepted *metav1.Condition
89+
for i := range statuses[0].Conditions {
90+
if statuses[0].Conditions[i].Type == string(gatewayv1.ListenerConditionAccepted) {
91+
accepted = &statuses[0].Conditions[i]
92+
}
93+
}
94+
require.NotNil(t, accepted, "listener must report an Accepted condition")
95+
assert.Equal(t, tc.wantStatus, accepted.Status)
96+
assert.Equal(t, string(tc.wantReason), accepted.Reason)
97+
assert.Len(t, statuses[0].SupportedKinds, tc.wantKinds)
98+
assert.Zero(t, statuses[0].AttachedRoutes)
99+
})
100+
}
101+
}
102+
103+
// The Gateway's own Accepted condition follows its listeners: one bad listener
104+
// is enough for ListenersNotValid, and the status separates "some listeners
105+
// work" from "none do".
106+
func TestGatewayAcceptanceFromListeners(t *testing.T) {
107+
listener := func(name string, accepted bool) gatewayv1.ListenerStatus {
108+
status := metav1.ConditionTrue
109+
if !accepted {
110+
status = metav1.ConditionFalse
111+
}
112+
return gatewayv1.ListenerStatus{
113+
Name: gatewayv1.SectionName(name),
114+
Conditions: []metav1.Condition{{
115+
Type: string(gatewayv1.ListenerConditionAccepted),
116+
Status: status,
117+
}},
118+
}
119+
}
120+
121+
for _, tc := range []struct {
122+
name string
123+
listeners []gatewayv1.ListenerStatus
124+
wantInvalid bool
125+
wantStatus bool
126+
}{
127+
{
128+
name: "all listeners accepted",
129+
listeners: []gatewayv1.ListenerStatus{listener("http", true), listener("https", true)},
130+
wantInvalid: false,
131+
},
132+
{
133+
name: "one of two listeners rejected",
134+
listeners: []gatewayv1.ListenerStatus{listener("http", true), listener("invalid", false)},
135+
wantInvalid: true,
136+
wantStatus: true,
137+
},
138+
{
139+
name: "every listener rejected",
140+
listeners: []gatewayv1.ListenerStatus{listener("invalid", false)},
141+
wantInvalid: true,
142+
wantStatus: false,
143+
},
144+
{
145+
name: "no listeners",
146+
listeners: nil,
147+
wantInvalid: false,
148+
},
149+
} {
150+
t.Run(tc.name, func(t *testing.T) {
151+
status, invalid := gatewayAcceptanceFromListeners(tc.listeners)
152+
assert.Equal(t, tc.wantInvalid, invalid)
153+
if invalid {
154+
assert.Equal(t, tc.wantStatus, status)
155+
}
156+
})
157+
}
158+
}
159+
160+
// Accepted=False used to report Reason=Accepted, which says nothing about what
161+
// went wrong.
162+
func TestSetGatewayConditionAccepted_Reason(t *testing.T) {
163+
gw := &gatewayv1.Gateway{ObjectMeta: metav1.ObjectMeta{Namespace: "default", Name: "gw"}}
164+
165+
SetGatewayConditionAccepted(gw, false, gatewayv1.GatewayReasonInvalidParameters, "parametersRef is not resolvable")
166+
167+
require.Len(t, gw.Status.Conditions, 1)
168+
assert.Equal(t, string(gatewayv1.GatewayConditionAccepted), gw.Status.Conditions[0].Type)
169+
assert.Equal(t, metav1.ConditionFalse, gw.Status.Conditions[0].Status)
170+
assert.Equal(t, string(gatewayv1.GatewayReasonInvalidParameters), gw.Status.Conditions[0].Reason)
171+
}

0 commit comments

Comments
 (0)