Add option to delete tenant's pvcs on tenant deletion (#251)

Author: cesnietor

k8s/console/base/kustomization.yaml

@@ -8,4 +8,4 @@ resources:
   - console-configmap.yaml
   - console-service.yaml
   - console-deployment.yaml
-  - minio-operator.yaml
+  - https://github.com/minio/operator/?ref=v3.0.10

k8s/console/base/minio-operator.yaml

@@ -28,6 +28,12 @@ rules:
       - create
       - list
       - patch
+  - apiGroups:
+      - ""
+    resources:
+      - persistentvolumeclaims
+    verbs:
+      - deletecollection
   - apiGroups:
       - "storage.k8s.io"
     resources:

k8s/operator-console/base/console-cluster-role.yaml

@@ -8,4 +8,4 @@ resources:
   - console-configmap.yaml
   - console-service.yaml
   - console-deployment.yaml
-  - minio-operator.yaml
+  - https://github.com/minio/operator/?ref=v3.0.10

k8s/operator-console/base/kustomization.yaml

@@ -110,7 +110,7 @@ func registerTenantHandlers(api *operations.ConsoleAPI) {
 		err := getDeleteTenantResponse(session, params)
 		if err != nil {
 			log.Println(err)
-			return admin_api.NewTenantInfoDefault(500).WithPayload(&models.Error{Code: 500, Message: swag.String("Unable to delete tenant")})
+			return admin_api.NewTenantInfoDefault(500).WithPayload(&models.Error{Code: 500, Message: swag.String(err.Error())})
 		}
 		return admin_api.NewTenantInfoOK()
 
@@ -145,25 +145,53 @@ func registerTenantHandlers(api *operations.ConsoleAPI) {
 	})
 }
 
