Skip to content
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.

Sign up for GitHub

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

Set Service address in Gateway Status #1141

Merged
merged 4 commits into from
Oct 17, 2023
+748 −80
Merged

Set Service address in Gateway Status #1141

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
dc2e775
Set Service address in Gateway Status
sjberman Oct 12, 2023
73049cb
First round of reviews
sjberman Oct 16, 2023
a9f6192
Fix predicate and locking
sjberman Oct 17, 2023
3ebbdd5
Remove unnecessary predicate funcs
sjberman Oct 17, 2023
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
1 change: 0 additions & 1 deletion deploy/helm-chart/templates/rbac.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ rules:
- namespaces
- services
- secrets
- nodes
verbs:
- list
- watch
Expand Down
1 change: 0 additions & 1 deletion deploy/manifests/nginx-gateway.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ rules:
- namespaces
- services
- secrets
- nodes
verbs:
- list
- watch
Expand Down
34 changes: 34 additions & 0 deletions internal/framework/controller/predicate/service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ package predicate

import (
apiv1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/intstr"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/event"
"sigs.k8s.io/controller-runtime/pkg/predicate"
)
Expand Down Expand Up @@ -67,6 +69,16 @@ func (ServicePortsChangedPredicate) Update(e event.UpdateEvent) bool {
// GatewayServicePredicate implements predicate functions for this Pod's Service.
type GatewayServicePredicate struct {
predicate.Funcs
NSName types.NamespacedName
}

// Create implements the default CreateEvent filter for the Gateway Service.
func (gsp GatewayServicePredicate) Create(e event.CreateEvent) bool {
sjberman marked this conversation as resolved.
Show resolved Hide resolved
if e.Object == nil {
return false
}

return client.ObjectKeyFromObject(e.Object) == gsp.NSName
}

// Update implements the default UpdateEvent filter for the Gateway Service.
Expand All @@ -88,6 +100,10 @@ func (gsp GatewayServicePredicate) Update(e event.UpdateEvent) bool {
return false
}

if client.ObjectKeyFromObject(newSvc) != gsp.NSName {
return false
}

if oldSvc.Spec.Type != newSvc.Spec.Type {
return true
}
Expand All @@ -109,3 +125,21 @@ func (gsp GatewayServicePredicate) Update(e event.UpdateEvent) bool {

return false
}

// Delete implements the default DeleteEvent filter for the Gateway Service.
func (gsp GatewayServicePredicate) Delete(e event.DeleteEvent) bool {
if e.Object == nil {
return false
}

return client.ObjectKeyFromObject(e.Object) == gsp.NSName
}

// Generic implements the default GenericEvent filter for the Gateway Service.
func (gsp GatewayServicePredicate) Generic(e event.GenericEvent) bool {
if e.Object == nil {
return false
}

return client.ObjectKeyFromObject(e.Object) == gsp.NSName
}
53 changes: 48 additions & 5 deletions internal/framework/controller/predicate/service_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import (

. "github.com/onsi/gomega"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/intstr"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/event"
Expand Down Expand Up @@ -240,11 +242,21 @@ func TestServicePortsChangedPredicate_Update(t *testing.T) {
func TestServicePortsChangedPredicate(t *testing.T) {
g := NewWithT(t)

p := ServicePortsChangedPredicate{}
p := GatewayServicePredicate{NSName: types.NamespacedName{Namespace: "nginx-gateway", Name: "nginx"}}

g.Expect(p.Delete(event.DeleteEvent{Object: &v1.Service{}})).To(BeTrue())
g.Expect(p.Create(event.CreateEvent{Object: &v1.Service{}})).To(BeTrue())
g.Expect(p.Generic(event.GenericEvent{Object: &v1.Service{}})).To(BeTrue())
g.Expect(p.Delete(event.DeleteEvent{Object: &v1.Service{}})).To(BeFalse())
g.Expect(p.Create(event.CreateEvent{Object: &v1.Service{}})).To(BeFalse())
g.Expect(p.Generic(event.GenericEvent{Object: &v1.Service{}})).To(BeFalse())

correctSvc := &v1.Service{
ObjectMeta: metav1.ObjectMeta{
Namespace: "nginx-gateway",
Name: "nginx",
},
}
g.Expect(p.Delete(event.DeleteEvent{Object: correctSvc})).To(BeTrue())
g.Expect(p.Create(event.CreateEvent{Object: correctSvc})).To(BeTrue())
g.Expect(p.Generic(event.GenericEvent{Object: correctSvc})).To(BeTrue())
}

func TestGatewayServicePredicate_Update(t *testing.T) {
Expand Down Expand Up @@ -278,6 +290,17 @@ func TestGatewayServicePredicate_Update(t *testing.T) {
objectNew: &v1.Namespace{},
expUpdate: false,
},
{
msg: "Service not watched",
objectOld: &v1.Service{},
objectNew: &v1.Service{
ObjectMeta: metav1.ObjectMeta{
Namespace: "nginx-gateway",
Name: "not-watched",
},
},
expUpdate: false,
},
{
msg: "something irrelevant changed",
objectOld: &v1.Service{
Expand All @@ -286,6 +309,10 @@ func TestGatewayServicePredicate_Update(t *testing.T) {
},
},
objectNew: &v1.Service{
ObjectMeta: metav1.ObjectMeta{
Namespace: "nginx-gateway",
Name: "nginx",
},
Spec: v1.ServiceSpec{
ClusterIP: "5.6.7.8",
},
Expand All @@ -300,6 +327,10 @@ func TestGatewayServicePredicate_Update(t *testing.T) {
},
},
objectNew: &v1.Service{
ObjectMeta: metav1.ObjectMeta{
Namespace: "nginx-gateway",
Name: "nginx",
},
Spec: v1.ServiceSpec{
Type: v1.ServiceTypeNodePort,
},
Expand All @@ -323,6 +354,10 @@ func TestGatewayServicePredicate_Update(t *testing.T) {
},
},
objectNew: &v1.Service{
ObjectMeta: metav1.ObjectMeta{
Namespace: "nginx-gateway",
Name: "nginx",
},
Spec: v1.ServiceSpec{
Type: v1.ServiceTypeNodePort,
}, Status: v1.ServiceStatus{
Expand Down Expand Up @@ -357,6 +392,10 @@ func TestGatewayServicePredicate_Update(t *testing.T) {
},
},
objectNew: &v1.Service{
ObjectMeta: metav1.ObjectMeta{
Namespace: "nginx-gateway",
Name: "nginx",
},
Spec: v1.ServiceSpec{
Type: v1.ServiceTypeNodePort,
}, Status: v1.ServiceStatus{
Expand Down Expand Up @@ -388,6 +427,10 @@ func TestGatewayServicePredicate_Update(t *testing.T) {
},
},
objectNew: &v1.Service{
ObjectMeta: metav1.ObjectMeta{
Namespace: "nginx-gateway",
Name: "nginx",
},
Spec: v1.ServiceSpec{
Type: v1.ServiceTypeNodePort,
}, Status: v1.ServiceStatus{
Expand All @@ -404,7 +447,7 @@ func TestGatewayServicePredicate_Update(t *testing.T) {
},
}

p := GatewayServicePredicate{}
p := GatewayServicePredicate{NSName: types.NamespacedName{Namespace: "nginx-gateway", Name: "nginx"}}

for _, tc := range testcases {
t.Run(tc.msg, func(t *testing.T) {
Expand Down
5 changes: 3 additions & 2 deletions internal/framework/status/updater.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -255,17 +255,18 @@ func (upd *UpdaterImpl) writeStatuses(

// UpdateAddresses is called when the Gateway Status needs its addresses updated.
func (upd *UpdaterImpl) UpdateAddresses(ctx context.Context, addresses []v1beta1.GatewayStatusAddress) {
defer upd.lock.Unlock()
upd.lock.Lock()

for name, status := range upd.lastStatuses.gatewayAPI.GatewayStatuses {
kate-osborn marked this conversation as resolved.
Show resolved Hide resolved
if status.Ignored {
continue
}
status.Addresses = addresses
upd.lastStatuses.gatewayAPI.GatewayStatuses[name] = status
}
upd.lock.Unlock()

upd.Update(ctx, upd.lastStatuses.gatewayAPI)
upd.updateGatewayAPI(ctx, upd.lastStatuses.gatewayAPI)
}

// NewRetryUpdateFunc returns a function which will be used in wait.ExponentialBackoffWithContext.
Expand Down
6 changes: 2 additions & 4 deletions internal/mode/static/handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,9 +147,8 @@ func (h *eventHandlerImpl) handleEvent(ctx context.Context, logger logr.Logger,
logger.Error(err, "Setting GatewayStatusAddress to Pod IP Address")
}
h.cfg.statusUpdater.UpdateAddresses(ctx, gwAddresses)
} else {
h.cfg.processor.CaptureUpsertChange(e.Resource)
}
h.cfg.processor.CaptureUpsertChange(e.Resource)
default:
h.cfg.processor.CaptureUpsertChange(e.Resource)
}
Expand All @@ -165,9 +164,8 @@ func (h *eventHandlerImpl) handleEvent(ctx context.Context, logger logr.Logger,
logger.Error(err, "Setting GatewayStatusAddress to Pod IP Address")
}
h.cfg.statusUpdater.UpdateAddresses(ctx, gwAddresses)
} else {
h.cfg.processor.CaptureDeleteChange(e.Type, e.NamespacedName)
}
h.cfg.processor.CaptureDeleteChange(e.Type, e.NamespacedName)
default:
h.cfg.processor.CaptureDeleteChange(e.Type, e.NamespacedName)
}
Expand Down
3 changes: 1 addition & 2 deletions internal/mode/static/manager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -275,8 +275,7 @@ func registerControllers(
options: func() []controller.Option {
svcNSName := types.NamespacedName{Namespace: cfg.GatewayPodConfig.Namespace, Name: cfg.GatewayPodConfig.ServiceName}
return []controller.Option{
controller.WithNamespacedNameFilter(filter.CreateSingleResourceFilter(svcNSName)),
controller.WithK8sPredicate(predicate.GatewayServicePredicate{}),
controller.WithK8sPredicate(predicate.GatewayServicePredicate{NSName: svcNSName}),
}
}(),
},
Expand Down
Loading