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

FC volume plugin should support World Wide Identifier (WWID) parameter as an unique volume identifier #48639

Closed
3 tasks
mtanino opened this issue Jul 7, 2017 · 5 comments · Fixed by #48741
Closed
3 tasks
Labels
kind/bug Categorizes issue or PR as related to a bug. kind/feature Categorizes issue or PR as related to a new feature. sig/storage Categorizes an issue or PR as relevant to SIG Storage.

Comments

@mtanino
Copy link

mtanino commented Jul 7, 2017

Is this a BUG REPORT or FEATURE REQUEST?: BUG

/kind bug
/kind feature

What happened:

Currently FC volume plugin supports two parameters "targetWWNs" and "lun" to specify volume from the /dev/disk/by-path/ directory. However as mentioned on the Red Hat's persistent naming documentation, devices referenced by by-path might be changed.

This means that volume for Pod A is potentially used by Pod B on the same node and this causes security problem and also deta destruction.

https://access.redhat.com/documentation/en-US/Red_Hat_Enterprise_Linux/6/html/Storage_Administration_Guide/persistent_naming.html

It is generally not appropriate for applications to use these path-based names. This is
because the storage device these paths reference may change, potentially causing incorrect
data to be written to the device. Path-based names are also not appropriate for multipath
devices, because the path-based names may be mistaken for separate storage devices, leading
to uncoordinated access and unintended modifications of the data.

Also LUN format can be contained hex value, but current LUN of FC plugin only permits integer value. Therefore LUN is restricted the value between 0 and 255 inclusive.

ls -l /dev/disk/by-path/
lrwxrwxrwx. 1 root root  9 Jul  6 05:12 pci-0000:05:00.0-fc-0xXXXXXXXXXXXXXXXX-lun-1 -> ../../sdc
lrwxrwxrwx. 1 root root  9 Jul  6 05:14 pci-0000:05:00.0-fc-0xXXXXXXXXXXXXXXXX-lun-10 -> ../../sdd
lrwxrwxrwx. 1 root root  9 Jul  6 05:25 pci-0000:05:00.0-fc-0xXXXXXXXXXXXXXXXX-lun-20 -> ../../sde
lrwxrwxrwx. 1 root root  9 Jul  6 05:51 pci-0000:05:00.0-fc-0xXXXXXXXXXXXXXXXX-lun-255 -> ../../sdi
lrwxrwxrwx. 1 root root  9 Jul  6 05:50 pci-0000:05:00.0-fc-0xXXXXXXXXXXXXXXXX-lun-0x0100000000000000 -> ../../sdh   (LUN-> 256)
lrwxrwxrwx. 1 root root  9 Jul  6 05:40 pci-0000:05:00.0-fc-0xXXXXXXXXXXXXXXXX-lun-0x03e8000000000000 -> ../../sdf   (LUN->1000)
lrwxrwxrwx. 1 root root  9 Jul  6 05:45 pci-0000:05:00.0-fc-0xXXXXXXXXXXXXXXXX-lun-0x07d0000000000000 -> ../../sdg   (LUN->2000)

or

LUN specification for FC Persistent Volumes #45024

For example, the /dev/disk/by-path for one of these devices looks like:
ccw-0.0.50c0-fc-0x500507680b2281fb-lun-0x027b000000000000 -> ../../sda

What you expected to happen:

Instead of using "targetWWNs" and "lun", "WWID" is recommended to be used in reliably identifying devices. (See following documents)

WWID can be obtained by SCSI Inquiry page 0x83 as mentioned on the following documents. The symlink including WWID for /dev/sdX is exposed under the /dev/disk/by-id/ directory. Therefore we can use "by-id" instead of "by-path".

This identifier can be obtained by issuing a SCSI Inquiry to retrieve the Device Identification
Vital Product Data (page 0x83) or Unit Serial Number (page 0x80). The mappings from these
WWIDs to the current /dev/sd names can be seen in the symlinks maintained in the
/dev/disk/by-id/ directory.

Anything else we need to know?:

I'll push a PR to support "WWID" for FC volume plugin. Current target version is v1.8.

  • TODO
  • Support "WWID" in FCVolumeSource (API change)
  • Support "WWID" in FC volume plugin
  • Deprecate current "targetWWNs" and "lun" in the future (discussion needed)
@k8s-ci-robot k8s-ci-robot added kind/bug Categorizes issue or PR as related to a bug. kind/feature Categorizes issue or PR as related to a new feature. labels Jul 7, 2017
@k8s-github-robot
Copy link

@mtanino There are no sig labels on this issue. Please add a sig label by:
(1) mentioning a sig: @kubernetes/sig-<team-name>-misc
e.g., @kubernetes/sig-api-machinery-* for API Machinery
(2) specifying the label manually: /sig <label>
e.g., /sig scalability for sig/scalability

Note: method (1) will trigger a notification to the team. You can find the team list here and label list here

@k8s-github-robot k8s-github-robot added the needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. label Jul 7, 2017
@mtanino
Copy link
Author

mtanino commented Jul 7, 2017

/sig storage

@k8s-ci-robot k8s-ci-robot added the sig/storage Categorizes an issue or PR as relevant to SIG Storage. label Jul 7, 2017
@k8s-github-robot k8s-github-robot removed the needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. label Jul 7, 2017
@mtanino
Copy link
Author

mtanino commented Jul 7, 2017

/cc @msau42 @saad-ali @rootfs @childsb

@mtanino
Copy link
Author

mtanino commented Jul 10, 2017

@jsafrane

mtanino pushed a commit to mtanino/kubernetes that referenced this issue Aug 10, 2017
This PR adds World Wide Identifier (WWID) parameter to
FCVolumeSource as an unique volume identifier.

fixes kubernetes#48639
MikeSpreitzer pushed a commit to MikeSpreitzer/kubernetes that referenced this issue Aug 11, 2017
Automatic merge from submit-queue

FC volume plugin: Support WWID for volume identifier

**What this PR does / why we need it**:

This PR adds World Wide Identifier (WWID) parameter to FCVolumeSource as an unique volume identifier.

**Which issue this PR fixes**: fixes kubernetes#48639 

**Special notes for your reviewer**:

/cc @rootfs @jsafrane @msau42 

**Release note**:

```
FC volume plugin: Support WWID for volume identifier
```
@jimmyseto
Copy link

Is this a BUG REPORT or FEATURE REQUEST?: BUG

/kind bug
/kind feature

What happened:

Currently FC volume plugin supports two parameters "targetWWNs" and "lun" to specify volume from the /dev/disk/by-path/ directory. However as mentioned on the Red Hat's persistent naming documentation, devices referenced by by-path might be changed.

This means that volume for Pod A is potentially used by Pod B on the same node and this causes security problem and also deta destruction.

https://access.redhat.com/documentation/en-US/Red_Hat_Enterprise_Linux/6/html/Storage_Administration_Guide/persistent_naming.html

It is generally not appropriate for applications to use these path-based names. This is
because the storage device these paths reference may change, potentially causing incorrect
data to be written to the device. Path-based names are also not appropriate for multipath
devices, because the path-based names may be mistaken for separate storage devices, leading
to uncoordinated access and unintended modifications of the data.

Also LUN format can be contained hex value, but current LUN of FC plugin only permits integer value. Therefore LUN is restricted the value between 0 and 255 inclusive.

ls -l /dev/disk/by-path/
lrwxrwxrwx. 1 root root  9 Jul  6 05:12 pci-0000:05:00.0-fc-0xXXXXXXXXXXXXXXXX-lun-1 -> ../../sdc
lrwxrwxrwx. 1 root root  9 Jul  6 05:14 pci-0000:05:00.0-fc-0xXXXXXXXXXXXXXXXX-lun-10 -> ../../sdd
lrwxrwxrwx. 1 root root  9 Jul  6 05:25 pci-0000:05:00.0-fc-0xXXXXXXXXXXXXXXXX-lun-20 -> ../../sde
lrwxrwxrwx. 1 root root  9 Jul  6 05:51 pci-0000:05:00.0-fc-0xXXXXXXXXXXXXXXXX-lun-255 -> ../../sdi
lrwxrwxrwx. 1 root root  9 Jul  6 05:50 pci-0000:05:00.0-fc-0xXXXXXXXXXXXXXXXX-lun-0x0100000000000000 -> ../../sdh   (LUN-> 256)
lrwxrwxrwx. 1 root root  9 Jul  6 05:40 pci-0000:05:00.0-fc-0xXXXXXXXXXXXXXXXX-lun-0x03e8000000000000 -> ../../sdf   (LUN->1000)
lrwxrwxrwx. 1 root root  9 Jul  6 05:45 pci-0000:05:00.0-fc-0xXXXXXXXXXXXXXXXX-lun-0x07d0000000000000 -> ../../sdg   (LUN->2000)

or

LUN specification for FC Persistent Volumes #45024

For example, the /dev/disk/by-path for one of these devices looks like:
ccw-0.0.50c0-fc-0x500507680b2281fb-lun-0x027b000000000000 -> ../../sda

What you expected to happen:

Instead of using "targetWWNs" and "lun", "WWID" is recommended to be used in reliably identifying devices. (See following documents)

WWID can be obtained by SCSI Inquiry page 0x83 as mentioned on the following documents. The symlink including WWID for /dev/sdX is exposed under the /dev/disk/by-id/ directory. Therefore we can use "by-id" instead of "by-path".

* https://access.redhat.com/documentation/en-US/Red_Hat_Enterprise_Linux/6/html/Storage_Administration_Guide/persistent_naming.html#persistent_naming-wwid
This identifier can be obtained by issuing a SCSI Inquiry to retrieve the Device Identification
Vital Product Data (page 0x83) or Unit Serial Number (page 0x80). The mappings from these
WWIDs to the current /dev/sd names can be seen in the symlinks maintained in the
/dev/disk/by-id/ directory.
* https://pubs.vmware.com/vsphere-50/index.jsp?topic=%2Fcom.vmware.vsphere.storage.doc_50%2FGUID-A36810F4-00EC-4EA8-A242-2A0DBBF56731.html

* https://msdn.microsoft.com/en-us/library/windows/desktop/aa384634(v=vs.85).aspx

Anything else we need to know?:

I'll push a PR to support "WWID" for FC volume plugin. Current target version is v1.8.

* TODO

* [ ]   Support "WWID" in FCVolumeSource (API change)

* [ ]   Support "WWID" in FC volume plugin

* [ ]   Deprecate current "targetWWNs" and "lun" in the future (discussion needed)

@mtanino - I'm curious as to why the wwids field was made to be an array of strings, as opposed to a string. An FC volume only has one WWID, to my understanding. Can you elaborate on this? Thanks in advance for your help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug. kind/feature Categorizes issue or PR as related to a new feature. sig/storage Categorizes an issue or PR as relevant to SIG Storage.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants