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

Make driver functionality to work and adopt latest csi-lib-iscsi and csi spec 1.5.0 #70

Merged
merged 2 commits into from
Dec 3, 2021

Conversation

humblec
Copy link
Contributor

@humblec humblec commented Nov 25, 2021

Considering csi-lib-iscsi has gone through some refactoring, it is
good to consume latest commit and also to use latest
csi spec ie 1.5.0.

This commit update the dependency of below projects:

  • github.com/container-storage-interface/spec v1.5.0
  • github.com/kubernetes-csi/csi-lib-iscsi v0.0.0-20211110090527-5c802c48a124
  • github.com/kubernetes-csi/csi-lib-utils v0.10.0
  • google.golang.org/grpc v1.38.0

The ISCSI driver functionality has been tested with this PR and it looks to be working as expected 👍
Please refer # #70 (comment)

Signed-off-by: Humble Chirammal [email protected]

@k8s-ci-robot k8s-ci-robot added the do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. label Nov 25, 2021
@k8s-ci-robot k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Nov 25, 2021
@humblec
Copy link
Contributor Author

humblec commented Nov 25, 2021

/release-note-none

@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Nov 25, 2021
@humblec humblec changed the title Make use of latest csi-lib-iscsi and csi spec 1.5.0 [WIP] Make use of latest csi-lib-iscsi and csi spec 1.5.0 Nov 25, 2021
@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 25, 2021
@andyzhangx
Copy link
Member

can you run make && make sanity-test on your local env? not sure why github action does not work, let me add a CI test pipeline to run sanity test first.

@humblec
Copy link
Contributor Author

humblec commented Nov 25, 2021

let me add a CI test pipeline to run sanity test first.

That would be cool!

can you run make && make sanity-test on your local env?

@andyzhangx indeed it was ran in my local setup :) and I wanted to put the fixes as followup commits 47f010f here in the PR , so marked this as WIP, once the functional verification is done I will remove the WIP. 👍

    ...
/home/hchiramm/go/src/github.com/kubernetes-csi/csi-test/pkg/sanity/node.go:737
------------------------------
SSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSSS
Ran 10 of 78 Specs in 0.050 seconds
SUCCESS! -- 10 Passed | 0 Failed | 0 Pending | 68 Skipped
pkill -f iscsiplugin
Deleting CSI sanity test binary

@andyzhangx
Copy link
Member

aha, I see, the sanity test is already running there, need to rename to Sanity tests, here is the PR: #71

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 29, 2021
Considering csi-lib-iscsi has good amount of fixes, it is
good to consume latest commit and also good to use latest
csi spec ie 1.5.0.

This commit update the dependency of below projects:

- github.com/container-storage-interface/spec v1.5.0
- github.com/kubernetes-csi/csi-lib-iscsi v0.0.0-20211110090527-5c802c48a124
- github.com/kubernetes-csi/csi-lib-utils v0.10.0
- google.golang.org/grpc v1.38.0

Signed-off-by: Humble Chirammal <[email protected]>
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 2, 2021
@humblec humblec changed the title [WIP] Make use of latest csi-lib-iscsi and csi spec 1.5.0 Make driver functionality to work and adopt latest csi-lib-iscsi and csi spec 1.5.0 Dec 2, 2021
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 2, 2021
@humblec humblec force-pushed the release-1 branch 2 times, most recently from 59522ac to 77d5614 Compare December 2, 2021 13:34
@humblec
Copy link
Contributor Author

humblec commented Dec 2, 2021



root@hchiramm csi-driver-iscsi]# kubectl get csidriver
NAME               ATTACHREQUIRED   PODINFOONMOUNT   STORAGECAPACITY   TOKENREQUESTS   REQUIRESREPUBLISH   MODES        AGE
iscsi.csi.k8s.io   false            false            false             <unset>         false               Persistent   12m
[root@hchiramm csi-driver-iscsi]# 

