release conflicted ip of stateless workload to trigger assigning a new one #3081

Icarus9913 · 2024-01-12T02:44:43Z

Once starting a stateless Pod, the spiderpool will reuse the first allocated IPs from the SpiderEndpoint resource for the Pod if it met some errors. If the coordinator detected the IP conflict, spiderpool will still reuse the conflicted IPs. With this, the Pod would never starts.

So, I added a feature that let coordinator send a signal to spiderpool-agent component to release its whole NICs IPs and let the spiderpool reallocate IPs for the Pod. It might let the Pod starts correctly.

Signed-off-by: Icarus9913 [email protected]

codecov · 2024-01-12T02:47:50Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (46b150e) 81.07% compared to head (c7e0ce3) 81.12%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3081      +/-   ##
==========================================
+ Coverage   81.07%   81.12%   +0.05%     
==========================================
  Files          50       50              
  Lines        5358     5373      +15     
==========================================
+ Hits         4344     4359      +15     
  Misses        856      856              
  Partials      158      158

Flag	Coverage Δ
unittests	`81.12% <100.00%> (+0.05%)`	⬆️

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

Files	Coverage Δ
...orkloadendpointmanager/workloadendpoint_manager.go	`100.00% <100.00%> (ø)`

weizhoublue · 2024-01-15T14:48:40Z

(1) 这应该在相关 doc 中有说明：对于无状态 pod，XXXX
(2) 有用例闭环

docs/concepts/coordinator-zh_CN.md

cmd/coordinator/cmd/command_add.go

cmd/spiderpool-agent/cmd/config.go

weizhoublue · 2024-01-21T14:38:26Z

docs/concepts/coordinator.md

@@ -57,7 +57,8 @@ For more information about the Underlay Pod not being able to access the Cluster
 ## Detect Pod IP conflicts(alpha)

 IP conflicts are unacceptable for underlay networks, which can cause serious problems. When creating a pod, we can use the `coordinator` to detect whether the IP of the pod conflicts, and support both IPv4 and IPv6 addresses. By sending an ARP or NDP probe message,
-If the MAC address of the reply packet is not the pod itself, we consider the IP to be conflicting and reject the creation of the pod with conflicting IP addresses:
+If the MAC address of the reply packet is not the pod itself, we consider the IP to be conflicting and reject the creation of the pod with conflicting IP addresses.


这个翻译过来的有点中式
If the MAC address of the reply packet is belonged to the pod itself, the IP address is recognized as conflicted ....
@windsonsea 给一些建议

weizhoublue · 2024-01-21T14:39:54Z

pkg/ipam/release.go

+	log := logutils.FromContext(ctx)
+
+	var pod *corev1.Pod
+	// with IP release, we must need Pod UID


it needs to offer Pod UID when releasing ip

weizhoublue · 2024-01-21T14:40:40Z

pkg/ipam/release.go

+				return fmt.Errorf("failed to get the top controller of the Pod %s/%s, error: %v", pod.Namespace, pod.Name, err)
+			}
+
+			// do not release conflict IPs for stable Pod


do not release conflict IPs for stateful Pod

pkg/ipam/release.go

weizhoublue · 2024-01-21T14:43:42Z

pkg/ipam/release.go

+				return nil
+			}
+		} else {
+			log.Warn("EnableReleaseConflictIPs is disabled, skip to release IPs")


is there IP information in the log ?

weizhoublue · 2024-01-21T14:44:02Z

pkg/ipam/release.go

+		}
+	}
+
+	// release SpiderEndpoint IPs


release IP for stateless endpoint

该接口为统一接口，不区分是否为stateless

I seems to be dedicated for stateless endpoint after line 257 , so I mean that the comment could be the following, and help code reviewer a lot

// release IP for stateless endpoint

pkg/ipam/release.go

api/v1/agent/openapi.yaml

weizhoublue · 2024-01-28T14:42:40Z

test/e2e/coordinator/macvlan-overlay-one/macvlan_overlay_one_test.go

+				output, err := frame.DockerExecCommand(ctx, common.VlanGatewayContainer, commandV6Str)
+				Expect(err).NotTo(HaveOccurred(), "Failed to exec %s for Node %s, error is: %v, log: %v", commandV6Str, common.VlanGatewayContainer, err, string(output))
+			}
+		})


need a case for stateful pod whose IP should not be released ? add to the test document and it could be implemented next month

Signed-off-by: Icarus9913 <[email protected]>

Icarus9913 added pr/not-ready not ready for merging release/feature-new release note for new feature labels Jan 12, 2024

Icarus9913 requested review from weizhoublue and windsonsea as code owners January 12, 2024 02:44

Icarus9913 force-pushed the feat/wk/ip-conflict-release branch 2 times, most recently from 1f20326 to d07922c Compare January 15, 2024 10:06

weizhoublue changed the title ~~add ip release for ip conflict situation~~ release conflict ip of stateless workload to trigger assigning a new one Jan 15, 2024

weizhoublue changed the title ~~release conflict ip of stateless workload to trigger assigning a new one~~ release conflicted ip of stateless workload to trigger assigning a new one Jan 15, 2024

Icarus9913 force-pushed the feat/wk/ip-conflict-release branch 2 times, most recently from 8400439 to 098b161 Compare January 18, 2024 09:09

Icarus9913 requested review from ty-dc and bzsuni as code owners January 18, 2024 09:09

Icarus9913 force-pushed the feat/wk/ip-conflict-release branch 4 times, most recently from 37e77f5 to 7efdb15 Compare January 19, 2024 02:57

Icarus9913 added pr/ready-review This pull is ready for review and removed pr/not-ready not ready for merging labels Jan 19, 2024

weizhoublue reviewed Jan 19, 2024

View reviewed changes

docs/concepts/coordinator-zh_CN.md Outdated Show resolved Hide resolved

Icarus9913 force-pushed the feat/wk/ip-conflict-release branch from 7efdb15 to c2d1c72 Compare January 19, 2024 09:38