Fix: Statefulset pod should change IP when recreating with a changed pool in annotation #3669

lou-lan · 2024-06-26T09:52:18Z

codecov · 2024-06-26T09:56:18Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.16%. Comparing base (5566f50) to head (670dab2).
Report is 10 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3669      +/-   ##
==========================================
- Coverage   81.21%   81.16%   -0.05%     
==========================================
  Files          50       50              
  Lines        4391     4391              
==========================================
- Hits         3566     3564       -2     
- Misses        669      670       +1     
- Partials      156      157       +1

Flag	Coverage Δ
unittests	`81.16% <ø> (-0.05%)`	⬇️

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

see 1 file with indirect coverage changes

weizhoublue · 2024-06-26T10:48:14Z

need add a new E2E for this bug

weizhoublue · 2024-06-26T10:57:45Z

pkg/statefulsetmanager/sts_manager.go

@@ -70,7 +73,7 @@ func (sm *statefulSetManager) ListStatefulSets(ctx context.Context, cached bool,
 // IsValidStatefulSetPod only serves for StatefulSet pod, it will check the pod whether need to be cleaned up with the given params podNS, podName.
 // Once the pod's controller StatefulSet was deleted, the pod's corresponding IPPool IP and Endpoint need to be cleaned up.
 // Or the pod's controller StatefulSet decreased its replicas and the pod's index is out of replicas, it needs to be cleaned up too.
-func (sm *statefulSetManager) IsValidStatefulSetPod(ctx context.Context, namespace, podName, podControllerType string) (bool, error) {
+func (sm *statefulSetManager) IsValidStatefulSetPod(ctx context.Context, namespace, podName, podControllerType string, endpoint *spiderpoolv2beta1.SpiderEndpoint) (bool, error) {


我觉得这个 api 是验证 pod 是否是一个有效的 statefulset pod 副本，是否在副本数范围内。而新加入的代码却使得该函数超出了本身的功能定位，成为一个非常杂的函数。它甚至影响了 GC 等场景的IP回收场景的行为，会产生bug ，对现有代码逻辑影响比较大

新代码我建议是独立为新一个函数来调用，如 verifyStaterfulSetPodIPValidity() ，来确认当前的 st pod 的 ip 是否是有效的，是否符合 annotation

pkg/ipam/release.go

weizhoublue · 2024-06-27T11:09:43Z

pkg/ipam/allocate.go

@@ -98,6 +145,22 @@ func (i *ipam) Allocate(ctx context.Context, addArgs *models.IpamAddArgs) (*mode
 	return addResp, nil
 }

+func checkEndpointInPool(endpointMap, poolMap map[string]map[string]struct{}) bool {


这个 api 可以移动到 utils.go 下，然后换个表述可能会更好理解函数的作用

checkNicPoolExistence( current, expected map[string]map[string]struct{} ) bool

pkg/ipam/allocate.go

weizhoublue · 2024-06-27T11:18:14Z

pkg/ipam/allocate.go

+					endpointMap[ip.NIC][*ip.IPv6Pool] = struct{}{}
+				}
+			}
+			if !checkEndpointInPool(endpointMap, poolMap) {


如果不需要释放，也需要一条 debug 日志，知道代码做了什么检查
stateful pod 的 ip （什么ip，属于什么ippool）符合当前期望的 ippool （ ippool 打印出来），不需要释放 ip

pkg/ipam/allocate.go

weizhoublue · 2024-06-28T03:07:48Z

modify the relevant doc

lou-lan · 2024-06-28T06:27:43Z

need add a new E2E for this bug

A00009

pkg/ipam/allocate.go

docs/usage/statefulset.md

Signed-off-by: lou-lan <loulan@loulan.me>

Fix: Statefulset pod should change IP when recreating with a changed pool in annotation Signed-off-by: robot <tao.yang@daocloud.io>

lou-lan requested a review from weizhoublue as a code owner June 26, 2024 09:52

lou-lan requested review from cyclinder and ty-dc June 26, 2024 09:52

lou-lan force-pushed the fix/inconsistent-pool-names branch from 21368fd to 039d17f Compare June 26, 2024 09:53

lou-lan added the release/bug label Jun 26, 2024

lou-lan changed the title ~~Fix create pod use old ip when update StatefulSet pool name with anno…~~ Fix Pod use old IP when update StatefulSet pool name with annotation Jun 26, 2024

lou-lan force-pushed the fix/inconsistent-pool-names branch 2 times, most recently from b244c4b to f514448 Compare June 26, 2024 10:03

weizhoublue reviewed Jun 26, 2024

View reviewed changes

pkg/ipam/release.go Outdated Show resolved Hide resolved

weizhoublue reviewed Jun 26, 2024

View reviewed changes

pkg/ipam/release.go Outdated Show resolved Hide resolved

lou-lan force-pushed the fix/inconsistent-pool-names branch 3 times, most recently from 27f42c8 to 7032602 Compare June 27, 2024 09:02

weizhoublue reviewed Jun 27, 2024

View reviewed changes

pkg/ipam/allocate.go Show resolved Hide resolved

weizhoublue reviewed Jun 27, 2024

View reviewed changes

pkg/ipam/allocate.go Show resolved Hide resolved

weizhoublue reviewed Jun 27, 2024

View reviewed changes