From e54084739dff14b2e160d06f165d261b814e28f5 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 ++++++ .../test-operator.clusterserviceversion.yaml | 35 +++++++++++++++++++ internal/controller/common.go | 12 +++++++ internal/controller/common_controller.go | 31 ++++++++++++++-- 17 files changed, 183 insertions(+), 2 deletions(-) diff --git a/api/bases/test.openstack.org_ansibletests.yaml b/api/bases/test.openstack.org_ansibletests.yaml index bfedd1c7..0f905bd3 100644 --- a/api/bases/test.openstack.org_ansibletests.yaml +++ b/api/bases/test.openstack.org_ansibletests.yaml @@ -879,6 +879,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: |- @@ -1110,6 +1115,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 9dbca724..95846968 100644 --- a/api/bases/test.openstack.org_horizontests.yaml +++ b/api/bases/test.openstack.org_horizontests.yaml @@ -886,6 +886,11 @@ spec: default: false description: Parallel 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: |- diff --git a/api/bases/test.openstack.org_tempests.yaml b/api/bases/test.openstack.org_tempests.yaml index 394fd0d0..7f54e32a 100644 --- a/api/bases/test.openstack.org_tempests.yaml +++ b/api/bases/test.openstack.org_tempests.yaml @@ -878,6 +878,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: |- @@ -1451,6 +1456,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 e7ce768e..68a624ff 100644 --- a/api/bases/test.openstack.org_tobikoes.yaml +++ b/api/bases/test.openstack.org_tobikoes.yaml @@ -884,6 +884,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 @@ -1144,6 +1149,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 de589f43..55840887 100644 --- a/api/v1beta1/horizontest_types.go +++ b/api/v1beta1/horizontest_types.go @@ -163,6 +163,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 bfedd1c7..0f905bd3 100644 --- a/config/crd/bases/test.openstack.org_ansibletests.yaml +++ b/config/crd/bases/test.openstack.org_ansibletests.yaml @@ -879,6 +879,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: |- @@ -1110,6 +1115,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 9dbca724..95846968 100644 --- a/config/crd/bases/test.openstack.org_horizontests.yaml +++ b/config/crd/bases/test.openstack.org_horizontests.yaml @@ -886,6 +886,11 @@ spec: default: false description: Parallel 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: |- diff --git a/config/crd/bases/test.openstack.org_tempests.yaml b/config/crd/bases/test.openstack.org_tempests.yaml index 394fd0d0..7f54e32a 100644 --- a/config/crd/bases/test.openstack.org_tempests.yaml +++ b/config/crd/bases/test.openstack.org_tempests.yaml @@ -878,6 +878,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: |- @@ -1451,6 +1456,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 e7ce768e..68a624ff 100644 --- a/config/crd/bases/test.openstack.org_tobikoes.yaml +++ b/config/crd/bases/test.openstack.org_tobikoes.yaml @@ -884,6 +884,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 @@ -1144,6 +1149,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/config/manifests/bases/test-operator.clusterserviceversion.yaml b/config/manifests/bases/test-operator.clusterserviceversion.yaml index e1e4b260..0e5074e5 100644 --- a/config/manifests/bases/test-operator.clusterserviceversion.yaml +++ b/config/manifests/bases/test-operator.clusterserviceversion.yaml @@ -108,6 +108,11 @@ spec: secure.yaml displayName: Open Stack Config Secret path: openStackConfigSecret + - description: |- + Timeout after which a test pod in the Pending state is considered + stuck and its execution is terminated. + displayName: Pending Timeout + path: pendingTimeout - description: |- Use with caution! This parameter specifies whether test-operator should spawn test pods with allowedPrivilegedEscalation: true, readOnlyRootFilesystem: false, @@ -209,6 +214,11 @@ spec: secure.yaml displayName: Open Stack Config Secret path: workflow[0].openStackConfigSecret + - description: |- + Timeout after which a test pod in the Pending state is considered + stuck and its execution is terminated. + displayName: Pending Timeout + path: workflow[0].pendingTimeout - description: |- Use with caution! This parameter specifies whether test-operator should spawn test pods with allowedPrivilegedEscalation: true and the default capabilities on @@ -337,6 +347,11 @@ spec: - description: Parallel displayName: Parallel path: parallel + - description: |- + Timeout after which a test pod in the Pending state is considered + stuck and its execution is terminated. + displayName: Pending Timeout + path: pendingTimeout - description: |- Use with caution! This parameter specifies whether test-operator should spawn test pods with allowedPrivilegedEscalation: true, readOnlyRootFilesystem: false, @@ -461,6 +476,11 @@ spec: behaviour then set this option to true. displayName: Parallel path: parallel + - description: |- + Timeout after which a test pod in the Pending state is considered + stuck and its execution is terminated. + displayName: Pending Timeout + path: pendingTimeout - description: |- Use with caution! This parameter specifies whether test-operator should spawn test pods with allowedPrivilegedEscalation: true, readOnlyRootFilesystem: false, @@ -777,6 +797,11 @@ spec: behaviour then set this option to true. displayName: Parallel path: workflow[0].parallel + - description: |- + Timeout after which a test pod in the Pending state is considered + stuck and its execution is terminated. + displayName: Pending Timeout + path: workflow[0].pendingTimeout - description: |- Use with caution! This parameter specifies whether test-operator should spawn test pods with allowedPrivilegedEscalation: true and the default capabilities on @@ -1108,6 +1133,11 @@ spec: - description: Optional patch to apply to the Tobiko repository. displayName: Patch path: patch + - description: |- + Timeout after which a test pod in the Pending state is considered + stuck and its execution is terminated. + displayName: Pending Timeout + path: pendingTimeout - description: Boolean specifying whether tobiko tests create new resources or re-use those previously created displayName: Prevent Create @@ -1221,6 +1251,11 @@ spec: - description: Optional patch to apply to the Tobiko repository for this step. displayName: Patch path: workflow[0].patch + - description: |- + Timeout after which a test pod in the Pending state is considered + stuck and its execution is terminated. + displayName: Pending Timeout + path: workflow[0].pendingTimeout - description: Boolean specifying whether tobiko tests create new resources or re-use those previously created displayName: Prevent Create diff --git a/internal/controller/common.go b/internal/controller/common.go index dd373ad2..2e77cfae 100644 --- a/internal/controller/common.go +++ b/internal/controller/common.go @@ -42,6 +42,7 @@ const ( workflowStepLabel = "workflowStep" instanceNameLabel = "instanceName" operatorNameLabel = "operator" + pendingTimeoutAnnotation = "test.openstack.org/pending-timeout" testOperatorLockName = "test-operator-lock" testOperatorLockOwnerField = "owner" testOperatorBaseDir = "/etc/test_operator/" @@ -59,6 +60,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 @@ -204,6 +207,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 44ad7194..d6c26acf 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() } @@ -209,8 +211,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)