alexlebens/external-snapshotter
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge pull request #47 from xing-yang/delete_volume_finalizer

Browse Source 
Add Delete Volume Finalizer
This commit is contained in:
Kubernetes Prow Robot
2019-04-29 04:38:19 -07:00
committed by GitHub
parent b36da39ab6 603855fc1b
commit a1b3374fac
5 changed files with 366 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
deploy/kubernetes/rbac.yaml
View File

@@ -25,7 +25,7 @@ rules:
verbs: ["get", "list", "watch"]
- apiGroups: [""]
resources: ["persistentvolumeclaims"]
verbs: ["get", "list", "watch"]
verbs: ["get", "list", "watch", "update"]
- apiGroups: ["storage.k8s.io"]
resources: ["storageclasses"]
verbs: ["get", "list", "watch"]

171
pkg/controller/framework_test.go
View File

@@ -21,6 +21,7 @@ import (
"errors"
"fmt"
"reflect"
sysruntime "runtime"
"strconv"
"strings"
"sync"
@@ -53,6 +54,7 @@ import (
"k8s.io/client-go/tools/cache"
"k8s.io/client-go/tools/record"
"k8s.io/klog"
"k8s.io/kubernetes/pkg/util/slice"
)
// This is a unit test framework for snapshot controller.
@@ -110,7 +112,8 @@ type controllerTest struct {
// List of expected CSI list snapshot calls
expectedListCalls []listCall
// Function to call as the test.
test testCall
test testCall
expectSuccess bool
}
type testCall func(ctrl *csiSnapshotController, reactor *snapshotReactor, test controllerTest) error
@@ -177,6 +180,11 @@ func withContentFinalizer(content *crdv1.VolumeSnapshotContent) *crdv1.VolumeSna
return content
}
func withPVCFinalizer(pvc *v1.PersistentVolumeClaim) *v1.PersistentVolumeClaim {
pvc.ObjectMeta.Finalizers = append(pvc.ObjectMeta.Finalizers, PVCFinalizer)
return pvc
}
// React is a callback called by fake kubeClient from the controller.
// In other words, every snapshot/content change performed by the controller ends
// here.
@@ -331,6 +339,32 @@ func (r *snapshotReactor) React(action core.Action) (handled bool, ret runtime.O
klog.V(4).Infof("GetClaim: claim %s not found", name)
return true, nil, fmt.Errorf("cannot find claim %s", name)
case action.Matches("update", "persistentvolumeclaims"):
obj := action.(core.UpdateAction).GetObject()
claim := obj.(*v1.PersistentVolumeClaim)
// Check and bump object version
storedClaim, found := r.claims[claim.Name]
if found {
storedVer, _ := strconv.Atoi(storedClaim.ResourceVersion)
requestedVer, _ := strconv.Atoi(claim.ResourceVersion)
if storedVer != requestedVer {
return true, obj, errVersionConflict
}
// Don't modify the existing object
claim = claim.DeepCopy()
claim.ResourceVersion = strconv.Itoa(storedVer + 1)
} else {
return true, nil, fmt.Errorf("cannot update claim %s: claim not found", claim.Name)
}
// Store the updated object to appropriate places.
r.claims[claim.Name] = claim
r.changedObjects = append(r.changedObjects, claim)
r.changedSinceLastSync++
klog.V(4).Infof("saved updated claim %s", claim.Name)
return true, claim, nil
case action.Matches("get", "storageclasses"):
name := action.(core.GetAction).GetName()
storageClass, found := r.storageClasses[name]
@@ -550,6 +584,9 @@ func (r *snapshotReactor) syncAll() {
for _, v := range r.contents {
r.changedObjects = append(r.changedObjects, v)
}
for _, pvc := range r.claims {
r.changedObjects = append(r.changedObjects, pvc)
}
r.changedSinceLastSync = 0
}
@@ -699,6 +736,7 @@ func newSnapshotReactor(kubeClient *kubefake.Clientset, client *fake.Clientset,
client.AddReactor("delete", "volumesnapshotcontents", reactor.React)
client.AddReactor("delete", "volumesnapshots", reactor.React)
kubeClient.AddReactor("get", "persistentvolumeclaims", reactor.React)
kubeClient.AddReactor("update", "persistentvolumeclaims", reactor.React)
kubeClient.AddReactor("get", "persistentvolumes", reactor.React)
kubeClient.AddReactor("get", "storageclasses", reactor.React)
kubeClient.AddReactor("get", "secrets", reactor.React)
@@ -746,6 +784,7 @@ func newTestController(kubeClient kubernetes.Interface, clientset clientset.Inte
ctrl.contentListerSynced = alwaysReady
ctrl.snapshotListerSynced = alwaysReady
ctrl.classListerSynced = alwaysReady
ctrl.pvcListerSynced = alwaysReady
return ctrl, nil
}
@@ -845,7 +884,7 @@ func newSnapshotArray(name, className, boundToContent, snapshotUID, claimName st
}
// newClaim returns a new claim with given attributes
func newClaim(name, claimUID, capacity, boundToVolume string, phase v1.PersistentVolumeClaimPhase, class *string) *v1.PersistentVolumeClaim {
func newClaim(name, claimUID, capacity, boundToVolume string, phase v1.PersistentVolumeClaimPhase, class *string, bFinalizer bool) *v1.PersistentVolumeClaim {
claim := v1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
Name: name,
@@ -877,6 +916,9 @@ func newClaim(name, claimUID, capacity, boundToVolume string, phase v1.Persisten
claim.Status.Capacity = claim.Spec.Resources.Requests
}
if bFinalizer {
return withPVCFinalizer(&claim)
}
return &claim
}
@@ -884,7 +926,15 @@ func newClaim(name, claimUID, capacity, boundToVolume string, phase v1.Persisten
// newClaim() with the same parameters.
func newClaimArray(name, claimUID, capacity, boundToVolume string, phase v1.PersistentVolumeClaimPhase, class *string) []*v1.PersistentVolumeClaim {
return []*v1.PersistentVolumeClaim{
newClaim(name, claimUID, capacity, boundToVolume, phase, class),
newClaim(name, claimUID, capacity, boundToVolume, phase, class, false),
}
}
// newClaimArrayFinalizer returns array with a single claim that would be returned by
// newClaim() with the same parameters plus finalizer.
func newClaimArrayFinalizer(name, claimUID, capacity, boundToVolume string, phase v1.PersistentVolumeClaimPhase, class *string) []*v1.PersistentVolumeClaim {
return []*v1.PersistentVolumeClaim{
newClaim(name, claimUID, capacity, boundToVolume, phase, class, true),
}
}
@@ -961,6 +1011,14 @@ func testSyncContent(ctrl *csiSnapshotController, reactor *snapshotReactor, test
return ctrl.syncContent(test.initialContents[0])
}
func testAddPVCFinalizer(ctrl *csiSnapshotController, reactor *snapshotReactor, test controllerTest) error {
return ctrl.ensureSnapshotSourceFinalizer(test.initialSnapshots[0])
}
func testRemovePVCFinalizer(ctrl *csiSnapshotController, reactor *snapshotReactor, test controllerTest) error {
return ctrl.checkandRemoveSnapshotSourceFinalizer(test.initialSnapshots[0])
}
var (
classEmpty string
classGold = "gold"
@@ -1097,6 +1155,113 @@ func runSyncTests(t *testing.T, tests []controllerTest, snapshotClasses []*crdv1
}
}
// This tests ensureSnapshotSourceFinalizer and checkandRemoveSnapshotSourceFinalizer
func runPVCFinalizerTests(t *testing.T, tests []controllerTest, snapshotClasses []*crdv1.VolumeSnapshotClass) {
snapshotscheme.AddToScheme(scheme.Scheme)
for _, test := range tests {
klog.V(4).Infof("starting test %q", test.name)
// Initialize the controller
kubeClient := &kubefake.Clientset{}
client := &fake.Clientset{}
ctrl, err := newTestController(kubeClient, client, nil, t, test)
if err != nil {
t.Fatalf("Test %q construct persistent content failed: %v", test.name, err)
}
reactor := newSnapshotReactor(kubeClient, client, ctrl, nil, nil, test.errors)
for _, snapshot := range test.initialSnapshots {
ctrl.snapshotStore.Add(snapshot)
reactor.snapshots[snapshot.Name] = snapshot
}
for _, content := range test.initialContents {
if ctrl.isDriverMatch(test.initialContents[0]) {
ctrl.contentStore.Add(content)
reactor.contents[content.Name] = content
}
}
pvcIndexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{})
for _, claim := range test.initialClaims {
reactor.claims[claim.Name] = claim
pvcIndexer.Add(claim)
}
ctrl.pvcLister = corelisters.NewPersistentVolumeClaimLister(pvcIndexer)
for _, volume := range test.initialVolumes {
reactor.volumes[volume.Name] = volume
}
for _, storageClass := range test.initialStorageClasses {
reactor.storageClasses[storageClass.Name] = storageClass
}
for _, secret := range test.initialSecrets {
reactor.secrets[secret.Name] = secret
}
// Inject classes into controller via a custom lister.
indexer := cache.NewIndexer(cache.MetaNamespaceKeyFunc, cache.Indexers{})
for _, class := range snapshotClasses {
indexer.Add(class)
}
ctrl.classLister = storagelisters.NewVolumeSnapshotClassLister(indexer)
// Run the tested functions
err = test.test(ctrl, reactor, test)
if err != nil {
t.Errorf("Test %q failed: %v", test.name, err)
}
// Verify PVCFinalizer tests results
evaluatePVCFinalizerTests(ctrl, reactor, test, t)
}
}
// Evaluate PVCFinalizer tests results
func evaluatePVCFinalizerTests(ctrl *csiSnapshotController, reactor *snapshotReactor, test controllerTest, t *testing.T) {
// Evaluate results
bHasPVCFinalizer := false
name := sysruntime.FuncForPC(reflect.ValueOf(test.test).Pointer()).Name()
index := strings.LastIndex(name, ".")
if index == -1 {
t.Errorf("Test %q: failed to test finalizer - invalid test call name [%s]", test.name, name)
return
}
names := []rune(name)
funcName := string(names[index+1 : len(name)])
klog.V(4).Infof("test %q: PVCFinalizer test func name: [%s]", test.name, funcName)
if funcName == "testAddPVCFinalizer" {
for _, pvc := range reactor.claims {
if test.initialClaims[0].Name == pvc.Name {
if !slice.ContainsString(test.initialClaims[0].ObjectMeta.Finalizers, PVCFinalizer, nil) && slice.ContainsString(pvc.ObjectMeta.Finalizers, PVCFinalizer, nil) {
klog.V(4).Infof("test %q succeeded. PVCFinalizer is added to PVC %s", test.name, pvc.Name)
bHasPVCFinalizer = true
}
break
}
}
if test.expectSuccess && !bHasPVCFinalizer {
t.Errorf("Test %q: failed to add finalizer to PVC %s", test.name, test.initialClaims[0].Name)
}
}
bHasPVCFinalizer = true
if funcName == "testRemovePVCFinalizer" {
for _, pvc := range reactor.claims {
if test.initialClaims[0].Name == pvc.Name {
if slice.ContainsString(test.initialClaims[0].ObjectMeta.Finalizers, PVCFinalizer, nil) && !slice.ContainsString(pvc.ObjectMeta.Finalizers, PVCFinalizer, nil) {
klog.V(4).Infof("test %q succeeded. PVCFinalizer is removed from PVC %s", test.name, pvc.Name)
bHasPVCFinalizer = false
}
break
}
}
if test.expectSuccess && bHasPVCFinalizer {
t.Errorf("Test %q: failed to remove finalizer from PVC %s", test.name, test.initialClaims[0].Name)
}
}
}
func getSize(size int64) *resource.Quantity {
return resource.NewQuantity(size, resource.BinarySI)
}

129
pkg/controller/snapshot_controller.go
View File

@@ -192,11 +192,19 @@ func (ctrl *csiSnapshotController) syncSnapshot(snapshot *crdv1.VolumeSnapshot)
return ctrl.addSnapshotFinalizer(snapshot)
}
klog.V(5).Infof("syncSnapshot[%s]: check if we should remove finalizer on snapshot source and remove it if we can", snapshotKey(snapshot))
// Check if we should remove finalizer on snapshot source and remove it if we can.
errFinalizer := ctrl.checkandRemoveSnapshotSourceFinalizer(snapshot)
if errFinalizer != nil {
klog.Errorf("error check and remove snapshot source finalizer for snapshot [%s]: %v", snapshot.Name, errFinalizer)
// Log an event and keep the original error from syncUnready/ReadySnapshot
ctrl.eventRecorder.Event(snapshot, v1.EventTypeWarning, "ErrorSnapshotSourceFinalizer", "Error check and remove PVC Finalizer for VolumeSnapshot")
}
if !snapshot.Status.ReadyToUse {
return ctrl.syncUnreadySnapshot(snapshot)
}
return ctrl.syncReadySnapshot(snapshot)
}
// syncReadySnapshot checks the snapshot which has been bound to snapshot content successfully before.
@@ -367,7 +375,6 @@ func (ctrl *csiSnapshotController) createSnapshot(snapshot *crdv1.VolumeSnapshot
// We will get an "snapshot update" event soon, this is not a big error
klog.V(4).Infof("createSnapshot [%s]: cannot update internal cache: %v", snapshotKey(snapshotObj), updateErr)
}
return nil
})
return nil
@@ -612,6 +619,14 @@ func (ctrl *csiSnapshotController) createSnapshotOperation(snapshot *crdv1.Volum
return snapshot, nil
}
// If PVC is not being deleted and finalizer is not added yet, a finalizer should be added.
klog.V(5).Infof("createSnapshotOperation: Check if PVC is not being deleted and add Finalizer for source of snapshot [%s] if needed", snapshot.Name)
err := ctrl.ensureSnapshotSourceFinalizer(snapshot)
if err != nil {
klog.Errorf("createSnapshotOperation failed to add finalizer for source of snapshot %s", err)
return nil, err
}
class, volume, contentName, snapshotterCredentials, err := ctrl.getCreateSnapshotInput(snapshot)
if err != nil {
return nil, fmt.Errorf("failed to get input parameters to create snapshot %s: %q", snapshot.Name, err)
@@ -1068,3 +1083,113 @@ func (ctrl *csiSnapshotController) removeSnapshotFinalizer(snapshot *crdv1.Volum
klog.V(5).Infof("Removed protection finalizer from volume snapshot %s", snapshotKey(snapshot))
return nil
}
// ensureSnapshotSourceFinalizer checks if a Finalizer needs to be added for the snapshot source;
// if true, adds a Finalizer for VolumeSnapshot Source PVC
func (ctrl *csiSnapshotController) ensureSnapshotSourceFinalizer(snapshot *crdv1.VolumeSnapshot) error {
// Get snapshot source which is a PVC
pvc, err := ctrl.getClaimFromVolumeSnapshot(snapshot)
if err != nil {
klog.Infof("cannot get claim from snapshot [%s]: [%v] Claim may be deleted already.", snapshot.Name, err)
return nil
}
// If PVC is not being deleted and PVCFinalizer is not added yet, the PVCFinalizer should be added.
if pvc.ObjectMeta.DeletionTimestamp == nil && !slice.ContainsString(pvc.ObjectMeta.Finalizers, PVCFinalizer, nil) {
// Add the finalizer
pvcClone := pvc.DeepCopy()
pvcClone.ObjectMeta.Finalizers = append(pvcClone.ObjectMeta.Finalizers, PVCFinalizer)
_, err = ctrl.client.CoreV1().PersistentVolumeClaims(pvcClone.Namespace).Update(pvcClone)
if err != nil {
klog.Errorf("cannot add finalizer on claim [%s] for snapshot [%s]: [%v]", pvc.Name, snapshot.Name, err)
return newControllerUpdateError(pvcClone.Name, err.Error())
}
klog.Infof("Added protection finalizer to persistent volume claim %s", pvc.Name)
}
return nil
}
// removeSnapshotSourceFinalizer removes a Finalizer for VolumeSnapshot Source PVC.
func (ctrl *csiSnapshotController) removeSnapshotSourceFinalizer(snapshot *crdv1.VolumeSnapshot) error {
// Get snapshot source which is a PVC
pvc, err := ctrl.getClaimFromVolumeSnapshot(snapshot)
if err != nil {
klog.Infof("cannot get claim from snapshot [%s]: [%v] Claim may be deleted already. No need to remove finalizer on the claim.", snapshot.Name, err)
return nil
}
pvcClone := pvc.DeepCopy()
pvcClone.ObjectMeta.Finalizers = slice.RemoveString(pvcClone.ObjectMeta.Finalizers, PVCFinalizer, nil)
_, err = ctrl.client.CoreV1().PersistentVolumeClaims(pvcClone.Namespace).Update(pvcClone)
if err != nil {
return newControllerUpdateError(pvcClone.Name, err.Error())
}
klog.V(5).Infof("Removed protection finalizer from persistent volume claim %s", pvc.Name)
return nil
}
// isSnapshotSourceBeingUsed checks if a PVC is being used as a source to create a snapshot
func (ctrl *csiSnapshotController) isSnapshotSourceBeingUsed(snapshot *crdv1.VolumeSnapshot) bool {
klog.V(5).Infof("isSnapshotSourceBeingUsed[%s]: started", snapshotKey(snapshot))
// Get snapshot source which is a PVC
pvc, err := ctrl.getClaimFromVolumeSnapshot(snapshot)
if err != nil {
klog.Infof("isSnapshotSourceBeingUsed: cannot to get claim from snapshot: %v", err)
return false
}
// Going through snapshots in the cache (snapshotLister). If a snapshot's PVC source
// is the same as the input snapshot's PVC source and snapshot's ReadyToUse status
// is false, the snapshot is still being created from the PVC and the PVC is in-use.
snapshots, err := ctrl.snapshotLister.VolumeSnapshots(snapshot.Namespace).List(labels.Everything())
if err != nil {
return false
}
for _, snap := range snapshots {
// Skip static bound snapshot without a PVC source
if snap.Spec.Source == nil {
klog.V(4).Infof("Skipping static bound snapshot %s when checking PVC %s/%s", snap.Name, pvc.Namespace, pvc.Name)
continue
}
if pvc.Name == snap.Spec.Source.Name && snap.Status.ReadyToUse == false {
klog.V(2).Infof("Keeping PVC %s/%s, it is used by snapshot %s/%s", pvc.Namespace, pvc.Name, snap.Namespace, snap.Name)
return true
}
}
klog.V(5).Infof("isSnapshotSourceBeingUsed: no snapshot is being created from PVC %s/%s", pvc.Namespace, pvc.Name)
return false
}
// checkandRemoveSnapshotSourceFinalizer checks if the snapshot source finalizer should be removed
// and removed it if needed.
func (ctrl *csiSnapshotController) checkandRemoveSnapshotSourceFinalizer(snapshot *crdv1.VolumeSnapshot) error {
// Get snapshot source which is a PVC
pvc, err := ctrl.getClaimFromVolumeSnapshot(snapshot)
if err != nil {
klog.Infof("cannot get claim from snapshot [%s]: [%v] Claim may be deleted already. No need to remove finalizer on the claim.", snapshot.Name, err)
return nil
}
klog.V(5).Infof("checkandRemoveSnapshotSourceFinalizer for snapshot [%s]: snapshot status [%#v]", snapshot.Name, snapshot.Status)
// Check if there is a Finalizer on PVC to be removed
if slice.ContainsString(pvc.ObjectMeta.Finalizers, PVCFinalizer, nil) {
// There is a Finalizer on PVC. Check if PVC is used
// and remove finalizer if it's not used.
isUsed := ctrl.isSnapshotSourceBeingUsed(snapshot)
if !isUsed {
klog.Infof("checkandRemoveSnapshotSourceFinalizer[%s]: Remove Finalizer for PVC %s as it is not used by snapshots in creation", snapshot.Name, pvc.Name)
err = ctrl.removeSnapshotSourceFinalizer(snapshot)
if err != nil {
klog.Errorf("checkandRemoveSnapshotSourceFinalizer [%s]: removeSnapshotSourceFinalizer failed to remove finalizer %v", snapshot.Name, err)
return err
}
}
}
return nil
}

67
pkg/controller/snapshot_finalizer_test.go Normal file
View File

@@ -0,0 +1,67 @@
/*
Copyright 2019 The Kubernetes Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
package controller
import (
"testing"
"k8s.io/api/core/v1"
)
// Test single call to ensureSnapshotSourceFinalizer and checkandRemoveSnapshotSourceFinalizer,
// expecting PVCFinalizer to be added or removed
func TestPVCFinalizer(t *testing.T) {
tests := []controllerTest{
{
name: "1-1 - successful add PVC finalizer",
initialSnapshots: newSnapshotArray("snap6-2", classSilver, "", "snapuid6-2", "claim6-2", false, nil, nil, nil),
initialClaims: newClaimArray("claim6-2", "pvc-uid6-2", "1Gi", "volume6-2", v1.ClaimBound, &classEmpty),
test: testAddPVCFinalizer,
expectSuccess: true,
},
{
name: "1-2 - won't add PVC finalizer; already added",
initialSnapshots: newSnapshotArray("snap6-2", classSilver, "", "snapuid6-2", "claim6-2", false, nil, nil, nil),
initialClaims: newClaimArrayFinalizer("claim6-2", "pvc-uid6-2", "1Gi", "volume6-2", v1.ClaimBound, &classEmpty),
test: testAddPVCFinalizer,
expectSuccess: false,
},
{
name: "1-3 - successful remove PVC finalizer",
initialSnapshots: newSnapshotArray("snap6-2", classSilver, "", "snapuid6-2", "claim6-2", false, nil, nil, nil),
initialClaims: newClaimArrayFinalizer("claim6-2", "pvc-uid6-2", "1Gi", "volume6-2", v1.ClaimBound, &classEmpty),
test: testRemovePVCFinalizer,
expectSuccess: true,
},
{
name: "1-4 - won't remove PVC finalizer; already removed",
initialSnapshots: newSnapshotArray("snap6-2", classSilver, "", "snapuid6-2", "claim6-2", false, nil, nil, nil),
initialClaims: newClaimArray("claim6-2", "pvc-uid6-2", "1Gi", "volume6-2", v1.ClaimBound, &classEmpty),
test: testRemovePVCFinalizer,
expectSuccess: false,
},
{
name: "1-5 - won't remove PVC finalizer; PVC in-use",
initialSnapshots: newSnapshotArray("snap6-2", classSilver, "", "snapuid6-2", "claim6-2", false, nil, nil, nil),
initialClaims: newClaimArray("claim6-2", "pvc-uid6-2", "1Gi", "volume6-2", v1.ClaimBound, &classEmpty),
test: testRemovePVCFinalizer,
expectSuccess: false,
},
}
runPVCFinalizerTests(t, tests, snapshotClasses)
}

3
pkg/controller/util.go
View File

@@ -75,6 +75,9 @@ var snapshotterSecretParams = deprecatedSecretParamsMap{
secretNamespaceKey: prefixedSnapshotterSecretNamespaceKey,
}
// Name of finalizer on PVCs that have been used as a source to create VolumeSnapshots
const PVCFinalizer = "snapshot.storage.kubernetes.io/pvc-protection"
func snapshotKey(vs *crdv1.VolumeSnapshot) string {
return fmt.Sprintf("%s/%s", vs.Namespace, vs.Name)
}