Skip to content

Commit 44472e9

Browse files
authored
Add 503 status code when there are zero upstream endpoints (#3406)
Add 503 status code when there are zero upstream endpoints. Problem: When using NGINX Plus, the generated state file for an upstream was not correctly generating the 503 server for upstreams with zero endpoints. Solution: On upstreams with zero endpoints, generate the correct nginx conf in the state file.
1 parent 209f028 commit 44472e9

File tree

6 files changed

+45
-11
lines changed

6 files changed

+45
-11
lines changed

internal/controller/nginx/agent/agent.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515

1616
"github.com/nginx/nginx-gateway-fabric/internal/controller/nginx/agent/broadcast"
1717
agentgrpc "github.com/nginx/nginx-gateway-fabric/internal/controller/nginx/agent/grpc"
18+
"github.com/nginx/nginx-gateway-fabric/internal/controller/nginx/types"
1819
"github.com/nginx/nginx-gateway-fabric/internal/controller/state/dataplane"
1920
"github.com/nginx/nginx-gateway-fabric/internal/controller/state/resolver"
2021
"github.com/nginx/nginx-gateway-fabric/internal/controller/status"
@@ -179,6 +180,16 @@ func buildStreamUpstreamServers(upstream dataplane.Upstream) *pb.UpdateStreamSer
179180
}
180181