-// deleteTenantAction performs the actions of deleting a tenant
-func deleteTenantAction(ctx context.Context, operatorClient OperatorClient, nameSpace, instanceName string) error {
-	err := operatorClient.TenantDelete(ctx, nameSpace, instanceName, metav1.DeleteOptions{})
-	if err != nil {
-		return err
-	}
-	return nil
-}
-
 // getDeleteTenantResponse gets the output of deleting a minio instance
 func getDeleteTenantResponse(session *models.Principal, params admin_api.DeleteTenantParams) error {
 	opClientClientSet, err := cluster.OperatorClient(session.SessionToken)
 	if err != nil {
 		return err
 	}
+	// get Kubernetes Client
+	clientset, err := cluster.K8sClient(session.SessionToken)
+	if err != nil {
+		return err
+	}
 	opClient := &operatorClient{
 		client: opClientClientSet,
 	}
-	return deleteTenantAction(context.Background(), opClient, params.Namespace, params.Tenant)
+	deleteTenantPVCs := false
+	if params.Body != nil {
+		deleteTenantPVCs = params.Body.DeletePvcs
+	}
+	return deleteTenantAction(context.Background(), opClient, clientset.CoreV1(), params.Namespace, params.Tenant, deleteTenantPVCs)
+}
+
+// deleteTenantAction performs the actions of deleting a tenant
+//
+// It also adds the option of deleting the tenant's underlying pvcs if deletePvcs set
+func deleteTenantAction(
+	ctx context.Context,
+	operatorClient OperatorClient,
+	clientset v1.CoreV1Interface,
+	namespace, tenantName string,
+	deletePvcs bool) error {
+
+	err := operatorClient.TenantDelete(ctx, namespace, tenantName, metav1.DeleteOptions{})
+	if err != nil {
+		// try to delete pvc even if the tenant doesn't exist anymore but only if deletePvcs is set to true,
+		// else, we return the error
+		if (deletePvcs && !k8sErrors.IsNotFound(err)) || !deletePvcs {
+			return err
+		}
+	}
+
+	if deletePvcs {
+		opts := metav1.ListOptions{
+			LabelSelector: fmt.Sprintf("%s=%s", operator.TenantLabel, tenantName),
+		}
+		return clientset.PersistentVolumeClaims(namespace).DeleteCollection(ctx, metav1.DeleteOptions{}, opts)
+	}
+	return nil
 }
 
 func getTenantScheme(mi *operator.Tenant) string {

k8s/operator-console/base/minio-operator.yaml

@@ -33,9 +33,11 @@ import (
 	operator "github.com/minio/operator/pkg/apis/minio.min.io/v1"
 	v1 "github.com/minio/operator/pkg/apis/minio.min.io/v1"
 	corev1 "k8s.io/api/core/v1"
+	k8sErrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 	types "k8s.io/apimachinery/pkg/types"
 	"k8s.io/client-go/kubernetes/fake"
 )
@@ -335,12 +337,13 @@ func Test_TenantInfo(t *testing.T) {
 
 func Test_deleteTenantAction(t *testing.T) {
 	opClient := opClientMock{}
-
 	type args struct {
 		ctx              context.Context
 		operatorClient   OperatorClient
 		nameSpace        string
 		tenantName       string
+		deletePvcs       bool
+		objs             []runtime.Object
 		mockTenantDelete func(ctx context.Context, namespace string, tenantName string, options metav1.DeleteOptions) error
 	}
 	tests := []struct {
@@ -355,6 +358,7 @@ func Test_deleteTenantAction(t *testing.T) {
 				operatorClient: opClient,
 				nameSpace:      "default",
 				tenantName:     "minio-tenant",
+				deletePvcs:     false,
 				mockTenantDelete: func(ctx context.Context, namespace string, tenantName string, options metav1.DeleteOptions) error {
 					return nil
 				},
@@ -368,17 +372,155 @@ func Test_deleteTenantAction(t *testing.T) {
 				operatorClient: opClient,
 				nameSpace:      "default",
 				tenantName:     "minio-tenant",
+				deletePvcs:     false,
 				mockTenantDelete: func(ctx context.Context, namespace string, tenantName string, options metav1.DeleteOptions) error {
 					return errors.New("something happened")
 				},
 			},
 			wantErr: true,
 		},
+		{
+			// Delete only PVCs of the defined tenant on the specific namespace
+			name: "Delete PVCs on Tenant Deletion",
+			args: args{
+				ctx:            context.Background(),
+				operatorClient: opClient,
+				nameSpace:      "minio-tenant",
+				tenantName:     "tenant1",
+				deletePvcs:     true,
+				objs: []runtime.Object{
+					&corev1.PersistentVolumeClaim{
+						ObjectMeta: metav1.ObjectMeta{
+							Name:      "PVC1",
+							Namespace: "minio-tenant",
+							Labels: map[string]string{
+								operator.TenantLabel: "tenant1",
+								operator.ZoneLabel:   "zone-1",
+							},
+						},
+					},
+				},
+				mockTenantDelete: func(ctx context.Context, namespace string, tenantName string, options metav1.DeleteOptions) error {
+					return nil
+				},
+			},
+			wantErr: false,
+		},
+		{
+			// Do not delete underlying pvcs
+			name: "Don't Delete PVCs on Tenant Deletion",
+			args: args{
+				ctx:            context.Background(),
+				operatorClient: opClient,
+				nameSpace:      "minio-tenant",
+				tenantName:     "tenant1",
+				deletePvcs:     false,
+				objs: []runtime.Object{
+					&corev1.PersistentVolumeClaim{
+						ObjectMeta: metav1.ObjectMeta{
+							Name:      "PVC1",
+							Namespace: "minio-tenant",
+							Labels: map[string]string{
+								operator.TenantLabel: "tenant1",
+								operator.ZoneLabel:   "zone-1",
+							},
+						},
+					},
+				},
+				mockTenantDelete: func(ctx context.Context, namespace string, tenantName string, options metav1.DeleteOptions) error {
+					return nil
+				},
+			},
+			wantErr: false,
+		},
+		{
+			// If error is different than NotFound, PVC deletion should not continue
+			name: "Don't delete pvcs if error Deleting Tenant, return",
+			args: args{
+				ctx:            context.Background(),
+				operatorClient: opClient,
+				nameSpace:      "minio-tenant",
+				tenantName:     "tenant1",
+				deletePvcs:     true,
+				objs: []runtime.Object{
+					&corev1.PersistentVolumeClaim{
+						ObjectMeta: metav1.ObjectMeta{
+							Name:      "PVC1",
+							Namespace: "minio-tenant",
+							Labels: map[string]string{
+								operator.TenantLabel: "tenant1",
+								operator.ZoneLabel:   "zone-1",
+							},
+						},
+					},
+				},
+				mockTenantDelete: func(ctx context.Context, namespace string, tenantName string, options metav1.DeleteOptions) error {
+					return errors.New("error returned")
+				},
+			},
+			wantErr: true,
+		},
+		{
+			// If error is NotFound while trying to Delete Tenant, PVC deletion should continue
+			name: "Delete pvcs if tenant not found",
+			args: args{
+				ctx:            context.Background(),
+				operatorClient: opClient,
+				nameSpace:      "minio-tenant",
+				tenantName:     "tenant1",
+				deletePvcs:     true,
+				objs: []runtime.Object{
+					&corev1.PersistentVolumeClaim{
+						ObjectMeta: metav1.ObjectMeta{
+							Name:      "PVC1",
+							Namespace: "minio-tenant",
+							Labels: map[string]string{
+								operator.TenantLabel: "tenant1",
+								operator.ZoneLabel:   "zone-1",
+							},
+						},
+					},
+				},
+				mockTenantDelete: func(ctx context.Context, namespace string, tenantName string, options metav1.DeleteOptions) error {
+					return k8sErrors.NewNotFound(schema.GroupResource{}, "tenant1")
+				},
+			},
+			wantErr: false,
+		},
+		{
+			// If error is NotFound while trying to Delete Tenant and pvcdeletion=false,
+			// error should be returned
+			name: "Don't delete pvcs and return error if tenant not found",
+			args: args{
+				ctx:            context.Background(),
+				operatorClient: opClient,
+				nameSpace:      "minio-tenant",
+				tenantName:     "tenant1",
+				deletePvcs:     false,
+				objs: []runtime.Object{
+					&corev1.PersistentVolumeClaim{
+						ObjectMeta: metav1.ObjectMeta{
+							Name:      "PVC1",
+							Namespace: "minio-tenant",
+							Labels: map[string]string{
+								operator.TenantLabel: "tenant1",
+								operator.ZoneLabel:   "zone-1",
+							},
+						},
+					},
+				},
+				mockTenantDelete: func(ctx context.Context, namespace string, tenantName string, options metav1.DeleteOptions) error {
+					return k8sErrors.NewNotFound(schema.GroupResource{}, "tenant1")
+				},
+			},
+			wantErr: true,
+		},
 	}
 	for _, tt := range tests {
 		opClientTenantDeleteMock = tt.args.mockTenantDelete
+		kubeClient := fake.NewSimpleClientset(tt.args.objs...)
 		t.Run(tt.name, func(t *testing.T) {
-			if err := deleteTenantAction(tt.args.ctx, tt.args.operatorClient, tt.args.nameSpace, tt.args.tenantName); (err != nil) != tt.wantErr {
+			if err := deleteTenantAction(tt.args.ctx, tt.args.operatorClient, kubeClient.CoreV1(), tt.args.nameSpace, tt.args.tenantName, tt.args.deletePvcs); (err != nil) != tt.wantErr {
 				t.Errorf("deleteTenantAction() error = %v, wantErr %v", err, tt.wantErr)
 			}
 		})
@@ -768,7 +910,7 @@ func Test_UpdateTenantAction(t *testing.T) {
 		cnsClient := fake.NewSimpleClientset(tt.objs...)
 		t.Run(tt.name, func(t *testing.T) {
 			if err := updateTenantAction(tt.args.ctx, tt.args.operatorClient, cnsClient.CoreV1(), tt.args.httpCl, tt.args.nameSpace, tt.args.params); (err != nil) != tt.wantErr {
-				t.Errorf("deleteTenantAction() error = %v, wantErr %v", err, tt.wantErr)
+				t.Errorf("updateTenantAction() error = %v, wantErr %v", err, tt.wantErr)
 			}
 		})
 	}