Add CSI driver for gluster block (loopback device) by poornimag · Pull Request #105 · gluster/gluster-csi-driver

poornimag · 2018-11-25T09:21:51Z

This patch intends to add glusterblock csi driver which
exports a loopback device formatted as xfs. The current
implementaion expects a mount point as a parameter, where
the block devices will be created.

The original patch was taken from [1]

[1] master...Madhu-1:lo-block

Co-Authored-by: Poornima G pgurusid@redhat.com
Signed-off-by: Poornima G pgurusid@redhat.com

Describe what this PR does
Provide some context for the reviewer

Is there anything that requires special attention?
Do you have any questions? Did you do something clever?

Related issues:
Mention any github issues relevant to this PR

centos-ci · 2018-11-25T09:26:00Z

Can one of the admins verify this patch?

Madhu-1 · 2018-11-26T08:21:29Z

add to whitelist

Madhu-1 · 2018-11-26T08:25:06Z

@poornimag CI is failing

nixpanic · 2018-11-26T09:04:15Z

+
+# Install dependencies
+RUN yum -y install centos-release-gluster && \
+    yum -y install glusterfs-fuse kmod-xfs xfsdump xfsprogs dmapi -y && \


I do not think there is a kmod-xfs or dmapi package in CentOS.

nixpanic · 2018-11-26T09:04:53Z

+ARG builddate="(unknown)"
+
+LABEL build-date="${builddate}"
+LABEL io.k8s.description="FUSE-based CSI driver for Gluster file access"


"for Gluster file acces"? Both here and a few lines below.

nixpanic · 2018-11-26T09:05:17Z

@@ -0,0 +1,321 @@
+package glusterblock


missing copyright notice.

nixpanic · 2018-11-26T09:07:18Z

Is there a design or README for this gluster-block feature?

Madhu-1 · 2018-11-26T10:04:19Z

+              mountPath: /var/lib/csi/sockets/pluginproxy/
+
+        - name: glusterblock
+          image: docker.io/poornimag/glusterblock-csi-driver


image name needs to be updated to use docker.io/gluster

this is applicable in all places

Madhu-1 · 2018-11-26T10:05:16Z

+          args:
+            - "--nodeid=$(NODE_ID)"
+            - "--endpoint=$(CSI_ENDPOINT)"
+            - "--resturl=$(REST_URL)"


as of now, we are not communicating with gd2. I think we can remove this argument

this is applicable in all places

Madhu-1 · 2018-11-26T10:05:46Z

+                  fieldPath: spec.nodeName
+            - name: CSI_ENDPOINT
+              value: unix://plugin/csi.sock
+            - name: REST_URL


this is also not required.

this is applicable in all places

Madhu-1 · 2018-11-26T10:09:13Z

+		return nil, status.Error(codes.AlreadyExists, "block volume already exists")
+	}
+	fileName := hostPath + volumeName
+	_, err = os.Create(fileName)


before creating the new files, I think we need to check free spaces on the BHV.

Madhu-1 · 2018-11-26T10:09:54Z