I1202 13:05:02.878282       8 utils.go:64] GRPC request: {}
I1202 13:05:02.879757       8 identityserver.go:32] Using default GetPluginInfo
I1202 13:05:02.879762       8 utils.go:69] GRPC response: {"name":"iscsi.csi.k8s.io","vendor_version":"1.0.0"}
I1202 13:05:02.999608       8 utils.go:63] GRPC call: /csi.v1.Identity/GetPluginInfo
I1202 13:05:02.999632       8 utils.go:64] GRPC request: {}


[root@hchiramm static-pv-name]# kubectl get pods
NAME                   READY   STATUS    RESTARTS   AGE
csi-iscsi-node-ndchq   3/3     Running   0          10m
[root@hchiramm static-pv-name]# 

[root@hchiramm csi-driver-iscsi]# kubectl get pvc
NAME        STATUS   VOLUME           CAPACITY   ACCESS MODES   STORAGECLASS   AGE
iscsi-pvc   Bound    static-pv-name   1Gi        RWO                           12m

----------------

PV :

Message:         
Source:
    Type:              CSI (a Container Storage Interface (CSI) volume source)
    Driver:            iscsi.csi.k8s.io
    FSType:            
    VolumeHandle:      hum-iscsi-share
    ReadOnly:          false
    VolumeAttributes:      discoveryCHAPAuth=false
                           iqn=iqn.2015-06.com.example.test:target1
                           iscsiInterface=default
                           lun=1
                           portals=[]
                           targetPortal=127.0.0.1:3260
Events:                <none>


[root@hchiramm csi-driver-iscsi]# kubectl create -f pod.yaml
pod/task-pv-pod created

[root@hchiramm csi-driver-iscsi]# ls -l /dev/disk/by-path/* |grep iscsi
lrwxrwxrwx 1 root root  9 Dec  2 20:57 /dev/disk/by-path/ip-127.0.0.1:3260-iscsi-iqn.2015-06.com.example.test:target1-lun-1 -> ../../sda

[root@hchiramm csi-driver-iscsi]# lsblk |grep mount
sda                               8:0    0     1G  0 disk /var/lib/kubelet/pods/d8718ae2-58b0-417f-99ec-59d5389b5889/volumes/kubernetes.io~csi/static-pv-name/mount
[root@hchiramm csi-driver-iscsi]# 
[root@hchiramm csi-driver-iscsi]# kubectl exec -ti task-pv-pod -- bash
root@task-pv-pod:/# 
root@task-pv-pod:/# cd /usr/share/nginx/html/
root@task-pv-pod:/usr/share/nginx/html# touch iscsi
root@task-pv-pod:/usr/share/nginx/html# echo "iscsi" >isci
root@task-pv-pod:/usr/share/nginx/html# ls
isci  iscsi  lost+found  test
root@task-pv-pod:/usr/share/nginx/html# cat isci 
iscsi
root@task-pv-pod:/usr/share/nginx/html# exit
exit
[root@hchiramm csi-driver-iscsi]# kubectl get pods
NAME                   READY   STATUS    RESTARTS   AGE
csi-iscsi-node-ndchq   3/3     Running   0          11m
task-pv-pod            1/1     Running   0          70s

[root@hchiramm csi-driver-iscsi]# kubectl delete pod task-pv-pod
pod "task-pv-pod" deleted
[root@hchiramm csi-driver-iscsi]# 

[root@hchiramm csi-driver-iscsi]# ls -l /dev/disk/by-path/*

[root@hchiramm csi-driver-iscsi]# 

@humblec
Copy link
Contributor Author

humblec commented Dec 2, 2021

/assign @j-griffith
/assign @andyzhangx

- Get rid of Multipath field from the Connector build

As we have done refactoring of iscsi lib to have a multipath
logic from it, while builing the connector we dont need multipath
formation from the buildSCSIConnector anymore. This commit get rid
of the same and leave the logic to the iscsi lib.

- Add lun information to the connector builder

Signed-off-by: Humble Chirammal <[email protected]>
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: humblec, j-griffith

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@j-griffith
Copy link

/LGTM

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 3, 2021
@k8s-ci-robot k8s-ci-robot merged commit 7c729b1 into kubernetes-csi:master Dec 3, 2021
Dockerfile Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesn't merit a release note. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants