From 494f1442c79d687914677bb09b8b0188ccef23f3 Mon Sep 17 00:00:00 2001 From: Marcel Boehm Date: Thu, 18 Jan 2024 16:09:36 +0100 Subject: [PATCH 1/2] Allow overriding of CreationTimeout --- api/v1beta1/loadbalancerset_types.go | 7 ++++++ .../loadbalancerset_controller.go | 12 ++++++++-- .../loadbalancerset_controller_test.go | 24 ++++++++++++++++++- 3 files changed, 40 insertions(+), 3 deletions(-) diff --git a/api/v1beta1/loadbalancerset_types.go b/api/v1beta1/loadbalancerset_types.go index 52bf5776..473e8eef 100644 --- a/api/v1beta1/loadbalancerset_types.go +++ b/api/v1beta1/loadbalancerset_types.go @@ -4,6 +4,13 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) +const ( + // CreationTimeoutAnnotation can be set to override the defaut duration of + // 10 minutes, after which a new LoadBalancerMachine is deleted (by the LBM) + // if it reports no conditions yet. + CreationTimeoutAnnotation = "yawol.stackit.cloud/creationTimeout" +) + // +kubebuilder:object:root=true // +kubebuilder:resource:shortName=lbs // +kubebuilder:subresource:status diff --git a/controllers/yawol-controller/loadbalancerset/loadbalancerset_controller.go b/controllers/yawol-controller/loadbalancerset/loadbalancerset_controller.go index 5cc655c9..768054e1 100644 --- a/controllers/yawol-controller/loadbalancerset/loadbalancerset_controller.go +++ b/controllers/yawol-controller/loadbalancerset/loadbalancerset_controller.go @@ -247,10 +247,18 @@ func findFirstMachineForDeletion(machines []yawolv1beta1.LoadBalancerMachine) (y // True if a condition is not good for 5 minutes func shouldMachineBeDeleted(machine *yawolv1beta1.LoadBalancerMachine) (shouldDelete bool, reason string) { before3Minutes := metav1.Time{Time: time.Now().Add(-3 * time.Minute)} - before10Minutes := metav1.Time{Time: time.Now().Add(-10 * time.Minute)} + + creationTimeoutDuration := 10 * time.Minute + if v := machine.Annotations[yawolv1beta1.CreationTimeoutAnnotation]; v != "" { + // silently ignore errors + if parsed, err := time.ParseDuration(v); err == nil { + creationTimeoutDuration = parsed + } + } + creationTimeout := time.Now().Add(-1 * creationTimeoutDuration) // in the first 10 minutes we tolerate empty conditions - if machine.CreationTimestamp.After(before10Minutes.Time) && + if machine.CreationTimestamp.After(creationTimeout) && (machine.Status.Conditions == nil || len(*machine.Status.Conditions) == 0) { return false, "" diff --git a/controllers/yawol-controller/loadbalancerset/loadbalancerset_controller_test.go b/controllers/yawol-controller/loadbalancerset/loadbalancerset_controller_test.go index 0b3c9a53..a8603f0e 100644 --- a/controllers/yawol-controller/loadbalancerset/loadbalancerset_controller_test.go +++ b/controllers/yawol-controller/loadbalancerset/loadbalancerset_controller_test.go @@ -382,7 +382,29 @@ func TestIsMachineReady(t *testing.T) { func TestShouldMachineBeDeleted(t *testing.T) { t.Run("Do not delete if there are no conditions shortly after creation", func(t *testing.T) { machine := &yawolv1beta1.LoadBalancerMachine{ - ObjectMeta: metav1.ObjectMeta{CreationTimestamp: metav1.Time{Time: time.Now().Add(-9 * time.Minute)}}, + ObjectMeta: metav1.ObjectMeta{ + CreationTimestamp: metav1.Time{Time: time.Now().Add(-9 * time.Minute)}, + }, + Status: yawolv1beta1.LoadBalancerMachineStatus{ + Conditions: nil, + }, + } + got, _ := shouldMachineBeDeleted(machine) + want := false + + if !reflect.DeepEqual(got, want) { + t.Errorf("Expected %v got %v", want, got) + } + }) + + t.Run("Do not delete if there are no conditions and the grace period has been adjusted", func(t *testing.T) { + machine := &yawolv1beta1.LoadBalancerMachine{ + ObjectMeta: metav1.ObjectMeta{ + CreationTimestamp: metav1.Time{Time: time.Now().Add(-30 * time.Minute)}, + Annotations: map[string]string{ + yawolv1beta1.CreationTimeoutAnnotation: "1h", + }, + }, Status: yawolv1beta1.LoadBalancerMachineStatus{ Conditions: nil, }, From 54d366b19c5354bba6c9147f17639d73d69bb3b6 Mon Sep 17 00:00:00 2001 From: Marcel Boehm Date: Fri, 19 Jan 2024 09:50:20 +0100 Subject: [PATCH 2/2] PR feedback --- api/v1beta1/loadbalancermachine_types.go | 7 ++++++ api/v1beta1/loadbalancerset_types.go | 7 ------ .../loadbalancerset_controller.go | 2 +- .../loadbalancerset_controller_test.go | 22 ++++++++++++++++++- 4 files changed, 29 insertions(+), 9 deletions(-) diff --git a/api/v1beta1/loadbalancermachine_types.go b/api/v1beta1/loadbalancermachine_types.go index 7dae5d20..63903164 100644 --- a/api/v1beta1/loadbalancermachine_types.go +++ b/api/v1beta1/loadbalancermachine_types.go @@ -5,6 +5,13 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) +const ( + // LoadBalancerMachineCreationTimeoutAnnotation can be set on a LoadBalancerMachine to override + // the default duration of 10 minutes, after which a new LoadBalancerMachine + // is deleted (by the LBM) if it reports no conditions yet. + LoadBalancerMachineCreationTimeoutAnnotation = "yawol.stackit.cloud/creationTimeout" +) + // +kubebuilder:object:root=true // +kubebuilder:resource:shortName=lbm // +kubebuilder:subresource:status diff --git a/api/v1beta1/loadbalancerset_types.go b/api/v1beta1/loadbalancerset_types.go index 473e8eef..52bf5776 100644 --- a/api/v1beta1/loadbalancerset_types.go +++ b/api/v1beta1/loadbalancerset_types.go @@ -4,13 +4,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -const ( - // CreationTimeoutAnnotation can be set to override the defaut duration of - // 10 minutes, after which a new LoadBalancerMachine is deleted (by the LBM) - // if it reports no conditions yet. - CreationTimeoutAnnotation = "yawol.stackit.cloud/creationTimeout" -) - // +kubebuilder:object:root=true // +kubebuilder:resource:shortName=lbs // +kubebuilder:subresource:status diff --git a/controllers/yawol-controller/loadbalancerset/loadbalancerset_controller.go b/controllers/yawol-controller/loadbalancerset/loadbalancerset_controller.go index 768054e1..ac6be2bd 100644 --- a/controllers/yawol-controller/loadbalancerset/loadbalancerset_controller.go +++ b/controllers/yawol-controller/loadbalancerset/loadbalancerset_controller.go @@ -249,7 +249,7 @@ func shouldMachineBeDeleted(machine *yawolv1beta1.LoadBalancerMachine) (shouldDe before3Minutes := metav1.Time{Time: time.Now().Add(-3 * time.Minute)} creationTimeoutDuration := 10 * time.Minute - if v := machine.Annotations[yawolv1beta1.CreationTimeoutAnnotation]; v != "" { + if v := machine.Annotations[yawolv1beta1.LoadBalancerMachineCreationTimeoutAnnotation]; v != "" { // silently ignore errors if parsed, err := time.ParseDuration(v); err == nil { creationTimeoutDuration = parsed diff --git a/controllers/yawol-controller/loadbalancerset/loadbalancerset_controller_test.go b/controllers/yawol-controller/loadbalancerset/loadbalancerset_controller_test.go index a8603f0e..23b94f7e 100644 --- a/controllers/yawol-controller/loadbalancerset/loadbalancerset_controller_test.go +++ b/controllers/yawol-controller/loadbalancerset/loadbalancerset_controller_test.go @@ -402,7 +402,7 @@ func TestShouldMachineBeDeleted(t *testing.T) { ObjectMeta: metav1.ObjectMeta{ CreationTimestamp: metav1.Time{Time: time.Now().Add(-30 * time.Minute)}, Annotations: map[string]string{ - yawolv1beta1.CreationTimeoutAnnotation: "1h", + yawolv1beta1.LoadBalancerMachineCreationTimeoutAnnotation: "1h", }, }, Status: yawolv1beta1.LoadBalancerMachineStatus{ @@ -417,6 +417,26 @@ func TestShouldMachineBeDeleted(t *testing.T) { } }) + t.Run("Delete if there are no conditions after a custom grace period", func(t *testing.T) { + machine := &yawolv1beta1.LoadBalancerMachine{ + ObjectMeta: metav1.ObjectMeta{ + CreationTimestamp: metav1.Time{Time: time.Now().Add(-30 * time.Minute)}, + Annotations: map[string]string{ + yawolv1beta1.LoadBalancerMachineCreationTimeoutAnnotation: "15m", + }, + }, + Status: yawolv1beta1.LoadBalancerMachineStatus{ + Conditions: nil, + }, + } + got, _ := shouldMachineBeDeleted(machine) + want := true + + if !reflect.DeepEqual(got, want) { + t.Errorf("Expected %v got %v", want, got) + } + }) + t.Run("Do not delete if failed condition is not older than 5 minutes", func(t *testing.T) { machine := &yawolv1beta1.LoadBalancerMachine{ Status: yawolv1beta1.LoadBalancerMachineStatus{