feature: run a clean-up job when uninstalling #3339

ty-dc · 2024-04-07T01:49:14Z

Thanks for contributing!

What type of PR is this?

What this PR does / why we need it:
close #2653

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Remove all spiderpool mutatingwebhookconfigurations
Remove all spiderpool validatingwebhookconfigurations
Remove all spiderpool CR：SpiderIPPool，SpiderSubnet、SpiderEndpoint, etc.

codecov · 2024-04-07T01:52:09Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.31%. Comparing base (dc8e9af) to head (a9fc861).

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #3339   +/-   ##
=======================================
  Coverage   81.31%   81.31%           
=======================================
  Files          50       50           
  Lines        4352     4352           
=======================================
  Hits         3539     3539           
  Misses        661      661           
  Partials      152      152

Flag	Coverage Δ
unittests	`81.31% <ø> (ø)`

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

weizhoublue · 2024-04-07T03:28:25Z

could it be tested in a E2E case ?

weizhoublue · 2024-04-07T03:33:16Z

cmd/spiderpool-controller/cmd/clean.go

+
+			err = c.Delete(ctx, &item)
+			if err != nil {
+				return err


for cleaning job, it should not be interrupted, and clean as much as possible

cmd/spiderpool-controller/cmd/clean.go

ty-dc · 2024-04-10T03:11:13Z

Run codecov/[email protected]
  with:
    directory: ./
    files: coverage.out
    flags: unittests
    name: my-codecov-umbrella
    fail_ci_if_error: true
    verbose: true

should look like this:

Run codecov/[email protected]
  with:
    directory: ./
    files: coverage.out
    flags: unittests
    name: my-codecov-umbrella
    fail_ci_if_error: true
    verbose: true
    token: ***

cmd/spiderpool-controller/cmd/clean.go

weizhoublue · 2024-04-11T04:57:20Z

cmd/spiderpool-controller/cmd/clean.go

+		}
+	}
+
+	return nil


中间有哪个清除失败了，最终返回错误，反应在 job 的状态中

weizhoublue · 2024-04-12T03:13:00Z

cmd/spiderpool-controller/cmd/clean.go

+			if err != nil {
+				return fmt.Errorf("failed to update spiderEndpoint's: %v finalizers to be empty: %v ", &item, err)
+			}
+			logger.Sugar().Infof("succeeded to update spiderEndpoint's: %v finalizers to be empty", &item)


succeeded to clean the finalizers of spiderEndpoint xxxx

weizhoublue · 2024-04-15T05:47:49Z

charts/spiderpool/values.yaml

@@ -757,6 +757,10 @@ spiderpoolController:
      ## @param spiderpoolController.tls.auto.extraDnsNames extra DNS names of server cert for auto method
      extraDnsNames: []

+  cleanup:


mention it in the document of uninstalling

weizhoublue · 2024-04-15T05:50:25Z

cmd/spiderpool-controller/cmd/clean.go

+	if err == nil {
+		err := c.Delete(ctx, mObj)
+		if err != nil {
+			return fmt.Errorf("failed to delete MutatingWebhookConfiguration: %v.", err)


卸载是尽最大可能删除东西，任何删除失败的动作，不要 return ，而是日志后 continue 继续删除其他

weizhoublue · 2024-04-15T05:52:51Z

@cyclinder 现在垃圾资源的清除，除了 Spiderpool 本身，其它项目的垃圾资源是否还需要帮助删除？

windsonsea

Headings can be shorter and better

windsonsea · 2024-04-15T08:30:42Z

docs/usage/install/uninstall.md

+
+     Replace `<spiderpool-name>` with the name of the Spiderpool you want to uninstall and `<spiderpool-namespace>` with the namespace of the Spiderpool.
+
+### In versions higher than 0.9.2


Above v0.9.2

windsonsea · 2024-04-15T08:30:55Z

docs/usage/install/uninstall.md

+kubectl get spidercoordinators.spiderpool.spidernet.io -o name | wc -l
+```
+
+### In versions lower than 0.9.2


Suggested change

### In versions lower than 0.9.2

### Below v0.9.2

windsonsea · 2024-04-15T08:31:37Z

docs/usage/install/uninstall-zh_CN.md

+
+    将 `<spiderpool-name>` 替换为要卸载的 Spiderpool 的名称，将 `<spiderpool-namespace>` 替换为 Spiderpool 所在的命名空间。
+
+### 在高于 0.9.2 的版本


Suggested change

### 在高于 0.9.2 的版本

### 0.9.2 以上版本

windsonsea · 2024-04-15T08:31:53Z

docs/usage/install/uninstall-zh_CN.md

+kubectl get spidercoordinators.spiderpool.spidernet.io -o name | wc -l
+```
+
+### 在低于 0.9.2 的版本


Suggested change

### 在低于 0.9.2 的版本

### 0.9.2 以下版本

cyclinder

Can you update your PR description to list all the resources you clean? I think we should clean up the below resources in order:

All the CRD instances: IPPools,Subnets,SpiderMultusConfig...
The CRD definitions.
Pods that have not been completely deleted, Example sriov-network-config-daemon, see clean sriov pod #2822

ty-dc · 2024-04-16T02:21:50Z

Can you update your PR description to list all the resources you clean? I think we should clean up the below resources in order:

All the CRD instances: IPPools,Subnets,SpiderMultusConfig...

The CRD definitions. CRD 定义。

Pods that have not been completely deleted, Example sriov-network-config-daemon, see clean sriov pod #2822

updated，Step 2 Do it happily. But step 3, what if the customer installed it themselves? Is there any way we can know whether a resource like sirov was installed via spiderpool?

docs/usage/install/uninstall-zh_CN.md

weizhoublue · 2024-04-16T09:35:58Z

docs/usage/install/uninstall-zh_CN.md

+
+### 0.9.2 以上版本
+
+在 0.9.2 之后引入了自动清理 Spiderpool 资源的功能，它通过 `spiderpoolController.cleanup.enabled` 配置项来启用，该值默认为 `true`，您可以通过如下方式验证与 Spiderpool 相关的资源数量是否自动被清理。


为什么是 0.9.2 ，而不是下一个 0.10 什么的版本
这个也不会进 0.9.3

Signed-off-by: tao.yang <[email protected]>

ty-dc added the release/feature-new release note for new feature label Apr 7, 2024

ty-dc requested review from weizhoublue and Icarus9913 as code owners April 7, 2024 01:49

ty-dc force-pushed the feature/uninstall branch 4 times, most recently from 953fa75 to b17898e Compare April 7, 2024 02:18

ty-dc added the pr/not-ready not ready for merging label Apr 7, 2024

weizhoublue reviewed Apr 7, 2024

View reviewed changes

cmd/spiderpool-controller/cmd/clean.go Show resolved Hide resolved

ty-dc force-pushed the feature/uninstall branch 6 times, most recently from e2440a5 to 06ebb91 Compare April 9, 2024 02:23

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

weizhoublue reviewed Apr 11, 2024

View reviewed changes