Repository: minio/minio (language: go)

Issue:
Able to delete non-empty buckets and data is not deleted
## NOTE

Current version ([RELEASE.2023-04-20T17-56-55Z](https://github.com/minio/minio/releases/tag/RELEASE.2023-04-20T17-56-55Z)) does not check if a bucket is empty when deleted and retains the content which is available again when a bucket with the same name is created again.
This seams to be related when having multiple server pools.

## Expected Behavior
When a non-empty bucket is trying to be deleted return the same error message as previously:

```bash
aws --endpoint https://my-minio s3 rb s3://foobar 
remove_bucket failed: s3://foobar An error occurred (BucketNotEmpty) when calling the DeleteBucket operation: The bucket you tried to delete is not empty
```

## Current Behavior
A non-empty bucket can be deleted and after creating it again the old content is still present

## Possible Solution
n/a

## Steps to Reproduce (for bugs)
```bash
aws --endpoint https://my-minio s3 mb s3://foobar
make_bucket: foobar
```

```bash
aws --endpoint https://my-minio s3 ls
2023-04-25 19:19:37 foobar
```

```bash
aws --endpoint https://my-minio s3 cp 20230414_171256.jpg s3://foobar/20230414_171256.jpg
upload: Downloads/20230414_171256.jpg to s3://foobar/20230414_171256.jpg
```

```bash
aws --endpoint https://my-minio s3 cp 20230414_171256.jpg s3 ls s3://foobar/
2023-04-25 19:20:26    1478891 20230414_171256.jpg
```

```bash
aws --endpoint https://my-minio s3 cp 20230414_171256.jpg s3 rb s3://foobar 
remove_bucket: foobar
```

```bash
aws --endpoint https://my-minio s3 cp 20230414_171256.jpg s3 ls s3://foobar/
An error occurred (NoSuchBucket) when calling the ListObjectsV2 operation: The specified bucket does not exist
```

```bash
aws --endpoint https://my-minio s3 cp 20230414_171256.jpg s3 mb s3://foobar                                                                 
make_bucket: foobar
```

```bash
aws --endpoint https://my-minio s3 cp 20230414_171256.jpg s3 ls s3://foobar/                                                                
2023-04-25 19:20:26    1478891 20230414_171256.jpg
```

## Context
9 server pools with 4-6 servers each

## Regression
looks like https://github.com/minio/minio/pull/16859 introduced the bug.

## Your Environment
* Version used (`minio --version`): RELEASE.2023-04-20T17-56-55Z
* Server setup and configuration: 9 server pools, no tls
* Operating System and version (`uname -a`): docker



Discussion / hints:
> ```shell
> ```shell
> aws --endpoint https://my-minio s3 ls
> 2023-04-25 19:19:37 foobar
> ```
> 
> 
>     
>       
>     
> 
>       
>     
> 
>     
>   
> ```shell
> aws --endpoint https://my-minio s3 cp 20230414_171256.jpg s3://foobar/20230414_171256.jpg
> upload: Downloads/20230414_171256.jpg to s3://foobar/20230414_171256.jpg
> ```
> 
> 
>     
>       
>     
> 
>       
>     
> 
>     
>   
> ```shell
> aws --endpoint https://my-minio s3 cp 20230414_171256.jpg s3 ls s3://foobar/
> upload: Downloads/20230414_171256.jpg to s3://foobar/20230414_171256.jpg
> ```
> 
> 
>     
>       
>     
> 
>       
>     
> 
>     
>   
> ```shell
> aws --endpoint https://my-minio s3 cp 20230414_171256.jpg s3 ls s3://foobar/
> 2023-04-25 19:20:26    1478891 20230414_171256.jpg
> ```
> 
> 
>     
>       
>     
> 
>       
>     
> 
>     
>   
> ```shell
> aws --endpoint https://my-minio s3 cp 20230414_171256.jpg s3 rb s3://foobar 
> remove_bucket: foobar
> ```
> ```

These commands look all interleaved on each other you are doing `cp` and `rb` in the same command line. 
This was only to demonstrate how to reproduce it, all commands where executed in this order on the same command line with multiple seconds in between each other.
I just updated the commands to reproduce it: there was one wrong output that I messed up during copying. Now it is accurate. 
It looks like your 9 pools are not behaving correctly, can you share your server configuration here including all the command line flags?
Using the k8s operator. see attached manifest.
Same behaviour on a fresh/empty cluster with the same manifest. Rolling back to _RELEASE.2023-03-20T20-16-18Z_ also solves the problem.

```yaml
---
apiVersion: minio.min.io/v2
kind: Tenant
metadata:
  name: minio
  namespace: minio
spec:
  credsSecret:
    name: minio-user-root
  exposeServices: {}
  image: minio/minio:RELEASE.2023-04-20T17-56-55Z
  mountPath: /export
  env:
    - name: MINIO_REGION
      value: us-west-1
    - name: MINIO_BROWSER
      value: "on"
    - name: MINIO_STORAGE_CLASS_STANDARD
      value: EC:2
    - name: MINIO_STORAGE_CLASS_RRS
      value: EC:2
    - name: MINIO_PROMETHEUS_AUTH_TYPE
      value: public
  pools:
    - name: directpv-0
      affinity:
        podAntiAffinity:
          requiredDuringSchedulingIgnoredDuringExecution:
            - labelSelector:
                matchExpressions:
                  - key: v1.min.io/pool
                    operator: In
                    values:
                      - directpv-0
              topologyKey: kubernetes.io/hostname
      resources:
        requests:
          cpu: "3"
          memory: 8Gi
        limits:
          cpu: "8"
          memory: 8Gi
      securityContext:
        fsGroup: 1000
        fsGroupChangePolicy: Always
        runAsGroup: 1000
        runAsNonRoot: true
        runAsUser: 1000
      servers: 4
      volumeClaimTemplate:
        metadata:
          name: data
        spec:
          accessModes:
            - ReadWriteOnce
          resources:
            requests:
              storage: "7990000000000"
          storageClassName: directpv-min-io
      volumesPerServer: 1
    - name: directpv-1
      affinity:
        podAntiAffinity:
          requiredDuringSchedulingIgnoredDuringExecution:
            - labelSelector:
                matchExpressions:
                  - key: v1.min.io/pool
                    operator: In
                    values:
                      - directpv-1
              topologyKey: kubernetes.io/hostname
      resources:
        requests:
          cpu: "3"
          memory: 8Gi
        limits:
          cpu: "8"
          memory: 8Gi
      securityContext:
        fsGroup: 1000
        fsGroupChangePolicy: Always
        runAsGroup: 1000
        runAsNonRoot: true
        runAsUser: 1000
      servers: 4
      volumeClaimTemplate:
        metadata:
          name: data
        spec:
          accessModes:
            - ReadWriteOnce
          resources:
            requests:
              storage: "7990000000000"
          storageClassName: directpv-min-io
      volumesPerServer: 1
    - name: directpv-2
      affinity:
        podAntiAffinity:
          requiredDuringSchedulingIgnoredDuringExecution:
            - labelSelector:
                matchExpressions:
                  - key: v1.min.io/pool
                    operator: In
                    values:
                      - directpv-2
              topologyKey: kubernetes.io/hostname
      resources:
        requests:
          cpu: "3"
          memory: 8Gi
        limits:
          cpu: "8"
          memory: 8Gi
      securityContext:
        fsGroup: 1000
        fsGroupChangePolicy: Always
        runAsGroup: 1000
        runAsNonRoot: true
        runAsUser: 1000
      servers: 4
      volumeClaimTemplate:
        metadata:
          name: data
        spec:
          accessModes:
            - ReadWriteOnce
          resources:
            requests:
              storage: "7990000000000"
          storageClassName: directpv-min-io
      volumesPerServer: 1
    - name: directpv-3
      affinity:
        podAntiAffinity:
          requiredDuringSchedulingIgnoredDuringExecution:
            - labelSelector:
                matchExpressions:
                  - key: v1.min.io/pool
                    operator: In
                    values:
                      - directpv-3
              topologyKey: kubernetes.io/hostname
      resources:
        requests:
          cpu: "3"
          memory: 8Gi
        limits:
          cpu: "8"
          memory: 8Gi
      securityContext:
        fsGroup: 1000
        fsGroupChangePolicy: Always
        runAsGroup: 1000
        runAsNonRoot: true
        runAsUser: 1000
      servers: 4
      volumeClaimTemplate:
        metadata:
          name: data
        spec:
          accessModes:
            - ReadWriteOnce
          resources:
            requests:
              storage: "7990000000000"
          storageClassName: directpv-min-io
      volumesPerServer: 1
    - name: directpv-4
      affinity:
        podAntiAffinity:
          requiredDuringSchedulingIgnoredDuringExecution:
            - labelSelector:
                matchExpressions:
                  - key: v1.min.io/pool
                    operator: In
                    values:
                      - directpv-4
              topologyKey: kubernetes.io/hostname
      resources:
        requests:
          cpu: "3"
          memory: 8Gi
        limits:
          cpu: "8"
          memory: 8Gi
      securityContext:
        fsGroup: 1000
        fsGroupChangePolicy: Always
        runAsGroup: 1000
        runAsNonRoot: true
        runAsUser: 1000
      servers: 4
      volumeClaimTemplate:
        metadata:
          name: data
        spec:
          accessModes:
            - ReadWriteOnce
          resources:
            requests:
              storage: "7990000000000"
          storageClassName: directpv-min-io
      volumesPerServer: 1
    - name: directpv-5
      affinity:
        podAntiAffinity:
          requiredDuringSchedulingIgnoredDuringExecution:
            - labelSelector:
                matchExpressions:
                  - key: v1.min.io/pool
                    operator: In
                    values:
                      - directpv-5
              topologyKey: kubernetes.io/hostname
      resources:
        requests:
          cpu: "3"
          memory: 8Gi
        limits:
          cpu: "8"
          memory: 8Gi
      securityContext:
        fsGroup: 1000
        fsGroupChangePolicy: Always
        runAsGroup: 1000
        runAsNonRoot: true
        runAsUser: 1000
      servers: 4
      volumeClaimTemplate:
        metadata:
          name: data
        spec:
          accessModes:
            - ReadWriteOnce
          resources:
            requests:
              storage: "7990000000000"
          storageClassName: directpv-min-io
      volumesPerServer: 1
    - name: directpv-6
      affinity:
        podAntiAffinity:
          requiredDuringSchedulingIgnoredDuringExecution:
            - labelSelector:
                matchExpressions:
                  - key: v1.min.io/pool
                    operator: In
                    values:
                      - directpv-6
              topologyKey: kubernetes.io/hostname
      resources:
        requests:
          cpu: "3"
          memory: 8Gi
        limits:
          cpu: "8"
          memory: 8Gi
      securityContext:
        fsGroup: 1000
        fsGroupChangePolicy: Always
        runAsGroup: 1000
        runAsNonRoot: true
        runAsUser: 1000
      servers: 6
      volumeClaimTemplate:
        metadata:
          name: data
        spec:
          accessModes:
            - ReadWriteOnce
          resources:
            requests:
              storage: "7990000000000"
          storageClassName: directpv-min-io
      volumesPerServer: 1
    - name: directpv-7
      affinity:
        podAntiAffinity:
          requiredDuringSchedulingIgnoredDuringExecution:
            - labelSelector:
                matchExpressions:
                  - key: v1.min.io/pool
                    operator: In
                    values:
                      - directpv-7
              topologyKey: kubernetes.io/hostname
      resources:
        requests:
          cpu: "3"
          memory: 8Gi
        limits:
          cpu: "8"
          memory: 8Gi
      securityContext:
        fsGroup: 1000
        fsGroupChangePolicy: Always
        runAsGroup: 1000
        runAsNonRoot: true
        runAsUser: 1000
      servers: 6
      volumeClaimTemplate:
        metadata:
          name: data
        spec:
          accessModes:
            - ReadWriteOnce
          resources:
            requests:
              storage: "7990000000000"
          storageClassName: directpv-min-io
      volumesPerServer: 1
    - name: directpv-8
      affinity:
        podAntiAffinity:
          requiredDuringSchedulingIgnoredDuringExecution:
            - labelSelector:
                matchExpressions:
                  - key: v1.min.io/pool
                    operator: In
                    values:
                      - directpv-8
              topologyKey: kubernetes.io/hostname
      resources:
        requests:
          cpu: "3"
          memory: 8Gi
        limits:
          cpu: "8"
          memory: 8Gi
      securityContext:
        fsGroup: 1000
        fsGroupChangePolicy: Always
        runAsGroup: 1000
        runAsNonRoot: true
        runAsUser: 1000
      servers: 6
      volumeClaimTemplate:
        metadata:
          name: data
        spec:
          accessModes:
            - ReadWriteOnce
          resources:
            requests:
              storage: "7990000000000"
          storageClassName: directpv-min-io
      volumesPerServer: 1
  requestAutoCert: false
```
```
commit dc8fdcb9c9e868dc66f2b930145344554d6c1ab6
Author: Poorna <poornas@users.noreply.github.com>
Date:   Thu Mar 30 11:54:08 2023 -0700

    fix: error checking in DeleteBucket (#16929)

diff --git a/cmd/erasure-server-pool.go b/cmd/erasure-server-pool.go
index 0bafdc31f..fb60c9b3e 100644
--- a/cmd/erasure-server-pool.go
+++ b/cmd/erasure-server-pool.go
@@ -1658,14 +1658,14 @@ func (z *erasureServerPools) DeleteBucket(ctx context.Context, bucket string, op
        }
 
        err := z.s3Peer.DeleteBucket(ctx, bucket, opts)
-       if err == nil || errors.Is(err, errVolumeNotFound) {
+       if err == nil || isErrBucketNotFound(err) {
                // If site replication is configured, hold on to deleted bucket state until sites sync
                if opts.SRDeleteOp == MarkDelete {
                        z.s3Peer.MakeBucket(context.Background(), pathJoin(minioMetaBucket, bucketMetaPrefix, deletedBucketsPrefix, bucket), MakeBucketOptions{})
                }
        }
 
-       if err != nil && !errors.Is(err, errVolumeNotFound) {
+       if err != nil && !isErrBucketNotFound(err) {
                if !opts.NoRecreate {
                        z.s3Peer.MakeBucket(ctx, bucket, MakeBucketOptions{})
                }
```

This is the only relevant change that could have an affect here strange. 
> looks like #16859 introduced the bug.

I think I know the reason, it is the quorum calculation that is adding the problem here. Since with 9 pools 6 pools would return success since the object might be placed in let's say 9th pool or 8th pool that were never deleted. 

Looks like we can't rely on quorum here for bucket deletes in pooled setup, instead we have to fail if any of the DELETE fails.

Fix patch:
diff --git a/cmd/peer-s3-client.go b/cmd/peer-s3-client.go
index 2bc831d6658bb..b3b9c81d90d57 100644
--- a/cmd/peer-s3-client.go
+++ b/cmd/peer-s3-client.go
@@ -273,9 +273,24 @@ func (sys *S3PeerSys) DeleteBucket(ctx context.Context, bucket string, opts Dele
 	errs := g.Wait()
 	errs = append(errs, deleteBucketLocal(ctx, bucket, opts))
 
-	quorum := (len(sys.allPeerClients) / 2) + 1
-	err := reduceWriteQuorumErrs(ctx, errs, bucketOpIgnoredErrs, quorum)
-	return toObjectErr(err, bucket)
+	var errReturn error
+	for _, err := range errs {
+		if errReturn == nil && err != nil {
+			// always return first error
+			errReturn = toObjectErr(err, bucket)
+			break
+		}
+	}
+
+	for _, err := range errs {
+		if err == nil && errReturn != nil {
+			// re-create successful deletes, since we are return an error.
+			sys.MakeBucket(ctx, bucket, MakeBucketOptions{})
+			break
+		}
+	}
+
+	return errReturn
 }
 
 // DeleteBucket deletes bucket on a peer


Test patch: