From 18fe6440bd605f6d67e9ab0b2e615513e8ff9d08 Mon Sep 17 00:00:00 2001 From: Katarina Strenkova Date: Thu, 23 Jul 2026 05:13:57 -0400 Subject: [PATCH] Add logic for pending pods timeout Recently a way to distinguish pending pods from running pods was added. This brings new opportunities for improvement in test-operator, including early exist for stuck (pending) pods. This PR introduces parameter PendingTimeout, which will allow users to set the maximum time they are willing to wait for a pod in a pending state until it is marked as stuck. Once the limit is exceeded we can mark it as stuck and move onto the next pod. This feature will also help in times, when the pods are stuck and therefore exceed the full testing time limit, resulting in no log collection. With this change the job should end early and have logs reporting the pending state. One thing to note is that it is important to make sure pods that exceeded the pending timeout should be considered as failed by jobs. There should be no false positives by introducing this change! --- .../test.openstack.org_ansibletests.yaml | 10 ++++++ .../test.openstack.org_horizontests.yaml | 5 +++ api/bases/test.openstack.org_tempests.yaml | 10 ++++++ api/bases/test.openstack.org_tobikoes.yaml | 10 ++++++ api/v1beta1/ansibletest_types.go | 5 +++ api/v1beta1/common.go | 12 +++++++ api/v1beta1/horizontest_types.go | 5 +++ api/v1beta1/tempest_types.go | 5 +++ api/v1beta1/tobiko_types.go | 5 +++ api/v1beta1/zz_generated.deepcopy.go | 5 +++ .../test.openstack.org_ansibletests.yaml | 10 ++++++ .../test.openstack.org_horizontests.yaml | 5 +++ .../bases/test.openstack.org_tempests.yaml | 10 ++++++ .../bases/test.openstack.org_tobikoes.yaml | 10 ++++++ internal/controller/common.go | 12 +++++++ internal/controller/common_controller.go | 31 +++++++++++++++++-- 16 files changed, 148 insertions(+), 2 deletions(-) diff --git a/api/bases/test.openstack.org_ansibletests.yaml b/api/bases/test.openstack.org_ansibletests.yaml index 263e0f35..aecc1016 100644 --- a/api/bases/test.openstack.org_ansibletests.yaml +++ b/api/bases/test.openstack.org_ansibletests.yaml @@ -1276,6 +1276,11 @@ spec: description: OpenStackConfigSecret is the name of the Secret containing the secure.yaml type: string + pendingTimeout: + description: |- + Timeout after which a test pod in the Pending state is considered + stuck and its execution is terminated. + type: integer privileged: default: false description: |- @@ -1507,6 +1512,11 @@ spec: description: OpenStackConfigSecret is the name of the Secret containing the secure.yaml type: string + pendingTimeout: + description: |- + Timeout after which a test pod in the Pending state is considered + stuck and its execution is terminated. + type: integer privileged: description: |- Use with caution! This parameter specifies whether test-operator should spawn test diff --git a/api/bases/test.openstack.org_horizontests.yaml b/api/bases/test.openstack.org_horizontests.yaml index 2cc24c40..461ec646 100644 --- a/api/bases/test.openstack.org_horizontests.yaml +++ b/api/bases/test.openstack.org_horizontests.yaml @@ -1304,6 +1304,11 @@ spec: tests. maxLength: 253 type: string + pendingTimeout: + description: |- + Timeout after which a test pod in the Pending state is considered + stuck and its execution is terminated. + type: integer privileged: default: false description: |- diff --git a/api/bases/test.openstack.org_tempests.yaml b/api/bases/test.openstack.org_tempests.yaml index 9a38721c..f428ec68 100644 --- a/api/bases/test.openstack.org_tempests.yaml +++ b/api/bases/test.openstack.org_tempests.yaml @@ -1275,6 +1275,11 @@ spec: instances of test-operator related CRs exist. If you want to turn off this behaviour then set this option to true. type: boolean + pendingTimeout: + description: |- + Timeout after which a test pod in the Pending state is considered + stuck and its execution is terminated. + type: integer privileged: default: false description: |- @@ -1848,6 +1853,11 @@ spec: instances of test-operator related CRs exist. If you want to turn off this behaviour then set this option to true. type: boolean + pendingTimeout: + description: |- + Timeout after which a test pod in the Pending state is considered + stuck and its execution is terminated. + type: integer privileged: description: |- Use with caution! This parameter specifies whether test-operator should spawn test diff --git a/api/bases/test.openstack.org_tobikoes.yaml b/api/bases/test.openstack.org_tobikoes.yaml index 606ab81c..5b39c319 100644 --- a/api/bases/test.openstack.org_tobikoes.yaml +++ b/api/bases/test.openstack.org_tobikoes.yaml @@ -1281,6 +1281,11 @@ spec: format: uri type: string type: object + pendingTimeout: + description: |- + Timeout after which a test pod in the Pending state is considered + stuck and its execution is terminated. + type: integer preventCreate: default: false description: Boolean specifying whether tobiko tests create new resources @@ -1541,6 +1546,11 @@ spec: format: uri type: string type: object + pendingTimeout: + description: |- + Timeout after which a test pod in the Pending state is considered + stuck and its execution is terminated. + type: integer preventCreate: description: Boolean specifying whether tobiko tests create new resources or re-use those previously created diff --git a/api/v1beta1/ansibletest_types.go b/api/v1beta1/ansibletest_types.go index 1c5c5414..d94b2eff 100644 --- a/api/v1beta1/ansibletest_types.go +++ b/api/v1beta1/ansibletest_types.go @@ -226,6 +226,11 @@ func (instance *AnsibleTest) GetConditions() *condition.Conditions { return &instance.Status.Conditions } +// GetPendingTimeout - return the pending timeout +func (instance *AnsibleTest) GetPendingTimeout() int { + return instance.Spec.PendingTimeout +} + // GetStorageClass - return the storage class name func (instance *AnsibleTest) GetStorageClass() string { return instance.Spec.StorageClass diff --git a/api/v1beta1/common.go b/api/v1beta1/common.go index 154b2043..1ab2de9c 100644 --- a/api/v1beta1/common.go +++ b/api/v1beta1/common.go @@ -95,6 +95,12 @@ type CommonOptions struct { // ExtraMounts containing conf files, credentials and storage volumes ExtraMounts []ExtraVolMounts `json:"extraMounts,omitempty"` + // +operator-sdk:csv:customresourcedefinitions:type=spec + // +kubebuilder:validation:Optional + // Timeout after which a test pod in the Pending state is considered + // stuck and its execution is terminated. + PendingTimeout int `json:"pendingTimeout"` + // +kubebuilder:validation:Optional // +operator-sdk:csv:customresourcedefinitions:type=spec // This value contains a nodeSelector value that is applied to test pods @@ -186,6 +192,12 @@ type WorkflowCommonOptions struct { // Please use ExtraMounts parameter instead! ExtraConfigmapsMounts *[]ExtraConfigmapsMounts `json:"extraConfigmapsMounts,omitempty"` + // +operator-sdk:csv:customresourcedefinitions:type=spec + // +kubebuilder:validation:Optional + // Timeout after which a test pod in the Pending state is considered + // stuck and its execution is terminated. + PendingTimeout *int `json:"pendingTimeout,omitempty"` + // +kubebuilder:validation:Optional // +operator-sdk:csv:customresourcedefinitions:type=spec // This value contains a nodeSelector value that is applied to test pods diff --git a/api/v1beta1/horizontest_types.go b/api/v1beta1/horizontest_types.go index eca3590b..4ec2ee7f 100644 --- a/api/v1beta1/horizontest_types.go +++ b/api/v1beta1/horizontest_types.go @@ -204,6 +204,11 @@ func (instance *HorizonTest) GetConditions() *condition.Conditions { return &instance.Status.Conditions } +// GetPendingTimeout - return the pending timeout +func (instance *HorizonTest) GetPendingTimeout() int { + return instance.Spec.PendingTimeout +} + // GetStorageClass - return the storage class name func (instance *HorizonTest) GetStorageClass() string { return instance.Spec.StorageClass diff --git a/api/v1beta1/tempest_types.go b/api/v1beta1/tempest_types.go index ea98116d..320072f0 100644 --- a/api/v1beta1/tempest_types.go +++ b/api/v1beta1/tempest_types.go @@ -528,6 +528,11 @@ func (instance *Tempest) GetConditions() *condition.Conditions { return &instance.Status.Conditions } +// GetPendingTimeout - return the pending timeout +func (instance *Tempest) GetPendingTimeout() int { + return instance.Spec.PendingTimeout +} + // GetStorageClass - return the storage class name func (instance *Tempest) GetStorageClass() string { return instance.Spec.StorageClass diff --git a/api/v1beta1/tobiko_types.go b/api/v1beta1/tobiko_types.go index d01ab889..f0c70a8f 100644 --- a/api/v1beta1/tobiko_types.go +++ b/api/v1beta1/tobiko_types.go @@ -266,6 +266,11 @@ func (instance *Tobiko) GetConditions() *condition.Conditions { return &instance.Status.Conditions } +// GetPendingTimeout - return the pending timeout +func (instance *Tobiko) GetPendingTimeout() int { + return instance.Spec.PendingTimeout +} + // GetStorageClass - return the storage class name func (instance *Tobiko) GetStorageClass() string { return instance.Spec.StorageClass diff --git a/api/v1beta1/zz_generated.deepcopy.go b/api/v1beta1/zz_generated.deepcopy.go index 82d5abbe..6d91aa19 100644 --- a/api/v1beta1/zz_generated.deepcopy.go +++ b/api/v1beta1/zz_generated.deepcopy.go @@ -746,6 +746,11 @@ func (in *WorkflowCommonOptions) DeepCopyInto(out *WorkflowCommonOptions) { copy(*out, *in) } } + if in.PendingTimeout != nil { + in, out := &in.PendingTimeout, &out.PendingTimeout + *out = new(int) + **out = **in + } if in.NodeSelector != nil { in, out := &in.NodeSelector, &out.NodeSelector *out = new(map[string]string) diff --git a/config/crd/bases/test.openstack.org_ansibletests.yaml b/config/crd/bases/test.openstack.org_ansibletests.yaml index 263e0f35..aecc1016 100644 --- a/config/crd/bases/test.openstack.org_ansibletests.yaml +++ b/config/crd/bases/test.openstack.org_ansibletests.yaml @@ -1276,6 +1276,11 @@ spec: description: OpenStackConfigSecret is the name of the Secret containing the secure.yaml type: string + pendingTimeout: + description: |- + Timeout after which a test pod in the Pending state is considered + stuck and its execution is terminated. + type: integer privileged: default: false description: |- @@ -1507,6 +1512,11 @@ spec: description: OpenStackConfigSecret is the name of the Secret containing the secure.yaml type: string + pendingTimeout: + description: |- + Timeout after which a test pod in the Pending state is considered + stuck and its execution is terminated. + type: integer privileged: description: |- Use with caution! This parameter specifies whether test-operator should spawn test diff --git a/config/crd/bases/test.openstack.org_horizontests.yaml b/config/crd/bases/test.openstack.org_horizontests.yaml index 2cc24c40..461ec646 100644 --- a/config/crd/bases/test.openstack.org_horizontests.yaml +++ b/config/crd/bases/test.openstack.org_horizontests.yaml @@ -1304,6 +1304,11 @@ spec: tests. maxLength: 253 type: string + pendingTimeout: + description: |- + Timeout after which a test pod in the Pending state is considered + stuck and its execution is terminated. + type: integer privileged: default: false description: |- diff --git a/config/crd/bases/test.openstack.org_tempests.yaml b/config/crd/bases/test.openstack.org_tempests.yaml index 9a38721c..f428ec68 100644 --- a/config/crd/bases/test.openstack.org_tempests.yaml +++ b/config/crd/bases/test.openstack.org_tempests.yaml @@ -1275,6 +1275,11 @@ spec: instances of test-operator related CRs exist. If you want to turn off this behaviour then set this option to true. type: boolean + pendingTimeout: + description: |- + Timeout after which a test pod in the Pending state is considered + stuck and its execution is terminated. + type: integer privileged: default: false description: |- @@ -1848,6 +1853,11 @@ spec: instances of test-operator related CRs exist. If you want to turn off this behaviour then set this option to true. type: boolean + pendingTimeout: + description: |- + Timeout after which a test pod in the Pending state is considered + stuck and its execution is terminated. + type: integer privileged: description: |- Use with caution! This parameter specifies whether test-operator should spawn test diff --git a/config/crd/bases/test.openstack.org_tobikoes.yaml b/config/crd/bases/test.openstack.org_tobikoes.yaml index 606ab81c..5b39c319 100644 --- a/config/crd/bases/test.openstack.org_tobikoes.yaml +++ b/config/crd/bases/test.openstack.org_tobikoes.yaml @@ -1281,6 +1281,11 @@ spec: format: uri type: string type: object + pendingTimeout: + description: |- + Timeout after which a test pod in the Pending state is considered + stuck and its execution is terminated. + type: integer preventCreate: default: false description: Boolean specifying whether tobiko tests create new resources @@ -1541,6 +1546,11 @@ spec: format: uri type: string type: object + pendingTimeout: + description: |- + Timeout after which a test pod in the Pending state is considered + stuck and its execution is terminated. + type: integer preventCreate: description: Boolean specifying whether tobiko tests create new resources or re-use those previously created diff --git a/internal/controller/common.go b/internal/controller/common.go index dffa413d..6a3f3785 100644 --- a/internal/controller/common.go +++ b/internal/controller/common.go @@ -43,6 +43,7 @@ const ( workflowStepLabel = "workflowStep" instanceNameLabel = "instanceName" operatorNameLabel = "operator" + pendingTimeoutAnnotation = "test.openstack.org/pending-timeout" testOperatorLockName = "test-operator-lock" testOperatorLockOwnerField = "owner" testOperatorBaseDir = "/etc/test_operator/" @@ -58,6 +59,8 @@ const ( InfoWaitingOnPod = "Waiting on either pod to finish or release of the lock." // InfoPendingPod is the info message when waiting for a pending pod to start InfoPendingPod = "Waiting for pending pod to start running." + // InfoPendingPodTimeout is the info message when a pending pod exceeds its timeout + InfoPendingPodTimeout = "Pod exceeded pending timeout." // InfoTestingCompleted is the info message when all testing is completed InfoTestingCompleted = "Testing completed. All pods spawned by the test-operator finished." // InfoCreatingFirstPod is the info message when creating the first test pod @@ -203,6 +206,15 @@ func (r *Reconciler) NextAction( return Failure, workflowStepIdx, err } + // if the last pod has exceeded pending timeout + if lastPod.Annotations[pendingTimeoutAnnotation] == "true" { + if !isLastPodIndex(workflowStepIdx, workflowLength) { + workflowStepIdx++ + return CreateNextPod, workflowStepIdx, nil + } + return EndTesting, workflowStepIdx, nil + } + switch lastPod.Status.Phase { case corev1.PodPending: // If the last pod is in Pending state -> CheckPending diff --git a/internal/controller/common_controller.go b/internal/controller/common_controller.go index 41718f97..a945bd44 100644 --- a/internal/controller/common_controller.go +++ b/internal/controller/common_controller.go @@ -20,6 +20,7 @@ import ( "context" "fmt" "strconv" + "time" "github.com/go-logr/logr" "github.com/openstack-k8s-operators/lib-common/modules/common" @@ -36,6 +37,7 @@ import ( type TestResource interface { client.Object GetConditions() *condition.Conditions + GetPendingTimeout() int GetStorageClass() string SetObservedGeneration() } @@ -211,8 +213,33 @@ func CommonReconcile[T TestResource]( switch nextAction { case CheckPending: - Log.Info(InfoPendingPod) - return ctrl.Result{RequeueAfter: RequeueAfterValue}, nil + pendingTimeout := time.Duration(instance.GetPendingTimeout()) * time.Second + + lastPod, err := r.GetLastPod(ctx, instance) + if err != nil { + return ctrl.Result{}, err + } + + if pendingTimeout <= 0 || time.Since(lastPod.CreationTimestamp.Time) <= pendingTimeout { + Log.Info(InfoPendingPod) + return ctrl.Result{RequeueAfter: RequeueAfterValue}, nil + } + + Log.Info(InfoPendingPodTimeout) + + if lastPod.Annotations == nil { + lastPod.Annotations = make(map[string]string) + } + lastPod.Annotations[pendingTimeoutAnnotation] = "true" + + var deadline int64 = 1 + lastPod.Spec.ActiveDeadlineSeconds = &deadline + + if err := r.Client.Update(ctx, lastPod); err != nil { + return ctrl.Result{}, err + } + + return ctrl.Result{Requeue: true}, nil case Wait: Log.Info(InfoWaitingOnPod)