Skip to content

Commit 157c011

Browse files
pleshakovpleshakov
authored
Handle multiple Gateway resources (#139)
Previously, NGINX Gateway only supported a single Gateway resource that must have had 'nginx-gateway/gateway' namespace/name. This commit removes that hard-corded requirement: Now NGINX Gateway will select the Gateway resource among all resources that belong to NGINX Gateway (i.e. reference NGINX Gateway corresponding GatewayClass). In case of multiple Gateway resources, NGINX Gateway will use a deterministic conflict resolution strategy: it will choose the oldest resource based on the creation timestamp. If the timestamps are equal, NGINX Gateway will chose the resource that appears alphabetically first based on its namespace and then name.
1 parent 12b6b70 commit 157c011

19 files changed

+1276
-676
lines changed

‎cmd/gateway/main.go‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import (
55
"os"
66

77
flag "github.com/spf13/pflag"
8-
"k8s.io/apimachinery/pkg/types"
98
"sigs.k8s.io/controller-runtime/pkg/log/zap"
109

1110
"github.com/nginxinc/nginx-kubernetes-gateway/internal/config"
@@ -40,13 +39,8 @@ func main() {
4039

4140
logger := zap.New()
4241
conf := config.Config{
43-
GatewayCtlrName: *gatewayCtlrName,
44-
Logger: logger,
45-
// FIXME(pleshakov) Allow the cluster operator to customize this value
46-
GatewayNsName: types.NamespacedName{
47-
Namespace: "nginx-gateway",
48-
Name: "gateway",
49-
},
42+
GatewayCtlrName: *gatewayCtlrName,
43+
Logger: logger,
5044
GatewayClassName: *gatewayClassName,
5145
}
5246

‎internal/implementations/gateway/gateway.go‎

Lines changed: 6 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
package implementation
22

33
import (
4-
"fmt"
5-
64
"github.com/go-logr/logr"
75
"k8s.io/apimachinery/pkg/types"
86
"sigs.k8s.io/gateway-api/apis/v1alpha2"
@@ -13,32 +11,22 @@ import (
1311
)
1412

1513
type gatewayImplementation struct {
16-
conf config.Config
14+
logger logr.Logger
1715
eventCh chan<- interface{}
1816
}
1917

2018
func NewGatewayImplementation(conf config.Config, eventCh chan<- interface{}) sdk.GatewayImpl {
2119
return &gatewayImplementation{
22-
conf: conf,
20+
logger: conf.Logger,
2321
eventCh: eventCh,
2422
}
2523
}
2624

27-
func (impl *gatewayImplementation) Logger() logr.Logger {
28-
return impl.conf.Logger
29-
}
25+
// FIXME(pleshakov) All Implementations (Gateway, HTTPRoute, ...) look similar. Consider writing a general-purpose
26+
// component to implement all implementations. This will avoid the duplication code and tests.
3027

3128
func (impl *gatewayImplementation) Upsert(gw *v1alpha2.Gateway) {
32-
if gw.Namespace != impl.conf.GatewayNsName.Namespace || gw.Name != impl.conf.GatewayNsName.Name {
33-
msg := fmt.Sprintf("Gateway was upserted but ignored because this controller only supports the Gateway %s", impl.conf.GatewayNsName)
34-
impl.Logger().Info(msg,
35-
"namespace", gw.Namespace,
36-
"name", gw.Name,
37-
)
38-
return
39-
}
40-
41-
impl.Logger().Info("Gateway was upserted",
29+
impl.logger.Info("Gateway was upserted",
4230
"namespace", gw.Namespace,
4331
"name", gw.Name,
4432
)
@@ -49,16 +37,7 @@ func (impl *gatewayImplementation) Upsert(gw *v1alpha2.Gateway) {
4937
}
5038

5139
func (impl *gatewayImplementation) Remove(nsname types.NamespacedName) {
52-
if nsname != impl.conf.GatewayNsName {
53-
msg := fmt.Sprintf("Gateway was removed but ignored because this controller only supports the Gateway %s", impl.conf.GatewayNsName)
54-
impl.Logger().Info(msg,
55-
"namespace", nsname.Namespace,
56-
"name", nsname.Name,
57-
)
58-
return
59-
}
60-
61-
impl.Logger().Info("Gateway was removed",
40+
impl.logger.Info("Gateway was removed",
6241
"namespace", nsname.Namespace,
6342
"name", nsname.Name,
6443
)

‎internal/implementations/gateway/gateway_test.go‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
package implementation_test
2+
3+
import (
4+
. "github.com/onsi/ginkgo/v2"
5+
. "github.com/onsi/gomega"
6+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
7+
"k8s.io/apimachinery/pkg/types"
8+
"sigs.k8s.io/controller-runtime/pkg/log/zap"
9+
"sigs.k8s.io/gateway-api/apis/v1alpha2"
10+
11+
"github.com/nginxinc/nginx-kubernetes-gateway/internal/config"
12+
"github.com/nginxinc/nginx-kubernetes-gateway/internal/events"
13+
implementation "github.com/nginxinc/nginx-kubernetes-gateway/internal/implementations/gateway"
14+
"github.com/nginxinc/nginx-kubernetes-gateway/pkg/sdk"
15+
)
16+
17+
var _ = Describe("GatewayImplementation", func() {
18+
var (
19+
eventCh chan interface{}
20+
impl sdk.GatewayImpl
21+
)
22+
23+
BeforeEach(func() {
24+
eventCh = make(chan interface{})
25+
26+
impl = implementation.NewGatewayImplementation(config.Config{
27+
Logger: zap.New(),
28+
}, eventCh)
29+
})
30+
31+
Describe("Implementation processes Gateways", func() {
32+
It("should process upsert", func() {
33+
gc := &v1alpha2.Gateway{
34+
ObjectMeta: metav1.ObjectMeta{
35+
Namespace: "test-add",
36+
Name: "gateway",
37+
},
38+
}
39+
40+
go func() {
41+
impl.Upsert(gc)
42+
}()
43+
44+
Eventually(eventCh).Should(Receive(Equal(&events.UpsertEvent{Resource: gc})))
45+
})
46+
47+
It("should process remove", func() {
48+
nsname := types.NamespacedName{Namespace: "test-remove", Name: "gateway"}
49+
50+
go func() {
51+
impl.Remove(nsname)
52+
}()
53+
54+
Eventually(eventCh).Should(Receive(Equal(
55+
&events.DeleteEvent{
56+
NamespacedName: nsname,
57+
Type: &v1alpha2.Gateway{},
58+
})))
59+
})
60+
})
61+
})

‎internal/implementations/gateway/implementation_suit_test.go‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
package implementation_test
2+
3+
import (
4+
"testing"
5+
6+
. "github.com/onsi/ginkgo/v2"
7+
. "github.com/onsi/gomega"
8+
)
9+
10+
func TestImplementation(t *testing.T) {
11+
RegisterFailHandler(Fail)
12+
RunSpecs(t, "Implementation Suite")
13+
}

‎internal/manager/manager.go‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,6 @@ func Start(cfg config.Config) error {
7070
}
7171

7272
processor := state.NewChangeProcessorImpl(state.ChangeProcessorConfig{
73-
GatewayNsName: cfg.GatewayNsName,
7473
GatewayCtlrName: cfg.GatewayCtlrName,
7574
GatewayClassName: cfg.GatewayClassName,
7675
})
@@ -79,7 +78,6 @@ func Start(cfg config.Config) error {
7978
nginxFileMgr := file.NewManagerImpl()
8079
nginxRuntimeMgr := ngxruntime.NewManagerImpl()
8180
statusUpdater := status.NewUpdater(status.UpdaterConfig{
82-
GatewayNsName: cfg.GatewayNsName,
8381
GatewayCtlrName: cfg.GatewayCtlrName,
8482
GatewayClassName: cfg.GatewayClassName,
8583
Client: mgr.GetClient(),

‎internal/state/change_processor.go‎

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,12 @@ func NewChangeProcessorImpl(cfg ChangeProcessorConfig) *ChangeProcessorImpl {
5555
}
5656
}
5757

58+
// FIXME(pleshakov)
59+
// Currently, changes (upserts/delete) trigger rebuilding of the configuration, even if the change doesn't change
60+
// the configuration or the statuses of the resources. For example, a change in a Gateway resource that doesn't
61+
// belong to the NGINX Gateway or an HTTPRoute that doesn't belong to any of the Gateways of the NGINX Gateway.
62+
// Find a way to ignore changes that don't affect the configuration and/or statuses of the resources.
63+
5864
func (c *ChangeProcessorImpl) CaptureUpsertChange(obj client.Object) {
5965
c.lock.Lock()
6066
defer c.lock.Unlock()
@@ -72,14 +78,12 @@ func (c *ChangeProcessorImpl) CaptureUpsertChange(obj client.Object) {
7278
}
7379
c.store.gc = o
7480
case *v1alpha2.Gateway:
75-
if o.Namespace != c.cfg.GatewayNsName.Namespace || o.Name != c.cfg.GatewayNsName.Name {
76-
panic(fmt.Errorf("gateway resource must be %s/%s, got %s/%s", c.cfg.GatewayNsName.Namespace, c.cfg.GatewayNsName.Name, o.Namespace, o.Name))
77-
}
7881
// if the resource spec hasn't changed (its generation is the same), ignore the upsert
79-
if c.store.gw != nil && c.store.gw.Generation == o.Generation {
82+
prev, exist := c.store.gateways[getNamespacedName(obj)]
83+
if exist && o.Generation == prev.Generation {
8084
c.changed = false
8185
}
82-
c.store.gw = o
86+
c.store.gateways[getNamespacedName(obj)] = o
8387
case *v1alpha2.HTTPRoute:
8488
// if the resource spec hasn't changed (its generation is the same), ignore the upsert
8589
prev, exist := c.store.httpRoutes[getNamespacedName(obj)]
@@ -98,17 +102,14 @@ func (c *ChangeProcessorImpl) CaptureDeleteChange(resourceType client.Object, ns
98102

99103
c.changed = true
100104

101-
switch o := resourceType.(type) {
105+
switch resourceType.(type) {
102106
case *v1alpha2.GatewayClass:
103107
if nsname.Name != c.cfg.GatewayClassName {
104108
panic(fmt.Errorf("gatewayclass resource must be %s, got %s", c.cfg.GatewayClassName, nsname.Name))
105109
}
106110
c.store.gc = nil
107111
case *v1alpha2.Gateway:
108-
if nsname != c.cfg.GatewayNsName {
109-
panic(fmt.Errorf("gateway resource must be %s/%s, got %s/%s", c.cfg.GatewayNsName.Namespace, c.cfg.GatewayNsName.Name, o.Namespace, o.Name))
110-
}
111-
c.store.gw = nil
112+
delete(c.store.gateways, nsname)
112113
case *v1alpha2.HTTPRoute:
113114
delete(c.store.httpRoutes, nsname)
114115
default:
@@ -126,7 +127,7 @@ func (c *ChangeProcessorImpl) Process() (changed bool, conf Configuration, statu
126127

127128
c.changed = false
128129

129-
graph := buildGraph(c.store, c.cfg.GatewayNsName, c.cfg.GatewayCtlrName, c.cfg.GatewayClassName)
130+
graph := buildGraph(c.store, c.cfg.GatewayCtlrName, c.cfg.GatewayClassName)
130131

131132
conf = buildConfiguration(graph)
132133
statuses = buildStatuses(graph)

0 commit comments

Comments
 (0)