Skip to content

Commit eca8cd4

Browse files
pleshakovpleshakov
committed
Update status reporting to use new types
This commit updates status reporting to use the Statuses type, introduced in 0a39e3f, as a source of status information about resources.
1 parent 0e9ce25 commit eca8cd4

15 files changed

+734
-166
lines changed

‎deploy/manifests/nginx-gateway.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ rules:
3636
- gateway.networking.k8s.io
3737
resources:
3838
- httproutes/status
39+
- gateways/status
3940
verbs:
4041
- update
4142
---

‎internal/events/loop.go

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"github.com/nginxinc/nginx-kubernetes-gateway/internal/nginx/file"
1414
"github.com/nginxinc/nginx-kubernetes-gateway/internal/nginx/runtime"
1515
"github.com/nginxinc/nginx-kubernetes-gateway/internal/state"
16+
"github.com/nginxinc/nginx-kubernetes-gateway/internal/status"
1617
)
1718

1819
// EventLoop is the main event loop of the Gateway.
@@ -24,6 +25,7 @@ type EventLoop struct {
2425
logger logr.Logger
2526
nginxFileMgr file.Manager
2627
nginxRuntimeMgr runtime.Manager
28+
statusUpdater status.Updater
2729
}
2830

2931
// NewEventLoop creates a new EventLoop.
@@ -35,6 +37,7 @@ func NewEventLoop(
3537
logger logr.Logger,
3638
nginxFileMgr file.Manager,
3739
nginxRuntimeMgr runtime.Manager,
40+
statusUpdater status.Updater,
3841
) *EventLoop {
3942
return &EventLoop{
4043
processor: processor,
@@ -44,6 +47,7 @@ func NewEventLoop(
4447
logger: logger.WithName("eventLoop"),
4548
nginxFileMgr: nginxFileMgr,
4649
nginxRuntimeMgr: nginxRuntimeMgr,
50+
statusUpdater: statusUpdater,
4751
}
4852
}
4953

@@ -85,13 +89,7 @@ func (el *EventLoop) handleEvent(ctx context.Context, event interface{}) {
8589
el.logger.Error(err, "Failed to update NGINX configuration")
8690
}
8791

88-
// FIXME(pleshakov) Update resource statuses instead of printing to stdout
89-
for name, s := range statuses.ListenerStatuses {
90-
fmt.Printf("Listener %q, Statuses: %v\n", name, s)
91-
}
92-
for nsname, s := range statuses.HTTPRouteStatuses {
93-
fmt.Printf("HTTPRoute %q, Statuses: %v\n", nsname, s)
94-
}
92+
el.statusUpdater.Update(ctx, statuses)
9593
}
9694

9795
func (el *EventLoop) updateNginx(ctx context.Context, conf newstate.Configuration) error {

‎internal/events/loop_test.go

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"github.com/nginxinc/nginx-kubernetes-gateway/internal/nginx/file/filefakes"
2222
"github.com/nginxinc/nginx-kubernetes-gateway/internal/nginx/runtime/runtimefakes"
2323
"github.com/nginxinc/nginx-kubernetes-gateway/internal/state/statefakes"
24+
"github.com/nginxinc/nginx-kubernetes-gateway/internal/status/statusfakes"
2425
)
2526

