Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent calling controller-expand if volume in-use #86

Merged
merged 2 commits into from
Jun 10, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions deploy/kubernetes/rbac.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ rules:
- apiGroups: [""]
resources: ["persistentvolumeclaims"]
verbs: ["get", "list", "watch"]
- apiGroups: [""]
resources: ["pods"]
verbs: ["get", "list", "watch"]
- apiGroups: [""]
resources: ["persistentvolumeclaims/status"]
verbs: ["patch"]
Expand Down
129 changes: 115 additions & 14 deletions pkg/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,13 @@ import (

"github.com/kubernetes-csi/external-resizer/pkg/resizer"
"github.com/kubernetes-csi/external-resizer/pkg/util"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
v1 "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"
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
"k8s.io/apimachinery/pkg/util/wait"
"k8s.io/client-go/informers"
"k8s.io/client-go/kubernetes"
Expand Down Expand Up @@ -56,6 +59,11 @@ type resizeController struct {
pvSynced cache.InformerSynced
pvcLister corelisters.PersistentVolumeClaimLister
pvcSynced cache.InformerSynced

usedPVCs *inUsePVCStore

podLister corelisters.PodLister
podListerSynced cache.InformerSynced
}

// NewResizeController returns a ResizeController.
Expand All @@ -69,6 +77,9 @@ func NewResizeController(
pvInformer := informerFactory.Core().V1().PersistentVolumes()
pvcInformer := informerFactory.Core().V1().PersistentVolumeClaims()

// list pods so as we can identify PVC that are in-use
podInformer := informerFactory.Core().V1().Pods()

eventBroadcaster := record.NewBroadcaster()
eventBroadcaster.StartLogging(klog.Infof)
eventBroadcaster.StartRecordingToSink(&corev1.EventSinkImpl{Interface: kubeClient.CoreV1().Events(v1.NamespaceAll)})
Expand All @@ -79,15 +90,18 @@ func NewResizeController(
pvcRateLimiter, fmt.Sprintf("%s-pvc", name))

ctrl := &resizeController{
name: name,
resizer: resizer,
kubeClient: kubeClient,
pvLister: pvInformer.Lister(),
pvSynced: pvInformer.Informer().HasSynced,
pvcLister: pvcInformer.Lister(),
pvcSynced: pvcInformer.Informer().HasSynced,
claimQueue: claimQueue,
eventRecorder: eventRecorder,
name: name,
resizer: resizer,
kubeClient: kubeClient,
pvLister: pvInformer.Lister(),
pvSynced: pvInformer.Informer().HasSynced,
pvcLister: pvcInformer.Lister(),
pvcSynced: pvcInformer.Informer().HasSynced,
podLister: podInformer.Lister(),
podListerSynced: podInformer.Informer().HasSynced,
claimQueue: claimQueue,
eventRecorder: eventRecorder,
usedPVCs: newUsedPVCStore(),
}

// Add a resync period as the PVC's request size can be resized again when we handling
Expand All @@ -98,17 +112,48 @@ func NewResizeController(
DeleteFunc: ctrl.deletePVC,
}, resyncPeriod)

podInformer.Informer().AddEventHandlerWithResyncPeriod(cache.ResourceEventHandlerFuncs{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see that the in-tree PVC protection controller also checks the pod. What is the advantage of checking thru the pod vs checking the VolumeAttachment object?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because not all volume types support attach/detach and hence there may not be an VolumeAttachment object.

AddFunc: ctrl.addPod,
DeleteFunc: ctrl.deletePod,
UpdateFunc: ctrl.updatePod,
}, resyncPeriod)

return ctrl
}

func (ctrl *resizeController) addPVC(obj interface{}) {
objKey, err := getPVCKey(obj)
objKey, err := getObjectKey(obj)
if err != nil {
return
}
ctrl.claimQueue.Add(objKey)
}

func (ctrl *resizeController) addPod(obj interface{}) {
pod := parsePod(obj)
if pod == nil {
return
}
ctrl.usedPVCs.addPod(pod)
}

func (ctrl *resizeController) deletePod(obj interface{}) {
pod := parsePod(obj)
if pod == nil {
return
}
ctrl.usedPVCs.removePod(pod)
}

func (ctrl *resizeController) updatePod(oldObj, newObj interface{}) {
pod := parsePod(newObj)
if pod == nil {
return
}

ctrl.usedPVCs.addPod(pod)
}