181182
func buildUpstreamServers(upstream dataplane.Upstream) []*structpb.Struct {
183+
if len(upstream.Endpoints) == 0 {
184+
return []*structpb.Struct{
185+
{
186+
Fields: map[string]*structpb.Value{
187+
"server": structpb.NewStringValue(types.Nginx503Server),
188+
},
189+
},
190+
}
191+
}
192+
182193
servers := make([]*structpb.Struct, 0, len(upstream.Endpoints))
183194

184195
for _, endpoint := range upstream.Endpoints {

internal/controller/nginx/agent/agent_test.go

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"sigs.k8s.io/controller-runtime/pkg/client/fake"
1313

1414
"github.com/nginx/nginx-gateway-fabric/internal/controller/nginx/agent/broadcast/broadcastfakes"
15+
"github.com/nginx/nginx-gateway-fabric/internal/controller/nginx/types"
1516
"github.com/nginx/nginx-gateway-fabric/internal/controller/state/dataplane"
1617
"github.com/nginx/nginx-gateway-fabric/internal/controller/state/resolver"
1718
"github.com/nginx/nginx-gateway-fabric/internal/controller/status"
@@ -179,6 +180,10 @@ func TestUpdateUpstreamServers(t *testing.T) {
179180
},
180181
},
181182
},
183+
{
184+
Name: "empty-upstream",
185+
Endpoints: []resolver.Endpoint{},
186+
},
182187
},
183188
StreamUpstreams: []dataplane.Upstream{
184189
{
@@ -212,6 +217,20 @@ func TestUpdateUpstreamServers(t *testing.T) {
212217
},
213218
},
214219
},
220+
{
221+
Action: &pb.NGINXPlusAction_UpdateHttpUpstreamServers{
222+
UpdateHttpUpstreamServers: &pb.UpdateHTTPUpstreamServers{
223+
HttpUpstreamName: "empty-upstream",
224+
Servers: []*structpb.Struct{
225+
{
226+
Fields: map[string]*structpb.Value{
227+
"server": structpb.NewStringValue(types.Nginx503Server),
228+
},
229+
},
230+
},
231+
},
232+
},
233+
},
215234
{
216235
Action: &pb.NGINXPlusAction_UpdateStreamServers{
217236
UpdateStreamServers: &pb.UpdateStreamServers{
@@ -234,13 +253,14 @@ func TestUpdateUpstreamServers(t *testing.T) {
234253
g.Expect(fakeBroadcaster.SendCallCount()).To(Equal(0))
235254
} else if test.buildUpstreams {
236255
g.Expect(deployment.GetNGINXPlusActions()).To(Equal(expActions))
237-
g.Expect(fakeBroadcaster.SendCallCount()).To(Equal(2))
256+
g.Expect(fakeBroadcaster.SendCallCount()).To(Equal(3))
238257
}
239258

240259
if test.expErr {
241260
expErr := errors.Join(
242261
fmt.Errorf("couldn't update upstream via the API: %w", testErr),
243262
fmt.Errorf("couldn't update upstream via the API: %w", testErr),
263+
fmt.Errorf("couldn't update upstream via the API: %w", testErr),
244264
)
245265

246266
g.Expect(deployment.GetLatestUpstreamError()).To(Equal(expErr))

internal/controller/nginx/config/upstreams.go

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"github.com/nginx/nginx-gateway-fabric/internal/controller/nginx/config/http"
88
"github.com/nginx/nginx-gateway-fabric/internal/controller/nginx/config/policies/upstreamsettings"
99
"github.com/nginx/nginx-gateway-fabric/internal/controller/nginx/config/stream"
10+
"github.com/nginx/nginx-gateway-fabric/internal/controller/nginx/types"
1011
"github.com/nginx/nginx-gateway-fabric/internal/controller/state/dataplane"
1112
"github.com/nginx/nginx-gateway-fabric/internal/framework/helpers"
1213
)
@@ -17,8 +18,6 @@ var (
1718
)
1819

1920
const (
20-
// nginx503Server is used as a backend for services that cannot be resolved (have no IP address).
21-
nginx503Server = "unix:/var/run/nginx/nginx-503-server.sock"
2221
// nginx500Server is used as a server for the invalid backend ref upstream.
2322
nginx500Server = "unix:/var/run/nginx/nginx-500-server.sock"
2423
// invalidBackendRef is used as an upstream name for invalid backend references.
@@ -159,7 +158,7 @@ func (g GeneratorImpl) createUpstream(
159158
StateFile: stateFile,
160159
Servers: []http.UpstreamServer{
161160
{
162-
Address: nginx503Server,
161+
Address: types.Nginx503Server,
163162
},
164163
},
165164
}

internal/controller/nginx/config/upstreams_test.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"github.com/nginx/nginx-gateway-fabric/internal/controller/nginx/config/policies"
1212
"github.com/nginx/nginx-gateway-fabric/internal/controller/nginx/config/policies/upstreamsettings"
1313
"github.com/nginx/nginx-gateway-fabric/internal/controller/nginx/config/stream"
14+
"github.com/nginx/nginx-gateway-fabric/internal/controller/nginx/types"
1415
"github.com/nginx/nginx-gateway-fabric/internal/controller/state/dataplane"
1516
"github.com/nginx/nginx-gateway-fabric/internal/controller/state/resolver"
1617
"github.com/nginx/nginx-gateway-fabric/internal/framework/helpers"
@@ -216,7 +217,7 @@ func TestCreateUpstreams(t *testing.T) {
216217
ZoneSize: ossZoneSize,
217218
Servers: []http.UpstreamServer{
218219
{
219-
Address: nginx503Server,
220+
Address: types.Nginx503Server,
220221
},
221222
},
222223
},
@@ -277,7 +278,7 @@ func TestCreateUpstream(t *testing.T) {
277278
ZoneSize: ossZoneSize,
278279
Servers: []http.UpstreamServer{
279280
{
280-
Address: nginx503Server,
281+
Address: types.Nginx503Server,
281282
},
282283
},
283284
},
@@ -293,7 +294,7 @@ func TestCreateUpstream(t *testing.T) {
293294
ZoneSize: ossZoneSize,
294295
Servers: []http.UpstreamServer{
295296
{
296-
Address: nginx503Server,
297+
Address: types.Nginx503Server,
297298
},
298299
},
299300
},
@@ -581,7 +582,7 @@ func TestCreateUpstreamPlus(t *testing.T) {
581582
StateFile: stateDir + "/no-endpoints.conf",
582583
Servers: []http.UpstreamServer{
583584
{
584-
Address: nginx503Server,
585+
Address: types.Nginx503Server,
585586
},
586587
},
587588
},
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
package types
2+
3+
const (
4+
// Nginx503Server is used as a backend for services that cannot be resolved (have no IP address).
5+
Nginx503Server = "unix:/var/run/nginx/nginx-503-server.sock"
6+
)

tests/suite/graceful_recovery_test.go

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -585,9 +585,6 @@ func getUnexpectedNginxErrorLogs(nginxPodName, namespace string) string {
585585
"connect() failed (111: Connection refused)",
586586
"could not be resolved (host not found) during usage report",
587587
"server returned 429",
588-
// FIXME(salonichf5) remove this error message check
589-
// when https://github.com/nginx/nginx-gateway-fabric/issues/2090 is completed.
590-
"no live upstreams while connecting to upstream",
591588
}
592589

593590
unexpectedErrors := ""

0 commit comments

Comments
 (0)