Skip to content

Commit

Permalink
fix(cvr delete, finalizer): remove finalizers to ensure successful de…
Browse files Browse the repository at this point in the history
…letion (#955)

This commit removes the finalizers from a cstor volume replica object. This
ensures removal of cstor volume replica object from kubernetes. In addition,
it will try to delete the underlying volume if cvr controller gets the delete
event. With the removal of finalizer there is no more retry mechanism to
actually delete the underlying volume if there were issues encountered during
earlier deletes.

Why was finalizer put in first place?
CstorVolumeReplica aliased CVR has finalizers that prohibits deletion unless
the volume gets deleted. This is a check to ensure proper cleanup even if
delete events are missed & / or underlying volume was not deleted in earlier
attempts.

Observations:
There were observations that point to the fact that when the namespace
(that contains these volumes) as well when the operator (that contains the
CRD schema definitions) are deleted, CVR finalizers pose a significant problem
to the overall deletion & later re-creation procedure. In fact the PODs that
are responsible for actual volume deletion & confirming the delete status is
no more available when its namespace or the openebs operator get deleted.

Future Work:
A better design needs to be implemented that ensures volume cleanup & hence
space reclamation.

fixes openebs/openebs#2374
fixes openebs/openebs#2349

Signed-off-by: AmitKumarDas <amit.das@mayadata.io>
  • Loading branch information
Amit Kumar Das authored and vishnuitta committed Feb 28, 2019
1 parent 6c5beaa commit d04834e
Show file tree
Hide file tree
Showing 2 changed files with 0 additions and 32 deletions.
31 changes: 0 additions & 31 deletions cmd/cstor-pool-mgmt/controller/replica-controller/handler.go
Expand Up @@ -17,7 +17,6 @@ limitations under the License.
package replicacontroller

import (
"encoding/json"
"fmt"
"os"
"reflect"
Expand All @@ -30,7 +29,6 @@ import (
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/runtime"
"k8s.io/client-go/tools/cache"
)
Expand Down Expand Up @@ -133,11 +131,6 @@ func (c *CStorVolumeReplicaController) cVREventHandler(operation common.QueueOpe
c.recorder.Event(cVR, corev1.EventTypeWarning, string(common.FailureDestroy), string(common.MessageResourceFailDestroy))
return string(apis.CVRStatusDeletionFailed), err
}
// removeFinalizer is to remove finalizer of cVR resource.
err = c.removeFinalizer(cVR)
if err != nil {
return string(apis.CVRStatusOffline), err
}
return "", nil
case common.QOpSync:
glog.Infof("Synchronizing CstorVolumeReplica status for volume %s", cVR.ObjectMeta.Name)
Expand Down Expand Up @@ -218,30 +211,6 @@ func (c *CStorVolumeReplicaController) getVolumeReplicaResource(key string) (*ap
return cStorVolumeReplicaUpdated, nil
}

// removeFinalizer is to remove finalizer of CStorVolumeReplica resource.
func (c *CStorVolumeReplicaController) removeFinalizer(cVR *apis.CStorVolumeReplica) error {
glog.Errorf("Removing finalizers for %s", cVR.Name)
// The Patch method requires an array of elements
// therefore creating array of one element
cvrPatch := make([]CVRPatch, 1)
// setting operation as remove
cvrPatch[0].Op = "remove"
// object to be removed is finalizers
cvrPatch[0].Path = "/metadata/finalizers"
cvrPatchJSON, err := json.Marshal(cvrPatch)
if err != nil {
glog.Errorf("Error marshaling cvrPatch object: %s", err)
return err
}
_, err = c.clientset.OpenebsV1alpha1().CStorVolumeReplicas(cVR.Namespace).Patch(cVR.Name, types.JSONPatchType, cvrPatchJSON)
if err != nil {
glog.Errorf("Finalizer patch failed for %s: %s", cVR.Name, err)
return err
}
glog.Infof("Removed Finalizer: %v, %v", cVR.ObjectMeta.Name, string(cVR.GetUID()))
return nil
}

// IsRightCStorVolumeReplica is to check if the cvr request is for particular pod/application.
func IsRightCStorVolumeReplica(cVR *apis.CStorVolumeReplica) bool {
if os.Getenv(string(common.OpenEBSIOCStorID)) == string(cVR.ObjectMeta.Labels["cstorpool.openebs.io/uid"]) {
Expand Down
1 change: 0 additions & 1 deletion pkg/install/v1alpha1/cstor_volume.go
Expand Up @@ -682,7 +682,6 @@ spec:
openebs.io/storage-class-ref: |
name: {{ .Volume.storageclass }}
resourceVersion: {{ .TaskResult.creategetsc.storageClassVersion }}
finalizers: ["cstorvolumereplica.openebs.io/finalizer"]
spec:
capacity: {{ .Volume.capacity }}
targetIP: {{ .TaskResult.cvolcreateputsvc.clusterIP }}
Expand Down

0 comments on commit d04834e

Please sign in to comment.