Added debug log of AllocatedIPCount of ippool #3926

ty-dc · 2024-08-22T13:54:05Z

Thanks for contributing!

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes ##3771

Special notes for your reviewer:

codecov · 2024-08-22T13:57:51Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.84%. Comparing base (b64851c) to head (a9069ca).
Report is 6 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #3926   +/-   ##
=======================================
  Coverage   80.83%   80.84%           
=======================================
  Files          51       51           
  Lines        4514     4516    +2     
=======================================
+ Hits         3649     3651    +2     
  Misses        699      699           
  Partials      166      166

Flag	Coverage Δ
unittests	`80.84% <100.00%> (+<0.01%)`	⬆️

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

Files with missing lines	Coverage Δ
pkg/ippoolmanager/ippool_manager.go	`89.38% <100.00%> (+0.09%)`	⬆️

weizhoublue · 2024-08-22T16:05:32Z

pkg/gcmanager/scanAll_IPPool.go

@@ -371,6 +379,18 @@ func (s *SpiderGC) executeScanAll(ctx context.Context) {
 					}
 				}
 			}
+
+			if *pool.Status.AllocatedIPCount != int64(tempAllocatedIPCount) {


这肯定是会产生新 bug 的，
更新 ipool 都是基于 resource version，这个统计必须是在基于相同的 resource version
前面在统计 ip 使用量过程中，一旦成功做了 gc ip ， resource version 就变了。再拿着老的基于老 resource version 统计的 ip用量来更新，如果此时有 ip 分配或者释放的并发，或者 ip 池 ip 的添加，这个老的统计ip 用量再刷进去就是错误的

UpdateAllocatedIPCount 判断了更新 apierrors.IsConflict(err) ？如果 resource version 变了，打印日志，更新失败。

你已经再想想，你统计用量时的version 和你刷新的version 是同一个么？

的确不是同一个。

weizhoublue · 2024-08-23T06:19:56Z

pkg/ippoolmanager/ippool_manager.go

+func (im *ipPoolManager) UpdateAllocatedIPCount(ctx context.Context, poolName string, allocatedIPCount *int64) error {
+	logger := logutils.FromContext(ctx)
+
+	ipPool, err := im.GetIPPoolByName(ctx, poolName, constant.IgnoreCache)


这个version 有啥用，不是统计状态时的version，这期间的统计值变了多少哦，你没弄明白version的意义

整个pr思路错了

已纠正。

weizhoublue · 2024-08-23T07:05:44Z

pkg/gcmanager/scanAll_IPPool.go

@@ -352,6 +359,7 @@ func (s *SpiderGC) executeScanAll(ctx context.Context) {

 			GCIP:
 				if flagGCIPPoolIP {
+					tempAllocatedIPCount--


这个代码写的复杂了，不就统计下已用 ip 中的数组成员数量，就出来了？
也不需要加来减去的

weizhoublue · 2024-08-23T07:20:24Z

需要找出用量统计的根因，gc并不能解决真正的问题，只是事后补救，这是两个纬度

ty-dc · 2024-08-23T08:25:30Z

需要找出用量统计的根因，gc并不能解决真正的问题，只是事后补救，这是两个纬度

原本分配 IP 的流程，是在已有的 allcateIPCount 数量++，如果分配地址时，并发进行一个在分配、一个在释放，而释放的时候 AllocatedIPCount 还没有完成 -- 这个动作，而分配的时候拿到的是未 -- 的 AllocatedIPCount ，并进行了 ++ ，从而导致出现了统计出现不正确的现象（猜测，暂未能证实猜想）

ty-dc · 2024-08-23T08:33:57Z

test/e2e/reclaim/reclaim_test.go

@@ -586,6 +586,23 @@ var _ = Describe("test ip with reclaim ip case", Label("reclaim"), func() {
 			// Delete Pod
 			Expect(frame.DeletePod(podName, namespace)).To(Succeed(), "Failed to delete pod %v/%v\n", namespace, podName)
 			GinkgoWriter.Printf("succeed to delete pod %v/%v\n", namespace, podName)
+
+			// Check whether the dirty IP data is recovered successfully and whether the AllocatedIPCount decreases and meets expectations?


用例是在手动更新 AllocatedIPCount 为一个错误的值，期望在新的改进引入后，分配 IP 或则释放 IP，或则 gc all 回收异常 IP，都能够纠正这个 AllocatedIPCount 值。

IPPool 的状态是健壮的。

Signed-off-by: ty-dc <tao.yang@daocloud.io>

weizhoublue · 2024-09-09T12:20:15Z

(1) 这个日志真有能帮助 debug 问题根因么
（2）这个 pr 的 title 并不能 fix

ty-dc · 2024-09-10T07:32:14Z

(1) 这个日志真有能帮助 debug 问题根因么

当前没有任何日志去获悉 ippool 的 AllocatedIPCount 数量在什么时候开始出现异常的。AllocatedIPCount++，AllocatedIPCount-- 只出现在文中两处，补充这样的日志，当 IPPool 分配与释放 IP 打印当前的 AllocatedIPCount，能够知道在哪一次分配和释放出现的记录错误的问题。

weizhoublue · 2024-09-12T01:45:02Z

(1) 这个日志真有能帮助 debug 问题根因么

当前没有任何日志去获悉 ippool 的 AllocatedIPCount 数量在什么时候开始出现异常的。AllocatedIPCount++，AllocatedIPCount-- 只出现在文中两处，补充这样的日志，当 IPPool 分配与释放 IP 打印当前的 AllocatedIPCount，能够知道在哪一次分配和释放出现的记录错误的问题。

你再想想，你这行日志真的能进行生产监控么
一直在这里 debug 级别打印数字

ty-dc added the pr/not-ready not ready for merging label Aug 22, 2024

ty-dc requested review from weizhoublue and bzsuni as code owners August 22, 2024 13:54

weizhoublue reviewed Aug 22, 2024

View reviewed changes

ty-dc force-pushed the fix/error-allocatedipcount branch from e1cda70 to d2cedea Compare August 23, 2024 02:16

weizhoublue reviewed Aug 23, 2024

View reviewed changes

ty-dc force-pushed the fix/error-allocatedipcount branch from d2cedea to 42941c7 Compare August 23, 2024 08:30

ty-dc commented Aug 23, 2024

View reviewed changes

ty-dc added release/bug cherrypick-release-v0.8 Cherry-pick the PR to branch release-v0.8. cherrypick-release-v0.9 cherrypick-release-v1.0 Cherry-pick the PR to branch release-v1.0. and removed pr/not-ready not ready for merging labels Aug 23, 2024

ty-dc force-pushed the fix/error-allocatedipcount branch from 42941c7 to e7ee784 Compare August 23, 2024 09:25

ty-dc mentioned this pull request Aug 26, 2024

coordinator: fix wrong policy route when there is more than 1 secondary nics #3873

Merged

weizhoublue added pr/not-ready not ready for merging and removed cherrypick-release-v0.8 Cherry-pick the PR to branch release-v0.8. cherrypick-release-v0.9 cherrypick-release-v1.0 Cherry-pick the PR to branch release-v1.0. labels Sep 3, 2024

ty-dc force-pushed the fix/error-allocatedipcount branch from e7ee784 to f59f549 Compare September 9, 2024 10:10

fix: the statistics of ippool's status.AllocatedIPCount are inaccurate

a9069ca

Signed-off-by: ty-dc <tao.yang@daocloud.io>

ty-dc force-pushed the fix/error-allocatedipcount branch from f59f549 to a9069ca Compare September 9, 2024 10:11

ty-dc removed the pr/not-ready not ready for merging label Sep 9, 2024

ty-dc changed the title ~~fix: the statistics of ippool's status.AllocatedIPCount are inaccurate~~ Added debug log of AllocatedIPCount of ippool Sep 10, 2024

ty-dc removed the release/bug label Sep 10, 2024

ty-dc added the release/none no release note label Sep 10, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added debug log of AllocatedIPCount of ippool #3926

Added debug log of AllocatedIPCount of ippool #3926

ty-dc commented Aug 22, 2024 •

edited

Loading

codecov bot commented Aug 22, 2024 •

edited

Loading

weizhoublue Aug 22, 2024 •

edited

Loading

ty-dc Aug 23, 2024

weizhoublue Aug 23, 2024

ty-dc Aug 23, 2024

weizhoublue Aug 23, 2024 •

edited

Loading

weizhoublue Aug 23, 2024

ty-dc Aug 23, 2024

weizhoublue Aug 23, 2024 •

edited

Loading

ty-dc Aug 23, 2024

weizhoublue commented Aug 23, 2024 •

edited

Loading

ty-dc commented Aug 23, 2024

ty-dc Aug 23, 2024 •

edited

Loading

weizhoublue commented Sep 9, 2024

ty-dc commented Sep 10, 2024

weizhoublue commented Sep 12, 2024 •

edited

Loading

Added debug log of AllocatedIPCount of ippool #3926

Are you sure you want to change the base?

Added debug log of AllocatedIPCount of ippool #3926

Conversation

ty-dc commented Aug 22, 2024 • edited Loading

Thanks for contributing!

What type of PR is this?

codecov bot commented Aug 22, 2024 • edited Loading

Codecov Report

weizhoublue Aug 22, 2024 • edited Loading

Choose a reason for hiding this comment

ty-dc Aug 23, 2024

Choose a reason for hiding this comment

weizhoublue Aug 23, 2024

Choose a reason for hiding this comment

ty-dc Aug 23, 2024

Choose a reason for hiding this comment

weizhoublue Aug 23, 2024 • edited Loading

Choose a reason for hiding this comment

weizhoublue Aug 23, 2024

Choose a reason for hiding this comment

ty-dc Aug 23, 2024

Choose a reason for hiding this comment

weizhoublue Aug 23, 2024 • edited Loading

Choose a reason for hiding this comment

ty-dc Aug 23, 2024

Choose a reason for hiding this comment

weizhoublue commented Aug 23, 2024 • edited Loading

ty-dc commented Aug 23, 2024

ty-dc Aug 23, 2024 • edited Loading

Choose a reason for hiding this comment

weizhoublue commented Sep 9, 2024

ty-dc commented Sep 10, 2024

weizhoublue commented Sep 12, 2024 • edited Loading

ty-dc commented Aug 22, 2024 •

edited

Loading

codecov bot commented Aug 22, 2024 •

edited

Loading

weizhoublue Aug 22, 2024 •

edited

Loading

weizhoublue Aug 23, 2024 •

edited

Loading

weizhoublue Aug 23, 2024 •

edited

Loading

weizhoublue commented Aug 23, 2024 •

edited

Loading

ty-dc Aug 23, 2024 •

edited

Loading

weizhoublue commented Sep 12, 2024 •

edited

Loading