add code suggestions from PR review

cmd/yawol-cloud-controller/main.go

@@ -101,8 +101,8 @@ func main() {
 		"K8s credentials for deploying the LoadBalancer resources.")
 	flag.Var(&classNames, "classname",
 		"Only listen to Services with the given className. Can be set multiple times. "+
-			"Always listen to "+helper.DefaultLoadbalancerClass+"."+
-			"See also --empty-classname.")
+			"If no classname is set it will defaults to "+helper.DefaultLoadbalancerClass+" "+
+			"and services without class. See also --empty-classname.")
 	flag.BoolVar(&emptyClassName, "empty-classname", true,
 		"Listen to services without a loadBalancerClass. Default is true.")
 	flag.IntVar(&leasesDurationInt, "leases-duration", 60,
@@ -121,7 +121,9 @@ func main() {
 	opts.BindFlags(flag.CommandLine)
 	flag.Parse()
 
-	classNames = append(classNames, helper.DefaultLoadbalancerClass)
+	if len(classNames) == 0 {
+		classNames = append(classNames, helper.DefaultLoadbalancerClass)
+	}
 	if emptyClassName {
 		classNames = append(classNames, "")
 	}

controllers/yawol-cloud-controller/targetcontroller/service_controller_test.go

@@ -638,6 +638,54 @@ var _ = Describe("Check loadbalancer reconcile", Serial, Ordered, func() {
 			}, time.Second*5, time.Millisecond*500).Should(Succeed())
 		})
 
+		It("create service without classname", func() {
+			By("create service")
+			service := v1.Service{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "class-name-service-test5",
+					Namespace: "default",
+				},
+				Spec: v1.ServiceSpec{
+					Ports: []v1.ServicePort{
+						{
+							Name:       "port1",
+							Protocol:   v1.ProtocolTCP,
+							Port:       12345,
+							TargetPort: intstr.IntOrString{IntVal: 12345},
+							NodePort:   30360,
+						},
+					},
+					Type: "LoadBalancer",
+				}}
+			Expect(k8sClient.Create(ctx, &service)).Should(Succeed())
+
+			By("check creation of LB")
+			Eventually(func() error {
+				err := k8sClient.Get(ctx, types.NamespacedName{
+					Name:      "default--class-name-service-test5",
+					Namespace: "default",
+				}, &lb)
+				return err
+			}, time.Second*5, time.Millisecond*500).Should(Succeed())
+
+			By("Check Event for creation")
+			Eventually(func() error {
+				eventList := v1.EventList{}
+				err := k8sClient.List(ctx, &eventList)
+				if err != nil {
+					return err
+				}
+				for _, event := range eventList.Items {
+					if event.InvolvedObject.Name == "class-name-service-test5" &&
+						event.InvolvedObject.Kind == "Service" &&
+						strings.Contains(event.Message, "LoadBalancer is in creation") {
+						return nil
+					}
+				}
+				return helper.ErrNoEventFound
+			}, time.Second*5, time.Millisecond*500).Should(Succeed())
+		})
+
 		It("create service with existingFloatingIP", func() {
 			By("create service")
 			service := v1.Service{

internal/helper/service.go

@@ -13,6 +13,7 @@ import (
 	coreV1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/client-go/tools/record"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
@@ -165,7 +166,7 @@ func GetLoadBalancerNameFromService(service *coreV1.Service) string {
 }
 
 func getLoadBalancerClass(service *coreV1.Service) string {
-	if className, ok := service.Annotations[yawolv1beta1.ServiceClassName]; ok {
+	if className := service.Annotations[yawolv1beta1.ServiceClassName]; className != "" {
 		return className
 	}
 
@@ -177,14 +178,7 @@ func getLoadBalancerClass(service *coreV1.Service) string {
 }
 
 func CheckLoadBalancerClasses(service *coreV1.Service, validClasses []string) bool {
-	serviceClassName := getLoadBalancerClass(service)
-
-	for _, validClass := range validClasses {
-		if validClass == serviceClassName {
-			return true
-		}
-	}
-	return false
+	return sets.New(validClasses...).Has(getLoadBalancerClass(service))
 }
 
 // ValidateService checks if the service is valid

internal/helper/service_test.go

@@ -0,0 +1,53 @@
+package helper
+
+import (
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+	corev1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/utils/pointer"
+
+	yawolv1beta1 "github.com/stackitcloud/yawol/api/v1beta1"
+)
+
+var _ = Describe("loadbalancerClasses", Serial, Ordered, func() {
+	svc := corev1.Service{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "test",
+			Namespace: "default",
+		},
+		Spec: corev1.ServiceSpec{},
+	}
+
+	It("should return correct loadbalancerClass from annotation", func() {
+		s := svc.DeepCopy()
+		s.Annotations = map[string]string{
+			yawolv1beta1.ServiceClassName: "foo",
+		}
+		expected := "foo"
+		Expect(getLoadBalancerClass(s)).To(Equal(expected))
+	})
+
+	It("should return correct loadbalancerClass from spec", func() {
+		s := svc.DeepCopy()
+		s.Spec.LoadBalancerClass = pointer.String("bar")
+		expected := "bar"
+		Expect(getLoadBalancerClass(s)).To(Equal(expected))
+	})
+
+	It("should return correct loadbalancerClass from annotation if also set in spec", func() {
+		s := svc.DeepCopy()
+		s.Annotations = map[string]string{
+			yawolv1beta1.ServiceClassName: "foo",
+		}
+		s.Spec.LoadBalancerClass = pointer.String("bar")
+		expected := "foo"
+		Expect(getLoadBalancerClass(s)).To(Equal(expected))
+	})
+
+	It("should return empty string if no class is set", func() {
+		s := svc.DeepCopy()
+		expected := ""
+		Expect(getLoadBalancerClass(s)).To(Equal(expected))
+	})
+})