diff --git a/README.md b/README.md index bcc5d13a..4e99fd57 100644 --- a/README.md +++ b/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) diff --git a/api/v1beta1/loadbalancer_types.go b/api/v1beta1/loadbalancer_types.go index 5fbfab0a..8a358297 100644 --- a/api/v1beta1/loadbalancer_types.go +++ b/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"` diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index 978933fb..1dfb25f7 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -7,6 +7,7 @@ package v1beta1 import ( "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" runtime "k8s.io/apimachinery/pkg/runtime" ) @@ -350,12 +351,12 @@ func (in *LoadBalancerOptions) DeepCopyInto(out *LoadBalancerOptions) { } if in.TCPIdleTimeout != nil { in, out := &in.TCPIdleTimeout, &out.TCPIdleTimeout - *out = new(int) + *out = new(metav1.Duration) **out = **in } if in.UDPIdleTimeout != nil { in, out := &in.UDPIdleTimeout, &out.UDPIdleTimeout - *out = new(int) + *out = new(metav1.Duration) **out = **in } out.LogForward = in.LogForward diff --git a/charts/yawol-controller/crds/yawol.stackit.cloud_loadbalancers.yaml b/charts/yawol-controller/crds/yawol.stackit.cloud_loadbalancers.yaml index 8fa81aa8..8eaac797 100644 --- a/charts/yawol-controller/crds/yawol.stackit.cloud_loadbalancers.yaml +++ b/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 diff --git a/controllers/yawol-cloud-controller/targetcontroller/service_controller.go b/controllers/yawol-cloud-controller/targetcontroller/service_controller.go index e0896768..253b9728 100644 --- a/controllers/yawol-cloud-controller/targetcontroller/service_controller.go +++ b/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") diff --git a/controllers/yawol-cloud-controller/targetcontroller/service_controller_test.go b/controllers/yawol-cloud-controller/targetcontroller/service_controller_test.go index 4c5e3d2c..f5e33784 100644 --- a/controllers/yawol-cloud-controller/targetcontroller/service_controller_test.go +++ b/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()) + + }) }) }) diff --git a/internal/helper/loadbalancer.go b/internal/helper/loadbalancer.go index 95a1f068..b4433e0f 100644 --- a/internal/helper/loadbalancer.go +++ b/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}) diff --git a/internal/helper/service.go b/internal/helper/service.go index 799a37ea..7aef047d 100644 --- a/internal/helper/service.go +++ b/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} } } diff --git a/internal/helper/yawollet.go b/internal/helper/yawollet.go index b21bf1f8..8ea1d8bb 100644 --- a/internal/helper/yawollet.go +++ b/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{