From 48a0decc49feedef358b658bee613ddc66e6a6e7 Mon Sep 17 00:00:00 2001 From: ShyamsundarR Date: Fri, 15 Nov 2019 15:14:06 -0500 Subject: [PATCH] CSI: Make attach mandatory in ceph CSIDriver object A recent PR #4172 added the ability for rook to create the CSIDriver object on a kubernetes cluster. The change also made it so that attachRequired value in the ceph CSI drivers were set to false. This leads to the entire attach skipping the AD controller that deals with RWO attachment enforcement. Thus, if a CephFS volume is marked RWO, 2 pods on different hosts can end up mounting the same and consuming it at the same time. Further for RBD, currently the image watchers are the only ones that prevent a double mount of an image, other than the AD controller enforcement of single attach for RWO RBD PVs. This has proven not to be reliable in the face of MON failures, as the watcher information is lost, and a second mount is allowed. Thus, we need to bring back the attach requirement to leverage kubernetes checks and balances for RWO volume type, and prevent unwanted multi-attach use of the same. This PR hence undoes some of the changes introduced by the PR #4172, to bring the attacher side car back, and also declare the attachRequired for both RBD and CephFS CSI plugins as true. NOTE: Initially the attachRequired was set to false as this improved attach times considerably on kubernetes, as the entire phase was being avoided. Signed-off-by: ShyamsundarR --- Documentation/ceph-upgrade.md | 1 - cluster/charts/rook-ceph/values.yaml | 1 - .../cephfs/csi-cephfsplugin-provisioner-dep.yaml | 16 ++++++++++++++++ .../rbd/csi-rbdplugin-provisioner-dep.yaml | 16 ++++++++++++++++ cluster/examples/kubernetes/ceph/operator.yaml | 1 - pkg/operator/ceph/csi/spec.go | 2 +- 6 files changed, 33 insertions(+), 4 deletions(-) diff --git a/Documentation/ceph-upgrade.md b/Documentation/ceph-upgrade.md index d28b5aeee..717b832f6 100644 --- a/Documentation/ceph-upgrade.md +++ b/Documentation/ceph-upgrade.md @@ -293,7 +293,6 @@ below, which you should change to match where your images are located. value: "quay.io/k8scsi/csi-provisioner:v1.3.0" - name: ROOK_CSI_SNAPSHOTTER_IMAGE value: "quay.io/k8scsi/csi-snapshotter:v1.2.0" - #ROOK_CSI_ATTACHER_IMAGE is required if Kubernetes version is 1.13.x - name: ROOK_CSI_ATTACHER_IMAGE value: "quay.io/k8scsi/csi-attacher:v1.2.0" ``` diff --git a/cluster/charts/rook-ceph/values.yaml b/cluster/charts/rook-ceph/values.yaml index b92be0a34..1d5579667 100644 --- a/cluster/charts/rook-ceph/values.yaml +++ b/cluster/charts/rook-ceph/values.yaml @@ -81,7 +81,6 @@ csi: #image: quay.io/k8scsi/csi-provisioner:v1.3.0 #snapshotter: #image: quay.io/k8scsi/csi-snapshotter:v1.2.0 - # attacher is required if Kubernetes version is 1.13.x #attacher: #image: quay.io/k8scsi/csi-attacher:v1.2.0 diff --git a/cluster/examples/kubernetes/ceph/csi/template/cephfs/csi-cephfsplugin-provisioner-dep.yaml b/cluster/examples/kubernetes/ceph/csi/template/cephfs/csi-cephfsplugin-provisioner-dep.yaml index 484c398cb..27988be9e 100644 --- a/cluster/examples/kubernetes/ceph/csi/template/cephfs/csi-cephfsplugin-provisioner-dep.yaml +++ b/cluster/examples/kubernetes/ceph/csi/template/cephfs/csi-cephfsplugin-provisioner-dep.yaml @@ -16,6 +16,22 @@ spec: spec: serviceAccount: rook-csi-cephfs-provisioner-sa containers: + - name: csi-attacher + image: {{ .AttacherImage }} + args: + - "--v=5" + - "--csi-address=$(ADDRESS)" + - "--leader-election=true" + - "--timeout=150s" + - "--leader-election-type=leases" + - "--leader-election-namespace={{ .Namespace }}" + env: + - name: ADDRESS + value: /csi/csi-provisioner.sock + imagePullPolicy: "IfNotPresent" + volumeMounts: + - name: socket-dir + mountPath: /csi - name: csi-provisioner image: {{ .ProvisionerImage }} args: diff --git a/cluster/examples/kubernetes/ceph/csi/template/rbd/csi-rbdplugin-provisioner-dep.yaml b/cluster/examples/kubernetes/ceph/csi/template/rbd/csi-rbdplugin-provisioner-dep.yaml index 1d494a1bb..a7edc21ba 100644 --- a/cluster/examples/kubernetes/ceph/csi/template/rbd/csi-rbdplugin-provisioner-dep.yaml +++ b/cluster/examples/kubernetes/ceph/csi/template/rbd/csi-rbdplugin-provisioner-dep.yaml @@ -33,6 +33,22 @@ spec: volumeMounts: - name: socket-dir mountPath: /csi + - name: csi-rbdplugin-attacher + image: {{ .AttacherImage }} + args: + - "--v=5" + - "--timeout=150s" + - "--csi-address=$(ADDRESS)" + - "--leader-election=true" + - "--leader-election-type=leases" + - "--leader-election-namespace={{ .Namespace }}" + env: + - name: ADDRESS + value: /csi/csi-provisioner.sock + imagePullPolicy: "IfNotPresent" + volumeMounts: + - name: socket-dir + mountPath: /csi - name: csi-snapshotter image: {{ .SnapshotterImage }} args: diff --git a/cluster/examples/kubernetes/ceph/operator.yaml b/cluster/examples/kubernetes/ceph/operator.yaml index 53a7f1cd4..16bf5d95b 100644 --- a/cluster/examples/kubernetes/ceph/operator.yaml +++ b/cluster/examples/kubernetes/ceph/operator.yaml @@ -179,7 +179,6 @@ spec: # value: "quay.io/k8scsi/csi-provisioner:v1.3.0" #- name: ROOK_CSI_SNAPSHOTTER_IMAGE # value: "quay.io/k8scsi/csi-snapshotter:v1.2.0" - # ROOK_CSI_ATTACHER_IMAGE is required if Kubernetes version is 1.13.x #- name: ROOK_CSI_ATTACHER_IMAGE # value: "quay.io/k8scsi/csi-attacher:v1.2.0" # kubelet directory path, if kubelet configured to use other than /var/lib/kubelet path. diff --git a/pkg/operator/ceph/csi/spec.go b/pkg/operator/ceph/csi/spec.go index 0427c94c3..a78f2b72d 100644 --- a/pkg/operator/ceph/csi/spec.go +++ b/pkg/operator/ceph/csi/spec.go @@ -330,7 +330,7 @@ func StartCSIDrivers(namespace string, clientset kubernetes.Interface, ver *vers // createCSIDriverInfo Registers CSI driver by creating a CSIDriver object func createCSIDriverInfo(clientset kubernetes.Interface, name string) error { - attach := false + attach := true mountInfo := false // Create CSIDriver object csiDriver := &k8scsi.CSIDriver{