2627
type unsupportedResource struct {
@@ -42,6 +43,7 @@ var _ = Describe("EventLoop", func() {
4243
fakeGenerator *configfakes.FakeGenerator
4344
fakeNginxFimeMgr *filefakes.FakeManager
4445
fakeNginxRuntimeMgr *runtimefakes.FakeManager
46+
fakeStatusUpdater *statusfakes.FakeUpdater
4547
cancel context.CancelFunc
4648
eventCh chan interface{}
4749
errorCh chan error
@@ -55,7 +57,8 @@ var _ = Describe("EventLoop", func() {
5557
fakeGenerator = &configfakes.FakeGenerator{}
5658
fakeNginxFimeMgr = &filefakes.FakeManager{}
5759
fakeNginxRuntimeMgr = &runtimefakes.FakeManager{}
58-
ctrl := events.NewEventLoop(fakeProcessor, fakeServiceStore, fakeGenerator, eventCh, zap.New(), fakeNginxFimeMgr, fakeNginxRuntimeMgr)
60+
fakeStatusUpdater = &statusfakes.FakeUpdater{}
61+
ctrl := events.NewEventLoop(fakeProcessor, fakeServiceStore, fakeGenerator, eventCh, zap.New(), fakeNginxFimeMgr, fakeNginxRuntimeMgr, fakeStatusUpdater)
5962

6063
var ctx context.Context
6164
ctx, cancel = context.WithCancel(context.Background())
@@ -82,7 +85,8 @@ var _ = Describe("EventLoop", func() {
8285
func(e *events.UpsertEvent) {
8386
fakeConf := newstate.Configuration{}
8487
changed := true
85-
fakeProcessor.ProcessReturns(changed, fakeConf, newstate.Statuses{})
88+
fakeStatuses := newstate.Statuses{}
89+
fakeProcessor.ProcessReturns(changed, fakeConf, fakeStatuses)
8690

8791
fakeCfg := []byte("fake")
8892
fakeGenerator.GenerateReturns(fakeCfg, config.Warnings{})
@@ -102,6 +106,10 @@ var _ = Describe("EventLoop", func() {
102106
Expect(cfg).Should(Equal(fakeCfg))
103107

104108
Eventually(fakeNginxRuntimeMgr.ReloadCallCount).Should(Equal(1))
109+
110+
Eventually(fakeStatusUpdater.UpdateCallCount).Should(Equal(1))
111+
_, statuses := fakeStatusUpdater.UpdateArgsForCall(0)
112+
Expect(statuses).Should(Equal(fakeStatuses))
105113
},
106114
Entry("HTTPRoute", &events.UpsertEvent{Resource: &v1alpha2.HTTPRoute{}}),
107115
Entry("Gateway", &events.UpsertEvent{Resource: &v1alpha2.Gateway{}}),

‎internal/manager/manager.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"github.com/nginxinc/nginx-kubernetes-gateway/internal/nginx/file"
2121
ngxruntime "github.com/nginxinc/nginx-kubernetes-gateway/internal/nginx/runtime"
2222
"github.com/nginxinc/nginx-kubernetes-gateway/internal/state"
23+
"github.com/nginxinc/nginx-kubernetes-gateway/internal/status"
2324
"github.com/nginxinc/nginx-kubernetes-gateway/pkg/sdk"
2425
)
2526

@@ -69,7 +70,8 @@ func Start(cfg config.Config) error {
6970
configGenerator := ngxcfg.NewGeneratorImpl(serviceStore)
7071
nginxFileMgr := file.NewManagerImpl()
7172
nginxRuntimeMgr := ngxruntime.NewManagerImpl()
72-
eventLoop := events.NewEventLoop(processor, serviceStore, configGenerator, eventCh, cfg.Logger, nginxFileMgr, nginxRuntimeMgr)
73+
statusUpdater := status.NewUpdater(cfg.GatewayCtlrName, cfg.GatewayNsName, mgr.GetClient(), cfg.Logger, status.NewRealClock())
74+
eventLoop := events.NewEventLoop(processor, serviceStore, configGenerator, eventCh, cfg.Logger, nginxFileMgr, nginxRuntimeMgr, statusUpdater)
7375

7476
err = mgr.Add(eventLoop)
7577
if err != nil {

‎internal/newstate/change_processor.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,17 @@ func (c *ChangeProcessorImpl) CaptureUpsertChange(obj client.Object) {
5656
if o.Namespace != c.gwNsName.Namespace || o.Name != c.gwNsName.Name {
5757
panic(fmt.Errorf("gateway resource must be %s/%s, got %s/%s", c.gwNsName.Namespace, c.gwNsName.Name, o.Namespace, o.Name))
5858
}
59+
// if the resource spec hasn't changed (its generation is the same), ignore the upsert
60+
if c.store.gw != nil && c.store.gw.Generation == o.Generation {
61+
c.changed = false
62+
}
5963
c.store.gw = o
6064
case *v1alpha2.HTTPRoute:
65+
// if the resource spec hasn't changed (its generation is the same), ignore the upsert
66+
prev, exist := c.store.httpRoutes[getNamespacedName(obj)]
67+
if exist && o.Generation == prev.Generation {
68+
c.changed = false
69+
}
6170
c.store.httpRoutes[getNamespacedName(obj)] = o
6271
default:
6372
panic(fmt.Errorf("ChangeProcessor doesn't support %T", obj))

‎internal/newstate/change_processor_test.go

Lines changed: 124 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,11 @@ import (
1414

1515
var _ = Describe("ChangeProcessor", func() {
1616
Describe("Normal cases of processing changes", func() {
17-
var hr1, hr2 *v1alpha2.HTTPRoute
18-
var gw *v1alpha2.Gateway
19-
var processor newstate.ChangeProcessor
17+
var (
18+
hr1, hr1Updated, hr2 *v1alpha2.HTTPRoute
19+
gw, gwUpdated *v1alpha2.Gateway
20+
processor newstate.ChangeProcessor
21+
)
2022

2123
BeforeEach(OncePerOrdered, func() {
2224
createRoute := func(name string, hostname string) *v1alpha2.HTTPRoute {
@@ -54,6 +56,10 @@ var _ = Describe("ChangeProcessor", func() {
5456
}
5557

5658
hr1 = createRoute("hr-1", "foo.example.com")
59+
60+
hr1Updated = hr1.DeepCopy()
61+
hr1Updated.Generation++
62+
5763
hr2 = createRoute("hr-2", "bar.example.com")
5864

5965
gw = &v1alpha2.Gateway{
@@ -73,6 +79,9 @@ var _ = Describe("ChangeProcessor", func() {
7379
},
7480
}
7581

82+
gwUpdated = gw.DeepCopy()
83+
gwUpdated.Generation++
84+
7685
processor = newstate.NewChangeProcessorImpl(types.NamespacedName{Namespace: "test", Name: "gateway"})
7786
})
7887

@@ -149,6 +158,116 @@ var _ = Describe("ChangeProcessor", func() {
149158
Expect(helpers.Diff(expectedStatuses, statuses)).To(BeEmpty())
150159
})
151160

161+
It("should return empty configuration and statuses after processing upserting the HTTPRoute without generation change", func() {
162+
hr1UpdatedSameGen := hr1.DeepCopy()
163+
// hr1UpdatedSameGen.Generation has not been changed
164+
processor.CaptureUpsertChange(hr1UpdatedSameGen)
165+
166+
changed, conf, statuses := processor.Process()
167+
Expect(changed).To(BeFalse())
168+
Expect(conf).To(BeZero())
169+
Expect(statuses).To(BeZero())
170+
})
171+
172+
It("should return updated configuration and statuses after upserting the HTTPRoute with generation change", func() {
173+
processor.CaptureUpsertChange(hr1Updated)
174+
175+
expectedConf := newstate.Configuration{
176+
HTTPServers: []newstate.HTTPServer{
177+
{
178+
Hostname: "foo.example.com",
179+
PathRules: []newstate.PathRule{
180+
{
181+
Path: "/",
182+
MatchRules: []newstate.MatchRule{
183+
{
184+
MatchIdx: 0,
185+
RuleIdx: 0,
186+
Source: hr1Updated,
187+
},
188+
},
189+
},
190+
},
191+
},
192+
},
193+
}
194+
expectedStatuses := newstate.Statuses{
195+
ListenerStatuses: map[string]newstate.ListenerStatus{
196+
"listener-80-1": {
197+
Valid: true,
198+
AttachedRoutes: 1,
199+
},
200+
},
201+
HTTPRouteStatuses: map[types.NamespacedName]newstate.HTTPRouteStatus{
202+
{Namespace: "test", Name: "hr-1"}: {
203+
ParentStatuses: map[string]newstate.ParentStatus{
204+
"listener-80-1": {Attached: true},
205+
},
206+
},
207+
},
208+
}
209+
210+
changed, conf, statuses := processor.Process()
211+
Expect(changed).To(BeTrue())
212+
Expect(helpers.Diff(expectedConf, conf)).To(BeEmpty())
213+
Expect(helpers.Diff(expectedStatuses, statuses)).To(BeEmpty())
214+
})
215+
216+
It("should return empty configuration and statuses after processing upserting the Gateway without generation change", func() {
217+
gwUpdatedSameGen := gw.DeepCopy()
218+
// gwUpdatedSameGen.Generation has not been changed
219+
processor.CaptureUpsertChange(gwUpdatedSameGen)
220+
221+
changed, conf, statuses := processor.Process()
222+
Expect(changed).To(BeFalse())
223+
Expect(conf).To(BeZero())
224+
Expect(statuses).To(BeZero())
225+
})
226+
227+
It("should return updated configuration and statuses after upserting the Gateway with generation change", func() {
228+
processor.CaptureUpsertChange(gwUpdated)
229+
230+
expectedConf := newstate.Configuration{
231+
HTTPServers: []newstate.HTTPServer{
232+
{
233+
Hostname: "foo.example.com",
234+
PathRules: []newstate.PathRule{
235+
{
236+
Path: "/",
237+
MatchRules: []newstate.MatchRule{
238+
{
239+
MatchIdx: 0,
240+
RuleIdx: 0,
241+
Source: hr1Updated,
242+
},
243+
},
244+
},
245+
},
246+
},
247+
},
248+
}
249+
expectedStatuses := newstate.Statuses{
250+
ListenerStatuses: map[string]newstate.ListenerStatus{
251+
"listener-80-1": {
252+
Valid: true,
253+
AttachedRoutes: 1,
254+
},
255+
},
256+
HTTPRouteStatuses: map[types.NamespacedName]newstate.HTTPRouteStatus{
257+
{Namespace: "test", Name: "hr-1"}: {
258+
ParentStatuses: map[string]newstate.ParentStatus{
259+
"listener-80-1": {Attached: true},
260+
},
261+
},
262+
},
263+
}
264+
265+
changed, conf, statuses := processor.Process()
266+
Expect(changed).To(BeTrue())
267+
Expect(helpers.Diff(expectedConf, conf)).To(BeEmpty())
268+
Expect(helpers.Diff(expectedStatuses, statuses)).To(BeEmpty())
269+
})
270+
152271
It("should return empty configuration and statuses after processing without capturing any changes", func() {
153272
changed, conf, statuses := processor.Process()
154273

@@ -186,7 +305,7 @@ var _ = Describe("ChangeProcessor", func() {
186305
{
187306
MatchIdx: 0,
188307
RuleIdx: 0,
189-
Source: hr1,
308+
Source: hr1Updated,
190309
},
191310
},
192311
},
@@ -235,7 +354,7 @@ var _ = Describe("ChangeProcessor", func() {
235354
{
236355
MatchIdx: 0,
237356
RuleIdx: 0,
238-
Source: hr1,
357+
Source: hr1Updated,
239358
},
240359
},
241360
},

‎internal/status/clock.go

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
package status
2+
3+
import "time"
4+
5+
// Clock returns the current local time.
6+
type Clock interface {
7+
Now() time.Time
8+
}
9+
10+
// Real clock returns the current local time.
11+
type RealClock struct {
12+
}
13+
14+
// NewRealClock creates a new RealClock.
15+
func NewRealClock() *RealClock {
16+
return &RealClock{}
17+
}
18+
19+
// Now returns the current local time.
20+
func (c *RealClock) Now() time.Time {
21+
return time.Now()
22+
}
23+
24+
// FakeClock allows you to control the returned time.
25+
type FakeClock struct {
26+
time time.Time
27+
}
28+
29+
// NewFakeClock creates a FakeClock. The clock will always return the specified time.
30+
func NewFakeClock(time time.Time) *FakeClock {
31+
return &FakeClock{time: time}
32+
}
33+
34+
// Now is a fake implementation of Now().
35+
func (c FakeClock) Now() time.Time {
36+
return c.time
37+
}

‎internal/status/clock_test.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
package status
2+
3+
import (
4+
"testing"
5+
"time"
6+
)
7+
8+
func TestFakeClock(t *testing.T) {
9+
time := time.Now()
10+
clock := NewFakeClock(time)
11+
12+
result := clock.Now()
13+
if result != time {
14+
t.Errorf("Now() returned %v but expected %v", result, time)
15+
}
16+
}

0 commit comments

Comments
 (0)