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.

Author: pleshakov

deploy/manifests/nginx-gateway.yaml

@@ -36,6 +36,7 @@ rules:
   - gateway.networking.k8s.io
   resources:
   - httproutes/status
+  - gateways/status
   verbs:
   - update
 ---

internal/events/loop.go

@@ -13,6 +13,7 @@ import (
 	"github.com/nginxinc/nginx-kubernetes-gateway/internal/nginx/file"
 	"github.com/nginxinc/nginx-kubernetes-gateway/internal/nginx/runtime"
 	"github.com/nginxinc/nginx-kubernetes-gateway/internal/state"
+	"github.com/nginxinc/nginx-kubernetes-gateway/internal/status"
 )
 
 // EventLoop is the main event loop of the Gateway.
@@ -24,6 +25,7 @@ type EventLoop struct {
 	logger          logr.Logger
 	nginxFileMgr    file.Manager
 	nginxRuntimeMgr runtime.Manager
+	statusUpdater   status.Updater
 }
 
 // NewEventLoop creates a new EventLoop.
@@ -35,6 +37,7 @@ func NewEventLoop(
 	logger logr.Logger,
 	nginxFileMgr file.Manager,
 	nginxRuntimeMgr runtime.Manager,
+	statusUpdater status.Updater,
 ) *EventLoop {
 	return &EventLoop{
 		processor:       processor,
@@ -44,6 +47,7 @@ func NewEventLoop(
 		logger:          logger.WithName("eventLoop"),
 		nginxFileMgr:    nginxFileMgr,
 		nginxRuntimeMgr: nginxRuntimeMgr,
+		statusUpdater:   statusUpdater,
 	}
 }
 
@@ -85,13 +89,7 @@ func (el *EventLoop) handleEvent(ctx context.Context, event interface{}) {
 		el.logger.Error(err, "Failed to update NGINX configuration")
 	}
 
-	// FIXME(pleshakov) Update resource statuses instead of printing to stdout
-	for name, s := range statuses.ListenerStatuses {
-		fmt.Printf("Listener %q, Statuses: %v\n", name, s)
-	}
-	for nsname, s := range statuses.HTTPRouteStatuses {
-		fmt.Printf("HTTPRoute %q, Statuses: %v\n", nsname, s)
-	}
+	el.statusUpdater.Update(ctx, statuses)
 }
 
 func (el *EventLoop) updateNginx(ctx context.Context, conf newstate.Configuration) error {

internal/events/loop_test.go

@@ -21,6 +21,7 @@ import (
 	"github.com/nginxinc/nginx-kubernetes-gateway/internal/nginx/file/filefakes"
 	"github.com/nginxinc/nginx-kubernetes-gateway/internal/nginx/runtime/runtimefakes"
 	"github.com/nginxinc/nginx-kubernetes-gateway/internal/state/statefakes"
+	"github.com/nginxinc/nginx-kubernetes-gateway/internal/status/statusfakes"
 )
 
 type unsupportedResource struct {
@@ -42,6 +43,7 @@ var _ = Describe("EventLoop", func() {
 		fakeGenerator       *configfakes.FakeGenerator
 		fakeNginxFimeMgr    *filefakes.FakeManager
 		fakeNginxRuntimeMgr *runtimefakes.FakeManager
+		fakeStatusUpdater   *statusfakes.FakeUpdater
 		cancel              context.CancelFunc
 		eventCh             chan interface{}
 		errorCh             chan error
@@ -55,7 +57,8 @@ var _ = Describe("EventLoop", func() {
 		fakeGenerator = &configfakes.FakeGenerator{}
 		fakeNginxFimeMgr = &filefakes.FakeManager{}
 		fakeNginxRuntimeMgr = &runtimefakes.FakeManager{}
-		ctrl := events.NewEventLoop(fakeProcessor, fakeServiceStore, fakeGenerator, eventCh, zap.New(), fakeNginxFimeMgr, fakeNginxRuntimeMgr)
+		fakeStatusUpdater = &statusfakes.FakeUpdater{}
+		ctrl := events.NewEventLoop(fakeProcessor, fakeServiceStore, fakeGenerator, eventCh, zap.New(), fakeNginxFimeMgr, fakeNginxRuntimeMgr, fakeStatusUpdater)
 
 		var ctx context.Context
 		ctx, cancel = context.WithCancel(context.Background())
@@ -82,7 +85,8 @@ var _ = Describe("EventLoop", func() {
 			func(e *events.UpsertEvent) {
 				fakeConf := newstate.Configuration{}
 				changed := true
-				fakeProcessor.ProcessReturns(changed, fakeConf, newstate.Statuses{})
+				fakeStatuses := newstate.Statuses{}
+				fakeProcessor.ProcessReturns(changed, fakeConf, fakeStatuses)
 
 				fakeCfg := []byte("fake")
 				fakeGenerator.GenerateReturns(fakeCfg, config.Warnings{})
@@ -102,6 +106,10 @@ var _ = Describe("EventLoop", func() {
 				Expect(cfg).Should(Equal(fakeCfg))
 
 				Eventually(fakeNginxRuntimeMgr.ReloadCallCount).Should(Equal(1))
+
+				Eventually(fakeStatusUpdater.UpdateCallCount).Should(Equal(1))
+				_, statuses := fakeStatusUpdater.UpdateArgsForCall(0)
+				Expect(statuses).Should(Equal(fakeStatuses))
 			},
 			Entry("HTTPRoute", &events.UpsertEvent{Resource: &v1alpha2.HTTPRoute{}}),
 			Entry("Gateway", &events.UpsertEvent{Resource: &v1alpha2.Gateway{}}),

internal/manager/manager.go

@@ -20,6 +20,7 @@ import (
 	"github.com/nginxinc/nginx-kubernetes-gateway/internal/nginx/file"
 	ngxruntime "github.com/nginxinc/nginx-kubernetes-gateway/internal/nginx/runtime"
 	"github.com/nginxinc/nginx-kubernetes-gateway/internal/state"
+	"github.com/nginxinc/nginx-kubernetes-gateway/internal/status"
 	"github.com/nginxinc/nginx-kubernetes-gateway/pkg/sdk"
 )
 
@@ -69,7 +70,8 @@ func Start(cfg config.Config) error {
 	configGenerator := ngxcfg.NewGeneratorImpl(serviceStore)
 	nginxFileMgr := file.NewManagerImpl()
 	nginxRuntimeMgr := ngxruntime.NewManagerImpl()
-	eventLoop := events.NewEventLoop(processor, serviceStore, configGenerator, eventCh, cfg.Logger, nginxFileMgr, nginxRuntimeMgr)
+	statusUpdater := status.NewUpdater(cfg.GatewayCtlrName, cfg.GatewayNsName, mgr.GetClient(), cfg.Logger, status.NewRealClock())
+	eventLoop := events.NewEventLoop(processor, serviceStore, configGenerator, eventCh, cfg.Logger, nginxFileMgr, nginxRuntimeMgr, statusUpdater)
 
 	err = mgr.Add(eventLoop)
 	if err != nil {

internal/newstate/change_processor.go

@@ -56,8 +56,17 @@ func (c *ChangeProcessorImpl) CaptureUpsertChange(obj client.Object) {
 		if o.Namespace != c.gwNsName.Namespace || o.Name != c.gwNsName.Name {
 			panic(fmt.Errorf("gateway resource must be %s/%s, got %s/%s", c.gwNsName.Namespace, c.gwNsName.Name, o.Namespace, o.Name))
 		}
+		// if the resource spec hasn't changed (its generation is the same), ignore the upsert
+		if c.store.gw != nil && c.store.gw.Generation == o.Generation {
+			c.changed = false
+		}
 		c.store.gw = o
 	case *v1alpha2.HTTPRoute:
+		// if the resource spec hasn't changed (its generation is the same), ignore the upsert
+		prev, exist := c.store.httpRoutes[getNamespacedName(obj)]
+		if exist && o.Generation == prev.Generation {
+			c.changed = false
+		}
 		c.store.httpRoutes[getNamespacedName(obj)] = o
 	default:
 		panic(fmt.Errorf("ChangeProcessor doesn't support %T", obj))

internal/newstate/change_processor_test.go

@@ -14,9 +14,11 @@ import (
 
 var _ = Describe("ChangeProcessor", func() {
 	Describe("Normal cases of processing changes", func() {
-		var hr1, hr2 *v1alpha2.HTTPRoute
-		var gw *v1alpha2.Gateway
-		var processor newstate.ChangeProcessor
+		var (
+			hr1, hr1Updated, hr2 *v1alpha2.HTTPRoute
+			gw, gwUpdated        *v1alpha2.Gateway
+			processor            newstate.ChangeProcessor
+		)
 
 		BeforeEach(OncePerOrdered, func() {
 			createRoute := func(name string, hostname string) *v1alpha2.HTTPRoute {
@@ -54,6 +56,10 @@ var _ = Describe("ChangeProcessor", func() {
 			}
 
 			hr1 = createRoute("hr-1", "foo.example.com")
+
+			hr1Updated = hr1.DeepCopy()
+			hr1Updated.Generation++
+
 			hr2 = createRoute("hr-2", "bar.example.com")
 
 			gw = &v1alpha2.Gateway{
@@ -73,6 +79,9 @@ var _ = Describe("ChangeProcessor", func() {
 				},
 			}
 
+			gwUpdated = gw.DeepCopy()
+			gwUpdated.Generation++
+
 			processor = newstate.NewChangeProcessorImpl(types.NamespacedName{Namespace: "test", Name: "gateway"})
 		})
 
@@ -149,6 +158,116 @@ var _ = Describe("ChangeProcessor", func() {
 				Expect(helpers.Diff(expectedStatuses, statuses)).To(BeEmpty())
 			})
 
+			It("should return empty configuration and statuses after processing upserting the HTTPRoute without generation change", func() {
+				hr1UpdatedSameGen := hr1.DeepCopy()
+				// hr1UpdatedSameGen.Generation has not been changed
+				processor.CaptureUpsertChange(hr1UpdatedSameGen)
+
+				changed, conf, statuses := processor.Process()
+				Expect(changed).To(BeFalse())
+				Expect(conf).To(BeZero())
+				Expect(statuses).To(BeZero())
+			})
+
+			It("should return updated configuration and statuses after upserting the HTTPRoute with generation change", func() {
+				processor.CaptureUpsertChange(hr1Updated)
+
+				expectedConf := newstate.Configuration{
+					HTTPServers: []newstate.HTTPServer{
+						{
+							Hostname: "foo.example.com",
+							PathRules: []newstate.PathRule{
+								{
+									Path: "/",
+									MatchRules: []newstate.MatchRule{
+										{
+											MatchIdx: 0,
+											RuleIdx:  0,
+											Source:   hr1Updated,
+										},
+									},
+								},
+							},
+						},
+					},
+				}
+				expectedStatuses := newstate.Statuses{
+					ListenerStatuses: map[string]newstate.ListenerStatus{
+						"listener-80-1": {
+							Valid:          true,
+							AttachedRoutes: 1,
+						},
+					},
+					HTTPRouteStatuses: map[types.NamespacedName]newstate.HTTPRouteStatus{
+						{Namespace: "test", Name: "hr-1"}: {
+							ParentStatuses: map[string]newstate.ParentStatus{
+								"listener-80-1": {Attached: true},
+							},
+						},
+					},
+				}
+
+				changed, conf, statuses := processor.Process()
+				Expect(changed).To(BeTrue())
+				Expect(helpers.Diff(expectedConf, conf)).To(BeEmpty())
+				Expect(helpers.Diff(expectedStatuses, statuses)).To(BeEmpty())
+			})
+
+			It("should return empty configuration and statuses after processing upserting the Gateway without generation change", func() {
+				gwUpdatedSameGen := gw.DeepCopy()
+				// gwUpdatedSameGen.Generation has not been changed
+				processor.CaptureUpsertChange(gwUpdatedSameGen)
+
+				changed, conf, statuses := processor.Process()
+				Expect(changed).To(BeFalse())
+				Expect(conf).To(BeZero())
+				Expect(statuses).To(BeZero())
+			})
+
+			It("should return updated configuration and statuses after upserting the Gateway with generation change", func() {
+				processor.CaptureUpsertChange(gwUpdated)
+
+				expectedConf := newstate.Configuration{
+					HTTPServers: []newstate.HTTPServer{
+						{
+							Hostname: "foo.example.com",
+							PathRules: []newstate.PathRule{
+								{
+									Path: "/",
+									MatchRules: []newstate.MatchRule{
+										{
+											MatchIdx: 0,
+											RuleIdx:  0,
+											Source:   hr1Updated,
+										},
+									},
+								},
+							},
+						},
+					},
+				}
+				expectedStatuses := newstate.Statuses{
+					ListenerStatuses: map[string]newstate.ListenerStatus{
+						"listener-80-1": {
+							Valid:          true,
+							AttachedRoutes: 1,
+						},
+					},
+					HTTPRouteStatuses: map[types.NamespacedName]newstate.HTTPRouteStatus{
+						{Namespace: "test", Name: "hr-1"}: {
+							ParentStatuses: map[string]newstate.ParentStatus{
+								"listener-80-1": {Attached: true},
+							},
+						},
+					},
+				}
+
+				changed, conf, statuses := processor.Process()
+				Expect(changed).To(BeTrue())
+				Expect(helpers.Diff(expectedConf, conf)).To(BeEmpty())
+				Expect(helpers.Diff(expectedStatuses, statuses)).To(BeEmpty())
+			})
+
 			It("should return empty configuration and statuses after processing without capturing any changes", func() {
 				changed, conf, statuses := processor.Process()
 
@@ -186,7 +305,7 @@ var _ = Describe("ChangeProcessor", func() {
 										{
 											MatchIdx: 0,
 											RuleIdx:  0,
-											Source:   hr1,
+											Source:   hr1Updated,
 										},
 									},
 								},
@@ -235,7 +354,7 @@ var _ = Describe("ChangeProcessor", func() {
 										{
 											MatchIdx: 0,
 											RuleIdx:  0,
-											Source:   hr1,
+											Source:   hr1Updated,
 										},
 									},
 								},

internal/status/clock.go

@@ -0,0 +1,37 @@
+package status
+
+import "time"
+
+// Clock returns the current local time.
+type Clock interface {
+	Now() time.Time
+}
+
+// Real clock returns the current local time.
+type RealClock struct {
+}
+
+// NewRealClock creates a new RealClock.
+func NewRealClock() *RealClock {
+	return &RealClock{}
+}
+
+// Now returns the current local time.
+func (c *RealClock) Now() time.Time {
+	return time.Now()
+}
+
+// FakeClock allows you to control the returned time.
+type FakeClock struct {
+	time time.Time
+}
+
+// NewFakeClock creates a FakeClock. The clock will always return the specified time.
+func NewFakeClock(time time.Time) *FakeClock {
+	return &FakeClock{time: time}
+}
+
+// Now is a fake implementation of Now().
+func (c FakeClock) Now() time.Time {
+	return c.time
+}

internal/status/clock_test.go

@@ -0,0 +1,16 @@
+package status
+
+import (
+	"testing"
+	"time"
+)
+
+func TestFakeClock(t *testing.T) {
+	time := time.Now()
+	clock := NewFakeClock(time)
+
+	result := clock.Now()
+	if result != time {
+		t.Errorf("Now() returned %v but expected %v", result, time)
+	}
+}