Skip to content

Commit

Permalink
fix(bug): ignore headless svc to prevent error messages (#763)
Browse files Browse the repository at this point in the history
# Description

* update service controller to ignore headless svc and re-trigger
reconcile when encountering one
* create test to validate UpdateRetinaSvc still throws an error if a
retina svc has no IPv4 (this is to make sure existing validation post
retinaSvc creation is not broken)

## Related Issue

If this pull request is related to any issue, please mention it here.
Additionally, make sure that the issue is assigned to you before
submitting this pull request.

fix #547 

## Checklist

- [x] I have read the [contributing
documentation](https://retina.sh/docs/contributing).
- [x] I signed and signed-off the commits (`git commit -S -s ...`). See
[this
documentation](https://docs.github.com/en/authentication/managing-commit-signature-verification/about-commit-signature-verification)
on signing commits.
- [x] I have correctly attributed the author(s) of the code.
- [x] I have tested the changes locally.
- [x] I have followed the project's style guidelines.
- [x] I have updated the documentation, if necessary.
- [x] I have added tests, if applicable.

## Screenshots (if applicable) or Testing Completed

Please add any relevant screenshots or GIFs to showcase the changes
made.

## Additional Notes

Add any additional notes or context about the pull request here.

---

Please refer to the [CONTRIBUTING.md](../CONTRIBUTING.md) file for more
information on how to contribute to this project.

Signed-off-by: Simone Rodigari <srodigari@microsoft.com>
  • Loading branch information
SRodi committed Sep 24, 2024
1 parent 9870ac4 commit c798308
Show file tree
Hide file tree
Showing 2 changed files with 143 additions and 1 deletion.
11 changes: 10 additions & 1 deletion pkg/controllers/daemon/service/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
errors "k8s.io/apimachinery/pkg/api/errors"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/predicate"

retinaCommon "github.com/microsoft/retina/pkg/common"
"github.com/microsoft/retina/pkg/controllers/cache"
Expand Down Expand Up @@ -76,7 +77,6 @@ func (r *ServiceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
}
ips := retinaCommon.IPAddresses{}
ips.IPv4 = net.ParseIP(service.Spec.ClusterIP)
net.ParseIP(service.Spec.ClusterIP)
var lbIP net.IP
if service.Status.LoadBalancer.Ingress != nil && len(service.Status.LoadBalancer.Ingress) > 0 {
lbIP = net.ParseIP(service.Status.LoadBalancer.Ingress[0].IP)
Expand All @@ -93,7 +93,16 @@ func (r *ServiceReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
// SetupWithManager sets up the controller with the Manager.
func (r *ServiceReconciler) SetupWithManager(mgr ctrl.Manager) error {
r.l.Info("Setting up Service controller")
// Define a predicate to filter out Services with ClusterIP == "None"
pred := predicate.NewPredicateFuncs(func(object client.Object) bool {
service, ok := object.(*corev1.Service)
if !ok {
return false
}
return service.Spec.ClusterIP != "None"
})
return ctrl.NewControllerManagedBy(mgr).
For(&corev1.Service{}).
WithEventFilter(pred).
Complete(r)
}
133 changes: 133 additions & 0 deletions pkg/controllers/daemon/service/controller_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@
package service

import (
"testing"

"github.com/stretchr/testify/assert"
corev1 "k8s.io/api/core/v1"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/event"
"sigs.k8s.io/controller-runtime/pkg/predicate"
)

func TestServiceFilter(t *testing.T) {
pred := predicate.NewPredicateFuncs(func(object client.Object) bool {
service, ok := object.(*corev1.Service)
if !ok {
return false
}
return service.Spec.ClusterIP != "None"
})

tests := []struct {
name string
event interface{}
expect bool
}{
{
name: "CreateEvent with ClusterIP None",
event: event.CreateEvent{
Object: &corev1.Service{
Spec: corev1.ServiceSpec{
ClusterIP: "None",
},
},
},
expect: false,
},
{
name: "CreateEvent with ClusterIP not None",
event: event.CreateEvent{
Object: &corev1.Service{
Spec: corev1.ServiceSpec{
ClusterIP: "10.0.0.1",
},
},
},
expect: true,
},
{
name: "UpdateEvent with ClusterIP None",
event: event.UpdateEvent{
ObjectNew: &corev1.Service{
Spec: corev1.ServiceSpec{
ClusterIP: "None",
},
},
},
expect: false,
},
{
name: "UpdateEvent with ClusterIP not None",
event: event.UpdateEvent{
ObjectNew: &corev1.Service{
Spec: corev1.ServiceSpec{
ClusterIP: "10.0.0.1",
},
},
},
expect: true,
},
{
name: "DeleteEvent with ClusterIP None",
event: event.DeleteEvent{
Object: &corev1.Service{
Spec: corev1.ServiceSpec{
ClusterIP: "None",
},
},
},
expect: false,
},
{
name: "DeleteEvent with ClusterIP not None",
event: event.DeleteEvent{
Object: &corev1.Service{
Spec: corev1.ServiceSpec{
ClusterIP: "10.0.0.1",
},
},
},
expect: true,
},
{
name: "GenericEvent with ClusterIP None",
event: event.GenericEvent{
Object: &corev1.Service{
Spec: corev1.ServiceSpec{
ClusterIP: "None",
},
},
},
expect: false,
},
{
name: "GenericEvent with ClusterIP not None",
event: event.GenericEvent{
Object: &corev1.Service{
Spec: corev1.ServiceSpec{
ClusterIP: "10.0.0.1",
},
},
},
expect: true,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
var result bool
switch e := tt.event.(type) {
case event.CreateEvent:
result = pred.Create(e)
case event.UpdateEvent:
result = pred.Update(e)
case event.DeleteEvent:
result = pred.Delete(e)
case event.GenericEvent:
result = pred.Generic(e)
}
assert.Equal(t, tt.expect, result)
})
}
}

0 comments on commit c798308

Please sign in to comment.