fix(bug): ignore headless svc to prevent error messages (#763)

# 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 <[email protected]>

pkg/controllers/daemon/service/controller.go

@@ -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"
@@ -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)
@@ -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)
 }

pkg/controllers/daemon/service/controller_test.go

@@ -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)
+		})
+	}
+}