Skip to content

Update statuses of HTTPRoutes #60

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Mar 11, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cmd/gateway/setup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
},
Expand Down
6 changes: 6 additions & 0 deletions deploy/manifests/nginx-gateway.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
37 changes: 16 additions & 21 deletions internal/events/loop.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
}
}

Expand All @@ -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
}
Expand All @@ -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
Expand All @@ -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
}

Expand All @@ -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)
Expand All @@ -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)
}
38 changes: 37 additions & 1 deletion internal/events/loop_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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

Expand All @@ -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{
Expand All @@ -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{
Expand All @@ -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))
})
})

Expand Down
15 changes: 15 additions & 0 deletions internal/helpers/helpers.go
Original file line number Diff line number Diff line change
@@ -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
}
17 changes: 13 additions & 4 deletions internal/manager/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"

Expand All @@ -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() {
Expand All @@ -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)
}
Expand All @@ -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()
Expand Down
11 changes: 7 additions & 4 deletions internal/state/configuration.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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{
{
Expand Down
Loading