+
+	if _, err = os.Stat(hostPath); os.IsNotExist(err) {
+		glog.Errorf("failed to create block volume as the block hosting path doesn't exist: %v", err)
+		return nil, err


the error should be like
return nil, status.Error(codes.InvalidArgument, err)

this is applicable in all places, we need to return applicable status codes while returning errors

Madhu-1 · 2018-11-26T10:10:26Z

+	}
+	if _, err = os.Stat(hostPath + volumeName); !os.IsNotExist(err) {
+		glog.Errorf("block volume %s already exixts")
+		return nil, status.Error(codes.AlreadyExists, "block volume already exists")


if block volume already exists, we need to send back the success response

Madhu-1 · 2018-11-26T10:11:05Z

+		return nil, err
+	}
+	if _, err = os.Stat(hostPath + volumeName); !os.IsNotExist(err) {
+		glog.Errorf("block volume %s already exixts")


typo exixts to exists

Madhu-1 · 2018-11-26T10:14:30Z

+	if err == nil {
+		deviceName := strings.Split(string(out), " \n")
+		device = deviceName[0]
+	}


what happens when we get some error here?

Madhu-1 · 2018-11-26T10:15:07Z

+	hostPath := cs.Config.BlockHostPath
+	fileName := hostPath + volumeID
+	err := os.Remove(fileName)
+	if err != nil {


if fileName not found we need to return success response

Madhu-1 · 2018-12-13T05:04:35Z

@poornimag any update on this PR?

atinmu · 2018-12-13T05:29:26Z

@poornimag Please have an issue filed and capture the same in the commit message. We'd like to get this targeted for GCS/0.5 release.

@Madhu-1 @JohnStrunk @humblec ^^

aravindavk

I see BlockHost path is accepted as config parameter. Who will create the block hosting volume?

Madhu-1 · 2018-12-13T05:33:20Z

@aravindavk earlier assumption was operator will create BHV and pass it as an argument during the CSI driver initialization

the design has changed so that CSI driver will take care of BHV. @poornimag will be updating PR with design doc.

atinmu · 2018-12-18T04:13:15Z

I don't see any progress on this PR yet. Please note we're targeting this PR for GCS/0.5. I'd also like to see an overall end to end consumption workflow based out of GCS stack for the loopback based rwo story somewhere, most probably in GCS wiki.

This patch intends to add glusterblock csi driver which exports a loopback device formatted as xfs. The current implementaion expects a mount point as a parameter, where the block devices will be created. The original patch was taken from [1] [1] gluster/gluster-csi-driver@master...Madhu-1:lo-block Co-Authored-by: Poornima G <pgurusid@redhat.com> Signed-off-by: Poornima G <pgurusid@redhat.com>

amarts · 2019-01-10T11:52:00Z

@poornimag is there any benchmark available for PV create time for this model? Would be great to see some graphs on PV create rate, and delete rate, may be upto 50 or 100 PVs? If yes, can you share it here, that would help people to understand the benefit of this approach.

poornimag · 2019-01-11T06:50:52Z

Would

Did some testing with this patch on the scalability of the block devices backed by loopback file hosted on Gluster volume. Here is the link to setup steps and the scale testing results.

This patch shall be re-written to use latest gd2 capabilities, will add blog and more details once we get the newer version.

JohnStrunk · 2019-01-14T19:27:56Z

3360 PVs in ~45 minutes, with a constant rate... very nice. 🎉

JohnStrunk · 2019-01-16T23:10:00Z

This is being implemented via #150.
Closing.

ghost assigned poornimag Nov 25, 2018

ghost added the in progress label Nov 25, 2018

nixpanic reviewed Nov 26, 2018

View reviewed changes

Madhu-1 requested changes Nov 26, 2018

View reviewed changes

aravindavk reviewed Dec 13, 2018

View reviewed changes

poornimag force-pushed the gluster-block-loopback branch from 46529dd to ea68c8f Compare December 15, 2018 06:42

poornimag force-pushed the gluster-block-loopback branch from ea68c8f to 030e9cd Compare December 21, 2018 13:06

poornimag force-pushed the gluster-block-loopback branch from 030e9cd to 8010ccf Compare January 10, 2019 11:05

Madhu-1 added the GCS/1.0 label Jan 10, 2019

JohnStrunk mentioned this pull request Jan 16, 2019

Add CSI driver for gluster block #150

Merged

JohnStrunk closed this Jan 16, 2019

ghost removed the in progress label Jan 16, 2019

Conversation

poornimag commented Nov 25, 2018

Uh oh!

centos-ci commented Nov 25, 2018

Uh oh!

Madhu-1 commented Nov 26, 2018

Uh oh!

Madhu-1 commented Nov 26, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nixpanic commented Nov 26, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Madhu-1 commented Dec 13, 2018

Uh oh!

atinmu commented Dec 13, 2018

Uh oh!

aravindavk left a comment

Choose a reason for hiding this comment

Uh oh!

Madhu-1 commented Dec 13, 2018

Uh oh!

atinmu commented Dec 18, 2018

Uh oh!

amarts commented Jan 10, 2019

Uh oh!

poornimag commented Jan 11, 2019

Uh oh!

JohnStrunk commented Jan 14, 2019

Uh oh!

JohnStrunk commented Jan 16, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants