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

feat: support pdb with v1 and v1beta version to control pod's deletion #1681

Merged
merged 35 commits into from
Jul 25, 2023

Conversation

jiuker
Copy link
Contributor

@jiuker jiuker commented Jul 12, 2023

support pdb

  • create tenant
  • expand tenant
  • decomm pool from tenant
  • delete tenant

Test steps:
1.start a tenant (4*4)
2. wait for tenant's status are ready
3.kubectl get pdb -A you will see that.
4. And delete the all pod. You will see delete the pod one by one.(For MinAvailable are 3)

@jiuker jiuker marked this pull request as draft July 12, 2023 08:11
@jiuker jiuker marked this pull request as ready for review July 18, 2023 03:13
guozhi.li added 2 commits July 18, 2023 11:22
@jiuker jiuker marked this pull request as draft July 18, 2023 03:43
@jiuker jiuker marked this pull request as ready for review July 18, 2023 04:23
pkg/controller/pdb.go Outdated Show resolved Hide resolved
pkg/controller/pdb.go Show resolved Hide resolved
@jiuker jiuker requested a review from shtripat July 18, 2023 06:33
@shtripat
Copy link
Contributor

@jiuker please add verification steps

@shtripat
Copy link
Contributor

I followed the below steps

  1. TAG=minio/operator:pdb make all
  2. kind create cluster --config kind-config.yaml
  3. kind load docker-image docker.io/minio/operator:pdb
  4. k kustomize . > operator-new.yaml
  5. Changed the operator image value in operator-new.yaml with docker.io/minio/operator:pdb
  6. k apply -f operator-new.yaml
  7. Once operator pods ready, created a tenant from operator console
  8. k logs minio-operator-7db874b9b-4pw9z -n minio-operator shows error as below
I0718 11:00:37.918185       1 pdb.go:259] PodDisruptionBudget: v1
E0718 11:00:37.921171       1 main-controller.go:676] error syncing 'tenant-ns/tenant-1': poddisruptionbudgets.policy "tenant-1-pool-0" is forbidden: User "system:serviceaccount:minio-operator:minio-operator" cannot get resource "poddisruptionbudgets" in API group "policy" in the namespace "tenant-ns"

Also there are no pdb created

$ k get pdb -A
No resources found

@jiuker
Copy link
Contributor Author

jiuker commented Jul 18, 2023

@shtripat I have fixed the ClusterRole. Test again plz. And you should update it.

@jiuker jiuker marked this pull request as draft July 18, 2023 13:14
guozhi.li added 2 commits July 19, 2023 18:33
@jiuker jiuker marked this pull request as ready for review July 19, 2023 10:37
@jiuker jiuker requested a review from shtripat July 19, 2023 10:37
@jiuker jiuker marked this pull request as draft July 19, 2023 10:41
@jiuker jiuker marked this pull request as ready for review July 19, 2023 10:45
@shtripat
Copy link
Contributor

Below scenarios work fine

  1. Create tenant
  2. Delete tenant
  3. Expand tenant

And PDBs get created for pools of the tenant.

Tried below scenario manually

  1. Edited tenant and removed one pool from the list
  2. deleted the stateful set for corresponding pool
  3. PDB doesnt get deleted and error in operator pod error syncing 'tenant-ns/tenant-1': poddisruptionbudgets.policy is forbidden: User "system:serviceaccount:minio-operator:minio-operator" cannot deletecollection resource "poddisruptionbudgets" in API group "policy" in the namespace "tenant-ns"

Copy link
Contributor

@shtripat shtripat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Verified below scenarios using kustomize plugin

  1. Create tenant (pdb creates)
  2. Expand tenant (pdb cerates for new pool)
  3. Decomm pool by editing YAML in console (pdb gets deleted for decomm'ed pool)
  4. Delete tenant (all pdbs get deleted)

@jiuker jiuker changed the title support pdb feat: support pdb with v1 and v1beta version to control pod's deletion Jul 20, 2023
@jiuker
Copy link
Contributor Author

jiuker commented Jul 20, 2023

@harshavardhana @pjuarezd PATL

@harshavardhana harshavardhana merged commit c482471 into minio:master Jul 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants