The current Disconnect method can only be used to remove iscsi sessions, it can not remove an individual volume from a Linux host.
A new method is required to remove an individual volume from a Linux host after it is unmounted.

In example/main.go the example uses the TargetIqn and TargetPortals fields in the connector:

The usage of these fields was removed in #13, which transfers those features into the Targets struct. The fields should be removed from the struct since they are no longer referenced and the main.go should be updated to reflect the proper state of the library. Or if the intent is to keep them in the struct as legacy support, the fields should be clearly marked as obsolete.

Check how getMultipathDevice is affected when user_friendly_names is enabled in a multipath system. (cf. https://github.com/kubernetes-csi/csi-lib-iscsi/pull/29/files/b0ce999d54f73c234a85e6ea074dd6154ea089f1#r741070348).

The goal of this repo is to provide a general iscsi lib for iSCSI based CSI drivers to utilize instead of each one writing their own wrappers around iscsiadm. It works well for that, but it is a "from scratch" effort meaning gaining trust and maturity for production environments is a hurdle.

One other consideration while writing this was to use the existing Kuberenetes iSCSI initiator utils. Most of the iscsiadm calls are localized to a single utils file, and now that we've moved the old iSCSI driver into it's own repo a good chunk of the work is already done here: https://github.com/kubernetes-csi/csi-driver-iscsi/blob/master/pkg/iscsi/iscsi_util.go

I think it's worth considering taking a look at pulling out that iscsi_util.go and providing a mechanism to access it from this library.

The main Connect method is too heavy, too many calls are introduced. usually when a device is created and mapped, we only need to rescan the session and wait till the disk appears.

So what about adding a new method for new device discovery that only run a rescan and return when the device is ready? Assume that we already know the TransportName and sessions are existing. It will have a better performance especially for large number of devices at a time.

At present we set misleading defaults for the main flags . We have to get rid of these flag defaults and we should a mandatory check that the flags are set.

This issue tracks ##29 (comment)

There is a typo in iscsi.go preventing compilation:
alex@Alexandrus-MacBook-Pro-2 csi-lib-iscsi % make
go clean -r -x
cd /Users/alex/code/csi-lib-iscsi
rm -f csi-lib-iscsi.test csi-lib-iscsi.test.exe
rm -rf _output
go build ./iscsi/

_/Users/alex/code/csi-lib-iscsi/iscsi

iscsi/iscsi.go:379:7: no new variables on left side of :=
make: *** [build] Error 2

Recently we've introduced in-tree a new mechanism to count volumes (block and filesystem) in use by pods. I'd be nice if we could use the same mechanism here.

Last week I pulled the Merge pull request #29 from csi-lib-iscsi and tested them out under RHEL 7.9 and Ubuntu 20.04 and found that the new lsblk changes did not work properly under ubuntu. The basic issue is that the lsblk output is different under Ubuntu and causes the iscsi Connect() call to fail because one device have no children.

Here is the output from each OS with ubuntu missing the data on 'sdd':

(1) ubuntu 20.04 -- lsblk from util-linux 2.34

$ /usr/bin/lsblk -rn -o NAME,KNAME,PKNAME,HCTL,TYPE,TRAN,SIZE /dev/disk/by-path/ip-10.235.212.199:3260-iscsi-iqn.2015-11.com.hpe:storage.msa2040.163639c929-lun-1 /dev/disk/by-path/ip-10.235.212.200:3260-iscsi-iqn.2015-11.com.hpe:storage.msa2040.163639c929-lun-1
sdd sdd 38:0:0:1 disk iscsi 1G
sde sde 37:0:0:1 disk iscsi 1G
3600c0ff00001277388d2976101000000 dm-3 sdd mpath 1G

(2) rhel 7.9 -- lsblk from util-linux 2.23.2

$ /usr/bin/lsblk -rn -o NAME,KNAME,PKNAME,HCTL,TYPE,TRAN,SIZE /dev/disk/by-path/ip-10.235.212.199:3260-iscsi-iqn.2015-11.com.hpe:storage.msa2040.163639c929-lun-1 /dev/disk/by-path/ip-10.235.212.200:3260-iscsi-iqn.2015-11.com.hpe:storage.msa2040.163639c929-lun-1
sdb sdb 24:0:0:1 disk iscsi 1G
3600c0ff0000127739ed1976101000000 dm-5 sdb mpath 1G
sdc sdc 23:0:0:1 disk iscsi 1G
3600c0ff0000127739ed1976101000000 dm-5 sdc mpath 1G

(3) debug output
DEBUG: 2021/11/20 14:02:40 iscsi.go:345: Session already exists, checking if device path "/dev/disk/by-path/ip-10.235.200.98:3260-iscsi-iqn.1988-11.com.dell:01.array.bc305ba247a8-lun-1" exists
DEBUG: 2021/11/20 14:02:40 iscsi.go:276: Appending device path: /dev/disk/by-path/ip-10.235.200.98:3260-iscsi-iqn.1988-11.com.dell:01.array.bc305ba247a8-lun-1
DEBUG: 2021/11/20 14:02:40 iscsi.go:477: Getting info about SCSI devices [/dev/disk/by-path/ip-10.235.200.97:3260-iscsi-iqn.1988-11.com.dell:01.array.bc305ba247a8-lun-1 /dev/disk/by-path/ip-10.235.200.98:3260-iscsi-iqn.1988-11.com.dell:01.array.bc305ba247a8-lun-1].
DEBUG: 2021/11/20 14:02:40 iscsi.go:510: /usr/bin/lsblk -rn -o NAME,KNAME,PKNAME,HCTL,TYPE,TRAN,SIZE /dev/disk/by-path/ip-10.235.200.97:3260-iscsi-iqn.1988-11.com.dell:01.array.bc305ba247a8-lun-1 /dev/disk/by-path/ip-10.235.200.98:3260-iscsi-iqn.1988-11.com.dell:01.array.bc305ba247a8-lun-1
DEBUG: 2021/11/20 14:02:40 iscsi.go:455: mount target is not a multipath device: device is not mapped to exactly one multipath device (is multipathd running?): []
DEBUG: 2021/11/20 14:02:40 iscsi.go:296: Connect failed: device is not mapped to exactly one multipath device (is multipathd running?): []
DEBUG: 2021/11/20 14:02:40 iscsi.go:605: Removing SCSI devices [{sdb 35:0:0:1 [] disk iscsi 1G} {sdc 36:0:0:1 [] disk iscsi 1G}].

Summary:

I looked into the issue and tried to determine if using 'lsblk' could be made to work, but I am not confident in any of my ideas and was looking for input or a correction to this issue.

Another thought is to possibly get 'multipath -ll' to work in both cases and that binary must already be installed if using multipath tools. I have discovered that the multipath output does produce device [Name, Children, HCTL, Size] but may have to get Type and Transport from /dev/disk/by-path/ or another source.

As stated here by @humblec, we should always print error messages without capital letters. I add this issue here to not overwhelm the related PR.

As pointed out in the discussion at: #1 (comment) it maybe worthwhile to discuss and advise CSI driver authors planning to use iSCSI where they may expect iscsid should be running.

Some options:

[1] Have CSI driver authors document and advise admins to have iscsid running in the hosts. iscsiadm may be packaged within a privileged container hosting the CSI driver and needs to be able to interact with iscsid in the host.

[2] Have iscsid running within the container hosting the CSI driver. This approach keeps the CSI driver containers self sufficient (except for the dependency on the iSCSI kernel mode module) but may lead to complications due to lack of namespace awareness around netlink (using host network namespace may solve this). If the host is also running iscsid as well, it will also create problems.

[3] Have iscsid running within a daemonset. This will have similar complications as [2].

Currently we have a debug mode that will provide log output of failed iscsiadm calls. It was just noticed that we log the iscsiadm call including arguments here, if the failing call happens to be a create/update db with CHAP secrets then we could end up logging the CHAP secrets which is of course no good.

We should scrub the arguments in the debug logging function for any secrets info.