Skip to content

Commit

Permalink
Merge pull request #67955 from jsafrane/csi-skip-attach-saad
Browse files Browse the repository at this point in the history
Automatic merge from submit-queue (batch tested with PRs 68161, 68023, 67909, 67955, 67731). If you want to cherry-pick this change to another branch, please follow the instructions here: https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md.

CSI: skip attach for non-attachable drivers

**What this PR does / why we need it**:
This is implementation of kubernetes/community#2523. CSI volumes that don't need attach/detach now don't need external attacher running.

WIP:
 * contains #67803 to get CSIDriver API. Ignore the first commit.
 * ~~missing e2e test~~

/sig storage

cc: @saad-ali @vladimirvivien @verult @msau42 @gnufied @davidz627 

**Release note**:
```release-note
CSI volume plugin does not need external attacher for non-attachable CSI volumes.
```
  • Loading branch information
Kubernetes Submit Queue committed Sep 5, 2018
2 parents 0df5d8d + f1cef9b commit 19c2538
Show file tree
Hide file tree
Showing 16 changed files with 574 additions and 99 deletions.
5 changes: 5 additions & 0 deletions pkg/features/kube_features.go
Expand Up @@ -386,6 +386,10 @@ const (
//
// Allow TTL controller to clean up Pods and Jobs after they finish.
TTLAfterFinished utilfeature.Feature = "TTLAfterFinished"
// owner: @jsafrane
// Kubernetes skips attaching CSI volumes that don't require attachment.
//
CSISkipAttach utilfeature.Feature = "CSISkipAttach"
)

func init() {
Expand Down Expand Up @@ -451,6 +455,7 @@ var defaultKubernetesFeatureGates = map[utilfeature.Feature]utilfeature.FeatureS
VolumeSnapshotDataSource: {Default: false, PreRelease: utilfeature.Alpha},
ProcMountType: {Default: false, PreRelease: utilfeature.Alpha},
TTLAfterFinished: {Default: false, PreRelease: utilfeature.Alpha},
CSISkipAttach: {Default: false, PreRelease: utilfeature.Alpha},

// inherited features from generic apiserver, relisted here to get a conflict if it is changed
// unintentionally on either side:
Expand Down
5 changes: 5 additions & 0 deletions pkg/volume/csi/BUILD
Expand Up @@ -29,6 +29,7 @@ go_library(
"//staging/src/k8s.io/client-go/kubernetes:go_default_library",
"//staging/src/k8s.io/csi-api/pkg/client/informers/externalversions:go_default_library",
"//staging/src/k8s.io/csi-api/pkg/client/informers/externalversions/csi/v1alpha1:go_default_library",
"//staging/src/k8s.io/csi-api/pkg/client/listers/csi/v1alpha1:go_default_library",
"//vendor/github.com/container-storage-interface/spec/lib/go/csi/v0:go_default_library",
"//vendor/github.com/golang/glog:go_default_library",
"//vendor/google.golang.org/grpc:go_default_library",
Expand Down Expand Up @@ -59,10 +60,14 @@ go_test(
"//staging/src/k8s.io/apimachinery/pkg/types:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/watch:go_default_library",
"//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library",
"//staging/src/k8s.io/client-go/kubernetes:go_default_library",
"//staging/src/k8s.io/client-go/kubernetes/fake:go_default_library",
"//staging/src/k8s.io/client-go/testing:go_default_library",
"//staging/src/k8s.io/client-go/util/testing:go_default_library",
"//staging/src/k8s.io/csi-api/pkg/apis/csi/v1alpha1:go_default_library",
"//staging/src/k8s.io/csi-api/pkg/client/clientset/versioned/fake:go_default_library",
"//vendor/github.com/container-storage-interface/spec/lib/go/csi/v0:go_default_library",
"//vendor/github.com/golang/glog:go_default_library",
],
)

Expand Down
45 changes: 32 additions & 13 deletions pkg/volume/csi/csi_attacher.go
Expand Up @@ -70,6 +70,16 @@ func (c *csiAttacher) Attach(spec *volume.Spec, nodeName types.NodeName) (string
return "", err
}

skip, err := c.plugin.skipAttach(csiSource.Driver)
if err != nil {
glog.Error(log("attacher.Attach failed to find if driver is attachable: %v", err))
return "", err
}
if skip {
glog.V(4).Infof(log("skipping attach for driver %s", csiSource.Driver))
return "", nil
}

node := string(nodeName)
pvName := spec.PersistentVolume.GetName()
attachID := getAttachmentName(csiSource.VolumeHandle, csiSource.Driver, node)
Expand Down Expand Up @@ -120,6 +130,16 @@ func (c *csiAttacher) WaitForAttach(spec *volume.Spec, attachID string, pod *v1.
return "", err
}

skip, err := c.plugin.skipAttach(source.Driver)
if err != nil {
glog.Error(log("attacher.Attach failed to find if driver is attachable: %v", err))
return "", err
}
if skip {
glog.V(4).Infof(log("Driver is not attachable, skip waiting for attach"))
return "", nil
}

return c.waitForVolumeAttachment(source.VolumeHandle, attachID, timeout)
}

Expand Down Expand Up @@ -221,11 +241,22 @@ func (c *csiAttacher) VolumesAreAttached(specs []*volume.Spec, nodeName types.No
glog.Error(log("attacher.VolumesAreAttached failed: %v", err))
continue
}
skip, err := c.plugin.skipAttach(source.Driver)
if err != nil {
glog.Error(log("Failed to check CSIDriver for %s: %s", source.Driver, err))
} else {
if skip {
// This volume is not attachable, pretend it's attached
attached[spec] = true
continue
}
}

attachID := getAttachmentName(source.VolumeHandle, source.Driver, string(nodeName))
glog.V(4).Info(log("probing attachment status for VolumeAttachment %v", attachID))
attach, err := c.k8s.StorageV1beta1().VolumeAttachments().Get(attachID, meta.GetOptions{})
if err != nil {
attached[spec] = false
glog.Error(log("attacher.VolumesAreAttached failed for attach.ID=%v: %v", attachID, err))
continue
}
Expand Down Expand Up @@ -325,19 +356,7 @@ func (c *csiAttacher) MountDevice(spec *volume.Spec, devicePath string, deviceMo

// Start MountDevice
nodeName := string(c.plugin.host.GetNodeName())
attachID := getAttachmentName(csiSource.VolumeHandle, csiSource.Driver, nodeName)

// search for attachment by VolumeAttachment.Spec.Source.PersistentVolumeName
attachment, err := c.k8s.StorageV1beta1().VolumeAttachments().Get(attachID, meta.GetOptions{})
if err != nil {
return err // This err already has enough context ("VolumeAttachment xyz not found")
}

if attachment == nil {
err = errors.New("no existing VolumeAttachment found")
return err
}
publishVolumeInfo := attachment.Status.AttachmentMetadata
publishVolumeInfo, err := c.plugin.getPublishVolumeInfo(c.k8s, csiSource.VolumeHandle, csiSource.Driver, nodeName)

nodeStageSecrets := map[string]string{}
if csiSource.NodeStageSecretRef != nil {
Expand Down

0 comments on commit 19c2538

Please sign in to comment.