Skip to content

Commit

Permalink
Fix Bug Deleting Tenant PVCs (#977)
Browse files Browse the repository at this point in the history
Signed-off-by: Daniel Valdivia <18384552+dvaldivia@users.noreply.github.com>
  • Loading branch information
dvaldivia committed Aug 24, 2021
1 parent 4306d0f commit 5978553
Show file tree
Hide file tree
Showing 3 changed files with 88 additions and 59 deletions.
53 changes: 28 additions & 25 deletions operatorapi/operator_tenants.go
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,14 @@ func getDeleteTenantResponse(session *models.Principal, params operator_api.Dele
if params.Body != nil {
deleteTenantPVCs = params.Body.DeletePvcs
}
if err = deleteTenantAction(context.Background(), opClient, clientset.CoreV1(), params.Namespace, params.Tenant, deleteTenantPVCs); err != nil {

tenant, err := opClient.TenantGet(params.HTTPRequest.Context(), params.Namespace, params.Tenant, metav1.GetOptions{})
if err != nil {
return prepareError(err)
}
tenant.EnsureDefaults()

if err = deleteTenantAction(params.HTTPRequest.Context(), opClient, clientset.CoreV1(), tenant, deleteTenantPVCs); err != nil {
return prepareError(err)
}
return nil
Expand All @@ -277,10 +284,10 @@ func deleteTenantAction(
ctx context.Context,
operatorClient OperatorClientI,
clientset v1.CoreV1Interface,
namespace, tenantName string,
tenant *miniov2.Tenant,
deletePvcs bool) error {

err := operatorClient.TenantDelete(ctx, namespace, tenantName, metav1.DeleteOptions{})
err := operatorClient.TenantDelete(ctx, tenant.Namespace, tenant.Name, 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
Expand All @@ -290,39 +297,35 @@ func deleteTenantAction(
}

if deletePvcs {
tenant, err := operatorClient.TenantGet(ctx, namespace, tenantName, metav1.GetOptions{})
if err != nil {
return err
}
tenant.EnsureDefaults()

// delete MinIO PVCs
opts := metav1.ListOptions{
LabelSelector: fmt.Sprintf("%s=%s", miniov2.TenantLabel, tenantName),
LabelSelector: fmt.Sprintf("%s=%s", miniov2.TenantLabel, tenant.Name),
}
err = clientset.PersistentVolumeClaims(tenant.Namespace).DeleteCollection(ctx, metav1.DeleteOptions{}, opts)
if err != nil {
return err
}
// delete postgres PVCs
if tenant.HasLogEnabled() {
opts := metav1.ListOptions{
LabelSelector: fmt.Sprintf("%s=%s", miniov2.LogDBInstanceLabel, tenant.LogStatefulsetName()),
}
err := clientset.PersistentVolumeClaims(tenant.Namespace).DeleteCollection(ctx, metav1.DeleteOptions{}, opts)
if err != nil {
return err
}

logOpts := metav1.ListOptions{
LabelSelector: fmt.Sprintf("%s=%s", miniov2.LogDBInstanceLabel, tenant.LogStatefulsetName()),
}
err := clientset.PersistentVolumeClaims(tenant.Namespace).DeleteCollection(ctx, metav1.DeleteOptions{}, logOpts)
if err != nil {
return err
}

// delete prometheus PVCs
if tenant.HasPrometheusEnabled() {
opts := metav1.ListOptions{
LabelSelector: fmt.Sprintf("%s=%s", miniov2.PrometheusInstanceLabel, tenant.PrometheusStatefulsetName()),
}
err := clientset.PersistentVolumeClaims(tenant.Namespace).DeleteCollection(ctx, metav1.DeleteOptions{}, opts)
if err != nil {
return err
}

promOpts := metav1.ListOptions{
LabelSelector: fmt.Sprintf("%s=%s", miniov2.PrometheusInstanceLabel, tenant.PrometheusStatefulsetName()),
}

if err := clientset.PersistentVolumeClaims(tenant.Namespace).DeleteCollection(ctx, metav1.DeleteOptions{}, promOpts); err != nil {
return err
}

// delete all tenant's secrets only if deletePvcs = true
return clientset.Secrets(tenant.Namespace).DeleteCollection(ctx, metav1.DeleteOptions{}, opts)
}
Expand Down
75 changes: 51 additions & 24 deletions operatorapi/operator_tenants_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -501,8 +501,7 @@ func Test_deleteTenantAction(t *testing.T) {
type args struct {
ctx context.Context
operatorClient OperatorClientI
nameSpace string
tenantName string
tenant *miniov2.Tenant
deletePvcs bool
objs []runtime.Object
mockTenantDelete func(ctx context.Context, namespace string, tenantName string, options metav1.DeleteOptions) error
Expand All @@ -517,9 +516,13 @@ func Test_deleteTenantAction(t *testing.T) {
args: args{
ctx: context.Background(),
operatorClient: opClient,
nameSpace: "default",
tenantName: "minio-tenant",
deletePvcs: false,
tenant: &miniov2.Tenant{
ObjectMeta: metav1.ObjectMeta{
Name: "default",
Namespace: "minio-tenant",
},
},
deletePvcs: false,
mockTenantDelete: func(ctx context.Context, namespace string, tenantName string, options metav1.DeleteOptions) error {
return nil
},
Expand All @@ -531,9 +534,13 @@ func Test_deleteTenantAction(t *testing.T) {
args: args{
ctx: context.Background(),
operatorClient: opClient,
nameSpace: "default",
tenantName: "minio-tenant",
deletePvcs: false,
tenant: &miniov2.Tenant{
ObjectMeta: metav1.ObjectMeta{
Name: "default",
Namespace: "minio-tenant",
},
},
deletePvcs: false,
mockTenantDelete: func(ctx context.Context, namespace string, tenantName string, options metav1.DeleteOptions) error {
return errors.New("something happened")
},
Expand All @@ -546,9 +553,13 @@ func Test_deleteTenantAction(t *testing.T) {
args: args{
ctx: context.Background(),
operatorClient: opClient,
nameSpace: "minio-tenant",
tenantName: "tenant1",
deletePvcs: true,
tenant: &miniov2.Tenant{
ObjectMeta: metav1.ObjectMeta{
Name: "tenant1",
Namespace: "minio-tenant",
},
},
deletePvcs: true,
objs: []runtime.Object{
&corev1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
Expand All @@ -573,9 +584,13 @@ func Test_deleteTenantAction(t *testing.T) {
args: args{
ctx: context.Background(),
operatorClient: opClient,
nameSpace: "minio-tenant",
tenantName: "tenant1",
deletePvcs: false,
tenant: &miniov2.Tenant{
ObjectMeta: metav1.ObjectMeta{
Name: "tenant1",
Namespace: "minio-tenant",
},
},
deletePvcs: false,
objs: []runtime.Object{
&corev1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
Expand All @@ -600,9 +615,13 @@ func Test_deleteTenantAction(t *testing.T) {
args: args{
ctx: context.Background(),
operatorClient: opClient,
nameSpace: "minio-tenant",
tenantName: "tenant1",
deletePvcs: true,
tenant: &miniov2.Tenant{
ObjectMeta: metav1.ObjectMeta{
Name: "tenant1",
Namespace: "minio-tenant",
},
},
deletePvcs: true,
objs: []runtime.Object{
&corev1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
Expand All @@ -627,9 +646,13 @@ func Test_deleteTenantAction(t *testing.T) {
args: args{
ctx: context.Background(),
operatorClient: opClient,
nameSpace: "minio-tenant",
tenantName: "tenant1",
deletePvcs: true,
tenant: &miniov2.Tenant{
ObjectMeta: metav1.ObjectMeta{
Name: "tenant1",
Namespace: "minio-tenant",
},
},
deletePvcs: true,
objs: []runtime.Object{
&corev1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
Expand All @@ -655,9 +678,13 @@ func Test_deleteTenantAction(t *testing.T) {
args: args{
ctx: context.Background(),
operatorClient: opClient,
nameSpace: "minio-tenant",
tenantName: "tenant1",
deletePvcs: false,
tenant: &miniov2.Tenant{
ObjectMeta: metav1.ObjectMeta{
Name: "tenant1",
Namespace: "minio-tenant",
},
},
deletePvcs: false,
objs: []runtime.Object{
&corev1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
Expand All @@ -681,7 +708,7 @@ func Test_deleteTenantAction(t *testing.T) {
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, kubeClient.CoreV1(), tt.args.nameSpace, tt.args.tenantName, tt.args.deletePvcs); (err != nil) != tt.wantErr {
if err := deleteTenantAction(tt.args.ctx, tt.args.operatorClient, kubeClient.CoreV1(), tt.args.tenant, tt.args.deletePvcs); (err != nil) != tt.wantErr {
t.Errorf("deleteTenantAction() error = %v, wantErr %v", err, tt.wantErr)
}
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,20 +145,19 @@ const BasicDashboard = ({ classes, usage }: IDashboardProps) => {

const makeServerArray = (usage: Usage | null) => {
if (usage != null) {
usage.servers.forEach(s => s.uptime = niceDays(s.uptime))
return usage.servers.sort(function (a, b) {
var nameA = a.endpoint.toUpperCase();
usage.servers.forEach((s) => (s.uptime = niceDays(s.uptime)));
return usage.servers.sort(function (a, b) {
var nameA = a.endpoint.toUpperCase();
var nameB = b.endpoint.toUpperCase();
if (nameA < nameB) {
return -1;
}
if (nameA > nameB) {
return 1;
}
}
if (nameA > nameB) {
return 1;
}
return 0;
});
}
else return [];
});
} else return [];
};

const serverArray = makeServerArray(usage);
Expand Down

0 comments on commit 5978553

Please sign in to comment.