Skip to content

Commit 0a4d542

Browse files
committed
optional VolumeAttachments check
Watching all VolumeAttachments in the cluster even when the CSI driver itself doesn't support attach/detach causes unnecessary overhead. It is now automatically enabled if and only if the driver reports the PUBLISH_UNPUBLISH_VOLUME controller capability. Checking the capability is better than checking the "skip attach" flag in the CSIDriver object because the capability is readily available and constant. It also has the advantage that the watch is disabled for drivers like csi-host-path-driver which do not support attach/detach but don't use "skip attach" (for whatever reason). (cherry picked from commit c53bc29)
1 parent 4e612cc commit 0a4d542

File tree

3 files changed

+31
-5
lines changed

3 files changed

+31
-5
lines changed

cmd/csi-provisioner/csi-provisioner.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,13 @@ func main() {
202202
claimLister := factory.Core().V1().PersistentVolumeClaims().Lister()
203203

204204
var csiNodeLister storagelistersv1.CSINodeLister
205-
vaLister := factory.Storage().V1().VolumeAttachments().Lister()
205+
var vaLister storagelistersv1.VolumeAttachmentLister
206+
if controllerCapabilities[csi.ControllerServiceCapability_RPC_PUBLISH_UNPUBLISH_VOLUME] {
207+
klog.Info("CSI driver supports PUBLISH_UNPUBLISH_VOLUME, watching VolumeAttachments")
208+
vaLister = factory.Storage().V1().VolumeAttachments().Lister()
209+
} else {
210+
klog.Info("CSI driver does not support PUBLISH_UNPUBLISH_VOLUME, not watching VolumeAttachments")
211+
}
206212
var nodeLister v1.NodeLister
207213
if ctrl.SupportsTopology(pluginCapabilities) {
208214
csiNodeLister = factory.Storage().V1().CSINodes().Lister()

deploy/kubernetes/rbac.yaml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,10 @@ rules:
5050
- apiGroups: [""]
5151
resources: ["nodes"]
5252
verbs: ["get", "list", "watch"]
53+
# Access to volumeattachments is only needed when the CSI driver
54+
# has the PUBLISH_UNPUBLISH_VOLUME controller capability.
55+
# In that case, external-provisioner will watch volumeattachments
56+
# to determine when it is safe to delete a volume.
5357
- apiGroups: ["storage.k8s.io"]
5458
resources: ["volumeattachments"]
5559
verbs: ["get", "list", "watch"]

pkg/controller/controller.go

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -271,7 +271,10 @@ func GetDriverCapabilities(conn *grpc.ClientConn, timeout time.Duration) (rpc.Pl
271271
return pluginCapabilities, controllerCapabilities, nil
272272
}
273273

274-
// NewCSIProvisioner creates new CSI provisioner
274+
// NewCSIProvisioner creates new CSI provisioner.
275+
//
276+
// vaLister is optional and only needed when VolumeAttachments are
277+
// meant to be checked before deleting a volume.
275278
func NewCSIProvisioner(client kubernetes.Interface,
276279
connectionTimeout time.Duration,
277280
identity string,
@@ -1046,6 +1049,21 @@ func (p *csiProvisioner) Delete(ctx context.Context, volume *v1.PersistentVolume
10461049
deleteCtx, cancel := context.WithTimeout(ctx, p.timeout)
10471050
defer cancel()
10481051

1052+
if err := p.canDeleteVolume(volume); err != nil {
1053+
return err
1054+
}
1055+
1056+
_, err = p.csiClient.DeleteVolume(deleteCtx, &req)
1057+
1058+
return err
1059+
}
1060+
1061+
func (p *csiProvisioner) canDeleteVolume(volume *v1.PersistentVolume) error {
1062+
if p.vaLister == nil {
1063+
// Nothing to check.
1064+
return nil
1065+
}
1066+
10491067
// Verify if volume is attached to a node before proceeding with deletion
10501068
vaList, err := p.vaLister.List(labels.Everything())
10511069
if err != nil {
@@ -1058,9 +1076,7 @@ func (p *csiProvisioner) Delete(ctx context.Context, volume *v1.PersistentVolume
10581076
}
10591077
}
10601078

1061-
_, err = p.csiClient.DeleteVolume(deleteCtx, &req)
1062-
1063-
return err
1079+
return nil
10641080
}
10651081

10661082
func (p *csiProvisioner) SupportsBlock(ctx context.Context) bool {

0 commit comments

Comments
 (0)