change udpIdleTimeout and tcpIdleTimeout from int to metav1.Duration (#…

…73)

* change udpIdleTimeout and tcpIdleTimeout from int to metav1.Duration

* fix loadbalancer_info metric udpIdleTimeout and tcpIdleTimeout to duration

README.md

@@ -212,10 +212,12 @@ metadata:
     yawol.stackit.cloud/logForward: "true"
     # defines loki URL for the log forwarding
     yawol.stackit.cloud/logForwardLokiURL: "http://example.com:3100/loki/api/v1/push"
-    # defines the TCP idle Timeout in seconds, default is 3600
-    yawol.stackit.cloud/tcpIdleTimeout: "300"
-    # defines the UDP idle Timeout in seconds, default is 60
-    yawol.stackit.cloud/udpIdleTimeout: "300"
+    # defines the TCP idle Timeout as duration, default is 1h
+    # make sure there is a valid unit (like "s", "m", "h"), otherwise this option is ignored
+    yawol.stackit.cloud/tcpIdleTimeout: "5m30s"
+    # defines the UDP idle Timeout as duration, default is 1m
+    # make sure there is a valid unit (like "s", "m", "h"), otherwise this option is ignored
+    yawol.stackit.cloud/udpIdleTimeout: "5m"
 ```
 
 See [our example service](example-setup/yawol-cloud-controller/service.yaml)

api/v1beta1/loadbalancer_types.go

@@ -110,14 +110,14 @@ type LoadBalancerOptions struct {
 	TCPProxyProtocolPortsFilter []int32 `json:"tcpProxyProtocolPortFilter,omitempty"`
 	// TCPIdleTimeout sets TCP idle Timeout for all TCP connections from this LoadBalancer.
 	// Value is in Seconds. With 0 you disable the idle timeout, be careful this can lead to side effects.
-	// Default is 3600s (1 hour).
+	// Default is 1h.
 	// +optional
-	TCPIdleTimeout *int `json:"tcpIdleTimeout,omitempty"`
+	TCPIdleTimeout *metav1.Duration `json:"tcpIdleTimeout,omitempty"`
 	// UDPIdleTimeout sets UDP idle Timeout for all UDP connections from this LoadBalancer.
 	// Value is in Seconds. With 0 you disable the idle timeout, be careful this can lead to side effects.
-	// Default is 60s (1 minute).
+	// Default is 1m.
 	// +optional
-	UDPIdleTimeout *int `json:"udpIdleTimeout,omitempty"`
+	UDPIdleTimeout *metav1.Duration `json:"udpIdleTimeout,omitempty"`
 	// LogForward enables log forward to a loki instance
 	// +optional
 	LogForward LoadBalancerLogForward `json:"logForward,omitempty"`

charts/yawol-controller/crds/yawol.stackit.cloud_loadbalancers.yaml

@@ -187,7 +187,7 @@ spec:
                       connections from this LoadBalancer. Value is in Seconds. With
                       0 you disable the idle timeout, be careful this can lead to
                       side effects. Default is 3600s (1 hour).
-                    type: integer
+                    type: string
                   tcpProxyProtocol:
                     description: TCPProxyProtocol enables HAProxy TCP Proxy Protocol
                     type: boolean
@@ -204,7 +204,7 @@ spec:
                       connections from this LoadBalancer. Value is in Seconds. With
                       0 you disable the idle timeout, be careful this can lead to
                       side effects. Default is 60s (1 minute).
-                    type: integer
+                    type: string
                 type: object
               ports:
                 description: Ports defines the Ports for the LoadBalancer (copy from

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

@@ -213,7 +213,7 @@ func (r *ServiceReconciler) createLoadBalancer(
 				},
 			},
 			DebugSettings: helper.GetDebugSettings(svc),
-			Options:       helper.GetOptions(svc),
+			Options:       helper.GetOptions(svc, r.Recorder),
 		},
 		Status: yawolv1beta1.LoadBalancerStatus{},
 	}
@@ -353,7 +353,7 @@ func (r *ServiceReconciler) reconcileOptions(
 	lb *yawolv1beta1.LoadBalancer,
 	svc *coreV1.Service,
 ) error {
-	newOptions := helper.GetOptions(svc)
+	newOptions := helper.GetOptions(svc, r.Recorder)
 	if !reflect.DeepEqual(newOptions.LoadBalancerSourceRanges, lb.Spec.Options.LoadBalancerSourceRanges) {
 		if err := r.patchLoadBalancerSourceRanges(ctx, lb, newOptions.LoadBalancerSourceRanges); err != nil {
 			r.Log.WithValues("service", svc.Name).Error(err, "could not patch loadbalancer.spec.options.LoadBalancerSourceRanges")

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

@@ -1528,7 +1528,7 @@ var _ = Describe("Check loadbalancer reconcile", func() {
 					Name:      "service-test21",
 					Namespace: "default",
 					Annotations: map[string]string{
-						yawolv1beta1.ServiceTCPIdleTimeout: "300",
+						yawolv1beta1.ServiceTCPIdleTimeout: "300s",
 					}},
 				Spec: v1.ServiceSpec{
 					Ports: []v1.ServicePort{
@@ -1550,7 +1550,7 @@ var _ = Describe("Check loadbalancer reconcile", func() {
 				if err != nil {
 					return err
 				}
-				if lb.Spec.Options.TCPIdleTimeout != nil && *lb.Spec.Options.TCPIdleTimeout == 300 {
+				if lb.Spec.Options.TCPIdleTimeout != nil && lb.Spec.Options.TCPIdleTimeout.Seconds() == float64(300) {
 					return nil
 				}
 				return fmt.Errorf("wrong options TCPIdleTimeout %v", lb.Spec.Options.TCPIdleTimeout)
@@ -1593,7 +1593,7 @@ var _ = Describe("Check loadbalancer reconcile", func() {
 			By("update svc and enable TCPIdleTimeout")
 			Expect(k8sClient.Get(ctx, client.ObjectKey{Name: service.Name, Namespace: service.Namespace}, &service)).Should(Succeed())
 			service.ObjectMeta.Annotations = map[string]string{
-				yawolv1beta1.ServiceTCPIdleTimeout: "301",
+				yawolv1beta1.ServiceTCPIdleTimeout: "301s",
 			}
 			Expect(k8sClient.Update(ctx, &service)).Should(Succeed())
 
@@ -1603,7 +1603,7 @@ var _ = Describe("Check loadbalancer reconcile", func() {
 				if err != nil {
 					return err
 				}
-				if lb.Spec.Options.TCPIdleTimeout != nil && *lb.Spec.Options.TCPIdleTimeout == 301 {
+				if lb.Spec.Options.TCPIdleTimeout != nil && lb.Spec.Options.TCPIdleTimeout.Seconds() == float64(301) {
 					return nil
 				}
 				return fmt.Errorf("wrong TCPIdleTimeout settings %v", lb.Spec.Options.TCPIdleTimeout)
@@ -1634,7 +1634,7 @@ var _ = Describe("Check loadbalancer reconcile", func() {
 					Name:      "service-test23",
 					Namespace: "default",
 					Annotations: map[string]string{
-						yawolv1beta1.ServiceUDPIdleTimeout: "300",
+						yawolv1beta1.ServiceUDPIdleTimeout: "5m",
 					}},
 				Spec: v1.ServiceSpec{
 					Ports: []v1.ServicePort{
@@ -1656,7 +1656,7 @@ var _ = Describe("Check loadbalancer reconcile", func() {
 				if err != nil {
 					return err
 				}
-				if lb.Spec.Options.UDPIdleTimeout != nil && *lb.Spec.Options.UDPIdleTimeout == 300 {
+				if lb.Spec.Options.UDPIdleTimeout != nil && lb.Spec.Options.UDPIdleTimeout.Seconds() == float64(300) {
 					return nil
 				}
 				return fmt.Errorf("wrong options UDPIdleTimeout %v", lb.Spec.Options.UDPIdleTimeout)
@@ -1699,7 +1699,7 @@ var _ = Describe("Check loadbalancer reconcile", func() {
 			By("update svc and enable UDPIdleTimeout")
 			Expect(k8sClient.Get(ctx, client.ObjectKey{Name: service.Name, Namespace: service.Namespace}, &service)).Should(Succeed())
 			service.ObjectMeta.Annotations = map[string]string{
-				yawolv1beta1.ServiceUDPIdleTimeout: "301",
+				yawolv1beta1.ServiceUDPIdleTimeout: "5m1s",
 			}
 			Expect(k8sClient.Update(ctx, &service)).Should(Succeed())
 
@@ -1709,7 +1709,7 @@ var _ = Describe("Check loadbalancer reconcile", func() {
 				if err != nil {
 					return err
 				}
-				if lb.Spec.Options.UDPIdleTimeout != nil && *lb.Spec.Options.UDPIdleTimeout == 301 {
+				if lb.Spec.Options.UDPIdleTimeout != nil && lb.Spec.Options.UDPIdleTimeout.Seconds() == float64(301) {
 					return nil
 				}
 				return fmt.Errorf("wrong UDPIdleTimeout settings %v", lb.Spec.Options.UDPIdleTimeout)
@@ -1732,5 +1732,59 @@ var _ = Describe("Check loadbalancer reconcile", func() {
 				return fmt.Errorf("wrong UDPIdleTimeout is not nil %v", lb.Spec.Options.UDPIdleTimeout)
 			}, time.Second*5, time.Millisecond*500).Should(Succeed())
 		})
+
+		It("create a service with invalid UDPIdleTimeout options", func() {
+			By("creating a service")
+			service := v1.Service{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "service-test25",
+					Namespace: "default",
+					Annotations: map[string]string{
+						yawolv1beta1.ServiceUDPIdleTimeout: "300",
+					}},
+				Spec: v1.ServiceSpec{
+					Ports: []v1.ServicePort{
+						{
+							Name:       "port1",
+							Protocol:   v1.ProtocolTCP,
+							Port:       12345,
+							TargetPort: intstr.IntOrString{IntVal: 12345},
+							NodePort:   30025,
+						},
+					},
+					Type: "LoadBalancer",
+				}}
+			Expect(k8sClient.Create(ctx, &service)).Should(Succeed())
+
+			By("check UDPIdleTimeout in LB")
+			Eventually(func() error {
+				err := k8sClient.Get(ctx, types.NamespacedName{Name: "default--service-test25", Namespace: "default"}, &lb)
+				if err != nil {
+					return err
+				}
+				if lb.Spec.Options.UDPIdleTimeout == nil {
+					return nil
+				}
+				return fmt.Errorf("wrong options UDPIdleTimeout %v", lb.Spec.Options.UDPIdleTimeout)
+			}, time.Second*5, time.Millisecond*500).Should(Succeed())
+
+			By("Check Event for parse error")
+			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 == "service-test25" &&
+						event.InvolvedObject.Kind == "Service" &&
+						strings.Contains(event.Message, yawolv1beta1.ServiceUDPIdleTimeout) {
+						return nil
+					}
+				}
+				return helper.ErrNoEventFound
+			}, time.Second*5, time.Millisecond*500).Should(Succeed())
+
+		})
 	})
 })

internal/helper/loadbalancer.go

@@ -337,11 +337,11 @@ func parseLoadBalancerInfoMetric(
 	}
 
 	if lb.Spec.Options.TCPIdleTimeout != nil {
-		labels["tcpIdleTimeout"] = strconv.Itoa(*lb.Spec.Options.TCPIdleTimeout)
+		labels["tcpIdleTimeout"] = lb.Spec.Options.TCPIdleTimeout.String()
 	}
 
 	if lb.Spec.Options.UDPIdleTimeout != nil {
-		labels["udpIdleTimeout"] = strconv.Itoa(*lb.Spec.Options.UDPIdleTimeout)
+		labels["udpIdleTimeout"] = lb.Spec.Options.UDPIdleTimeout.String()
 	}
 
 	loadbalancerInfoMetric.DeletePartialMatch(map[string]string{"lb": lb.Name, "namespace": lb.Namespace})

internal/helper/service.go

@@ -6,11 +6,14 @@ import (
 	"fmt"
 	"strconv"
 	"strings"
+	"time"
 
 	yawolv1beta1 "github.com/stackitcloud/yawol/api/v1beta1"
 
 	coreV1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/client-go/tools/record"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 )
 
@@ -42,7 +45,7 @@ func GetDebugSettings(svc *coreV1.Service) yawolv1beta1.LoadBalancerDebugSetting
 }
 
 // GetOptions return loadbalancer option settings for a service
-func GetOptions(svc *coreV1.Service) yawolv1beta1.LoadBalancerOptions {
+func GetOptions(svc *coreV1.Service, recorder record.EventRecorder) yawolv1beta1.LoadBalancerOptions {
 	options := yawolv1beta1.LoadBalancerOptions{}
 	if svc.Annotations[yawolv1beta1.ServiceInternalLoadbalancer] != "" {
 		options.InternalLB, _ = strconv.ParseBool(svc.Annotations[yawolv1beta1.ServiceInternalLoadbalancer])
@@ -66,16 +69,22 @@ func GetOptions(svc *coreV1.Service) yawolv1beta1.LoadBalancerOptions {
 	}
 
 	if svc.Annotations[yawolv1beta1.ServiceTCPIdleTimeout] != "" {
-		tcpIdleTimeout, err := strconv.Atoi(svc.Annotations[yawolv1beta1.ServiceTCPIdleTimeout])
-		if err == nil {
-			options.TCPIdleTimeout = &tcpIdleTimeout
+		tcpIdleTimeout, err := time.ParseDuration(svc.Annotations[yawolv1beta1.ServiceTCPIdleTimeout])
+		if err != nil {
+			recorder.Event(svc, coreV1.EventTypeWarning, "update",
+				"Could not parse "+yawolv1beta1.ServiceTCPIdleTimeout+" to time duration. Ignoring option.")
+		} else {
+			options.TCPIdleTimeout = &metav1.Duration{Duration: tcpIdleTimeout}
 		}
 	}
 
 	if svc.Annotations[yawolv1beta1.ServiceUDPIdleTimeout] != "" {
-		udpIdleTimeout, err := strconv.Atoi(svc.Annotations[yawolv1beta1.ServiceUDPIdleTimeout])
-		if err == nil {
-			options.UDPIdleTimeout = &udpIdleTimeout
+		udpIdleTimeout, err := time.ParseDuration(svc.Annotations[yawolv1beta1.ServiceUDPIdleTimeout])
+		if err != nil {
+			recorder.Event(svc, coreV1.EventTypeWarning, "update",
+				"Could not parse "+yawolv1beta1.ServiceUDPIdleTimeout+" to time duration. Ignoring option.")
+		} else {
+			options.UDPIdleTimeout = &metav1.Duration{Duration: udpIdleTimeout}
 		}
 	}
 

internal/helper/yawollet.go

@@ -13,6 +13,7 @@ import (
 	"github.com/golang/protobuf/ptypes/duration"
 	"github.com/golang/protobuf/ptypes/wrappers"
 	"google.golang.org/protobuf/types/known/anypb"
+	"google.golang.org/protobuf/types/known/durationpb"
 	corev1 "k8s.io/api/core/v1"
 	rbac "k8s.io/api/rbac/v1"
 	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -301,7 +302,7 @@ func createEnvoyTCPListener(
 	var idleTimeout *duration.Duration
 
 	if lb.Spec.Options.TCPIdleTimeout != nil {
-		idleTimeout = &duration.Duration{Seconds: int64(*lb.Spec.Options.TCPIdleTimeout)}
+		idleTimeout = durationpb.New(lb.Spec.Options.TCPIdleTimeout.Duration)
 	}
 
 	listenPort, err := anypb.New(&envoytcp.TcpProxy{
@@ -371,7 +372,7 @@ func createEnvoyUDPListener(
 
 	var idleTimeout *duration.Duration
 	if lb.Spec.Options.UDPIdleTimeout != nil {
-		idleTimeout = &duration.Duration{Seconds: int64(*lb.Spec.Options.UDPIdleTimeout)}
+		idleTimeout = durationpb.New(lb.Spec.Options.UDPIdleTimeout.Duration)
 	}
 
 	listenPort, err := anypb.New(&envoyudp.UdpProxyConfig{