func (ctrl *resizeController) updatePVC(oldObj, newObj interface{}) {
oldPVC, ok := oldObj.(*v1.PersistentVolumeClaim)
if !ok || oldPVC == nil {
Expand Down Expand Up @@ -162,14 +207,14 @@ func (ctrl *resizeController) updatePVC(oldObj, newObj interface{}) {
}

func (ctrl *resizeController) deletePVC(obj interface{}) {
objKey, err := getPVCKey(obj)
objKey, err := getObjectKey(obj)
if err != nil {
return
}
ctrl.claimQueue.Forget(objKey)
}

func getPVCKey(obj interface{}) (string, error) {
func getObjectKey(obj interface{}) (string, error) {
if unknown, ok := obj.(cache.DeletedFinalStateUnknown); ok && unknown.Obj != nil {
obj = unknown.Obj
}
Expand All @@ -191,8 +236,8 @@ func (ctrl *resizeController) Run(

stopCh := ctx.Done()

if !cache.WaitForCacheSync(stopCh, ctrl.pvSynced, ctrl.pvcSynced) {
klog.Errorf("Cannot sync pv/pvc caches")
if !cache.WaitForCacheSync(stopCh, ctrl.pvSynced, ctrl.pvcSynced, ctrl.podListerSynced) {
klog.Errorf("Cannot sync pod, pv or pvc caches")
return
}

Expand Down Expand Up @@ -322,6 +367,15 @@ func (ctrl *resizeController) resizePVC(pvc *v1.PersistentVolumeClaim, pv *v1.Pe
pvc = updatedPVC
}

// if pvc previously failed to expand because it can't be expanded when in-use
// we must not try expansion here
if ctrl.usedPVCs.hasInUseErrors(pvc) && ctrl.usedPVCs.checkForUse(pvc) {
// Record an event to indicate that resizer is not expanding the pvc
msg := fmt.Sprintf("Unable to expand %s because CSI driver %s only supports offline expansion and volume is currently in-use", util.PVCKey(pvc), ctrl.resizer.Name())
ctrl.eventRecorder.Event(pvc, v1.EventTypeWarning, util.VolumeResizeFailed, msg)
return fmt.Errorf(msg)
}

// Record an event to indicate that external resizer is resizing this volume.
ctrl.eventRecorder.Event(pvc, v1.EventTypeNormal, util.VolumeResizing,
fmt.Sprintf("External resizer is resizing volume %s", pv.Name))
Expand Down Expand Up @@ -352,12 +406,24 @@ func (ctrl *resizeController) resizePVC(pvc *v1.PersistentVolumeClaim, pv *v1.Pe
func (ctrl *resizeController) resizeVolume(
pvc *v1.PersistentVolumeClaim,
pv *v1.PersistentVolume) (resource.Quantity, bool, error) {

// before trying expansion we will remove the PVC from map
// that tracks PVCs which can't be expanded when in-use. If
// pvc indeed can not be expanded when in-use then it will be added
// back when expansion fails with in-use error.
ctrl.usedPVCs.removePVCWithInUseError(pvc)

requestSize := pvc.Spec.Resources.Requests[v1.ResourceStorage]

newSize, fsResizeRequired, err := ctrl.resizer.Resize(pv, requestSize)

if err != nil {
klog.Errorf("Resize volume %q by resizer %q failed: %v", pv.Name, ctrl.name, err)
// if this error was a in-use error then it must be tracked so as we don't retry without
// first verifying if volume is in-use
if inUseError(err) {
ctrl.usedPVCs.addPVCWithInUseError(pvc)
}
return newSize, fsResizeRequired, fmt.Errorf("resize volume %s failed: %v", pv.Name, err)
}
klog.V(4).Infof("Resize volume succeeded for volume %q, start to update PV's capacity", pv.Name)
Expand Down Expand Up @@ -422,3 +488,38 @@ func (ctrl *resizeController) markPVCAsFSResizeRequired(pvc *v1.PersistentVolume

return nil
}

func parsePod(obj interface{}) *v1.Pod {
if obj == nil {
return nil
}
pod, ok := obj.(*v1.Pod)
if !ok {
staleObj, ok := obj.(cache.DeletedFinalStateUnknown)
if !ok {
utilruntime.HandleError(fmt.Errorf("couldn't get stale object %#v", obj))
return nil
}
pod, ok = staleObj.Obj.(*v1.Pod)
if !ok {
utilruntime.HandleError(fmt.Errorf("stale object is not a Pod %#v", obj))
return nil
}
}
return pod
}

func inUseError(err error) bool {
st, ok := status.FromError(err)
if !ok {
// not a grpc error
return false
}
// if this is a failed precondition error then that means driver does not support expansion
// of in-use volumes
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add a comment linking to the CSI spec description about this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

// More info - https://github.com/container-storage-interface/spec/blob/master/spec.md#controllerexpandvolume-errors
if st.Code() == codes.FailedPrecondition {
return true
}
return false
}
Loading