Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: If the PodList contains extra Pods, the judgment will fail. #229

Closed
wants to merge 1 commit into from

Conversation

ty-dc
Copy link
Contributor

@ty-dc ty-dc commented Aug 5, 2024

DeletePodListUntilReady 函数传入了 podList 入参,但是当在删除 PodList 前获取到的 PodList 可能是包含了 terminal 状态的 Pod ,此时去对比重启前后的 Pod 数,那么就无法成功。因为 len(podListWithLabel.Items) 将一定不等于 len(podList.Items)

func (f *Framework) DeletePodListUntilReady(podList *corev1.PodList, timeOut time.Duration, opts ...client.DeleteOption) (*corev1.PodList, error) {

pod 删除处于 terminal 状态的持续时间是不可控的,虽然可以通过 优雅期强制删除,但是对于 spiderpool-controller 这类特殊的 Pod ,强制删除后,由于资源没有回收完全,新的 spiderpool-controller Pod 可能由于端口冲突等问题而重启,而重启后,spiderpool 的 CI 日志采集脚本会判定 Pod 重启过,而 CI 失败

在 DeletePodListUntilReady 中,增加 expectedPodNum 字段,用于判断你所需要的 Pod 数量,不再去通过传入的 len(PodList.Iterms)去判断 Pod 的副本数是否符合预期,这样行为由于环境的不同,pod 终结快慢等影响,稳定性不可控。
同时尽可能的不采取强制删除 Pod 的方式,避免出现端口冲突而重启的情况,被 CI 日志采集脚本捕获判定失败。

@ty-dc ty-dc requested a review from weizhoublue as a code owner August 5, 2024 03:37
@ty-dc ty-dc added the pr/ready-review This pull is ready for review label Aug 5, 2024
@ty-dc ty-dc force-pushed the fix/deletepod branch 2 times, most recently from e067d22 to 8abb4f8 Compare August 5, 2024 06:26
Copy link

codecov bot commented Aug 5, 2024

Codecov Report

Attention: Patch coverage is 0% with 3 lines in your changes missing coverage. Please review.

Project coverage is 68.80%. Comparing base (71bf7b1) to head (27794c4).
Report is 2 commits behind head on main.

Files Patch % Lines
framework/pod.go 0.00% 2 Missing ⚠️
framework/deployment.go 0.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #229      +/-   ##
==========================================
+ Coverage   67.76%   68.80%   +1.04%     
==========================================
  Files          17       17              
  Lines        1554     1247     -307     
==========================================
- Hits         1053      858     -195     
+ Misses        416      304     -112     
  Partials       85       85              
Flag Coverage Δ
unittests 68.80% <0.00%> (+1.04%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
framework/deployment.go 64.24% <0.00%> (+1.68%) ⬆️
framework/pod.go 57.45% <0.00%> (-1.48%) ⬇️

... and 14 files with indirect coverage changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr/ready-review This pull is ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant