Update statuses of HTTPRoutes (#60)

This commit adds a new component - status.Updater - which updates
statuses of HTTPRoutes. In the future, it will update statuses of the
other supported resources.

The added component has a few limitations (described in the comments)
that are required to be addressed before the Gateway is considered to be
ready for production use.

Author: pleshakov

cmd/gateway/setup_test.go

@@ -19,7 +19,7 @@ func MockValidator(name string, called *int, succeed bool) ValidatorContext {
 			*called++
 
 			if !succeed {
-				return errors.New("Mock error")
+				return errors.New("mock error")
 			}
 			return nil
 		},

deploy/manifests/nginx-gateway.yaml

@@ -30,6 +30,12 @@ rules:
   verbs:
   - list
   - watch
+- apiGroups:
+  - gateway.networking.k8s.io
+  resources:
+  - httproutes/status
+  verbs:
+  - update
 ---
 kind: ClusterRoleBinding
 apiVersion: rbac.authorization.k8s.io/v1

internal/events/loop.go

@@ -6,22 +6,25 @@ import (
 
 	"github.com/go-logr/logr"
 	"github.com/nginxinc/nginx-gateway-kubernetes/internal/state"
+	"github.com/nginxinc/nginx-gateway-kubernetes/internal/status"
 	"sigs.k8s.io/gateway-api/apis/v1alpha2"
 )
 
 // EventLoop is the main event loop of the Gateway.
 type EventLoop struct {
-	conf    state.Configuration
-	eventCh <-chan interface{}
-	logger  logr.Logger
+	conf          state.Configuration
+	eventCh       <-chan interface{}
+	logger        logr.Logger
+	statusUpdater status.Updater
 }
 
 // NewEventLoop creates a new EventLoop.
-func NewEventLoop(conf state.Configuration, eventCh <-chan interface{}, logger logr.Logger) *EventLoop {
+func NewEventLoop(conf state.Configuration, eventCh <-chan interface{}, statusUpdater status.Updater, logger logr.Logger) *EventLoop {
 	return &EventLoop{
-		conf:    conf,
-		eventCh: eventCh,
-		logger:  logger.WithName("eventLoop"),
+		conf:          conf,
+		eventCh:       eventCh,
+		statusUpdater: statusUpdater,
+		logger:        logger.WithName("eventLoop"),
 	}
 }
 
@@ -35,7 +38,7 @@ func (el *EventLoop) Start(ctx context.Context) error {
 		case <-ctx.Done():
 			return nil
 		case e := <-el.eventCh:
-			err := el.handleEvent(e)
+			err := el.handleEvent(ctx, e)
 			if err != nil {
 				return err
 			}
@@ -44,7 +47,7 @@ func (el *EventLoop) Start(ctx context.Context) error {
 }
 
 // TO-DO: think about how to avoid using an interface{} here
-func (el *EventLoop) handleEvent(event interface{}) error {
+func (el *EventLoop) handleEvent(ctx context.Context, event interface{}) error {
 	var changes []state.Change
 	var updates []state.StatusUpdate
 	var err error
@@ -55,15 +58,15 @@ func (el *EventLoop) handleEvent(event interface{}) error {
 	case *DeleteEvent:
 		changes, updates, err = el.propagateDelete(e)
 	default:
+		// TO-DO: panic
 		return fmt.Errorf("unknown event type %T", e)
 	}
 
 	if err != nil {
 		return err
 	}
 
-	el.processChangesAndStatusUpdates(changes, updates)
-
+	el.processChangesAndStatusUpdates(ctx, changes, updates)
 	return nil
 }
 
@@ -87,7 +90,7 @@ func (el *EventLoop) propagateDelete(e *DeleteEvent) ([]state.Change, []state.St
 	return nil, nil, fmt.Errorf("unknown resource type %T", e.Type)
 }
 
-func (el *EventLoop) processChangesAndStatusUpdates(changes []state.Change, updates []state.StatusUpdate) {
+func (el *EventLoop) processChangesAndStatusUpdates(ctx context.Context, changes []state.Change, updates []state.StatusUpdate) {
 	for _, c := range changes {
 		el.logger.Info("Processing a change",
 			"host", c.Host.Value)
@@ -96,13 +99,5 @@ func (el *EventLoop) processChangesAndStatusUpdates(changes []state.Change, upda
 		fmt.Printf("%+v\n", c)
 	}
 
-	for _, u := range updates {
-		// TO-DO: in the next iteration, the update will include the namespace/name of the resource instead of
-		// runtime.Object, so it will be easy to get the resource namespace/name and include it in the log output
-		el.logger.Info("Processing a status update",
-			"gvk", u.Object.GetObjectKind().GroupVersionKind().String())
-
-		// TO-DO: This code is temporary. We will remove it once we have a component that updates statuses.
-		fmt.Printf("%+v\n", u)
-	}
+	el.statusUpdater.ProcessStatusUpdates(ctx, updates)
 }

internal/events/loop_test.go

@@ -4,7 +4,9 @@ import (
 	"context"
 
 	"github.com/nginxinc/nginx-gateway-kubernetes/internal/events"
+	"github.com/nginxinc/nginx-gateway-kubernetes/internal/state"
 	"github.com/nginxinc/nginx-gateway-kubernetes/internal/state/statefakes"
+	"github.com/nginxinc/nginx-gateway-kubernetes/internal/status/statusfakes"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -30,14 +32,16 @@ func (r *unsupportedResource) DeepCopyObject() runtime.Object {
 var _ = Describe("EventLoop", func() {
 	var ctrl *events.EventLoop
 	var fakeConf *statefakes.FakeConfiguration
+	var fakeUpdater *statusfakes.FakeUpdater
 	var cancel context.CancelFunc
 	var eventCh chan interface{}
 	var errorCh chan error
 
 	BeforeEach(func() {
 		fakeConf = &statefakes.FakeConfiguration{}
 		eventCh = make(chan interface{})
-		ctrl = events.NewEventLoop(fakeConf, eventCh, zap.New())
+		fakeUpdater = &statusfakes.FakeUpdater{}
+		ctrl = events.NewEventLoop(fakeConf, eventCh, fakeUpdater, zap.New())
 
 		var ctx context.Context
 
@@ -59,6 +63,16 @@ var _ = Describe("EventLoop", func() {
 		})
 
 		It("should process upsert event", func() {
+			fakeStatusUpdates := []state.StatusUpdate{
+				{
+					NamespacedName: types.NamespacedName{},
+					Status:         nil,
+				},
+			}
+			// for now, we pass nil, because we don't need to test how EventLoop processes changes yet. We will start
+			// testing once we have NGINX Configuration Manager component.
+			fakeConf.UpsertHTTPRouteReturns(nil, fakeStatusUpdates)
+
 			hr := &v1alpha2.HTTPRoute{}
 
 			eventCh <- &events.UpsertEvent{
@@ -69,9 +83,25 @@ var _ = Describe("EventLoop", func() {
 			Eventually(func() *v1alpha2.HTTPRoute {
 				return fakeConf.UpsertHTTPRouteArgsForCall(0)
 			}).Should(Equal(hr))
+
+			Eventually(fakeUpdater.ProcessStatusUpdatesCallCount()).Should(Equal(1))
+			Eventually(func() []state.StatusUpdate {
+				_, updates := fakeUpdater.ProcessStatusUpdatesArgsForCall(0)
+				return updates
+			}).Should(Equal(fakeStatusUpdates))
 		})
 
 		It("should process delete event", func() {
+			fakeStatusUpdates := []state.StatusUpdate{
+				{
+					NamespacedName: types.NamespacedName{},
+					Status:         nil,
+				},
+			}
+			// for now, we pass nil, because we don't need to test how EventLoop processes changes yet. We will start
+			// testing once we have NGINX Configuration Manager component.
+			fakeConf.DeleteHTTPRouteReturns(nil, fakeStatusUpdates)
+
 			nsname := types.NamespacedName{Namespace: "test", Name: "route"}
 
 			eventCh <- &events.DeleteEvent{
@@ -83,6 +113,12 @@ var _ = Describe("EventLoop", func() {
 			Eventually(func() types.NamespacedName {
 				return fakeConf.DeleteHTTPRouteArgsForCall(0)
 			}).Should(Equal(nsname))
+
+			Eventually(fakeUpdater.ProcessStatusUpdatesCallCount()).Should(Equal(1))
+			Eventually(func() []state.StatusUpdate {
+				_, updates := fakeUpdater.ProcessStatusUpdatesArgsForCall(0)
+				return updates
+			}).Should(Equal(fakeStatusUpdates))
 		})
 	})
 

internal/helpers/helpers.go

@@ -0,0 +1,15 @@
+package helpers
+
+import "github.com/google/go-cmp/cmp"
+
+// Diff prints the diff between two structs.
+// It is useful in testing to compare two structs when they are large. In such a case, without Diff it will be difficult
+// to pinpoint the difference between the two structs.
+func Diff(x, y interface{}) string {
+	r := cmp.Diff(x, y)
+
+	if r != "" {
+		return "(-want +got)\n" + r
+	}
+	return r
+}

internal/manager/manager.go

@@ -2,6 +2,7 @@ package manager
 
 import (
 	"fmt"
+	"time"
 
 	"github.com/nginxinc/nginx-gateway-kubernetes/internal/config"
 	"github.com/nginxinc/nginx-gateway-kubernetes/internal/events"
@@ -10,6 +11,7 @@ import (
 	gcfg "github.com/nginxinc/nginx-gateway-kubernetes/internal/implementations/gatewayconfig"
 	hr "github.com/nginxinc/nginx-gateway-kubernetes/internal/implementations/httproute"
 	"github.com/nginxinc/nginx-gateway-kubernetes/internal/state"
+	"github.com/nginxinc/nginx-gateway-kubernetes/internal/status"
 	nginxgwv1alpha1 "github.com/nginxinc/nginx-gateway-kubernetes/pkg/apis/gateway/v1alpha1"
 	"github.com/nginxinc/nginx-gateway-kubernetes/pkg/sdk"
 
@@ -19,6 +21,9 @@ import (
 	gatewayv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2"
 )
 
+// clusterTimeout is a timeout for connections to the Kubernetes API
+const clusterTimeout = 10 * time.Second
+
 var scheme = runtime.NewScheme()
 
 func init() {
@@ -35,7 +40,10 @@ func Start(cfg config.Config) error {
 
 	eventCh := make(chan interface{})
 
-	mgr, err := manager.New(ctlr.GetConfigOrDie(), options)
+	clusterCfg := ctlr.GetConfigOrDie()
+	clusterCfg.Timeout = clusterTimeout
+
+	mgr, err := manager.New(clusterCfg, options)
 	if err != nil {
 		return fmt.Errorf("cannot build runtime manager: %w", err)
 	}
@@ -58,11 +66,12 @@ func Start(cfg config.Config) error {
 	}
 
 	conf := state.NewConfiguration(cfg.GatewayCtlrName, state.NewRealClock())
-	mainCtrl := events.NewEventLoop(conf, eventCh, cfg.Logger)
+	reporter := status.NewUpdater(mgr.GetClient(), cfg.Logger)
+	eventLoop := events.NewEventLoop(conf, eventCh, reporter, cfg.Logger)
 
-	err = mgr.Add(mainCtrl)
+	err = mgr.Add(eventLoop)
 	if err != nil {
-		return fmt.Errorf("cannot register main controller")
+		return fmt.Errorf("cannot register event loop: %w", err)
 	}
 
 	ctx := ctlr.SetupSignalHandler()

internal/state/configuration.go

@@ -6,7 +6,6 @@ import (
 	"strings"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
 	"sigs.k8s.io/gateway-api/apis/v1alpha2"
 )
@@ -148,8 +147,8 @@ type Change struct {
 
 // StatusUpdate represents an update to the status of a resource.
 type StatusUpdate struct {
-	// Object is the resource.
-	Object runtime.Object
+	// NamespacedName is the NamespacedName of the resource.
+	NamespacedName types.NamespacedName
 	// Status is the status field of the resource
 	// The Status include only the new conditions. This means that the status reporter component will need to merge
 	// the new conditions with the existing conditions of the resource.
@@ -235,12 +234,16 @@ func (c *configurationImpl) updateListeners() ([]Change, []StatusUpdate) {
 	// TO-DO: optimize it so that we only update the status of the affected (changed) httpRoutes
 	// getSortedKeys is used to ensure predictable order for unit tests
 	for _, key := range getSortedKeys(listener.httpRoutes) {
+		route := listener.httpRoutes[key]
 		update := StatusUpdate{
-			Object: listener.httpRoutes[key],
+			NamespacedName: types.NamespacedName{Namespace: route.Namespace, Name: route.Name},
 			Status: &v1alpha2.HTTPRouteStatus{
 				RouteStatus: v1alpha2.RouteStatus{
 					Parents: []v1alpha2.RouteParentStatus{
 						{
+							ParentRef: v1alpha2.ParentRef{
+								Name: "fake", // TO-DO: report the parent ref properly
+							},
 							ControllerName: v1alpha2.GatewayController(c.gatewayCtlrName),
 							Conditions: []metav1.Condition{
 								{